[Mesa-dev] [PATCH v3] compiler/glsl: invalidate float suffixes for glsl 1.10

[Lars Hamre]

Hi Ian and Roland,

I am new to mesa development, so please bare with my questions:

- Is there some protocol for testing against the private shader-db?
  (I assume private means I don't have access)

- Would the person checking PlaneShift be someone who owns the
  game and can test it against mesa master?

- If we were to change to solely emitting an error, would we remove
  the piglit tests in question or just accept the failures?

I am in agreement with Ian about preferring the driconf option so
we could adhere to spec; however, Roland brings up an
important point that this could break other applications which
potentially use the float suffix.

Thanks for the feedback.

Regards,
Lars


On Thu, Mar 31, 2016 at 10:35 AM, Roland Scheidegger <sroland at vmware.com>
wrote:

> Am 31.03.2016 um 04:17 schrieb Ian Romanick:
> > Arg. :(
> >
> > It turns out that shaders in the game PlaneShift use float suffixes in
> > GLSL 1.10 shaders... at least the version of the shaders in our private
> > shader-db repo do.  Once some one check to make sure the game still does
> > this, we will need some sort of patch.
> >
> > I see two likely options.
> >
> > 1. Just change the error to a warning.
> >
> > 2. Add a driconf option that changes the error to a warning.  Enable
> > this option for PlaneShift in the default .drirc.
> >
> > I'd prefer option #2, personally.
>
>
> I wouldn't be surprised if other apps did the same thing. Forbidding
> float suffixes sounds sort of unusual (and unexpected) to me. Albeit
> there weren't really any other data types then, so possibly noone else
> did it...
>
> Roland
>
> >
> > On 03/28/2016 05:42 PM, Lars Hamre wrote:
> >> NOTE: someone with access will need to commit this patch after the
> >>       review process
> >>
> >> v2: modify error message
> >> v3: parse the float instead of returning an ERROR_TOK
> >>
> >> Invalidates float suffixes for glsl 1.10
> >>
> >> Fixes the following piglit tests:
> >>
> tests/spec/glsl-1.10/compiler/literals/invalid-float-suffix-capital-f.vert
> >> tests/spec/glsl-1.10/compiler/literals/invalid-float-suffix-f.vert`
> >>
> >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=81585
> >>
> >> Signed-off-by: Lars Hamre <chemecse at gmail.com>
> >> Reviewed-by: Timothy Arceri <timothy.arceri at collabora.com>
> >> Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
> >>
> >> ---
> >>  src/compiler/glsl/glsl_lexer.ll | 7 +++++++
> >>  1 file changed, 7 insertions(+)
> >>
> >> diff --git a/src/compiler/glsl/glsl_lexer.ll
> b/src/compiler/glsl/glsl_lexer.ll
> >> index 1f12265..6084d71 100644
> >> --- a/src/compiler/glsl/glsl_lexer.ll
> >> +++ b/src/compiler/glsl/glsl_lexer.ll
> >> @@ -472,6 +472,13 @@ layout          {
> >>  \.[0-9]+([eE][+-]?[0-9]+)?[fF]?             |
> >>  [0-9]+\.([eE][+-]?[0-9]+)?[fF]?             |
> >>  [0-9]+[eE][+-]?[0-9]+[fF]?          {
> >> +                        struct _mesa_glsl_parse_state *state = yyextra;
> >> +                        char suffix = yytext[strlen(yytext) - 1];
> >> +                        if (!state->is_version(120, 0) &&
> >> +                            (suffix == 'f' || suffix == 'F')) {
> >> +                            _mesa_glsl_error(yylloc, state,
> >> +                                             "Float suffixes are
> invalid in GLSL 1.10");
> >> +                        }
> >>                          yylval->real = _mesa_strtof(yytext, NULL);
> >>                          return FLOATCONSTANT;
> >>                      }
> >> --
> >> 2.5.5
> >>
> >> _______________________________________________
> >> mesa-dev mailing list
> >> mesa-dev at lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >>
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160331/00a444bf/attachment.html>