On Tue, Jul 30, 2013 at 12:13 PM, Carl Worth <cwo...@cworth.org> 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? > > 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. > */
I think that this patch /should/ go in, since it was a fix for a new extension. 9.2 will be the first version of Mesa to ship with support for ARB_shading_language_420pack, so the branch should contain bug fixes for the extension. _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
