[Mesa-dev] [PATCH 03/22] i965/meta: Use explicit uniform locations in fast clear shader

[Ian Romanick]

On 02/18/2016 07:01 AM, Iago Toral wrote:
> On Wed, 2016-02-17 at 17:57 -0800, Ian Romanick wrote:
>> From: Ian Romanick <ian.d.romanick at intel.com>
>>
>> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
>> ---
>>  src/mesa/drivers/dri/i965/brw_meta_fast_clear.c | 14 ++++++++------
>>  1 file changed, 8 insertions(+), 6 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_meta_fast_clear.c b/src/mesa/drivers/dri/i965/brw_meta_fast_clear.c
>> index 37dffb9..3917df1 100644
>> --- a/src/mesa/drivers/dri/i965/brw_meta_fast_clear.c
>> +++ b/src/mesa/drivers/dri/i965/brw_meta_fast_clear.c
>> @@ -62,7 +62,6 @@ struct brw_fast_clear_state {
>>     struct gl_vertex_array_object *array_obj;
>>     GLuint vao;
>>     GLuint shader_prog;
>> -   GLint color_location;
>>  };
>>  
>>  static bool
>> @@ -118,7 +117,13 @@ brw_bind_rep_write_shader(struct brw_context *brw, float *color)
>>        "   gl_Position = position;\n"
>>        "}\n";
>>     const char *fs_source =
>> -      "uniform vec4 color;\n"
>> +      /* GL_ARB_explicit_uniform_location requires either GLSL 3.30 or
>> +       * GL_ARB_explicit_attrib_location, and only the later is universally
>> +       * supported.
>> +       */
>> +      "#extension GL_ARB_explicit_attrib_location: require\n"
>> +      "#extension GL_ARB_explicit_uniform_location: require\n"
> 
> We only use fast clears since gen6 so right now anything that goes
> through this code  is going to meet this requirements. I wonder if it
> would be worth it to assert on the supported GL version here, or at
> least that the FS compile worked though.

We support both of these extensions on all hardware used by the i965
driver.  In fact, every driver that supports GLSL enables these
extensions.  i965, i915, and Gallium state tracker unconditionally
enable these extensions.

We may want to add checks that the compile and link succeed, but I don't
think that's necessarily related to this change.  What do you think?

>> +      "layout(location=0) uniform vec4 color;\n"
>>        "void main()\n"
>>        "{\n"
>>        "   gl_FragColor = color;\n"
>> @@ -141,13 +146,10 @@ brw_bind_rep_write_shader(struct brw_context *brw, float *color)
>>        _mesa_BindAttribLocation(clear->shader_prog, 0, "position");
>>        _mesa_ObjectLabel(GL_PROGRAM, clear->shader_prog, -1, "meta repclear");
>>        _mesa_LinkProgram(clear->shader_prog);
>> -
>> -      clear->color_location =
>> -         _mesa_GetUniformLocation(clear->shader_prog, "color");
>>     }
>>  
>>     _mesa_UseProgram(clear->shader_prog);
>> -   _mesa_Uniform4fv(clear->color_location, 1, color);
>> +   _mesa_Uniform4fv(0, 1, color);
>>  }
>>  
>>  void
> 
>