Bug 42930

The telling bit from the log is:

fixme:d3d_shader:print_glsl_info_log Info log received from GLSL shader #1:
fixme:d3d_shader:print_glsl_info_log     error: Too many vertex shader uniform components

Before this change, some link errors wouldn't actually generate errors.  That could explain the incorrect rendering.

Is this message generated before the bisected commit?

Can you set a breakpoint at line 1912 in linker.cpp and dump some information?  Specifically, what are the values of sh->num_uniform_components and max_uniform_components[i]?

Line 1912 should be:

         linker_error(prog, "Too many %s shader uniform components",
		      shader_names[i]);

(In reply to comment #1)
> The telling bit from the log is:
> 
> fixme:d3d_shader:print_glsl_info_log Info log received from GLSL shader #1:
> fixme:d3d_shader:print_glsl_info_log     error: Too many vertex shader uniform
> components
> 
> Before this change, some link errors wouldn't actually generate errors.  That
> could explain the incorrect rendering.
> 
> Is this message generated before the bisected commit?
> 
> Can you set a breakpoint at line 1912 in linker.cpp and dump some information? 
> Specifically, what are the values of sh->num_uniform_components and
> max_uniform_components[i]?
> 
> Line 1912 should be:
> 
>          linker_error(prog, "Too many %s shader uniform components",
>               shader_names[i]);

Before the mentioned commit the "Too many vertex shader uniform
 components" message wasn't present. You can see how the error looked before in bug 42514.

Concerning the breakpoint I had some troubles with wine debugging so I had to use an glretrace to obtain it, I hope this isn't a problem.

Breakpoint 1, check_resources (ctx=0x8404fd8, prog=0x8431478)
    at linker.cpp:1913
1913			      shader_names[i]);
Missing separate debuginfos, use: debuginfo-install libXdamage-1.1.3-2.fc15.i686 libdrm-2.4.26-3.fc16.i686 libffi-3.0.10-1.fc16.i686 libtxc_dxtn-1.0.0-1.fc15.i686
(gdb) print  sh->num_uniform_components
$1 = 3936
(gdb) print max_uniform_components[i]
$2 = 1024

Please Ian, can we not report a failure when the max number of uniform components is exceeded? The r300 compiler can do some elimination of unused constants. This is especially useful with Wine, which sometimes declares unnecessarily large arrays, but doesn't use it all.

(In reply to comment #3)
> Please Ian, can we not report a failure when the max number of uniform
> components is exceeded? The r300 compiler can do some elimination of unused
> constants. This is especially useful with Wine, which sometimes declares
> unnecessarily large arrays, but doesn't use it all.

Not and be a valid GLSL implementation.  The spec is quite clear that we *MUST* report an error in this condition.  What is happening in the r300 compiler to eliminate these arrays?  Can this optimization happen at a higher level?  Do we have samples of these shaders?

The r300 compiler looks for the constants which are used and allocates new locations for them.

For example this:

DCL CONST[0..2000]
  0: MAD OUT[0], CONST[3], CONST[550], CONST[1300]
  1: END

is transformed into:

DCL CONST[0..2]
  0: MAD OUT[0], CONST[0], CONST[1], CONST[2]
  1: END

and a table for the new mapping is created:

0 -> 3
1 -> 550
2 -> 1300

which is used when uploading constants. This optimization is disabled if there is indirect addressing to the constant file.

I think the optimization was written before GLSL2 was merged. It can happen at a higher level (it may be even more efficient there), but the GLSL infrastructure must be able to break arrays into variables and still accept arrays in glUniform as if nothing happened.

IMO, MAX_xxx_UNIFORMS should be checked against the number of active uniforms, and not all those declared. Otherwise it would be limiting beyond the capabilities of the cards. And if the r300 compiler optimized that uniform away, it means it effectively is not active, even if the GLSL linker thinks so.

So, IMO, either the GLSL compiler does the optimization, or it grows the ability to query how many uniforms are actually active in the driver.

But hopefully it shouldn't be hard to make the optimization in the GLSL, and therefore avoid backward notificiation calls.

(In reply to comment #7)
> But hopefully it shouldn't be hard to make the optimization in the GLSL, and
> therefore avoid backward notificiation calls.

If only I had time to do this.

Well, the driver can also lie to the state tracker and to applications that it supports more uniform components than it really does. That would be worse than what we had before, but it would still be by an order of magniture better than what we have now (that is, EVE Online would work).

(In reply to comment #5)
> The r300 compiler looks for the constants which are used and allocates new
> locations for them.

I think we could achieve most this pretty easily in the linker.  After linking and before uniform location assignment, we'd need a pass that:

 - Records all constant-index accesses to each array.

 - Records whether or not there was a non-constant-idnex access to each array.

When assigning uniform locations, arrays that had only constant-index accesses would have only those accesses count against the uniform components limit.  During code generation the map of accessed elements could be used to automatically compact the array and remap indexes.

I just posted a patch to the mesa-dev mailing list that may fix this issue:

http://lists.freedesktop.org/archives/mesa-dev/2011-November/014913.html

(In reply to comment #10)
> I just posted a patch to the mesa-dev mailing list that may fix this issue:
> 
> http://lists.freedesktop.org/archives/mesa-dev/2011-November/014913.html

With your patch everything is back to normal.

I think then its ok to mark is at fixed.

Fixed by the following commit in Mesa master:

commit 151867b422d07b9e5845e95c2ebc30567809edc5
Author: Ian Romanick <ian.d.romanick@intel.com>
Date:   Mon Nov 21 11:42:37 2011 -0800

    linker: Remove erroneous multiply by 4 in uniform usage calculation
    
    The old count_uniform_size::num_shader_uniforms was actually
    calculating the number of components used.  Multiplying by 4 when
    setting gl_shader::num_uniform_components caused us to count 4x as
    many uniform components as were actually used.
    
    Signed-off-by: Ian Romanick <ian.d.romanick@intel.com>
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=42930
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=42966
    Acked-by: Marek Olšák <maraeo@gmail.com>
    Tested-by: Vinson Lee <vlee@vmware.com>
    Tested-by: Pavel Ondračka <pavel.ondracka@email.cz>
    Reviewed-and-tested-by: Kenneth Graunke <kenneth@whitecape.org>