Bug 42930 - [r300g, bisected] EVE online only shows black screen
Summary: [r300g, bisected] EVE online only shows black screen
Status: RESOLVED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: glsl-compiler (show other bugs)
Version: git
Hardware: All All
: medium normal
Assignee: mesa-dev
QA Contact:
URL: http://pavel.ondracka.cz/EVE.trace
Whiteboard:
Keywords: regression
Depends on:
Blocks: 42514
  Show dependency treegraph
 
Reported: 2011-11-14 12:47 UTC by Pavel Ondračka
Modified: 2011-11-22 11:57 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
terminal output (469.23 KB, text/plain)
2011-11-14 12:47 UTC, Pavel Ondračka
Details

Description Pavel Ondračka 2011-11-14 12:47:42 UTC
Created attachment 53558 [details]
terminal output

When starting EVE online there is only black screen, before there were some models misrendered (bug 42514) but otherwise it was working fine.

92f81590456103977988ee704d2e6119ca1b989d is the first bad commit
commit 92f81590456103977988ee704d2e6119ca1b989d
Author: Ian Romanick <ian.d.romanick@intel.com>
Date:   Tue Nov 8 12:37:19 2011 -0800

    linker: Validate resource usage in the linker
    
    This is also done in ir_to_mesa and st_glsl_to_tgsi, but that code
    will be removed soon.
    
    Signed-off-by: Ian Romanick <ian.d.romanick@intel.com>
    Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>


Terminal log attached, it was really huge so I took only first 5000 lines. BTW there is a EVE trace uploaded at: http://pavel.ondracka.cz/EVE.trace

mesa: fa704cc558ab321792b364dab43f1e960513bed0
Kernel: 3.0.0
GPU: RV530
Comment 1 Ian Romanick 2011-11-14 18:03:29 UTC
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]);
Comment 2 Pavel Ondračka 2011-11-15 04:53:16 UTC
(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
Comment 3 Marek Olšák 2011-11-15 05:20:10 UTC
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.
Comment 4 Ian Romanick 2011-11-15 10:49:37 UTC
(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?
Comment 5 Marek Olšák 2011-11-15 11:19:51 UTC
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.
Comment 6 Marek Olšák 2011-11-15 11:28:55 UTC
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.
Comment 7 Jose Fonseca 2011-11-15 11:29:15 UTC
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.
Comment 8 Marek Olšák 2011-11-15 11:38:47 UTC
(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).
Comment 9 Ian Romanick 2011-11-15 11:54:48 UTC
(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.
Comment 10 Ian Romanick 2011-11-21 11:54:03 UTC
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
Comment 11 Pavel Ondračka 2011-11-21 14:02:14 UTC
(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.
Comment 12 Niels P. 2011-11-22 11:47:32 UTC
I think then its ok to mark is at fixed.
Comment 13 Ian Romanick 2011-11-22 11:57:27 UTC
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>


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.