bad HLSL register mapping with structs (webgl uniform-location.html failure)

[GoogleCodeExporter]

(Reproduced with git commit 2e660d5627c8ef47daa)

This reproduces with structUniformShader.vert, but only with a hash function 
that results in a certain sort order, or with making that ordering explicit.  
I've attached a copy of structUniformShader.vert that renamed "u_struct" with 
"u_bstruct" to get it to sort in the right place to reproduce the bug.

With the original structUniformShader.vert, the generated output (essl_to_hlsl 
-b=h9) includes:

uniform float4 dx_ViewAdjust : register(c1);
uniform float _u_array[4] : register(c2);
uniform float4x4 _u_modelViewProjMatrix : register(c6);
uniform _MyStruct _u_struct : register(c10);

If u_struct is renamed to u_bstruct (so that it comes between u_array and 
u_modelViewProjMatrix), the output is:

uniform float4 dx_ViewAdjust : register(c1);
uniform float _u_array[4] : register(c2);
uniform _MyStruct _u_bstruct : register(c6);
uniform float4x4 _u_modelViewProjMatrix : register(c7);

Compiling this results in:

Error X4500: Overlapping register semantics not yet implemented 'c7'.  
Bsaically, _MyStruct seems to need two registers.  Changing c7 to c8 compiles.

Original issue reported on code.google.com by vladim...@gmail.com on 20 May 2014 at 5:34

• Blocking: #568, #505

Attachments:

• structUniformShader.vert

[GoogleCodeExporter]

Fumbling through this --

Std140BlockEncoder::advanceOffset just advances by 
gl::UniformComponentCount(type), which is 1 for GL_INT.

So this struct will advance the offset by 2 (one for each member of the 
struct), likely because the GL packing rules would have these structs packed.  
But it looks like HLSL struct members are not packed, and each one needs to eat 
up a full register in HLSL.

Original comment by vladim...@gmail.com on 20 May 2014 at 5:53

[GoogleCodeExporter]

looking now. if it's true ints are not packed you are correct, however I 
couldn't repro this with a sample application using the above shader, trying 
with essl_to_hlsl.

Original comment by jmad...@chromium.org on 20 May 2014 at 5:54

• Changed state: Started

[GoogleCodeExporter]

I think I see the problem -- it may be a difference in behaviour from D3D9 to 
D3D11. If you feel like finding a solution, feel free to make a patch, 
otherwise I'll look at it when I have time.

Original comment by jmad...@chromium.org on 20 May 2014 at 5:59

• Changed state: Accepted

[GoogleCodeExporter]

I just put up https://chromium-review.googlesource.com/200620 which fixes this 
problem, but maybe it's not the right kind of fix.

Original comment by vladim...@gmail.com on 20 May 2014 at 6:02

[GoogleCodeExporter]

Hmm, in 
http://msdn.microsoft.com/en-us/library/windows/desktop/bb509581%28v=vs.85%29.as
px :

> Differences between Direct3D 9 and Direct3D 10 and 11:
> 
> Unlike the auto-allocation of constants in Direct3D 9, which did not perform 
packing and instead assigned each variable to a set of float4 registers, HLSL 
constant variables follow packing rules in Direct3D 10 and 11.

Which I think does indeed say that D3D10/11 do packing, but D3D9 is not packed 
and everything is at least one float4.

Original comment by vladim...@gmail.com on 20 May 2014 at 6:11

[GoogleCodeExporter]

Original comment by jmad...@chromium.org on 20 May 2014 at 7:01

• Now blocking: #568

[GoogleCodeExporter]

This is actually a pretty bad breakage for D3D9. Shannon, we should fix this 
before the branch. I think this is also a good example of where having more 
testing for the D3D9/ES2 legacy path would be helpful.

Original comment by jmad...@chromium.org on 20 May 2014 at 7:02

[GoogleCodeExporter]

Before which branch? M36 branched more than a week ago, and if we need the fix 
there, it will need a merge, and should be addressed immediately. If it's an 
issue with the new master branch only, then we need just to avoid rolling 
Chrome until after a fix lands in ANGLE.

Original comment by shannonw...@chromium.org on 20 May 2014 at 7:10

[GoogleCodeExporter]

Sorry I confused you with my terminology -- before we roll the new ANGLE maser 
branch to Chrome.

Original comment by jmad...@chromium.org on 20 May 2014 at 7:11

[GoogleCodeExporter]

Thanks for clarification-- panic averted!

Original comment by shannonw...@chromium.org on 20 May 2014 at 7:13

• Added labels: Branch-Master, Roll-es3proto

[GoogleCodeExporter]

Original comment by jmad...@chromium.org on 26 May 2014 at 2:37

• Now blocking: #505

[GoogleCodeExporter]

CL committed: https://chromium-review.googlesource.com/#/c/200620/

Original comment by jmad...@chromium.org on 29 May 2014 at 9:00

• Changed state: Fixed