Correct, this doesn't try to tackle the link-time cases at all.
After sending these out, Anuj told me that he had similar patches
pending review. Unfortunate to duplicate effort, but it's what I get
for doing a bunch of random bug-stomping, I guess :)
Hopefully we can pick the best ideas from both series.
On Thu, Apr 17, 2014 at 6:29 PM, Kenneth Graunke <kenn...@whitecape.org> wrote:
> On 04/12/2014 10:30 PM, Chris Forbes wrote:
>> The ARB_fragment_coord_conventions spec, section 4.3.x (Input Layout
>> Qualifiers) says:
>>
>> "All redeclarations of gl_FragCoord in all fragment shaders in a
>> single program must have the same set of qualifiers. Within any
>> shader, the first redeclarations of gl_FragCoord must appear before
>> any use of gl_FragCoord."
>>
>> Fixes piglit's tests:
>> arb_fragment_coord_conventions/compiler/redeclaration-after-use
>> arb_fragment_coord_conventions/compiler/redeclaration-around-use
>>
>> glsl-1.50/compiler/fragment_coord_conventions/layout-qualifiers-conflicting-case-1
>>
>> glsl-1.50/compiler/fragment_coord_conventions/layout-qualifiers-conflicting-case-2
>>
>> Signed-off-by: Chris Forbes <chr...@ijw.co.nz>
>> ---
>> src/glsl/ast_to_hir.cpp | 20 ++++++++++++++++++++
>> 1 file changed, 20 insertions(+)
>>
>> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
>> index 9b9d511..d1eb3e2 100644
>> --- a/src/glsl/ast_to_hir.cpp
>> +++ b/src/glsl/ast_to_hir.cpp
>> @@ -2715,9 +2715,29 @@ get_variable_being_redeclared(ir_variable *var,
>> YYLTYPE loc,
>> /* Allow redeclaration of gl_FragCoord for ARB_fcc layout
>> * qualifiers.
>> */
>> +
>> + if (earlier->data.how_declared != ir_var_declared_implicitly
>> + && (earlier->data.origin_upper_left != var->data.origin_upper_left
>> + || earlier->data.pixel_center_integer !=
>> var->data.pixel_center_integer)) {
>> + _mesa_glsl_error(&loc, state,
>> + "Inconsistent redeclarations of gl_FragCoord");
>> + }
>> +
>> earlier->data.origin_upper_left = var->data.origin_upper_left;
>> earlier->data.pixel_center_integer = var->data.pixel_center_integer;
>>
>> + if (earlier->data.used &&
>> + earlier->data.how_declared == ir_var_declared_implicitly) {
>> + /* ARB_fragment_coord_conventions spec Section 4.3.x.1
>> + * (Input Layout Qualifier) says:
>> + *
>> + * "Within any shader, the first redeclarations of gl_FragCoord
>> + * must appear before any use of gl_FragCoord."
>> + */
>> + _mesa_glsl_error(&loc, state,
>> + "First redeclaration of gl_FragCoord must appear
>> before any use");
>> + }
>> +
>> /* According to section 4.3.7 of the GLSL 1.30 spec,
>> * the following built-in varaibles can be redeclared with an
>> * interpolation qualifier:
>>
>
> It seems like this will catch multiple, inconsistent redeclarations
> within a single fragment shader...but the spec text quoted indicates
> that redeclarations in all fragment shaders being linked together into a
> single program need to be consistent.
>
> I don't see any code to handle that in linker*.cpp, so presumably we
> ought to add that as well. Regardless, this is a good improvement!
>
> Both patches are:
> Reviewed-by: Kenneth Graunke <kenn...@whitecape.org>
>
> Thanks, Chris.
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
