[Mesa-dev] [PATCH 05/13] mesa: add double uniform support. (v4)

[Ilia Mirkin]

On Thu, Feb 5, 2015 at 11:17 AM, Brian Paul <brianp at vmware.com> wrote:
> On 02/05/2015 12:27 AM, Ilia Mirkin wrote:
>>
>> From: Dave Airlie <airlied at redhat.com>
>>
>> This adds support for the new uniform interfaces
>> from ARB_gpu_shader_fp64.
>>
>> v2:
>> support ARB_separate_shader_objects ProgramUniform*d* (Ian)
>> don't allow boolean uniforms to be updated (issue 15) (Ian)
>>
>> v3: fix size_mul
>> v4: Teach uniform update to take into account double precision (Topi)
>>
>> Signed-off-by: Dave Airlie <airlied at redhat.com>
>> ---
>>   src/mesa/main/uniform_query.cpp   |  27 +++---
>>   src/mesa/main/uniforms.c          | 185
>> ++++++++++++++++++++++++++++++++++----
>>   src/mesa/main/uniforms.h          |   3 +-
>>   src/mesa/program/ir_to_mesa.cpp   |  17 +++-
>>   src/mesa/program/prog_parameter.c |  16 ++--
>>   5 files changed, 210 insertions(+), 38 deletions(-)
>>
>> diff --git a/src/mesa/main/uniform_query.cpp
>> b/src/mesa/main/uniform_query.cpp
>> index d36f506..7db8c36 100644
>> --- a/src/mesa/main/uniform_query.cpp
>> +++ b/src/mesa/main/uniform_query.cpp
>> @@ -808,13 +810,14 @@ extern "C" void
>>   _mesa_uniform_matrix(struct gl_context *ctx, struct gl_shader_program
>> *shProg,
>>                      GLuint cols, GLuint rows,
>>                        GLint location, GLsizei count,
>> -                     GLboolean transpose, const GLfloat *values)
>> +                     GLboolean transpose,
>> +                     const GLvoid *values, GLenum type)
>
>
> You can use void instead of GLvoid.

All the other params are GL* -- seems really inconsistent to switch
back and forth.

>
>
>>   {
>>      unsigned offset;
>>      unsigned vectors;
>>      unsigned components;
>>      unsigned elements;
>> -
>> +   int size_mul = mesa_type_is_double(type) ? 2 : 1;
>
>
> I think you can do this instead:
>
>    unsigned size_mul;
>
>    assert(type == GL_FLOAT || type == GL_DOUBLE);
>    size_mul = type == GL_DOUBLE ? 2 : 1;

Right, only one or the other is ever passed in.

>
>
>>      struct gl_uniform_storage *const uni =
>>         validate_uniform_parameters(ctx, shProg, location, count,
>>                                     &offset, "glUniformMatrix");
>> @@ -852,7 +855,7 @@ _mesa_uniform_matrix(struct gl_context *ctx, struct
>> gl_shader_program *shProg,
>>      }
>>
>>      if (unlikely(ctx->_Shader->Flags & GLSL_UNIFORMS)) {
>> -      log_uniform(values, GLSL_TYPE_FLOAT, components, vectors, count,
>> +      log_uniform(values, uni->type->base_type, components, vectors,
>> count,
>>                   bool(transpose), shProg, location, uni);
>>      }
>>
>> @@ -879,11 +882,11 @@ _mesa_uniform_matrix(struct gl_context *ctx, struct
>> gl_shader_program *shProg,
>>
>>      if (!transpose) {
>>         memcpy(&uni->storage[elements * offset], values,
>> -            sizeof(uni->storage[0]) * elements * count);
>> +            sizeof(uni->storage[0]) * elements * count * size_mul);
>>      } else {
>>         /* Copy and transpose the matrix.
>>          */
>> -      const float *src = values;
>> +      const float *src = (const float *)values;
>>         float *dst = &uni->storage[elements * offset].f;
>>
>>         for (int i = 0; i < count; i++) {
>
>
> Don't you need to handle double values in the transpose code?

Yes! Nice find! Just made a copy of the existing code with doubles
src/dst. Seemed simplest without slowing down the float case.

>> diff --git a/src/mesa/program/ir_to_mesa.cpp
>> b/src/mesa/program/ir_to_mesa.cpp
>> index 725e51d..54413cd 100644
>> --- a/src/mesa/program/ir_to_mesa.cpp
>> +++ b/src/mesa/program/ir_to_mesa.cpp
>> @@ -606,6 +606,14 @@ type_size(const struct glsl_type *type)
>>           */
>>          return 1;
>>         }
>> +      break;
>> +   case GLSL_TYPE_DOUBLE:
>> +      if (type->is_matrix()) {
>> +        return type->matrix_columns * 2;
>> +      } else {
>> +         return 2;
>> +      }
>> +      break;
>
>
> Indentation looks funny there.  Using tabs?

Indeed. Fixed.