Bug 29137

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.

(In reply to comment #0)
> When running Sins of a Solar Empire under Wine all 3D objects are black, the
> only thing which is rendered correctly are menus and control panels.
> Main problem here is this massage: r300: Max size of the constant buffer is
> 256*4 floats.
> 
For a part that's our (Wine's) fault for depending on the driver to properly notice not all of those constants are actually used. On the other hand, trying to avoid this in Wine itself would be somewhat complicated. We mainly depend on the driver to notice that as long as you don't use non-const indices into an uniform array, it's really just a set of independent constants, of which you can eliminate the unused ones.

> Since the game was working fine with this hardware in Windows (and my RV530 is
> well above minimum requirements) I suspected this to be a Wine issue and I
> asked at their forums, sadly with little response. Finally soreau at #radeon
> said this in theory can be fixed in driver and advised to open a bug here.
> 
Yeah, not all Wine developers read the forums, and I'd only expect someone fairly familiar with the wined3d code to be able to say something useful on the subject.

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.

(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.

Created attachment 37525 [details] [review]
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.

(In reply to comment #4)
> Created an attachment (id=37525) [details]
> 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.

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.

One last question. Is it OK to report bugs spotted with this hack applied?

(In reply to comment #7)
> One last question. Is it OK to report bugs spotted with this hack applied?

The patch is fairly safe, but in general you shouldn't report (Wine) bugs with modified source. The other consideration is that at least currently there's a reasonably high chance that any (D3D/OpenGL) bugs you'll run into will be in the driver instead of Wine. I'm certainly willing to help if there's anything I can do to help track those down, but Wine bugzilla isn't the place for it.

(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.

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

(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.

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.

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".

(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.

*** Bug 29741 has been marked as a duplicate of this bug. ***

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.

If it is of any help, aplyint that wined3d hack makes all the wine applications I have tested render correctly except for two games:

One of them still complains about "r300: Max size of the constant buffer is 256*4 floats."

The other fails when a shader attempts to access the elements #30 and #31 in a 29-element array; presumably, without the patch, said array would be 32 elements long.

As far as I can see, this bug is the only thing preventing wine+glsl to work perfectly with mesa+r300g.

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.

My both test cases (SoaSE and SC2) don't have this problem any more and no regressions encountered so far. Thank you Marek. Closing.

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!