[Mesa-dev] [Mesa-stable] [PATCH 05/11] glsl: Don't monkey about with the interpolation modes

[Emil Velikov]

On 15 June 2016 at 04:54, Timothy Arceri <timothy.arceri at collabora.com> wrote:
> On Tue, 2016-06-14 at 19:01 -0700, Ian Romanick wrote:
>> From: Ian Romanick <ian.d.romanick at intel.com>
>>
>> Previously we'd munge the interpolation mode so that later checks in
>> the
>> GLSL linker would pass.  The caused problems for similar checks in
>> SSO
>> IO validation.  Instead, make the check smarter, use the same check
>> in
>> both places, and don't modify the interpolation mode.
>>
>> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96358
>> Cc: "12.0" <mesa-stable at lists.freedesktop.org>
>> Cc: Gregory Hainaut <gregory.hainaut at gmail.com>
>> Cc: Ilia Mirkin <imirkin at alum.mit.edu>
>> ---
>>  src/compiler/glsl/ast_to_hir.cpp    | 11 ----------
>>  src/compiler/glsl/link_varyings.cpp | 41
>> +++++++++++++++++++++++++++++++++----
>>  src/compiler/glsl/link_varyings.h   |  7 +++++++
>>  src/mesa/main/shader_query.cpp      |  6 +++++-
>>  4 files changed, 49 insertions(+), 16 deletions(-)
>>
>> diff --git a/src/compiler/glsl/ast_to_hir.cpp
>> b/src/compiler/glsl/ast_to_hir.cpp
>> index 7da734c..d675dfa 100644
>> --- a/src/compiler/glsl/ast_to_hir.cpp
>> +++ b/src/compiler/glsl/ast_to_hir.cpp
>> @@ -2991,17 +2991,6 @@ interpret_interpolation_qualifier(const struct
>> ast_type_qualifier *qual,
>>        interpolation = INTERP_QUALIFIER_NOPERSPECTIVE;
>>     else if (qual->flags.q.smooth)
>>        interpolation = INTERP_QUALIFIER_SMOOTH;
>> -   else if (state->es_shader &&
>> -            ((mode == ir_var_shader_in &&
>> -              state->stage != MESA_SHADER_VERTEX) ||
>> -             (mode == ir_var_shader_out &&
>> -              state->stage != MESA_SHADER_FRAGMENT)))
>> -      /* Section 4.3.9 (Interpolation) of the GLSL ES 3.00 spec
>> says:
>> -       *
>> -       *    "When no interpolation qualifier is present, smooth
>> interpolation
>> -       *    is used."
>> -       */
>> -      interpolation = INTERP_QUALIFIER_SMOOTH;
>>     else
>>        interpolation = INTERP_QUALIFIER_NONE;
>>
>> diff --git a/src/compiler/glsl/link_varyings.cpp
>> b/src/compiler/glsl/link_varyings.cpp
>> index 534393a..54491fc 100644
>> --- a/src/compiler/glsl/link_varyings.cpp
>> +++ b/src/compiler/glsl/link_varyings.cpp
>> @@ -201,6 +201,37 @@ anonymous_struct_type_matches(const glsl_type
>> *output_type,
>>             to_match->record_compare(output_type);
>>  }
>>
>> +bool
>> +interpolation_compatible(gl_shader_stage producer_stage,
>> +                         gl_shader_stage consumer_stage,
>> +                         enum glsl_interp_qualifier producer_interp,
>> +                         enum glsl_interp_qualifier consumer_interp,
>> +                         bool is_builtin_variable)
>> +{
>> +   if (producer_interp == consumer_interp)
>> +      return true;
>> +
>> +   if (is_builtin_variable)
>> +      return false;
>> +
>> +   /* Section 4.3.9 (Interpolation) of the GLSL ES 3.00 spec says:
>> +    *
>> +    *    When no interpolation qualifier is present, smooth
>> interpolation is
>> +    *    used.
>> +    */
>
> Note last time I was looking at this I couldn't find this text in the
> desktop spec so I don't think the following code can be applied to
> desktop gl.
>
>> +   if (producer_stage == MESA_SHADER_VERTEX &&
>> +       producer_interp == INTERP_QUALIFIER_NONE &&
>> +       consumer_interp == INTERP_QUALIFIER_SMOOTH)
>> +      return true;
>> +
>> +   if (consumer_stage == MESA_SHADER_FRAGMENT &&
>> +       consumer_interp == INTERP_QUALIFIER_NONE &&
>> +       producer_interp == INTERP_QUALIFIER_SMOOTH)
>> +      return true;
>
> Are you sure this is enough? What about a fragment shader with smooth
> and a geom shader with none? That shouldn't that return true also?
>
Please correct me if I'm wrong - seem like this patch wasn't
updated/superseded, was it ?

-Emil