[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