https://bugs.freedesktop.org/show_bug.cgi?id=29137

Sven Arvidsson sa@whiz.se changed:

What |Removed |Added ---------------------------------------------------------------------------- CC| |sa@whiz.se

--- Comment #1 from Sven Arvidsson sa@whiz.se 2010-07-18 05:58:01 PDT --- Another good test case for this is 3DMark2001: http://www.futuremark.com/download/3dmark2001/

I think most of the tests are affected, but its's most noticeable (missing objects) in Dragothic (High Detail) and Nature.

Both of these seems to be working okay if software rendering is forced.

https://bugs.freedesktop.org/show_bug.cgi?id=29137

--- Comment #3 from Pavel Ondračka drakkk@centrum.cz 2010-08-02 06:41:10 PDT --- (In reply to comment #2)

As a quick workaround you could modify Wine to use a slightly smaller array in shader_generate_glsl_declarations(), but we can't do that in general in Wine, because some applications depend on being able to use the higher indices, even if the total number of constants they use is nowhere near the limit.

I've had a look at shader_generate_glsl_declarations() function, however it is quite complicated and I don't have any clue how to do this. I don't even know if we are talking about some one liner patch or complete rewrite of this function :-( Henri, if this is just a simple change would you be so kind to write this patch for me, please? I don't know if this issue is going to be fixed in the r300g driver any time soon.

https://bugs.freedesktop.org/show_bug.cgi?id=29137

--- Comment #5 from Pavel Ondračka drakkk@centrum.cz 2010-08-02 07:31:59 PDT --- (In reply to comment #4)

Created an attachment (id=37525)

View: https://bugs.freedesktop.org/attachment.cgi?id=37525 Review: https://bugs.freedesktop.org/review?bug=29137&attachment=37525

wined3d hack

Something along the lines of the attached patch. You don't necessarily need the changes in shader.c, but they avoid a similar problem.

Thank you with this hack Sins of a Solar Empire is working fine.

https://bugs.freedesktop.org/show_bug.cgi?id=29137

--- Comment #7 from Pavel Ondračka drakkk@centrum.cz 2010-08-02 07:57:10 PDT --- One last question. Is it OK to report bugs spotted with this hack applied?

https://bugs.freedesktop.org/show_bug.cgi?id=29137

--- Comment #9 from Sven Arvidsson sa@whiz.se 2010-08-02 11:20:35 PDT --- (In reply to comment #6)

It won't certainly get fixed in r300g, we would have to reorder constants every draw call, which would incur noticable perfomance drop (in Gallium, constant buffers may be reused for several different shaders).

It might get fixed in the GLSL compiler, though, which resides in Mesa core. I'd rather wait until Intel merge their new GLSL compiler before looking into this issue.

Dunno if it's of any help (it being both different hardware and not Gallium) but the i965 driver works quite well with Wine+GLSL.

https://bugs.freedesktop.org/show_bug.cgi?id=29137

--- Comment #11 from Pavel Ondračka drakkk@centrum.cz 2010-08-20 16:07:31 PDT --- (In reply to comment #10)

There is a new GLSL compiler in Mesa. Does it fix your issue?

No it doesn't, there is no visible change.

https://bugs.freedesktop.org/show_bug.cgi?id=29137

--- Comment #13 from Henri Verbeet hverbeet@gmail.com 2010-08-21 06:02:33 PDT --- (In reply to comment #12)

Henri,

I was thinking about this issue and realized that the way you allocate constant space in wine is wrong. The problem is you create one large array "uniform vec4 VC[256]". If this array is addressed indirectly, that is using e.g. VC[index+20] where "index" cannot be evaluated at compile time, we must assume *any* element in the array may potentially be used and therefore we cannot eliminate unused elements, because there appears to be none.

Yes, but that's because of the way d3d9 works. I.e., you just have one big block of 256 float constants (in SM2 and SM3, SM1 only requires 96). Direct3D 10 and 11 constant buffers are much nicer in that regard. The problem is that wined3d needs a couple of extra constants for its own fixups, and often the driver needs a couple as well.

