[Mesa-dev] [PATCH] glsl: Generate compile errors for explicit blend indices < 0 or > 1.

[Ian Romanick]

On 09/11/2012 03:06 AM, Paul Berry wrote:
> On 31 August 2012 16:04, Kenneth Graunke <kenneth at whitecape.org
> <mailto:kenneth at whitecape.org>> wrote:
>
>     According to the GLSL 4.30 specification, this is a compile time error.
>     Earlier specifications don't specify a behavior, but since 0 and 1 are
>     the only valid indices for dual source blending, it makes sense to
>     generate the error.
>
>     Fixes (the fixed version of) piglit's layout-12.frag.
>
>     NOTE: This is a candidate for the 9.0 branch.
>
>     Signed-off-by: Kenneth Graunke <kenneth at whitecape.org
>     <mailto:kenneth at whitecape.org>>
>     ---
>       src/glsl/ast_to_hir.cpp | 17 +++++++++++++++--
>       1 file changed, 15 insertions(+), 2 deletions(-)
>
>     diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
>     index 02fe66b..becf6f9 100644
>     --- a/src/glsl/ast_to_hir.cpp
>     +++ b/src/glsl/ast_to_hir.cpp
>     @@ -2086,9 +2086,22 @@ apply_type_qualifier_to_variable(const struct
>     ast_type_qualifier *qual,
>               } else {
>                  var->location = qual->location;
>               }
>     +
>               if (qual->flags.q.explicit_index) {
>     -           var->explicit_index = true;
>     -           var->index = qual->index;
>     +            /* From the GLSL 4.30 specification: "It is also a
>     compile-time
>     +             * error if a fragment shader sets a layout index to
>     less than 0
>     +             * or greater than 1."
>
>
> Minor nit pick: would you mind referencing the section of the spec this
> quote comes from (4.4.2 Output Layout Qualifiers)?

I'll go a step further.  This should be formatted the way other spec 
references in the compiler are formatted:

	/* Page XXX (page YYY of the PDF) of the GLSL A.BB spec says:
	 *
	 *     "Blah."
	 */

It makes it a lot easier to find things using grep.

> Other than that, this patch is:
>
> Reviewed-by: Paul Berry <stereotype441 at gmail.com
> <mailto:stereotype441 at gmail.com>>

and

Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>

>     +             *
>     +             * Older specifications don't mandate a behavior; we
>     take this
>     +             * as a clarification and always generate the error.
>     +             */
>     +            if (qual->index < 0 || qual->index > 1) {
>     +               _mesa_glsl_error(loc, state,
>     +                                "explicit index may only be 0 or 1\n");
>     +            } else {
>     +               var->explicit_index = true;
>     +               var->index = qual->index;
>     +            }
>               }
>             }
>          } else if (qual->flags.q.explicit_index) {
>     --
>     1.7.11.4
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev