[Mesa-dev] [WIP 00/13] GL_ARB_explicit_uniform_location extension

[Tapani Pälli]

On 04/05/2014 12:33 AM, Ian Romanick wrote:
> On 03/27/2014 11:45 PM, Tapani Pälli wrote:
>> Hi;
>>
>> Patches implement the extension, no Piglit regressions and all the tests
>> for the extension pass (I've planned some more tests which are not yet
>> in though).
>>
>> Changes shortly:
>>
>>    - opt_dead_code optimization is modifed to build a list of removed
>>      uniform locations (that had explicit location) as this information
>>      is required by the extension. For this I had to modify here and
>>      there to have gl_shader_program structure available.
>>
>>    - link_uniforms is modified to first occupy all explicit locations
>>      and only then the rest (sorry about patch size but I'm having trouble
>>      to split it smaller)
>>
>>    - small parser changes, likely needs still more validation checks
>>
>> I would need some help determining what kind of version checks (gl, glsl)
>> should be made in the parser for this extension and also should I rather
>> just enable it for everyone or only for Intel, would be cool if someone
>> could test this implementation against some other platform.
> The extension spec says, "Requires OpenGL 3.3 or
> ARB_explicit_attrib_location."  I think we can enable it in every driver
> that also enables ARB_explicit_attrib_location... according to
> docs/GL3.txt, that's "all drivers that support GLSL."

OK, that's clear. I had some trouble with Nvidia that required me to set
GLSL version to 330, not even enabling ARB_explicit_attrib_location in
the shader helped, I think that is a bug in their driver because there
is no GLSL version requirement in the specification.

>> Also, I'm not sure how to actually get a good MAX_UNIFORM_LOCATIONS
>> value, current one is just thrown with a dice. For example this is 65536
>> for Nvidia binary driver (with gtx660) but it feels rather big to
>> *really* work .. or?
> I think this should be set based on GL_MAX_*_UNIFORM_COMPONENTS.  An
> application should be able to set a location for every uniform.  The
> worst that will happen if the value is huge is that the application will
> ask us to waste a bunch of memory.  Applications already have a lot of
> ways to ask us to waste memory. :)  If we encounter apps that, say, just
> use locations 2 and 65534, then we can look at using a different data
> structure to avoid the waste.

Right, I think the corner cases are quite academic with this extension.

>> Here's a branch with the patches:
>> http://cgit.freedesktop.org/~tpalli/mesa/log/?h=exp_uniform_loc
>>
>> Any comments appreciated, thanks;
>>
>> // Tapani
>>
>>
>> Tapani Pälli (13):
>>   glapi: add GL_ARB_explicit_uniform_location
>>   mesa: add enable bit for ARB_explicit_uniform_location
>>   mesa: add new enum MAX_UNIFORM_LOCATIONS and default value
>>   mesa: add a storage for inactive/removed uniform variables
>>   glsl: change do_common_optimization signature
>>   glsl: change do_dead_code signature, store uniform locations
>>   glsl/linker: change link_assign_uniform_locations signature
>>   glsl/linker: GL_ARB_explicit_uniform_location support
>>   mesa: support inactive uniforms in glUniform* functions
>>   glsl: add enable bit for ARB_explicit_uniform_location
>>   glsl: parser changes for GL_ARB_explicit_uniform_location
>>   intel: Enable GL_ARB_explicit_uniform_location
>>   docs: update ARB_explicit_uniform_location status
>>
>>  docs/GL3.txt                                 |   2 +-
>>  src/glsl/ast_to_hir.cpp                      |  13 ++
>>  src/glsl/glcpp/glcpp-parse.y                 |   3 +
>>  src/glsl/glsl_lexer.ll                       |   1 +
>>  src/glsl/glsl_parser_extras.cpp              |  11 +-
>>  src/glsl/glsl_parser_extras.h                |   2 +
>>  src/glsl/ir_optimization.h                   |   6 +-
>>  src/glsl/ir_uniform.h                        |   5 +-
>>  src/glsl/link_uniforms.cpp                   | 239 ++++++++++++++++++++++++---
>>  src/glsl/linker.cpp                          |  12 +-
>>  src/glsl/linker.h                            |   5 +-
>>  src/glsl/opt_dead_code.cpp                   |  40 ++++-
>>  src/mapi/glapi/gen/gl_API.xml                |   6 +
>>  src/mesa/drivers/dri/i965/brw_shader.cpp     |   3 +-
>>  src/mesa/drivers/dri/i965/intel_extensions.c |   1 +
>>  src/mesa/main/context.c                      |   3 +
>>  src/mesa/main/extensions.c                   |   1 +
>>  src/mesa/main/get.c                          |   1 +
>>  src/mesa/main/get_hash_params.py             |   1 +
>>  src/mesa/main/mtypes.h                       |  19 +++
>>  src/mesa/main/shaderobj.c                    |   7 +
>>  src/mesa/main/tests/enum_strings.cpp         |   1 +
>>  src/mesa/main/uniform_query.cpp              |  16 ++
>>  src/mesa/program/ir_to_mesa.cpp              |   2 +-
>>  src/mesa/state_tracker/st_glsl_to_tgsi.cpp   |   3 +-
>>  25 files changed, 367 insertions(+), 36 deletions(-)
>>