[PATCH 1/4] d3dx9: Return D3DERR_INVALIDCALL from ID3DXEffect::BeginPass() if the effect is not started.
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/d3dx9_36/effect.c | 6 ++++++ dlls/d3dx9_36/tests/effect.c | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/dlls/d3dx9_36/effect.c b/dlls/d3dx9_36/effect.c index 22afa649db3..e623c10035a 100644 --- a/dlls/d3dx9_36/effect.c +++ b/dlls/d3dx9_36/effect.c @@ -4001,6 +4001,12 @@ static HRESULT WINAPI d3dx_effect_BeginPass(ID3DXEffect *iface, UINT pass) TRACE("iface %p, pass %u\n", effect, pass); + if (!effect->started) + { + WARN("Effect is not started, returning D3DERR_INVALIDCALL.\n"); + return D3DERR_INVALIDCALL; + } + if (technique && pass < technique->pass_count && !effect->active_pass) { HRESULT hr; diff --git a/dlls/d3dx9_36/tests/effect.c b/dlls/d3dx9_36/tests/effect.c index ea90299f936..d4efc1101d9 100644 --- a/dlls/d3dx9_36/tests/effect.c +++ b/dlls/d3dx9_36/tests/effect.c @@ -3081,6 +3081,12 @@ static void test_effect_states(IDirect3DDevice9 *device) NULL, NULL, 0, NULL, &effect, NULL); ok(hr == D3D_OK, "Got result %x, expected 0 (D3D_OK).\n", hr); + hr = effect->lpVtbl->End(effect); + ok(hr == D3D_OK, "Got result %#x.\n", hr); + + hr = effect->lpVtbl->BeginPass(effect, 0); + ok(hr == D3DERR_INVALIDCALL, "Got result %#x.\n", hr); + /* State affected in passes saved/restored even if no pass was performed. States not present in passes are not saved & restored */ -- 2.34.1
And strip D3DXFX_* creation flags from them. Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/d3dx9_36/effect.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/dlls/d3dx9_36/effect.c b/dlls/d3dx9_36/effect.c index e623c10035a..b8c957245a6 100644 --- a/dlls/d3dx9_36/effect.c +++ b/dlls/d3dx9_36/effect.c @@ -6393,7 +6393,7 @@ static const char **parse_skip_constants_string(char *skip_constants_string, uns static HRESULT d3dx9_effect_init(struct d3dx_effect *effect, struct IDirect3DDevice9 *device, const char *data, SIZE_T data_size, const D3D_SHADER_MACRO *defines, ID3DInclude *include, - UINT eflags, ID3DBlob **errors, struct ID3DXEffectPool *pool, const char *skip_constants_string) + UINT flags, ID3DBlob **errors, struct ID3DXEffectPool *pool, const char *skip_constants_string) { #if D3DX_SDK_VERSION <= 36 UINT compile_flags = D3DCOMPILE_ENABLE_BACKWARDS_COMPATIBILITY; @@ -6409,9 +6409,9 @@ static HRESULT d3dx9_effect_init(struct d3dx_effect *effect, struct IDirect3DDev unsigned int i, j; HRESULT hr; - TRACE("effect %p, device %p, data %p, data_size %lu, defines %p, include %p, eflags %#x, errors %p, " + TRACE("effect %p, device %p, data %p, data_size %lu, defines %p, include %p, flags %#x, errors %p, " "pool %p, skip_constants %s.\n", - effect, device, data, data_size, defines, include, eflags, errors, pool, + effect, device, data, data_size, defines, include, flags, errors, pool, debugstr_a(skip_constants_string)); effect->ID3DXEffect_iface.lpVtbl = &ID3DXEffect_Vtbl; @@ -6426,7 +6426,7 @@ static HRESULT d3dx9_effect_init(struct d3dx_effect *effect, struct IDirect3DDev IDirect3DDevice9_AddRef(device); effect->device = device; - effect->flags = eflags; + effect->flags = flags; list_init(&effect->parameter_block_list); @@ -6436,8 +6436,9 @@ static HRESULT d3dx9_effect_init(struct d3dx_effect *effect, struct IDirect3DDev if (tag != d3dx9_effect_version(9, 1)) { TRACE("HLSL ASCII effect, trying to compile it.\n"); + compile_flags |= flags & ~(D3DXFX_NOT_CLONEABLE | D3DXFX_LARGEADDRESSAWARE); hr = D3DCompile(data, data_size, NULL, defines, include, - NULL, "fx_2_0", compile_flags, eflags, &bytecode, &temp_errors); + NULL, "fx_2_0", compile_flags, 0, &bytecode, &temp_errors); if (FAILED(hr)) { WARN("Failed to compile ASCII effect.\n"); -- 2.34.1
Signed-off-by: Matteo Bruni <mbruni(a)codeweavers.com>
On Tue, Feb 8, 2022 at 12:42 AM Zebediah Figura <zfigura(a)codeweavers.com> wrote:
And strip D3DXFX_* creation flags from them.
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/d3dx9_36/effect.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/dlls/d3dx9_36/effect.c b/dlls/d3dx9_36/effect.c index e623c10035a..b8c957245a6 100644 --- a/dlls/d3dx9_36/effect.c +++ b/dlls/d3dx9_36/effect.c @@ -6393,7 +6393,7 @@ static const char **parse_skip_constants_string(char *skip_constants_string, uns
static HRESULT d3dx9_effect_init(struct d3dx_effect *effect, struct IDirect3DDevice9 *device, const char *data, SIZE_T data_size, const D3D_SHADER_MACRO *defines, ID3DInclude *include, - UINT eflags, ID3DBlob **errors, struct ID3DXEffectPool *pool, const char *skip_constants_string) + UINT flags, ID3DBlob **errors, struct ID3DXEffectPool *pool, const char *skip_constants_string) { #if D3DX_SDK_VERSION <= 36 UINT compile_flags = D3DCOMPILE_ENABLE_BACKWARDS_COMPATIBILITY; @@ -6409,9 +6409,9 @@ static HRESULT d3dx9_effect_init(struct d3dx_effect *effect, struct IDirect3DDev unsigned int i, j; HRESULT hr;
- TRACE("effect %p, device %p, data %p, data_size %lu, defines %p, include %p, eflags %#x, errors %p, " + TRACE("effect %p, device %p, data %p, data_size %lu, defines %p, include %p, flags %#x, errors %p, " "pool %p, skip_constants %s.\n", - effect, device, data, data_size, defines, include, eflags, errors, pool, + effect, device, data, data_size, defines, include, flags, errors, pool, debugstr_a(skip_constants_string));
effect->ID3DXEffect_iface.lpVtbl = &ID3DXEffect_Vtbl; @@ -6426,7 +6426,7 @@ static HRESULT d3dx9_effect_init(struct d3dx_effect *effect, struct IDirect3DDev IDirect3DDevice9_AddRef(device); effect->device = device;
- effect->flags = eflags; + effect->flags = flags;
list_init(&effect->parameter_block_list);
@@ -6436,8 +6436,9 @@ static HRESULT d3dx9_effect_init(struct d3dx_effect *effect, struct IDirect3DDev if (tag != d3dx9_effect_version(9, 1)) { TRACE("HLSL ASCII effect, trying to compile it.\n"); + compile_flags |= flags & ~(D3DXFX_NOT_CLONEABLE | D3DXFX_LARGEADDRESSAWARE);
The D3DXFX_DONOTSAVE[*]STATE flags might also need to be filtered out. They conflict with D3DXSHADER_DEBUG, D3DXSHADER_SKIPVALIDATION and D3DXSHADER_SKIPOPTIMIZATION (and their D3DCOMPILE_ counterparts). It might be possible to verify the expected behavior for those, or at least for the validation flag.
hr = D3DCompile(data, data_size, NULL, defines, include, - NULL, "fx_2_0", compile_flags, eflags, &bytecode, &temp_errors); + NULL, "fx_2_0", compile_flags, 0, &bytecode, &temp_errors); if (FAILED(hr)) { WARN("Failed to compile ASCII effect.\n"); -- 2.34.1
On 2/9/22 05:54, Matteo Bruni wrote:
On Tue, Feb 8, 2022 at 12:42 AM Zebediah Figura <zfigura(a)codeweavers.com> wrote:
And strip D3DXFX_* creation flags from them.
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/d3dx9_36/effect.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/dlls/d3dx9_36/effect.c b/dlls/d3dx9_36/effect.c index e623c10035a..b8c957245a6 100644 --- a/dlls/d3dx9_36/effect.c +++ b/dlls/d3dx9_36/effect.c @@ -6393,7 +6393,7 @@ static const char **parse_skip_constants_string(char *skip_constants_string, uns
static HRESULT d3dx9_effect_init(struct d3dx_effect *effect, struct IDirect3DDevice9 *device, const char *data, SIZE_T data_size, const D3D_SHADER_MACRO *defines, ID3DInclude *include, - UINT eflags, ID3DBlob **errors, struct ID3DXEffectPool *pool, const char *skip_constants_string) + UINT flags, ID3DBlob **errors, struct ID3DXEffectPool *pool, const char *skip_constants_string) { #if D3DX_SDK_VERSION <= 36 UINT compile_flags = D3DCOMPILE_ENABLE_BACKWARDS_COMPATIBILITY; @@ -6409,9 +6409,9 @@ static HRESULT d3dx9_effect_init(struct d3dx_effect *effect, struct IDirect3DDev unsigned int i, j; HRESULT hr;
- TRACE("effect %p, device %p, data %p, data_size %lu, defines %p, include %p, eflags %#x, errors %p, " + TRACE("effect %p, device %p, data %p, data_size %lu, defines %p, include %p, flags %#x, errors %p, " "pool %p, skip_constants %s.\n", - effect, device, data, data_size, defines, include, eflags, errors, pool, + effect, device, data, data_size, defines, include, flags, errors, pool, debugstr_a(skip_constants_string));
effect->ID3DXEffect_iface.lpVtbl = &ID3DXEffect_Vtbl; @@ -6426,7 +6426,7 @@ static HRESULT d3dx9_effect_init(struct d3dx_effect *effect, struct IDirect3DDev IDirect3DDevice9_AddRef(device); effect->device = device;
- effect->flags = eflags; + effect->flags = flags;
list_init(&effect->parameter_block_list);
@@ -6436,8 +6436,9 @@ static HRESULT d3dx9_effect_init(struct d3dx_effect *effect, struct IDirect3DDev if (tag != d3dx9_effect_version(9, 1)) { TRACE("HLSL ASCII effect, trying to compile it.\n"); + compile_flags |= flags & ~(D3DXFX_NOT_CLONEABLE | D3DXFX_LARGEADDRESSAWARE);
The D3DXFX_DONOTSAVE[*]STATE flags might also need to be filtered out. They conflict with D3DXSHADER_DEBUG, D3DXSHADER_SKIPVALIDATION and D3DXSHADER_SKIPOPTIMIZATION (and their D3DCOMPILE_ counterparts). It might be possible to verify the expected behavior for those, or at least for the validation flag.
Do you know offhand how to test the validation flag? I don't know immediately any way to write an HLSL shader that will compile if and only if that flag is set. FWIW, the documentation does state that those flags are passed to ID3DXEffect::Begin(), not to creation functions [1]. Not that the documentation is always trustworthy, of course, but it is the reasonable behaviour. [1] https://docs.microsoft.com/en-us/windows/win32/direct3d9/d3dxfx
On Fri, Feb 11, 2022 at 5:00 AM Zebediah Figura <zfigura(a)codeweavers.com> wrote:
On 2/9/22 05:54, Matteo Bruni wrote:
On Tue, Feb 8, 2022 at 12:42 AM Zebediah Figura <zfigura(a)codeweavers.com> wrote:
And strip D3DXFX_* creation flags from them.
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/d3dx9_36/effect.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/dlls/d3dx9_36/effect.c b/dlls/d3dx9_36/effect.c index e623c10035a..b8c957245a6 100644 --- a/dlls/d3dx9_36/effect.c +++ b/dlls/d3dx9_36/effect.c @@ -6393,7 +6393,7 @@ static const char **parse_skip_constants_string(char *skip_constants_string, uns
static HRESULT d3dx9_effect_init(struct d3dx_effect *effect, struct IDirect3DDevice9 *device, const char *data, SIZE_T data_size, const D3D_SHADER_MACRO *defines, ID3DInclude *include, - UINT eflags, ID3DBlob **errors, struct ID3DXEffectPool *pool, const char *skip_constants_string) + UINT flags, ID3DBlob **errors, struct ID3DXEffectPool *pool, const char *skip_constants_string) { #if D3DX_SDK_VERSION <= 36 UINT compile_flags = D3DCOMPILE_ENABLE_BACKWARDS_COMPATIBILITY; @@ -6409,9 +6409,9 @@ static HRESULT d3dx9_effect_init(struct d3dx_effect *effect, struct IDirect3DDev unsigned int i, j; HRESULT hr;
- TRACE("effect %p, device %p, data %p, data_size %lu, defines %p, include %p, eflags %#x, errors %p, " + TRACE("effect %p, device %p, data %p, data_size %lu, defines %p, include %p, flags %#x, errors %p, " "pool %p, skip_constants %s.\n", - effect, device, data, data_size, defines, include, eflags, errors, pool, + effect, device, data, data_size, defines, include, flags, errors, pool, debugstr_a(skip_constants_string));
effect->ID3DXEffect_iface.lpVtbl = &ID3DXEffect_Vtbl; @@ -6426,7 +6426,7 @@ static HRESULT d3dx9_effect_init(struct d3dx_effect *effect, struct IDirect3DDev IDirect3DDevice9_AddRef(device); effect->device = device;
- effect->flags = eflags; + effect->flags = flags;
list_init(&effect->parameter_block_list);
@@ -6436,8 +6436,9 @@ static HRESULT d3dx9_effect_init(struct d3dx_effect *effect, struct IDirect3DDev if (tag != d3dx9_effect_version(9, 1)) { TRACE("HLSL ASCII effect, trying to compile it.\n"); + compile_flags |= flags & ~(D3DXFX_NOT_CLONEABLE | D3DXFX_LARGEADDRESSAWARE);
The D3DXFX_DONOTSAVE[*]STATE flags might also need to be filtered out. They conflict with D3DXSHADER_DEBUG, D3DXSHADER_SKIPVALIDATION and D3DXSHADER_SKIPOPTIMIZATION (and their D3DCOMPILE_ counterparts). It might be possible to verify the expected behavior for those, or at least for the validation flag.
Do you know offhand how to test the validation flag? I don't know immediately any way to write an HLSL shader that will compile if and only if that flag is set.
I thought that defining too many constants would be a quick and easy way to do that but it doesn't seem to be the case, so no, not really...
FWIW, the documentation does state that those flags are passed to ID3DXEffect::Begin(), not to creation functions [1]. Not that the documentation is always trustworthy, of course, but it is the reasonable behaviour.
[1] https://docs.microsoft.com/en-us/windows/win32/direct3d9/d3dxfx
Oh, right, of course. That means there shouldn't be any actual conflict with the D3DXSHADER_ / D3DXCOMPILE_ options. Given that and the above, I guess checking that passing those flags to D3DXCreateEffect() doesn't cause any immediate error might be good enough.
This fixes a crash when trying to set their value. Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/d3dx9_36/effect.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/dlls/d3dx9_36/effect.c b/dlls/d3dx9_36/effect.c index b8c957245a6..310e5c0bf17 100644 --- a/dlls/d3dx9_36/effect.c +++ b/dlls/d3dx9_36/effect.c @@ -1313,10 +1313,15 @@ static void *record_parameter(struct d3dx_effect *effect, struct d3dx_parameter static void set_dirty(struct d3dx_parameter *param) { - struct d3dx_shared_data *shared_data; struct d3dx_top_level_parameter *top_param = param->top_level_param; - ULONG64 new_update_version = next_update_version(top_param->version_counter); + struct d3dx_shared_data *shared_data; + ULONG64 new_update_version; + /* This is true for annotations. */ + if (!top_param) + return; + + new_update_version = next_update_version(top_param->version_counter); if ((shared_data = top_param->shared_data)) shared_data->update_version = new_update_version; else -- 2.34.1
Signed-off-by: Matteo Bruni <mbruni(a)codeweavers.com>
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/d3dx9_36/tests/effect.c | 63 ++++++++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/dlls/d3dx9_36/tests/effect.c b/dlls/d3dx9_36/tests/effect.c index d4efc1101d9..63321fba3b3 100644 --- a/dlls/d3dx9_36/tests/effect.c +++ b/dlls/d3dx9_36/tests/effect.c @@ -8075,6 +8075,38 @@ static void test_effect_parameter_block(void) HWND window; HRESULT hr; + static const DWORD annotation_code[] = + { +#if 0 + float f <float a = 1.0;>; + + float4 vs_main(float4 pos : POSITION) : POSITION + { + return pos; + } + + technique tech0 + { + pass p + { + VertexShader = compile vs_2_0 vs_main(); + } + } +#endif + 0xfeff0901, 0x00000080, 0x00000000, 0x00000003, 0x00000000, 0x0000004c, 0x00000000, 0x00000000, + 0x00000001, 0x00000001, 0x00000000, 0x3f800000, 0x00000003, 0x00000000, 0x00000044, 0x00000000, + 0x00000000, 0x00000001, 0x00000001, 0x00000002, 0x00000061, 0x00000002, 0x00000066, 0x00000001, + 0x00000010, 0x00000004, 0x00000000, 0x00000000, 0x00000000, 0x00000002, 0x00000070, 0x00000006, + 0x68636574, 0x00000030, 0x00000001, 0x00000001, 0x00000002, 0x00000002, 0x00000004, 0x00000020, + 0x00000000, 0x00000001, 0x00000028, 0x00000024, 0x00000074, 0x00000000, 0x00000001, 0x0000006c, + 0x00000000, 0x00000001, 0x00000092, 0x00000000, 0x00000058, 0x00000054, 0x00000000, 0x00000001, + 0x00000000, 0x00000000, 0xffffffff, 0x00000000, 0x00000000, 0x00000074, 0xfffe0200, 0x0014fffe, + 0x42415443, 0x0000001c, 0x00000023, 0xfffe0200, 0x00000000, 0x00000000, 0x20000400, 0x0000001c, + 0x325f7376, 0x4d00305f, 0x6f726369, 0x74666f73, 0x29522820, 0x534c4820, 0x6853204c, 0x72656461, + 0x6d6f4320, 0x656c6970, 0x30312072, 0xab00312e, 0x0200001f, 0x80000000, 0x900f0000, 0x02000001, + 0xc00f0000, 0x90e40000, 0x0000ffff, + }; + if (!(window = CreateWindowA("static", "d3dx9_test", WS_OVERLAPPEDWINDOW, 0, 0, 640, 480, NULL, NULL, NULL, NULL))) { @@ -8388,6 +8420,37 @@ static void test_effect_parameter_block(void) refcount = effect->lpVtbl->Release(effect); ok(!refcount, "Got unexpected refcount %u.\n", refcount); + hr = D3DXCreateEffect(device, annotation_code, sizeof(annotation_code), + NULL, NULL, 0, NULL, &effect, NULL); + ok(hr == D3D_OK, "Got result %#x.\n", hr); + + hr = effect->lpVtbl->GetFloat(effect, "f(a)a", &float_value); + ok(hr == D3D_OK, "Got result %#x.\n", hr); + ok(float_value == 1.0f, "Got float %.8e.\n", float_value); + hr = effect->lpVtbl->SetFloat(effect, "f(a)a", 2.0f); + ok(hr == D3D_OK, "Got result %#x.\n", hr); + hr = effect->lpVtbl->GetFloat(effect, "f(a)a", &float_value); + ok(hr == D3D_OK, "Got result %#x.\n", hr); + ok(float_value == 2.0f, "Got float %.8e.\n", float_value); + + hr = effect->lpVtbl->BeginParameterBlock(effect); + ok(hr == D3D_OK, "Got result %#x.\n", hr); + hr = effect->lpVtbl->SetFloat(effect, "f(a)a", 3.0f); + ok(hr == D3D_OK, "Got result %#x.\n", hr); + hr = effect->lpVtbl->GetFloat(effect, "f(a)a", &float_value); + ok(hr == D3D_OK, "Got result %#x.\n", hr); + ok(float_value == 2.0f, "Got float %.8e.\n", float_value); + block = effect->lpVtbl->EndParameterBlock(effect); + ok(!!block, "Got unexpected block %p.\n", block); + hr = effect->lpVtbl->ApplyParameterBlock(effect, block); + ok(hr == D3D_OK, "Got result %#x.\n", hr); + hr = effect->lpVtbl->GetFloat(effect, "f(a)a", &float_value); + ok(hr == D3D_OK, "Got result %#x.\n", hr); + ok(float_value == 3.0f, "Got float %.8e.\n", float_value); + + refcount = effect->lpVtbl->Release(effect); + ok(!refcount, "Got unexpected refcount %u.\n", refcount); + refcount = IDirect3DDevice9_Release(device); ok(!refcount, "Device has %u references left.\n", refcount); IDirect3D9_Release(d3d); -- 2.34.1
Signed-off-by: Matteo Bruni <mbruni(a)codeweavers.com>
participants (3)
-
Matteo Bruni -
Matteo Bruni -
Zebediah Figura