[Mesa-dev] [PATCH 1/6] mesa: add Uniform*d support to display lists

[Timothy Arceri]

On 20/06/18 15:16, Mathias Fröhlich wrote:
> Hi,
> 
> It seems that dlist.c is getting popular again, but can't we generate
> most of dlist.c using the api files?
> Probably saves a lot of typing, copying and pasting in the long run.
> Should also be less error prone in the long run.
> Not that I think code generation is required, but I believe it would
> help, especially as lots of that is already in place for generating
> the api tables.

Yes I thought about this but the effort required to write/test any 
generation is too high vs the number of extensions left that require 
dlist support.

I've just sent a series to add ARB_viewport_array display list support, 
with that we have maybe 3 more extension to go for GL 4.5 support. As 
for errors I've been creating extensive piglit tests (which should be 
done either way) and I expect the code I'm adding to be as error free as 
any generated code would be.

> 
> There is one comment inline below.
> 
> 
> 
> On Wednesday, 20 June 2018 03:58:34 CEST Timothy Arceri wrote:
>> This is required so we can enable fp64 support in compat profile.
>> ---
>>   src/mapi/glapi/gen/apiexec.py |  36 +--
>>   src/mesa/main/dlist.c         | 493 ++++++++++++++++++++++++++++++++++
>>   2 files changed, 511 insertions(+), 18 deletions(-)
>>
>> diff --git a/src/mapi/glapi/gen/apiexec.py b/src/mapi/glapi/gen/apiexec.py
>> index 20d6239ba14..00c80171274 100644
>> --- a/src/mapi/glapi/gen/apiexec.py
>> +++ b/src/mapi/glapi/gen/apiexec.py
>> @@ -85,24 +85,24 @@ functions = {
>>       # OpenGL 4.0 / GL_ARB_gpu_shader_fp64.  The extension spec says:
>>       #
>>       #     "OpenGL 3.2 and GLSL 1.50 are required."
>> -    "Uniform1d": exec_info(core=32),
>> -    "Uniform2d": exec_info(core=32),
>> -    "Uniform3d": exec_info(core=32),
>> -    "Uniform4d": exec_info(core=32),
>> -    "Uniform1dv": exec_info(core=32),
>> -    "Uniform2dv": exec_info(core=32),
>> -    "Uniform3dv": exec_info(core=32),
>> -    "Uniform4dv": exec_info(core=32),
>> -    "UniformMatrix2dv": exec_info(core=32),
>> -    "UniformMatrix3dv": exec_info(core=32),
>> -    "UniformMatrix4dv": exec_info(core=32),
>> -    "UniformMatrix2x3dv": exec_info(core=32),
>> -    "UniformMatrix2x4dv": exec_info(core=32),
>> -    "UniformMatrix3x2dv": exec_info(core=32),
>> -    "UniformMatrix3x4dv": exec_info(core=32),
>> -    "UniformMatrix4x2dv": exec_info(core=32),
>> -    "UniformMatrix4x3dv": exec_info(core=32),
>> -    "GetUniformdv": exec_info(core=32),
>> +    "Uniform1d": exec_info(compatibility=32, core=32),
>> +    "Uniform2d": exec_info(compatibility=32, core=32),
>> +    "Uniform3d": exec_info(compatibility=32, core=32),
>> +    "Uniform4d": exec_info(compatibility=32, core=32),
>> +    "Uniform1dv": exec_info(compatibility=32, core=32),
>> +    "Uniform2dv": exec_info(compatibility=32, core=32),
>> +    "Uniform3dv": exec_info(compatibility=32, core=32),
>> +    "Uniform4dv": exec_info(compatibility=32, core=32),
>> +    "UniformMatrix2dv": exec_info(compatibility=32, core=32),
>> +    "UniformMatrix3dv": exec_info(compatibility=32, core=32),
>> +    "UniformMatrix4dv": exec_info(compatibility=32, core=32),
>> +    "UniformMatrix2x3dv": exec_info(compatibility=32,core=32),
>> +    "UniformMatrix2x4dv": exec_info(compatibility=32, core=32),
>> +    "UniformMatrix3x2dv": exec_info(compatibility=32, core=32),
>> +    "UniformMatrix3x4dv": exec_info(compatibility=32, core=32),
>> +    "UniformMatrix4x2dv": exec_info(compatibility=32, core=32),
>> +    "UniformMatrix4x3dv": exec_info(compatibility=32, core=32),
>> +    "GetUniformdv": exec_info(compatibility=32, core=32),
>>   
>>       # OpenGL 4.1 / GL_ARB_vertex_attrib_64bit.  The extension spec says:
>>       #
>> diff --git a/src/mesa/main/dlist.c b/src/mesa/main/dlist.c
>> index 4fc451000b5..b0fbc17d017 100644
>> --- a/src/mesa/main/dlist.c
>> +++ b/src/mesa/main/dlist.c
>> @@ -365,6 +365,25 @@ typedef enum
>>      OPCODE_UNIFORM_3UIV,
>>      OPCODE_UNIFORM_4UIV,
>>   
>> +   /* GL_ARB_gpu_shader_fp64 */
>> +   OPCODE_UNIFORM_1D,
>> +   OPCODE_UNIFORM_2D,
>> +   OPCODE_UNIFORM_3D,
>> +   OPCODE_UNIFORM_4D,
>> +   OPCODE_UNIFORM_1DV,
>> +   OPCODE_UNIFORM_2DV,
>> +   OPCODE_UNIFORM_3DV,
>> +   OPCODE_UNIFORM_4DV,
>> +   OPCODE_UNIFORM_MATRIX22D,
>> +   OPCODE_UNIFORM_MATRIX33D,
>> +   OPCODE_UNIFORM_MATRIX44D,
>> +   OPCODE_UNIFORM_MATRIX23D,
>> +   OPCODE_UNIFORM_MATRIX32D,
>> +   OPCODE_UNIFORM_MATRIX24D,
>> +   OPCODE_UNIFORM_MATRIX42D,
>> +   OPCODE_UNIFORM_MATRIX34D,
>> +   OPCODE_UNIFORM_MATRIX43D,
>> +
>>      /* OpenGL 4.0 / GL_ARB_tessellation_shader */
>>      OPCODE_PATCH_PARAMETER_I,
>>      OPCODE_PATCH_PARAMETER_FV_INNER,
>> @@ -606,6 +625,22 @@ union uint64_pair
>>   };
>>   
>>   
>> +union float64_pair
>> +{
>> +   GLdouble d;
>> +   GLfloat f[2];
>> +};
> 
> May be you want to use GLuint ui[2] instead of GLfloat f[2].
> The reason is that you may end up with bit patterns that stem
> from finite double values but generate NaN's or infs when split
> into two floats in this way. Now if your application traps on NaN's
> I am not sure if the move operation already traps.
> If you use something integer values to transport arbitrary bit patterns
> to compose doubles form those it is guaranteed not to trap in any case.

Right, I'll rework this thanks for taking a look.


> 
> best
> 
> Mathias
> 
>