[Mesa-dev] [PATCH v2] glsl: fix consumer_stage restriction to separate shader objects

[Timothy Arceri]

On 25 January 2016 11:35:04 pm AEDT, "Samuel Iglesias Gonsálvez" <siglesias at igalia.com> wrote:
>This patch is still unreviewed.

I'll take another look tomorrow unless someone gets to it first. I've looked at it but didn't really look hard enough to understand the problem the previous patch was fixing I think I get it now.

>
>Sam
>
>On Tue, 2016-01-19 at 07:20 +0100, Samuel Iglesias Gonsálvez wrote:
>> Commit 781d278 did not restrict consumer_stage only to separate
>> shader
>> objects, which is when we don't know if the consumer stage would be a
>> fragment shader added later. In normal programs, when
>> consumer_stage == -1, it is because they are not consumed.
>> 
>> Fixes 4 piglit regressions added by commit 781d278 in radeonsi:
>> 
>> arb_gpu_shader_fp64-double-gettransformfeedbackvarying
>> arb_gpu_shader_fp64-tf-interleaved
>> arb_gpu_shader_fp64-tf-interleaved-aligned
>> arb_gpu_shader_fp64-tf-separate
>> 
>> v2:
>> - Simplify condition expression (Timothy).
>> 
>> Signed-off-by: Samuel Iglesias Gonsálvez <siglesias at igalia.com>
>> Tested-by: Michel Dänzer <michel.daenzer at amd.com>
>> ---
>>  src/glsl/link_varyings.cpp | 15 +++++++++------
>>  1 file changed, 9 insertions(+), 6 deletions(-)
>> 
>> diff --git a/src/glsl/link_varyings.cpp b/src/glsl/link_varyings.cpp
>> index 09f80d0..a27589d 100644
>> --- a/src/glsl/link_varyings.cpp
>> +++ b/src/glsl/link_varyings.cpp
>> @@ -824,7 +824,8 @@ public:
>>                     gl_shader_stage producer_stage,
>>                     gl_shader_stage consumer_stage);
>>     ~varying_matches();
>> -   void record(ir_variable *producer_var, ir_variable
>> *consumer_var);
>> +   void record(ir_variable *producer_var, ir_variable *consumer_var,
>> +               bool separate_shader);
>>     unsigned assign_locations(struct gl_shader_program *prog,
>>                               uint64_t reserved_slots, bool
>> separate_shader);
>>     void store_locations() const;
>> @@ -952,7 +953,8 @@ varying_matches::~varying_matches()
>>   * rendering.
>>   */
>>  void
>> -varying_matches::record(ir_variable *producer_var, ir_variable
>> *consumer_var)
>> +varying_matches::record(ir_variable *producer_var, ir_variable
>> *consumer_var,
>> +                        bool separate_shader)
>>  {
>>     assert(producer_var != NULL || consumer_var != NULL);
>>  
>> @@ -968,7 +970,8 @@ varying_matches::record(ir_variable
>> *producer_var, ir_variable *consumer_var)
>>     }
>>  
>>     if ((consumer_var == NULL && producer_var->type-
>> >contains_integer()) ||
>> -       (consumer_stage != -1 && consumer_stage !=
>> MESA_SHADER_FRAGMENT)) {
>> +       (consumer_stage != MESA_SHADER_FRAGMENT &&
>> +        (!separate_shader || consumer_stage != -1))) {
>>        /* Since this varying is not being consumed by the fragment
>> shader, its
>>         * interpolation type varying cannot possibly affect
>> rendering.
>>         * Also, this variable is non-flat and is (or contains) an
>> integer.
>> @@ -1667,7 +1670,7 @@ assign_varying_locations(struct gl_context
>> *ctx,
>>            */
>>           if (input_var || (prog->SeparateShader && consumer == NULL)
>> ||
>>               producer->Type == GL_TESS_CONTROL_SHADER) {
>> -            matches.record(output_var, input_var);
>> +            matches.record(output_var, input_var, prog-
>> >SeparateShader);
>>           }
>>  
>>           /* Only stream 0 outputs can be consumed in the next stage
>> */
>> @@ -1692,7 +1695,7 @@ assign_varying_locations(struct gl_context
>> *ctx,
>>               (input_var->data.mode != ir_var_shader_in))
>>              continue;
>>  
>> -         matches.record(NULL, input_var);
>> +         matches.record(NULL, input_var, prog->SeparateShader);
>>        }
>>     }
>>  
>> @@ -1711,7 +1714,7 @@ assign_varying_locations(struct gl_context
>> *ctx,
>>        }
>>  
>>        if (matched_candidate->toplevel_var-
>> >data.is_unmatched_generic_inout)
>> -         matches.record(matched_candidate->toplevel_var, NULL);
>> +         matches.record(matched_candidate->toplevel_var, NULL, prog-
>> >SeparateShader);
>>     }
>>  
>>     const uint64_t reserved_slots =
>_______________________________________________
>mesa-dev mailing list
>mesa-dev at lists.freedesktop.org
>http://lists.freedesktop.org/mailman/listinfo/mesa-dev