Hi,
Here's another problem with shaders.
Vertex shader input is marked up with tags that hint at the data usage - semantics. For the fixed pipeline, those tags must be 100% correct, because they tell the pipeline how to interpret the data - is that the position stream, or the color stream, or the texcoord stream. For shaders, however, I think this is not the case. Since the app has control of the shader, it doesn't need to give meanings to the different semantics.
I've looked at two demos, which demonstrate various problems. Demo #1: Instancing: http://www.humus.ca/index.php?page=3D&ID=52 This demo has hlsl shaders, which make use of the TEXCOORD8 semantic. This is an invalid semantic, according to the way we map things, since only 0-7 texcoords are supported. Furthermore, comments make it clear that this is not texture data. It would seem that the semantic is just meant to relay the data to the vertex shader input that is marked dcl_texcoord8. This means that we need to support all semantic names and indices 0-15 as possible labels for shader input data. MSDN even mentions that texcoord can be used for user-defined data. This presentation: http://www.ati.com/developer/gdc/D3DTutorial1_Shaders.pdf , has examples which make use of position > 0, normal > 0, etc, which are all invalid on the fixed function pipeline.
I am currently working on a very large patch, which will restructure strided data to address this problem, but that's not all.
Demo #2: Many Per Pixel Lights, http://www.zanir.szm.sk/dx/017_Many_Per_Pixel_Lights.zip This is a d3d8 demo. The shader inputs are not marked with a semantic - the declaration data is mapped 1:1 to the shader inputs, so a specific register number is designated as the D3DVSDE_DIFFUSE register. Now, consider that we use the semantic to implement shader fixup - flipping red and blue on color inputs. Previously this fixup did not work at all on d3d8 shaders (as far as I can tell), and I just made it work today, by storing a fake semantic for d3d8 shaders. The result is that in the demo above, everything turned green, and very wrong. Why? Looking at the demo it seems that it loads registers that contain user-defined data in all the declaration inputs, without paying any attention to the "meaning" of that register, since it controls the shader. As a result, we are flipping random data, that could be important, and not red and blue. In this particular case, it's a relative address token, which means that's absolutely critical that it gets the right value - that's why everything breaks.
Therefore, how can we rely on a semantic tag for shader fixups? Seems we can't do that. Also, I don't understand why we're applying fixups on shader input either. Can someone explain why this fixup is needed exactly? If we need to flip red and blue because the format is backwards, shouldn't this be done at the end of the pipeline, at the point of interpretation by GL. Flipping things at an intermediate point can affect all kinds of calculations in the shader. At the end of the pipeline we can also reliably tell what's to be interpreted as color data, instead of following semantic hints.
Any thoughts?
On 03/07/06, Ivan Gyurdiev ivg231@gmail.com wrote:
Demo #2: Many Per Pixel Lights, http://www.zanir.szm.sk/dx/017_Many_Per_Pixel_Lights.zip This is a d3d8 demo. The shader inputs are not marked with a semantic - the declaration data is mapped 1:1 to the shader inputs, so a specific register number is designated as the D3DVSDE_DIFFUSE register. Now, consider that we use the semantic to implement shader fixup - flipping red and blue on color inputs. Previously this fixup did not work at all on d3d8 shaders (as far as I can tell),
And it shouldn't. As you said, d3d8 shaders don't have a usage / semantic defined per register.
and I just made it work today, by storing a fake semantic for d3d8 shaders.
You can't do that. In d3d8 there's no way to figure out how a register is going to be used. For d3d8 the only way to determine if we need fixups is to look at the register's data type. That's exactly what the patch / hack I sent you a while ago did.
Therefore, how can we rely on a semantic tag for shader fixups? Seems we can't do that.
For d3d8 we can't, but for d3d8 the register shouldn't *have* that tag in the first place, since there's no way we can make up a correct one. The dolphin demo in the d3d8 sdk is another demo that breaks if you do this. (It stores texture coordinates in register 6, which happens to correspond to D3DVSDE_SPECULAR).
Also, I don't understand why we're applying fixups on shader input either. Can someone explain why this fixup is needed exactly? If we need to flip red and blue because the format is backwards, shouldn't this be done at the end of the pipeline, at the point of interpretation by GL. Flipping things at an intermediate point can affect all kinds of calculations in the shader. At the end of the pipeline we can also reliably tell what's to be interpreted as color data, instead of following semantic hints.
It's not an intermediate point, but rather how the data is visible inside the d3d shader. We don't look at the usage / semantic of the register as much as we look at the data type that usage implies (for d3d9 that is, d3d8 has an explicit data types and no usage). For the "color" datatype, the blue component of the color is the first component, x. We actually need to do it on input *because* of the calculations you talk about. Consider this: How would the data look if we did the fixups on the input data before sending it to the shader?
H. Verbeet wrote:
On 03/07/06, Ivan Gyurdiev ivg231@gmail.com wrote:
Demo #2: Many Per Pixel Lights, http://www.zanir.szm.sk/dx/017_Many_Per_Pixel_Lights.zip This is a d3d8 demo. The shader inputs are not marked with a semantic - the declaration data is mapped 1:1 to the shader inputs, so a specific register number is designated as the D3DVSDE_DIFFUSE register. Now, consider that we use the semantic to implement shader fixup - flipping red and blue on color inputs. Previously this fixup did not work at all on d3d8 shaders (as far as I can tell),
And it shouldn't. As you said, d3d8 shaders don't have a usage / semantic defined per register.
So, are the names in the D3DVSD_REGISTER enumeration meaningless? Why is D3DVSDE_POSITION called that way?
and I just made it work today, by storing a fake semantic for d3d8 shaders.
You can't do that. In d3d8 there's no way to figure out how a register is going to be used. For d3d8 the only way to determine if we need fixups is to look at the register's data type. That's exactly what the patch / hack I sent you a while ago did.
If that works for d3d8, why doesn't it work for d3d9? D3D9 have types as well on each semantic, and D3DCOLOR is one of the types.
Also, I don't understand why we're applying fixups on shader input either. Can someone explain why this fixup is needed exactly? If we need to flip red and blue because the format is backwards, shouldn't this be done at the end of the pipeline, at the point of interpretation by GL. Flipping things at an intermediate point can affect all kinds of calculations in the shader. At the end of the pipeline we can also reliably tell what's to be interpreted as color data, instead of following semantic hints.
It's not an intermediate point, but rather how the data is visible inside the d3d shader. We don't look at the usage / semantic of the register as much as we look at the data type that usage implies (for d3d9 that is, d3d8 has an explicit data types and no usage). For the "color" datatype, the blue component of the color is the first component, x. We actually need to do it on input *because* of the calculations you talk about. Consider this: How would the data look if we did the fixups on the input data before sending it to the shader?
I see - I must have misunderstood that we need the fixup to be applied because of the way OpenGL expects the color data after the shader is executed. You're saying that's just the standard way to assign a D3DCOLOR to x,y,z,w components...
On 03/07/06, Ivan Gyurdiev ivg231@gmail.com wrote:
and I just made it work today, by storing a fake semantic for d3d8 shaders.
By the way, I am still keeping that fake semantic around - it is useful
- it allows me to have a single code path for loading arrays, instead of
the two we currently have, which is a mess..
Well, it is wrong. However, as long as we've got some way of detecting that it's a fake, I suppose it might still work. Keep in mind though that the d3d8 path was added specifically to fix the problem you described earlier :-).
H. Verbeet wrote:
On 03/07/06, Ivan Gyurdiev ivg231@gmail.com wrote:
and I just made it work today, by storing a fake semantic for d3d8 shaders.
By the way, I am still keeping that fake semantic around - it is useful
- it allows me to have a single code path for loading arrays, instead of
the two we currently have, which is a mess..
Well, it is wrong.
How is it wrong? In one case you have a semantic, and you use that to load up the correct register (query shader, shader gives you the index to load). In the other case you preinitialize the semantics with the declaration register numbers - it's all the same thing really. I know this works, because I've already written the code.
Now, for the color fix, I agree with you - that part needs to be different for d3d8 vs d3d9 - that's the part that didn't work. Changes I've made also put the decision for whether something is color or not in its own function, instead of copying that for each backend. We just need to edit the function accordingly.
Keep in mind though that the d3d8 path was added specifically to fix the problem you described earlier :-).
I remember a bunch of dead code that looked like it was trying to write to arrayUsageMap, but wasn't actually writing anything :)
On 03/07/06, Ivan Gyurdiev ivg231@gmail.com wrote:
Well, it is wrong.
How is it wrong? In one case you have a semantic, and you use that to load up the correct register (query shader, shader gives you the index to load). In the other case you preinitialize the semantics with the declaration register numbers - it's all the same thing really. I know this works, because I've already written the code.
In the sense that you're making up a semantic based on the register number, but not necessarily the correct one. But perhaps if you take the data type into account when making up the semantic it might still work.
Keep in mind though that the d3d8 path was added specifically to fix the problem you described earlier :-).
I remember a bunch of dead code that looked like it was trying to write to arrayUsageMap, but wasn't actually writing anything :)
Maybe you're talking about a different piece of code. What I was talking about was http://source.winehq.org/source/dlls/wined3d/drawprim.c#L404
H. Verbeet wrote:
On 03/07/06, Ivan Gyurdiev ivg231@gmail.com wrote:
Well, it is wrong.
How is it wrong? In one case you have a semantic, and you use that to load up the correct register (query shader, shader gives you the index to load). In the other case you preinitialize the semantics with the declaration register numbers - it's all the same thing really. I know this works, because I've already written the code.
In the sense that you're making up a semantic based on the register number, but not necessarily the correct one.
That's not particularly important. The shader semantics aren't correct either - I see apps putting all kinds of data in TEXCOORD semantics that have nothing to do with textures. When I tried to run that through drawStridedSlow for software shaders it wouldn't work properly, since drawStridedSlow was interpreting the data and making sure a matching texture is found.
What's important is how you use the semantic to map to vshader inputs - and that will work correctly. It also goes in its own function (vshader_get_input(usage, usage_index) -> regnum (,writemask?) ), so no more accessing semantics map from within drawprim directly.
But perhaps if you take the data type into account when making up the semantic it might still work.
Why would I do that - just adds extra complexity that's not necessary, and doesn't really make sense - you can't tell if a FLOAT3 means normal or POSITION, or something else the app just made up and the shader knows how to interpret.
Keep in mind though that the d3d8 path was added specifically to fix the problem you described earlier :-).
I remember a bunch of dead code that looked like it was trying to write to arrayUsageMap, but wasn't actually writing anything :)
Maybe you're talking about a different piece of code.
Not really, just another piece of the same code that I removed earlier.
What I was talking about was http://source.winehq.org/source/dlls/wined3d/drawprim.c#L404
That looks like this in my tree:
if (useVertexShaderFunction && (data || streamVBO) ) {
IWineD3DVertexShader* vshader = This->stateBlock->vertexShader;
unsigned int idx; BOOL stride_used = vshader_get_input(vshader, element->Usage, element->UsageIndex, &idx); if (stride_used) { TRACE("Loaded shader array %u [usage=%s, usage_idx=%u, " "stream=%u, offset=%u, stride=%lu, VBO=%u]\n", idx, debug_d3ddeclusage(element->Usage), element->UsageIndex, element->Stream, element->Offset, stride, streamVBO);
strided[idx].lpData = data; strided[idx].dwType = element->Type; strided[idx].dwStride = stride; strided[idx].VBO = streamVBO; strided[idx].usage = element->Usage; strided[idx].usage_index = element->UsageIndex; } }
else { TRACE("Loaded fixed function array %u [usage=%s, usage_idx=%u, " "stream=%u, offset=%u, stride=%lu, VBO=%u]\n", fixed_idx, debug_d3ddeclusage(element->Usage), element->UsageIndex, element->Stream, element->Offset, stride, streamVBO);
strided[fixed_idx].lpData = data; strided[fixed_idx].dwType = element->Type; strided[fixed_idx].dwStride = stride; strided[fixed_idx].VBO = streamVBO; strided[fixed_idx].usage = element->Usage; strided[fixed_idx].usage_index = element->UsageIndex; fixed_idx++; }
then loadNumberedArrays is:
for (i = 0; i < nstreams; i++) {
if (strided[i].lpData == NULL) continue;
TRACE_(d3d_shader)("Loading array %u [usage=%s, usage_idx=%u, VBO=%u]\n", i, debug_d3ddeclusage(strided[i].usage), strided[i].usage_index, strided[i].VBO); if(curVBO != strided[i].VBO) { GL_EXTCALL(glBindBufferARB(GL_ARRAY_BUFFER_ARB, strided[i].VBO)); checkGLcall("glBindBufferARB"); curVBO = strided[i].VBO; } GL_EXTCALL(glVertexAttribPointerARB(i, WINED3D_ATR_SIZE(strided[i].dwType), WINED3D_ATR_GLTYPE(strided[i].dwType), WINED3D_ATR_NORMALIZED(strided[i].dwType), strided[i].dwStride, strided[i].lpData)); GL_EXTCALL(glEnableVertexAttribArrayARB(i)); }
On 03/07/06, Ivan Gyurdiev ivg231@gmail.com wrote:
That's not particularly important.
Well, it breaks color fixups :-) Something that has texcoord semantics doesn't necessarily have to contain texture coordinates, but it should contain something that has the same semantics.
But perhaps if you take the data type into account when making up the semantic it might still work.
Why would I do that - just adds extra complexity that's not necessary, and doesn't really make sense - you can't tell if a FLOAT3 means normal or POSITION, or something else the app just made up and the shader knows how to interpret.
Well, no, but you can probably tell that something that has a color datatype should have color semantics. But if we do fixups based on the datatype it's not all that relevant anymore, of course.
H. Verbeet wrote:
On 03/07/06, Ivan Gyurdiev ivg231@gmail.com wrote:
That's not particularly important.
Well, it breaks color fixups :-)
...color fixups will go in their own function that does this differently for 8 vs 9
BOOL vshader_input_is_color( IWineD3DVertexShader* iface, unsigned int regnum);
Something that has texcoord semantics doesn't necessarily have to contain texture coordinates, but it should contain something that has the same semantics.
Now you're talking semantics, and have me totally confused :)
"semantics" is just a word for some kind of handling you apply. In our case the handling will be a key relationship to the vshader input table.
Color fixups will be handled differently in any case, and if we can have a shared point for loading arrays then I think we should take advantage of that. I really thing more reusability is needed - just because something works slightly different does not mean it should be completely rewritten - that's the approach that everyone seems to be taking. Otherwise, how do you explain striking similarity between drawStridedSlow, the (disabled) drawSoftwareVS thing, process_vertices in device.c, fixups in vertexbuffer.... etc [ all of which have to be changed now to deal with different strided data format ]
On 03/07/06, Ivan Gyurdiev ivg231@gmail.com wrote:
So, are the names in the D3DVSD_REGISTER enumeration meaningless? From the shader's point of view, yes.
Why is D3DVSDE_POSITION called that way?
IIRC the names somehow correspond to the way inputs are mapped for fixed function, but I'm not too sure on the details.
If that works for d3d8, why doesn't it work for d3d9? D3D9 have types as well on each semantic, and D3DCOLOR is one of the types.
It probably would. At the time, the arrayUsageMap was more convenient to use though.
I see - I must have misunderstood that we need the fixup to be applied because of the way OpenGL expects the color data after the shader is executed.
It's because of the way OpenGL (or rather D3D :-)) expects the data on input to the vertex processing stage. Because vertex shaders replace the fixed function vertex processing stage, we can change the way it expects the data rather than fixing up the data itself.
You're saying that's just the standard way to assign a D3DCOLOR to x,y,z,w components...
Yes. Also see MSDN for D3DVSDT_D3DCOLOR vs D3DVSDT_UBYTE4.
-
H. Verbeet -
Ivan Gyurdiev