2013/6/11 Christian Costa titan.costa@gmail.com:
Fixes bug 22682.
dlls/d3dx9_36/d3dx9_36.spec | 2 +- dlls/d3dx9_36/shader.c | 50 ++++++++++++++++++++++++++++++++++++++ dlls/d3dx9_36/tests/shader.c | 55 ++++++++++++++++++++++++++++++++++++++++++ include/d3dx9shader.h | 1 + 4 files changed, 107 insertions(+), 1 deletion(-)
diff --git a/dlls/d3dx9_36/d3dx9_36.spec b/dlls/d3dx9_36/d3dx9_36.spec index f2c229a..0f1387e 100644 --- a/dlls/d3dx9_36/d3dx9_36.spec +++ b/dlls/d3dx9_36/d3dx9_36.spec @@ -159,7 +159,7 @@ @ stdcall D3DXGetPixelShaderProfile(ptr) @ stdcall D3DXGetShaderConstantTable(ptr ptr) @ stdcall D3DXGetShaderConstantTableEx(ptr long ptr) -@ stub D3DXGetShaderInputSemantics(ptr ptr ptr) +@ stdcall D3DXGetShaderInputSemantics(ptr ptr ptr) @ stub D3DXGetShaderOutputSemantics(ptr ptr ptr) @ stdcall D3DXGetShaderSamplers(ptr ptr ptr) @ stdcall D3DXGetShaderSize(ptr) diff --git a/dlls/d3dx9_36/shader.c b/dlls/d3dx9_36/shader.c index 75bc9b5..3b71b4b 100644 --- a/dlls/d3dx9_36/shader.c +++ b/dlls/d3dx9_36/shader.c @@ -1,6 +1,7 @@ /*
- Copyright 2008 Luis Busquets
- Copyright 2009 Matteo Bruni
- Copyright 2010, 2013 Christian Costa
- Copyright 2011 Travis Athougies
- This library is free software; you can redistribute it and/or
@@ -1778,3 +1779,52 @@ HRESULT WINAPI D3DXGetShaderSamplers(const DWORD *byte_code, const char **sample
return D3D_OK;}
+HRESULT WINAPI D3DXGetShaderInputSemantics(const DWORD *byte_code, D3DXSEMANTIC *semantics, UINT *count) +{
- const DWORD *ptr = byte_code;
- UINT i = 0;
- TRACE("byte_code = %p, semantics = %p, count = %p\n", byte_code, semantics, count);
- if (!byte_code)
return D3DERR_INVALIDCALL;- TRACE("Shader version: %#x\n", *ptr);
- /* Look for the END token, skipping the VERSION token */
- while (*++ptr != D3DSIO_END)
I think you should be a bit more careful in skipping non-opcode DWORDs, otherwise you could e.g. potentially match with constants from DEF instructions - very unlikely to happen in practice, sure, but since it can be easily avoided, why not? Take a look at shader_skip_opcode() from wined3d/shader_sm1.c. You can probably get away without having to write a table with the parameters count for each SM1 opcode by just skipping DWORDs with the most significant bit set (see shader_skip_unrecognized() from the same file). Of course you still have to special case DEF but that should be it.
- {
/* Skip comments */if ((*ptr & D3DSI_OPCODE_MASK) == D3DSIO_COMMENT){ptr += ((*ptr & D3DSI_COMMENTSIZE_MASK) >> D3DSI_COMMENTSIZE_SHIFT);}else if ((*ptr & D3DSI_OPCODE_MASK) == D3DSIO_DCL){DWORD param1 = *++ptr;DWORD param2 = *++ptr;DWORD usage = param1 & 0x1f;DWORD usage_index = (param1 >> 16) & 0xf;DWORD reg_type = (((param2 >> 11) & 0x3) << 3) | ((param2 >> 28) & 0x7);TRACE("D3DSIO_DCL param1: %#x, param2: %#x, usage: %u, usage_index: %u, reg_type: %u\n",param1, param2, usage, usage_index, reg_type);if (reg_type == D3DSPR_INPUT){if (semantics){semantics[i].Usage = usage;semantics[i].UsageIndex = usage_index;}i++;}}- }
- if (count)
*count = i;- return D3D_OK;
+}
Have you tried to implement D3DXGetShaderOutputSemantics too? I suspect most of the code will be pretty much the same, in that case you could move the common code to a helper function and use it from both. You don't necessarily need to do this right now, just mentioning a potential follow-up.
diff --git a/dlls/d3dx9_36/tests/shader.c b/dlls/d3dx9_36/tests/shader.c index f7be174..471cd8c 100644 --- a/dlls/d3dx9_36/tests/shader.c +++ b/dlls/d3dx9_36/tests/shader.c @@ -1,5 +1,6 @@ /*
- Copyright 2008 Luis Busquets
- Copyright 2010, 2013 Christian Costa
- Copyright 2011 Travis Athougies
- This library is free software; you can redistribute it and/or
@@ -271,6 +272,22 @@ static const D3DXCONSTANT_DESC ctab_samplers_expected[] = { {"sampler2", D3DXRS_SAMPLER, 3, 1, D3DXPC_OBJECT, D3DXPT_SAMPLER3D, 1, 1, 1, 0, 4, NULL}, {"notsampler", D3DXRS_FLOAT4, 2, 1, D3DXPC_VECTOR, D3DXPT_FLOAT, 1, 4, 1, 0, 16, NULL}};
+static const DWORD semantics_vs11[] = {
- 0xfffe0101, /* vs_1_1 */
- 0x0000001f, 0x80000000, 0x900f0000, /* dcl_position0 v0 (input) */
- 0x0000001f, 0x80000005, 0x900f0001, /* dcl_texcoord0 v1 (input) */
- 0x0000001f, 0x80010005, 0x900f0002, /* dcl_texcoord1 v2 (input) */
- 0x0000001f, 0x8000000a, 0xe00f0000, /* dcl_color0 oD0 (output) */
- 0x0000ffff}; /* END */
+static const DWORD semantics_vs30[] = {
- 0xfffe0300, /* vs_3_0 */
- 0x0000001f, 0x80000000, 0x900f0000, /* dcl_position0 v0 (input) */
- 0x0000001f, 0x80000005, 0x900f0001, /* dcl_texcoord0 v1 (input) */
- 0x0000001f, 0x80010005, 0x900f0002, /* dcl_texcoord1 v2 (input) */
- 0x0000001f, 0x8000000a, 0xe00f0000, /* dcl_color0 o0 (output) */
- 0x0000ffff}; /* END */
static void test_get_shader_size(void) { UINT shader_size, expected; @@ -1900,6 +1917,43 @@ static void test_get_shader_constant_variables(void) ok(count == 0, "Release failed, got %u, expected %u\n", count, 0); }
+static void test_get_shader_semantics(void) +{
- HRESULT ret;
- D3DXSEMANTIC semantics[MAXD3DDECLLENGTH];
- UINT count;
- /* Check parameters */
- ret = D3DXGetShaderInputSemantics(NULL, semantics, &count);
- ok(ret == D3DERR_INVALIDCALL, "Returned %#x, expected %#x\n", ret, D3DERR_INVALIDCALL);
- ret = D3DXGetShaderInputSemantics(semantics_vs11, semantics, NULL);
- ok(ret == D3D_OK, "Failed with %#xn", ret);
- count = 0xdeadbeef;
- ret = D3DXGetShaderInputSemantics(semantics_vs11, NULL, &count);
- ok(ret == D3D_OK, "Failed with %#xn", ret);
- ok(count == 3, "Got %u, expected 1\n", count);
- ret = D3DXGetShaderInputSemantics(semantics_vs11, semantics, &count);
- ok(ret == D3D_OK, "Failed with %#xn", ret);
- ok(count == 3, "Got %u, expected 1\n", count);
- ok(semantics[0].Usage == D3DDECLUSAGE_POSITION, "Got %u, expected %u\n", semantics[0].Usage, D3DDECLUSAGE_POSITION);
- ok(semantics[0].UsageIndex == 0, "Got %u, expected 0\n", semantics[0].UsageIndex);
- ok(semantics[1].Usage == D3DDECLUSAGE_TEXCOORD, "Got %u, expected %u\n", semantics[1].Usage, D3DDECLUSAGE_TEXCOORD);
- ok(semantics[1].UsageIndex == 0, "Got %u, expected 0\n", semantics[0].UsageIndex);
- ok(semantics[2].Usage == D3DDECLUSAGE_TEXCOORD, "Got %u, expected %u\n", semantics[2].Usage, D3DDECLUSAGE_TEXCOORD);
- ok(semantics[2].UsageIndex == 1, "Got %u, expected 1\n", semantics[0].UsageIndex);
- ret = D3DXGetShaderInputSemantics(semantics_vs30, semantics, &count);
- ok(ret == D3D_OK, "Failed with %#xn", ret);
- ok(count == 3, "Got %u, expected 1\n", count);
- ok(semantics[0].Usage == D3DDECLUSAGE_POSITION, "Got %u, expected %u\n", semantics[0].Usage, D3DDECLUSAGE_POSITION);
- ok(semantics[0].UsageIndex == 0, "Got %u, expected 0\n", semantics[0].UsageIndex);
- ok(semantics[1].Usage == D3DDECLUSAGE_TEXCOORD, "Got %u, expected %u\n", semantics[1].Usage, D3DDECLUSAGE_TEXCOORD);
- ok(semantics[1].UsageIndex == 0, "Got %u, expected 0\n", semantics[0].UsageIndex);
- ok(semantics[2].Usage == D3DDECLUSAGE_TEXCOORD, "Got %u, expected %u\n", semantics[2].Usage, D3DDECLUSAGE_TEXCOORD);
- ok(semantics[2].UsageIndex == 1, "Got %u, expected 1\n", semantics[0].UsageIndex);
+}
START_TEST(shader) { test_get_shader_size(); @@ -1911,4 +1965,5 @@ START_TEST(shader) test_get_sampler_index(); test_get_shader_samplers(); test_get_shader_constant_variables();
- test_get_shader_semantics();
} diff --git a/include/d3dx9shader.h b/include/d3dx9shader.h index 0b30519..f5ba1c4 100644 --- a/include/d3dx9shader.h +++ b/include/d3dx9shader.h @@ -269,6 +269,7 @@ DWORD WINAPI D3DXGetShaderVersion(const DWORD *byte_code); const char * WINAPI D3DXGetVertexShaderProfile(struct IDirect3DDevice9 *device); HRESULT WINAPI D3DXFindShaderComment(CONST DWORD* byte_code, DWORD fourcc, LPCVOID* data, UINT* size); HRESULT WINAPI D3DXGetShaderSamplers(CONST DWORD *byte_code, LPCSTR *samplers, UINT *count); +HRESULT WINAPI D3DXGetShaderInputSemantics(const DWORD *byte_code, D3DXSEMANTIC *semantics, UINT *count);
HRESULT WINAPI D3DXAssembleShaderFromFileA(const char *filename, const D3DXMACRO *defines, ID3DXInclude *include, DWORD flags, ID3DXBuffer **shader, ID3DXBuffer **error_messages);
+HRESULT WINAPI D3DXGetShaderInputSemantics(const DWORD *byte_code,
D3DXSEMANTIC *semantics, UINT *count)
+{
- const DWORD *ptr = byte_code;
- UINT i = 0;
- TRACE("byte_code = %p, semantics = %p, count = %p\n", byte_code,
semantics, count);
- if (!byte_code)
return D3DERR_INVALIDCALL;- TRACE("Shader version: %#x\n", *ptr);
- /* Look for the END token, skipping the VERSION token */
- while (*++ptr != D3DSIO_END)
I think you should be a bit more careful in skipping non-opcode DWORDs, otherwise you could e.g. potentially match with constants from DEF instructions - very unlikely to happen in practice, sure, but since it can be easily avoided, why not? Take a look at shader_skip_opcode() from wined3d/shader_sm1.c. You can probably get away without having to write a table with the parameters count for each SM1 opcode by just skipping DWORDs with the most significant bit set (see shader_skip_unrecognized() from the same file). Of course you still have to special case DEF but that should be it.
I was thinking about this kind of problem but followed what D3DXGetShaderSize does. So D3DXGetShaderSize will have to be fixed as well. So if I summarize:
if (sm > 2) handle instruction length else if (comment or def instruction) special handling for them else skip DWORD with bit 31 set
Is this correct?
- {
/* Skip comments */if ((*ptr & D3DSI_OPCODE_MASK) == D3DSIO_COMMENT){ptr += ((*ptr & D3DSI_COMMENTSIZE_MASK) >>D3DSI_COMMENTSIZE_SHIFT);
}else if ((*ptr & D3DSI_OPCODE_MASK) == D3DSIO_DCL){DWORD param1 = *++ptr;DWORD param2 = *++ptr;DWORD usage = param1 & 0x1f;DWORD usage_index = (param1 >> 16) & 0xf;DWORD reg_type = (((param2 >> 11) & 0x3) << 3) | ((param2
- & 0x7);
TRACE("D3DSIO_DCL param1: %#x, param2: %#x, usage: %u,usage_index: %u, reg_type: %u\n",
param1, param2, usage, usage_index, reg_type);if (reg_type == D3DSPR_INPUT){if (semantics){semantics[i].Usage = usage;semantics[i].UsageIndex = usage_index;}i++;}}- }
- if (count)
*count = i;- return D3D_OK;
+}
Have you tried to implement D3DXGetShaderOutputSemantics too? I suspect most of the code will be pretty much the same, in that case you could move the common code to a helper function and use it from both. You don't necessarily need to do this right now, just mentioning a potential follow-up.
I tought about it too. There are indeed similar. I planned to do it later
but I will do it in this patch since I have to update it anyway.
Thanks Christian
2013/6/12 Christian Costa titan.costa@gmail.com:
+HRESULT WINAPI D3DXGetShaderInputSemantics(const DWORD *byte_code, D3DXSEMANTIC *semantics, UINT *count) +{
- const DWORD *ptr = byte_code;
- UINT i = 0;
- TRACE("byte_code = %p, semantics = %p, count = %p\n", byte_code,
semantics, count);
- if (!byte_code)
return D3DERR_INVALIDCALL;- TRACE("Shader version: %#x\n", *ptr);
- /* Look for the END token, skipping the VERSION token */
- while (*++ptr != D3DSIO_END)
I think you should be a bit more careful in skipping non-opcode DWORDs, otherwise you could e.g. potentially match with constants from DEF instructions - very unlikely to happen in practice, sure, but since it can be easily avoided, why not? Take a look at shader_skip_opcode() from wined3d/shader_sm1.c. You can probably get away without having to write a table with the parameters count for each SM1 opcode by just skipping DWORDs with the most significant bit set (see shader_skip_unrecognized() from the same file). Of course you still have to special case DEF but that should be it.
I was thinking about this kind of problem but followed what D3DXGetShaderSize does. So D3DXGetShaderSize will have to be fixed as well. So if I summarize:
if (sm > 2) handle instruction length else if (comment or def instruction) special handling for them else skip DWORD with bit 31 set
Is this correct?
SM >= 2 for the instruction length case, but apart from that it should be good.
Ideally we'd use some tests, as Henri suggested, but that's not blocking this patch I think.
- {
/* Skip comments */if ((*ptr & D3DSI_OPCODE_MASK) == D3DSIO_COMMENT){ptr += ((*ptr & D3DSI_COMMENTSIZE_MASK) >>D3DSI_COMMENTSIZE_SHIFT);
}else if ((*ptr & D3DSI_OPCODE_MASK) == D3DSIO_DCL){DWORD param1 = *++ptr;DWORD param2 = *++ptr;DWORD usage = param1 & 0x1f;DWORD usage_index = (param1 >> 16) & 0xf;DWORD reg_type = (((param2 >> 11) & 0x3) << 3) | ((param2
- & 0x7);
TRACE("D3DSIO_DCL param1: %#x, param2: %#x, usage: %u,usage_index: %u, reg_type: %u\n",
param1, param2, usage, usage_index, reg_type);if (reg_type == D3DSPR_INPUT){if (semantics){semantics[i].Usage = usage;semantics[i].UsageIndex = usage_index;}i++;}}- }
- if (count)
*count = i;- return D3D_OK;
+}
Have you tried to implement D3DXGetShaderOutputSemantics too? I suspect most of the code will be pretty much the same, in that case you could move the common code to a helper function and use it from both. You don't necessarily need to do this right now, just mentioning a potential follow-up.
I tought about it too. There are indeed similar. I planned to do it later but I will do it in this patch since I have to update it anyway.
Thanks Christian
if (sm > 2) handle instruction length else if (comment or def instruction) special handling for them else skip DWORD with bit 31 set
Is this correct?
SM >= 2 for the instruction length case, but apart from that it should be good.
Oups. Tipo. It's what I meant.
Ideally we'd use some tests, as Henri suggested, but that's not
blocking this patch I think.
I'll try to add some of them for that case.
On 12 June 2013 00:17, Matteo Bruni matteo.mystral@gmail.com wrote:
I think you should be a bit more careful in skipping non-opcode DWORDs, otherwise you could e.g. potentially match with constants from DEF instructions - very unlikely to happen in practice, sure, but since it can be easily avoided, why not? Take a look at shader_skip_opcode() from wined3d/shader_sm1.c. You can probably get away without having to write a table with the parameters count for each SM1 opcode by just skipping DWORDs with the most significant bit set (see shader_skip_unrecognized() from the same file). Of course you still have to special case DEF but that should be it.
Ideally we'd have some more test shaders in dlls/d3dx9_36/tests/shader.c that would trigger these, and also run the various other shader processing functions against those.
-
Christian Costa -
Henri Verbeet -
Matteo Bruni