For shaders where no relative addressing is used, wined3d could declare separate uniforms instead of one big array. We'll probably make that change in the near/medium future, unless changes in Mesa make it a non-issue for us. I nevertheless think it would be a valid enhancement to the Mesa GLSL compiler to make that observation on its own.

We can do the optimization if there is no indirect addressing, but we cannot do this for all cases and say "it's fixed once and for all".

Similarly, wined3d can't split up the d3d9 constant array when relative addressing is used. This is not just a problem with Mesa for us, it affects e.g. fglrx and pre-GF8 nvidia hardware as well. The reason this isn't much of an issue on GF8+ nvidia hardware is that it simply reports 4096 uniforms as being available. Extensions like NV_shader_buffer_load suggest GF8+ hardware can simply access arbitrary buffer objects / video memory from a shader.

The workaround we use when relative addressing is used by a shader is to simply subtract our internal constants, in addition to our (most likely flawed) estimate of what the driver itself needs, from the number of constants available to the application. This breaks down when applications allocate from the end of the range of available constants. I.e., using c[255] while at the same time using relative addressing for e.g. vertex blending. http://bugs.winehq.org/show_bug.cgi?id=17818 is an example of a bug caused by this.

In short, I don't expect Mesa to be able to do much, if anything, to help solve this. There may be some things we can try on the Wine side though. For example, on hardware the supports immediate constants like r600 we could take advantage of that and just hardcode our fixups into the shaders, and have multiple versions of the shader instead. These fixups are things like compensating for differences in pixel center between d3d9 and GL, or texture origin when rendering to an FBO, which at least some drivers will have to do themselves anyway, but the other way around. If we get lucky they may even cancel out and get optimized away. The y-flip when rendering to FBO is especially sad, because it ends up being a multiplication by -1.0, which would often be free as a negate. The other way out is that while we can't determine which uniforms are read by the shader, we can determine which ones are set by the application. Constants that are never set would be our best bet for stuffing our own fixups in, although that strategy depends on the application really treating those as undefined, rather than relying on them being set to e.g. 0.0.

https://bugs.freedesktop.org/show_bug.cgi?id=29137

--- Comment #15 from Marek Olšák maraeo@gmail.com 2010-08-22 17:49:34 PDT --- I was discussing this with other devs and the conclusion is that this will not be optimized in Mesa's GLSL compiler because if you use at least one element of an array, the entire array becomes an "active" uniform and glUniform must then succeed for all (even unused) elements. Therefore all elements must be allocated in the constant register file after the compilation.

I am working on elimination of unused constants in the r300 compiler. I am already finished with vertex shaders, but still, if a shader uses relative addressing, there is nothing I can do. There is no way to make the shader compile successfully in this case.

As far as I remember, a D3D9 shader should declare just the constants it uses and/or ranges of used constants, not the entire constant space available. I don't remember the exact syntax of D3D9 assembly, but I doubt the HLSL compiler declares a sequence of definitions similar to this in an asm shader:

def c0 ... def c255

Instead, I think it just contains the definitions of constants the shader actually needs, and Wine should search for a max index and use that (+1) instead of 256. If one shader declares the 255th constant, let's make only this shader declare the array of 256 elements, but there is no need to declare it so large for every other shader out there.

https://bugs.freedesktop.org/show_bug.cgi?id=29137

--- Comment #17 from Marek Olšák maraeo@gmail.com 2010-08-24 18:07:05 PDT --- So I've implemented elimination of unused constants (and remapping their indices to fit in the constant file in hw, and other compiler improvements) in r300g. Wine support should be much better and indeed more Wine/D3D9 tests pass now.

If some shaders still fail due to these reasons: - relative addressing (indexing) of constants while using a too large uniform array - exceeding the maximum number of constants by other means

There is nothing I can do about it.

Please test.

https://bugs.freedesktop.org/show_bug.cgi?id=29137

--- Comment #19 from Rubén Fernández rubenf3000@gmail.com 2010-08-25 02:55:19 PDT --- Seconded. I have tested it with all the wine stuff I have. wine+GLSL behaved exactly like it does without GLSL in almost all cases, and even in those it didn't, it didn't complain about constant buffers or indexes. I consider this bug resolved, too. Excellent work, Marek!