[Mesa-dev] [PATCH 6/8] glsl: check total count of multi-slot double vertex attribs

[Dave Airlie]

On 5 May 2015 at 09:47, Ian Romanick <idr at freedesktop.org> wrote:
> On 04/29/2015 06:14 PM, Dave Airlie wrote:
>> From: Dave Airlie <airlied at redhat.com>
>>
>> The spec is vague all over the place about this, but this seems
>> to be the intent, we can probably make this optional later if
>> someone makes hw that cares and writes a driver.
>>
>> Basically we need to double count some of the d types but
>> only for totalling not for slot number assignment.
>>
>> Signed-off-by: Dave Airlie <airlied at redhat.com>
>> ---
>>  src/glsl/linker.cpp | 40 +++++++++++++++++++++++++++++++++++++++-
>>  1 file changed, 39 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
>> index 21fde94..5803f82 100644
>> --- a/src/glsl/linker.cpp
>> +++ b/src/glsl/linker.cpp
>> @@ -1975,6 +1975,7 @@ assign_attribute_or_color_locations(gl_shader_program *prog,
>>     } to_assign[16];
>>
>>     unsigned num_attr = 0;
>> +   unsigned total_attribs_size = 0;
>>
>>     foreach_in_list(ir_instruction, node, sh->ir) {
>>        ir_variable *const var = node->as_variable();
>> @@ -2016,12 +2017,41 @@ assign_attribute_or_color_locations(gl_shader_program *prog,
>>        }
>>        }
>>
>> +      const unsigned slots = var->type->count_attribute_slots();
>> +
>> +      /* From GL4.5 core spec, section 11.1.1 (Vertex Attributes):
>> +       *
>> +       * "A program with more than the value of MAX_VERTEX_ATTRIBS active
>> +       * attribute variables may fail to link, unless device-dependent
>> +       * optimizations are able to make the program fit within available
>> +       * hardware resources. For the purposes of this test, attribute variables
>> +       * of the type dvec3, dvec4, dmat2x3, dmat2x4, dmat3, dmat3x4, dmat4x3,
>> +       * and dmat4 may count as consuming twice as many attributes as equivalent
>> +       * single-precision types. While these types use the same number of
>> +       * generic attributes as their single-precision equivalents,
>> +       * implementations are permitted to consume two single-precision vectors
>> +       * of internal storage for each three- or four-component double-precision
>> +       * vector."
>> +       * Until someone has a good reason in Mesa, enforce that now.
>> +       */
>> +      if (target_index == MESA_SHADER_VERTEX) {
>> +      total_attribs_size += slots;
>> +      if (var->type->without_array() == glsl_type::dvec3_type ||
>> +          var->type->without_array() == glsl_type::dvec4_type ||
>> +          var->type->without_array() == glsl_type::dmat2x3_type ||
>> +          var->type->without_array() == glsl_type::dmat2x4_type ||
>> +          var->type->without_array() == glsl_type::dmat3_type ||
>> +          var->type->without_array() == glsl_type::dmat3x4_type ||
>> +          var->type->without_array() == glsl_type::dmat4x3_type ||
>> +          var->type->without_array() == glsl_type::dmat4_type)
>
> I think doing
>
>    if (var->type->without_array()->base_type == GLSL_TYPE_DOUBLE)
>
> should be sufficient here.

No it is only the double types that traverse two slots that matter,
the spec is quite clear that double and dvec2 don't get this treatment,
hence the cut-n-paste above.
>
> Also... we may have discussed this w.r.t. varyings in the original fp64
> series, why not just make count_attribute_slots() return 2*count for
> doubles?

Because they only take one attribute slot, if you create a shader
like

in dvec4 d1;
in dvec4 d2;

the GetAttribLocation should give you back 0, 1, not 0 and 2.

also BindAttribLocation with 0,1 should work, and explicit locations
of 0 and 1 should work.

So they can take up the space for two slots out of the 16 total,
but they can't use the indexes for two.

(I'll remove the extra blank line).

Dave.