On 09/11/2012 03:06 AM, Paul Berry wrote:
On 31 August 2012 16:04, Kenneth Graunke <kenn...@whitecape.org
<mailto:kenn...@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 <kenn...@whitecape.org
    <mailto:kenn...@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 <stereotype...@gmail.com
<mailto:stereotype...@gmail.com>>

and

Reviewed-by: Ian Romanick <ian.d.roman...@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@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to