[Mesa-dev] [PATCH v3] glsl: enforce invariant conditions for built-in variables
Lars Hamre
chemecse at gmail.com
Mon May 9 22:48:34 UTC 2016
Looks like it, I didn't realized VARYING_SLOT_POS covered both cases.
On Mon, May 9, 2016 at 5:17 PM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
> On Mon, May 9, 2016 at 5:10 PM, Lars Hamre <chemecse at gmail.com> wrote:
>> v3:
>> - compare varying slot locations rather than names (Ilia Mirkin)
>> v2:
>> - ES version check (Tapani Pälli)
>>
>> The conditions for which certain built-in special variables
>> can be declared invariant were not being checked.
>>
>> GLSL ES 1.00 specification, Section "Invariance and linkage" says:
>>
>> For the built-in special variables, gl_FragCoord can
>> only be declared invariant if and only if gl_Position is
>> declared invariant. Similarly gl_PointCoord can only be
>> declared invariant if and only if gl_PointSize is declared
>> invariant. It is an error to declare gl_FrontFacing as invariant.
>>
>> This fixes the following piglit tests in spec/glsl-es-1.00/linker:
>> glsl-fcoord-invariant
>> glsl-fface-invariant
>> glsl-pcoord-invariant
>>
>> Signed-off-by: Lars Hamre <chemecse at gmail.com>
>>
>> ---
>>
>> CC: Ilia Mirkin <imirkin at alum.mit.edu>
>>
>> NOTE: Someone with access will need to commit this after the
>> review process
>>
>> src/compiler/glsl/link_varyings.cpp | 43 +++++++++++++++++++++++++++++++++++--
>> 1 file changed, 41 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/compiler/glsl/link_varyings.cpp b/src/compiler/glsl/link_varyings.cpp
>> index 34e82c7..155a4d9 100644
>> --- a/src/compiler/glsl/link_varyings.cpp
>> +++ b/src/compiler/glsl/link_varyings.cpp
>> @@ -352,13 +352,23 @@ cross_validate_outputs_to_inputs(struct gl_shader_program *prog,
>> glsl_symbol_table parameters;
>> ir_variable *explicit_locations[MAX_VARYING][4] = { {NULL, NULL} };
>>
>> + bool is_gl_position_invariant = false;
>> + bool is_gl_point_size_invariant = false;
>> +
>> /* Find all shader outputs in the "producer" stage.
>> */
>> foreach_in_list(ir_instruction, node, producer->ir) {
>> ir_variable *const var = node->as_variable();
>>
>> if ((var == NULL) || (var->data.mode != ir_var_shader_out))
>> - continue;
>> + continue;
>> +
>> + if (prog->IsES && prog->Version < 300) {
>> + if (var->data.location == VARYING_SLOT_POS)
>> + is_gl_position_invariant = var->data.invariant;
>> + if (var->data.location == VARYING_SLOT_PSIZ)
>> + is_gl_point_size_invariant = var->data.invariant;
>> + }
>>
>> if (!var->data.explicit_location
>> || var->data.location < VARYING_SLOT_VAR0)
>> @@ -442,7 +452,36 @@ cross_validate_outputs_to_inputs(struct gl_shader_program *prog,
>> ir_variable *const input = node->as_variable();
>>
>> if ((input == NULL) || (input->data.mode != ir_var_shader_in))
>> - continue;
>> + continue;
>> +
>> + /*
>> + * GLSL ES 1.00 specification, Section "Invariance and linkage" says:
>> + *
>> + * "For the built-in special variables, gl_FragCoord can
>> + * only be declared invariant if and only if gl_Position is
>> + * declared invariant. Similarly gl_PointCoord can only be
>> + * declared invariant if and only if gl_PointSize is declared
>> + * invariant. It is an error to declare gl_FrontFacing as invariant."
>> + */
>> + if (prog->IsES && prog->Version < 300 && input->data.invariant) {
>> + if (input->data.location == VARYING_SLOT_FACE) {
>> + linker_error(prog,
>> + "gl_FrontFacing cannot be declared invariant");
>> + return;
>> + } else if (!is_gl_position_invariant &&
>> + !strcmp(input->name, "gl_FragCoord")) {
>
> I think you want input->data.location == VARYING_SLOT_POS here.
>
>> + linker_error(prog,
>> + "gl_FragCoord cannot be declared invariant "
>> + "unless gl_Position is also invariant");
>> + return;
>> + } else if (!is_gl_point_size_invariant &&
>> + input->data.location == VARYING_SLOT_PNTC) {
>> + linker_error(prog,
>> + "gl_PointCoord cannot be declared invariant "
>> + "unless gl_PointSize is also invariant");
>> + return;
>> + }
>> + }
>>
>> if (strcmp(input->name, "gl_Color") == 0 && input->data.used) {
>> const ir_variable *const front_color =
>> --
>> 2.5.5
>>
More information about the mesa-dev
mailing list