On 07/30/2013 12:13 PM, Carl Worth wrote:
Hi Ken,
In the last stable-release cycle you argued for not cherry picking
commit fcaa48d9cc8937e0ceb59dfd22ef5b6e6fd1a273 on the grounds that
we shouldn't change mesa to be strictly more strict in a stable release.
That is, adding a new error case to the compiler cannot actually cause a
valid program to start working, (which would be a nice bug fix), but
could potentially cause an invalid program to stop working (which could
be regarded as a regression).
The same reasoning seems to disqualify the below patch. Do you agree?
Maybe. If both NVIDIA and AMD correctly generate the error, the
probability of any application using it is epsilon. We should collect
that information and add it to the commit message.
I also think this patch may have just been a candidate for 9.2. :/
Anyone else have input on this issue? If we agree on the rationale, I'd
like to codify it with some language in our documentation for
stable-branch criteria. Something like:
Note: Not all bug fixes are automatically suitable for the
stable branch.
For example, a patch that makes the implementation more strict,
(such as detecting some invalid GLSL code that was previously
silently accepted), can be rejected by the stable-branch
maintainer. The rationale is that a patch like this can cause a
program which was previously working as desired to now stop
working due to the new error case. From the point of view of the
user of such an application, that's a regression that we do not
want to allow during a stable release.
-Carl
commit 17856726c94000bf16156f7f9acea77a271a6005
Author: Kenneth Graunke <kenn...@whitecape.org>
Date: Fri Jul 26 21:18:56 2013 -0700
glsl: Disallow auxiliary storage qualifiers on FS outputs.
This has always been an error; we just forgot to check for it.
Fixes Piglit's no-aux-qual-on-fs-output.frag.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=67333
Signed-off-by: Kenneth Graunke <kenn...@whitecape.org>
Reviewed-by: Matt Turner <matts...@gmail.com>
Cc: mesa-sta...@lists.freedesktop.org
diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
index 2569cde..598da92 100644
--- a/src/glsl/ast_to_hir.cpp
+++ b/src/glsl/ast_to_hir.cpp
@@ -2927,6 +2927,20 @@ ast_declarator_list::hir(exec_list *instructions,
"'centroid in' cannot be used in a vertex shader");
}
+ /* Section 4.3.6 of the GLSL 1.30 specification states:
+ * "It is an error to use centroid out in a fragment shader."
+ *
+ * The GL_ARB_shading_language_420pack extension specification states:
+ * "It is an error to use auxiliary storage qualifiers or interpolation
+ * qualifiers on an output in a fragment shader."
+ */
+ if (state->target == fragment_shader &&
+ this->type->qualifier.flags.q.out &&
+ this->type->qualifier.has_auxiliary_storage()) {
+ _mesa_glsl_error(&loc, state,
+ "auxiliary storage qualifiers cannot be used on "
+ "fragment shader outputs");
+ }
/* Precision qualifiers exists only in GLSL versions 1.00 and >= 1.30.
*/
_______________________________________________
mesa-stable mailing list
mesa-sta...@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-stable
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
