[Mesa-dev] [PATCH] Revert "glsl: warning: pragma `invariant(all)' not supported in GLSL ES 1.00"

Oliver McFadden oliver.mcfadden at linux.intel.com
Sun Aug 5 09:54:05 PDT 2012


On Tue, Jul 31, 2012 at 10:53:11AM -0700, Kenneth Graunke wrote:
> On 07/31/2012 08:41 AM, Eric Anholt wrote:
> > Kenneth Graunke <kenneth at whitecape.org> writes:
> > 
> >> This reverts commit e72f20641a6ea7875b6021aac13e778ada3b3d50.
> >>
> >> The commit has two problems:
> >> - The commit message itself quotes language from the GLSL ES 1.00
> >>   specification that indicates that it /does/ support the invariant(all)
> >>   pragma.  This appears to be the opposite of what the commit actually
> >>   does.
> >> - The check is wrong: state->Const.GLSL_100ES is set if the driver
> >>   supports GLSL ES 1.00, but doesn't mean that the language version
> >>   being parsed actually /is/ ES.  state->language_version == 100 is the
> >>   right way to do that.
> >>
> >> Cc: Oliver McFadden <oliver.mcfadden at linux.intel.com>
> >> Cc: Eric Anholt <eric at anholt.net>
> >> ---
> >>  src/glsl/glsl_parser.yy | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> Am I just missing something here?  I read through the commit several times
> >> trying to figure out what it was trying to do...it really seems to claim
> >>  #pragma invariant(all) is NOT supported, while quoting spec text that
> >> explicitly says it IS supported.  I'm confused.
> > 
> > Huh?  While the usage of Const is definitely wrong and I should have
> > caught it, the commit was trying to make the error no longer occur when
> > compiling an ES shader, and the logic looks reasonable for that.
> 
> Oh!  I completely read this backwards...sorry.  Knew I was missing
> something.
> 
> The thing that confused me was the subject line:
> glsl: warning: pragma `invariant(all)' not supported in GLSL ES 1.00"
> 
> which I interpreted as a claim that invariant(all) /isn't/ supported in
> ES...but it's not.  It's actually a cut and paste of the warning the
> GLSL compiler printed but which the patch removed for ES.
> 
> I've pushed a different patch instead, one which changes the logic to:
> 
> if (state->language_version == 110) {
>    ...emit "pragma not supported" warning...
> } else {
>    ...actually do it
> }
> 
> Sorry for the trouble.

Oops. I'm sorry for the trouble here, at least it has now been reverted
and corrected. Once again my apologies!

-- 
Oliver McFadden.


More information about the mesa-dev mailing list