<div class="gmail_quote">On 11 September 2012 00:40, Ian Romanick <span dir="ltr"><<a href="mailto:idr@freedesktop.org" target="_blank">idr@freedesktop.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im">On 09/11/2012 03:06 AM, Paul Berry wrote:<br>
</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">
On 31 August 2012 16:04, Kenneth Graunke <<a href="mailto:kenneth@whitecape.org" target="_blank">kenneth@whitecape.org</a><br></div><div class="im">
<mailto:<a href="mailto:kenneth@whitecape.org" target="_blank">kenneth@whitecape.org</a>><u></u>> wrote:<br>
<br>
According to the GLSL 4.30 specification, this is a compile time error.<br>
Earlier specifications don't specify a behavior, but since 0 and 1 are<br>
the only valid indices for dual source blending, it makes sense to<br>
generate the error.<br>
<br>
Fixes (the fixed version of) piglit's layout-12.frag.<br>
<br>
NOTE: This is a candidate for the 9.0 branch.<br>
<br>
Signed-off-by: Kenneth Graunke <<a href="mailto:kenneth@whitecape.org" target="_blank">kenneth@whitecape.org</a><br></div>
<mailto:<a href="mailto:kenneth@whitecape.org" target="_blank">kenneth@whitecape.org</a>><u></u>><div class="im"><br>
---<br>
src/glsl/ast_to_hir.cpp | 17 +++++++++++++++--<br>
1 file changed, 15 insertions(+), 2 deletions(-)<br>
<br>
diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp<br>
index 02fe66b..becf6f9 100644<br>
--- a/src/glsl/ast_to_hir.cpp<br>
+++ b/src/glsl/ast_to_hir.cpp<br>
@@ -2086,9 +2086,22 @@ apply_type_qualifier_to_<u></u>variable(const struct<br>
ast_type_qualifier *qual,<br>
} else {<br>
var->location = qual->location;<br>
}<br>
+<br>
if (qual->flags.q.explicit_index) {<br>
- var->explicit_index = true;<br>
- var->index = qual->index;<br>
+ /* From the GLSL 4.30 specification: "It is also a<br>
compile-time<br>
+ * error if a fragment shader sets a layout index to<br>
less than 0<br>
+ * or greater than 1."<br>
<br>
<br>
Minor nit pick: would you mind referencing the section of the spec this<br>
quote comes from (4.4.2 Output Layout Qualifiers)?<br>
</div></blockquote>
<br>
I'll go a step further. This should be formatted the way other spec references in the compiler are formatted:<br>
<br>
/* Page XXX (page YYY of the PDF) of the GLSL <a href="http://A.BB" target="_blank">A.BB</a> spec says:<br>
*<br>
* "Blah."<br>
*/<br>
<br>
It makes it a lot easier to find things using grep.<br></blockquote><div><br></div><div>Ooh, I'd never thought of the grep advantages of formatting the quotes consistently--I like that idea. I would also like to be able to find the quoted text in the spec itself easily (so I can see it in context). Unfortunately, it's often hard to do this based on the page number, because the specs get revised without notice to make error corrections, and this shifts the page numbers around. Also, there are up to four versions of some of the recent specs published in the registry (".core", ".core.withchanges", ".compatibility", and ".compatibility.withchanges"), so if we just cite the page number it's ambiguous which document we are referring to.</div>
<div><br></div><div>Can we adopt a standard way of formatting quotes that includes the section number too? Maybe something like</div><div><br></div><div>/* Page XXX (page YYY of the PDF) of GLSL <a href="http://A.BB">A.BB</a> (section S.T.P.Q) says:</div>
<div> *</div><div> * "Blah."</div><div> */</div><div><br></div><div>Note that with versions of the GL spec since 3.0, we can drop the "(page YYY of the PDF)" part, because they finally figured out how to get the page numbers in the pdf reader to match up between the page numbers printed on the page. Sadly, the GLSL specs still have inconsistent page numbering :(.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">
Other than that, this patch is:<br>
<br>
Reviewed-by: Paul Berry <<a href="mailto:stereotype441@gmail.com" target="_blank">stereotype441@gmail.com</a><br></div>
<mailto:<a href="mailto:stereotype441@gmail.com" target="_blank">stereotype441@gmail.<u></u>com</a>>><br>
</blockquote>
<br>
and<br>
<br>
Reviewed-by: Ian Romanick <<a href="mailto:ian.d.romanick@intel.com" target="_blank">ian.d.romanick@intel.com</a>><div class="HOEnZb"><div class="h5"><br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ *<br>
+ * Older specifications don't mandate a behavior; we<br>
take this<br>
+ * as a clarification and always generate the error.<br>
+ */<br>
+ if (qual->index < 0 || qual->index > 1) {<br>
+ _mesa_glsl_error(loc, state,<br>
+ "explicit index may only be 0 or 1\n");<br>
+ } else {<br>
+ var->explicit_index = true;<br>
+ var->index = qual->index;<br>
+ }<br>
}<br>
}<br>
} else if (qual->flags.q.explicit_index) {<br>
--<br>
1.7.11.4<br>
<br>
______________________________<u></u>_________________<br>
mesa-dev mailing list<br>
<a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a><br>
<a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev" target="_blank">http://lists.freedesktop.org/<u></u>mailman/listinfo/mesa-dev</a><br>
</blockquote>
<br>
</div></div></blockquote></div><br>