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(-) >> _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
