[PATCH] d3dx9/effect: Reset output handle for next after last technique in FindNextValidTechnique().
Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- dlls/d3dx9_36/effect.c | 2 +- dlls/d3dx9_36/tests/effect.c | 9 ++++----- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/dlls/d3dx9_36/effect.c b/dlls/d3dx9_36/effect.c index 710e999f27f..22afa649db3 100644 --- a/dlls/d3dx9_36/effect.c +++ b/dlls/d3dx9_36/effect.c @@ -3816,7 +3816,7 @@ static HRESULT WINAPI d3dx_effect_FindNextValidTechnique(ID3DXEffect *iface, D3D } } - *next_technique = get_technique_handle(&effect->techniques[0]); + *next_technique = NULL; return S_FALSE; } diff --git a/dlls/d3dx9_36/tests/effect.c b/dlls/d3dx9_36/tests/effect.c index ae6f65d52d5..48119ef2736 100644 --- a/dlls/d3dx9_36/tests/effect.c +++ b/dlls/d3dx9_36/tests/effect.c @@ -7892,9 +7892,9 @@ static void test_effect_find_next_valid_technique(void) D3DPRESENT_PARAMETERS present_parameters = {0}; IDirect3DDevice9 *device; D3DXTECHNIQUE_DESC desc; + D3DXHANDLE tech, tech2; ID3DXEffect *effect; IDirect3D9 *d3d; - D3DXHANDLE tech; ULONG refcount; HWND window; HRESULT hr; @@ -7939,11 +7939,10 @@ static void test_effect_find_next_valid_technique(void) ok(hr == D3D_OK, "Got result %#x.\n", hr); ok(!strcmp(desc.Name, "tech1"), "Got unexpected technique %s.\n", desc.Name); - hr = effect->lpVtbl->FindNextValidTechnique(effect, tech, &tech); + tech2 = tech; + hr = effect->lpVtbl->FindNextValidTechnique(effect, tech, &tech2); ok(hr == S_FALSE, "Got result %#x.\n", hr); - hr = effect->lpVtbl->GetTechniqueDesc(effect, tech, &desc); - ok(hr == D3D_OK, "Got result %#x.\n", hr); - ok(!strcmp(desc.Name, "tech0"), "Got unexpected technique %s.\n", desc.Name); + ok(!tech2, "Unexpected tech handle %p\n", tech2); effect->lpVtbl->Release(effect); -- 2.30.2
On Thu, Aug 12, 2021 at 4:58 PM Nikolay Sivov <nsivov(a)codeweavers.com> wrote:
Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- dlls/d3dx9_36/effect.c | 2 +- dlls/d3dx9_36/tests/effect.c | 9 ++++----- 2 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/dlls/d3dx9_36/effect.c b/dlls/d3dx9_36/effect.c index 710e999f27f..22afa649db3 100644 --- a/dlls/d3dx9_36/effect.c +++ b/dlls/d3dx9_36/effect.c @@ -3816,7 +3816,7 @@ static HRESULT WINAPI d3dx_effect_FindNextValidTechnique(ID3DXEffect *iface, D3D } }
- *next_technique = get_technique_handle(&effect->techniques[0]); + *next_technique = NULL; return S_FALSE; }
diff --git a/dlls/d3dx9_36/tests/effect.c b/dlls/d3dx9_36/tests/effect.c index ae6f65d52d5..48119ef2736 100644 --- a/dlls/d3dx9_36/tests/effect.c +++ b/dlls/d3dx9_36/tests/effect.c @@ -7892,9 +7892,9 @@ static void test_effect_find_next_valid_technique(void) D3DPRESENT_PARAMETERS present_parameters = {0}; IDirect3DDevice9 *device; D3DXTECHNIQUE_DESC desc; + D3DXHANDLE tech, tech2; ID3DXEffect *effect; IDirect3D9 *d3d; - D3DXHANDLE tech; ULONG refcount; HWND window; HRESULT hr; @@ -7939,11 +7939,10 @@ static void test_effect_find_next_valid_technique(void) ok(hr == D3D_OK, "Got result %#x.\n", hr); ok(!strcmp(desc.Name, "tech1"), "Got unexpected technique %s.\n", desc.Name);
- hr = effect->lpVtbl->FindNextValidTechnique(effect, tech, &tech); + tech2 = tech; + hr = effect->lpVtbl->FindNextValidTechnique(effect, tech, &tech2); ok(hr == S_FALSE, "Got result %#x.\n", hr); - hr = effect->lpVtbl->GetTechniqueDesc(effect, tech, &desc); - ok(hr == D3D_OK, "Got result %#x.\n", hr); - ok(!strcmp(desc.Name, "tech0"), "Got unexpected technique %s.\n", desc.Name); + ok(!tech2, "Unexpected tech handle %p\n", tech2);
effect->lpVtbl->Release(effect);
Something not obvious by just looking at the patch: how did this test pass on Windows previously? It turns out GetTechniqueDesc(effect, NULL, &desc); returns the descriptor of the first technique in the effect. In fact our implementation already does that. I don't see other tests of this particular GetTechniqueDesc() behavior right now, so I think I'm going to resend this patch with those lines reinstated and a comment to clarify what's happening.
On 8/17/21 5:50 PM, Matteo Bruni wrote:
On Thu, Aug 12, 2021 at 4:58 PM Nikolay Sivov <nsivov(a)codeweavers.com> wrote:
Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- dlls/d3dx9_36/effect.c | 2 +- dlls/d3dx9_36/tests/effect.c | 9 ++++----- 2 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/dlls/d3dx9_36/effect.c b/dlls/d3dx9_36/effect.c index 710e999f27f..22afa649db3 100644 --- a/dlls/d3dx9_36/effect.c +++ b/dlls/d3dx9_36/effect.c @@ -3816,7 +3816,7 @@ static HRESULT WINAPI d3dx_effect_FindNextValidTechnique(ID3DXEffect *iface, D3D } }
- *next_technique = get_technique_handle(&effect->techniques[0]); + *next_technique = NULL; return S_FALSE; }
diff --git a/dlls/d3dx9_36/tests/effect.c b/dlls/d3dx9_36/tests/effect.c index ae6f65d52d5..48119ef2736 100644 --- a/dlls/d3dx9_36/tests/effect.c +++ b/dlls/d3dx9_36/tests/effect.c @@ -7892,9 +7892,9 @@ static void test_effect_find_next_valid_technique(void) D3DPRESENT_PARAMETERS present_parameters = {0}; IDirect3DDevice9 *device; D3DXTECHNIQUE_DESC desc; + D3DXHANDLE tech, tech2; ID3DXEffect *effect; IDirect3D9 *d3d; - D3DXHANDLE tech; ULONG refcount; HWND window; HRESULT hr; @@ -7939,11 +7939,10 @@ static void test_effect_find_next_valid_technique(void) ok(hr == D3D_OK, "Got result %#x.\n", hr); ok(!strcmp(desc.Name, "tech1"), "Got unexpected technique %s.\n", desc.Name);
- hr = effect->lpVtbl->FindNextValidTechnique(effect, tech, &tech); + tech2 = tech; + hr = effect->lpVtbl->FindNextValidTechnique(effect, tech, &tech2); ok(hr == S_FALSE, "Got result %#x.\n", hr); - hr = effect->lpVtbl->GetTechniqueDesc(effect, tech, &desc); - ok(hr == D3D_OK, "Got result %#x.\n", hr); - ok(!strcmp(desc.Name, "tech0"), "Got unexpected technique %s.\n", desc.Name); + ok(!tech2, "Unexpected tech handle %p\n", tech2);
effect->lpVtbl->Release(effect); Something not obvious by just looking at the patch: how did this test pass on Windows previously? It turns out GetTechniqueDesc(effect, NULL, &desc); returns the descriptor of the first technique in the effect. In fact our implementation already does that. Removed FindNextValidTechnique() was called on last technique 'tech' -> "tech1". That returned S_FALSE, and reset 'tech' to NULL, on Windows. Removed GetTechniqueDesc() was then called on NULL handle. On whine however that picked first technique "tech0" unconditionally when S_FALSE is returned, masking GetTechniqueDesc() behavior. I don't see other tests of this particular GetTechniqueDesc() behavior right now, so I think I'm going to resend this patch with those lines reinstated and a comment to clarify what's happening. Yes, that'd be a right thing to do.
participants (2)
-
Matteo Bruni -
Nikolay Sivov