Signed-off-by: Paul Gofman gofmanp@gmail.com --- dlls/d3dx9_36/effect.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/dlls/d3dx9_36/effect.c b/dlls/d3dx9_36/effect.c index e69a1fd1f8..980313182a 100644 --- a/dlls/d3dx9_36/effect.c +++ b/dlls/d3dx9_36/effect.c @@ -517,7 +517,6 @@ static void free_sampler(struct d3dx_sampler *sampler) free_state(&sampler->states[i]); } HeapFree(GetProcessHeap(), 0, sampler->states); - HeapFree(GetProcessHeap(), 0, sampler); }
static void d3dx_pool_release_shared_parameter(struct d3dx_top_level_parameter *param); @@ -557,7 +556,7 @@ static void free_parameter_data(struct d3dx_parameter *param, BOOL child) break; } } - if (!child) + if (!child || is_param_type_sampler(param->type)) HeapFree(GetProcessHeap(), 0, param->data); }
Signed-off-by: Paul Gofman gofmanp@gmail.com --- dlls/d3dx9_36/tests/effect.c | 226 +++++++++++++++++++++++++++++++++++ 1 file changed, 226 insertions(+)
diff --git a/dlls/d3dx9_36/tests/effect.c b/dlls/d3dx9_36/tests/effect.c index 5c4a3c5e77..b9e89a6519 100644 --- a/dlls/d3dx9_36/tests/effect.c +++ b/dlls/d3dx9_36/tests/effect.c @@ -8001,6 +8001,231 @@ static void test_effect_find_next_valid_technique(void) DestroyWindow(window); }
+static void test_effect_parameter_block(void) +{ + static const D3DXMATRIX test_mat = + {{{ + -11.0f, -12.0f, 0.0f, 0.0f, + -21.0f, -22.0f, 0.0f, 0.0f, + -31.0f, -32.0f, 0.0f, 0.0f, + }}}; + D3DPRESENT_PARAMETERS present_parameters = {0}; + static const float float_array_zero[4]; + ID3DXEffect *effect, *effect2; + IDirect3DTexture9 *texture; + D3DXHANDLE block, handle; + IDirect3DDevice9 *device; + ID3DXEffectPool *pool; + float float_array[4]; + float float_value; + IDirect3D9 *d3d; + D3DXMATRIX mat; + ULONG refcount; + HWND window; + HRESULT hr; + + if (!(window = CreateWindowA("static", "d3dx9_test", WS_OVERLAPPEDWINDOW, 0, 0, + 640, 480, NULL, NULL, NULL, NULL))) + { + skip("Failed to create window.\n"); + return; + } + if (!(d3d = Direct3DCreate9(D3D_SDK_VERSION))) + { + skip("Failed to create IDirect3D9 object.\n"); + DestroyWindow(window); + return; + } + present_parameters.Windowed = TRUE; + present_parameters.SwapEffect = D3DSWAPEFFECT_DISCARD; + hr = IDirect3D9_CreateDevice(d3d, D3DADAPTER_DEFAULT, D3DDEVTYPE_HAL, window, + D3DCREATE_HARDWARE_VERTEXPROCESSING, &present_parameters, &device); + if (FAILED(hr)) + { + skip("Failed to create IDirect3DDevice9 object, hr %#x.\n", hr); + IDirect3D9_Release(d3d); + DestroyWindow(window); + return; + } + + hr = D3DXCreateEffectPool(&pool); + ok(hr == D3D_OK, "Got result %#x.\n", hr); + + hr = D3DXCreateEffect(device, test_effect_preshader_effect_blob, sizeof(test_effect_preshader_effect_blob), + NULL, NULL, 0, pool, &effect, NULL); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + hr = D3DXCreateEffect(device, test_effect_preshader_effect_blob, sizeof(test_effect_preshader_effect_blob), + NULL, NULL, 0, pool, &effect2, NULL); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + + hr = effect->lpVtbl->BeginParameterBlock(effect); + todo_wine ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + hr = effect->lpVtbl->BeginParameterBlock(effect); + todo_wine ok(hr == D3DERR_INVALIDCALL, "Got unexpected hr %#x.\n", hr); + block = effect->lpVtbl->EndParameterBlock(effect); + todo_wine ok(!!block, "Got unexpected block %p.\n", block); + handle = effect->lpVtbl->EndParameterBlock(effect); + ok(!handle, "Got unexpected handle %p.\n", handle); + + /* Block doesn't hold effect's reference. */ + effect->lpVtbl->AddRef(effect); + refcount = effect->lpVtbl->Release(effect); + ok(refcount == 1, "Got unexpected refcount %u.\n", refcount); + + hr = effect->lpVtbl->ApplyParameterBlock(effect, block); + todo_wine ok(hr == D3DERR_INVALIDCALL, "Got unexpected hr %#x.\n", hr); + hr = effect->lpVtbl->DeleteParameterBlock(effect, block); + todo_wine ok(hr == D3D_OK, "Got result %#x.\n", hr); + + hr = effect->lpVtbl->BeginParameterBlock(effect); + todo_wine ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + hr = effect->lpVtbl->SetFloat(effect, "vec3[0]", 1001.0f); + ok(hr == D3DERR_INVALIDCALL, "Got unexpected hr %#x.\n", hr); + hr = effect->lpVtbl->SetFloat(effect, "arr1[0]", 91.0f); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + block = effect->lpVtbl->EndParameterBlock(effect); + todo_wine ok(!!block, "Got unexpected block %p.\n", block); + hr = effect->lpVtbl->ApplyParameterBlock(effect, block); + todo_wine ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + + hr = effect->lpVtbl->DeleteParameterBlock(effect2, block); + todo_wine ok(hr == D3DERR_INVALIDCALL, "Got result %#x.\n", hr); + hr = effect->lpVtbl->DeleteParameterBlock(effect, block); + todo_wine ok(hr == D3D_OK, "Got result %#x.\n", hr); + + hr = effect->lpVtbl->ApplyParameterBlock(effect, "parameter_block"); + todo_wine ok(hr == D3DERR_INVALIDCALL, "Got unexpected hr %#x.\n", hr); + + hr = D3DXCreateTexture(device, D3DX_DEFAULT, D3DX_DEFAULT, D3DX_DEFAULT, 0, 0, D3DPOOL_DEFAULT, &texture); + ok(hr == D3D_OK, "Got result %#x, expected 0 (D3D_OK).\n", hr); + + hr = effect->lpVtbl->BeginParameterBlock(effect); + todo_wine ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + + hr = effect->lpVtbl->SetTexture(effect, "tex1", (IDirect3DBaseTexture9 *)texture); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + + /* Child parameters and array members are recorded separately (the whole parameter is not + * updated when parameter block is applied). + * Setting the same float value does not affect dirty state, but parameter is still + * recorded to parameter block. + * Setting shared parameter through effect2 is not recorded to effect parameter block. */ + hr = effect->lpVtbl->SetFloat(effect, "arr2[0]", 92.0f); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + hr = effect->lpVtbl->SetFloat(effect, "ts1[0].fv", 28.0f); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + float_array[0] = -29.0f; + hr = effect->lpVtbl->SetFloatArray(effect, "ts1[0].v2", float_array, 1); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + memset(&mat, 0, sizeof(mat)); + hr = effect->lpVtbl->SetMatrix(effect, "m3x2row", &test_mat); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + hr = effect->lpVtbl->SetMatrix(effect, "m3x2column", &test_mat); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + + hr = effect2->lpVtbl->SetFloat(effect2, "arr2[1]", -1.0f); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + /* Ensure that parameter is actually shared. */ + hr = effect->lpVtbl->GetFloat(effect, "arr2[1]", &float_value); + ok(float_value == -1.0f, "Unexpected value %g.\n", float_value); + + /* Object reference is not increased when object gets to parameter block + * but is also not lost when object is reset in effect parameter. + * Maybe native d3dx is using some copy on write strategy. */ + IDirect3DTexture9_AddRef(texture); + refcount = IDirect3DTexture9_Release(texture); + ok(refcount == 2, "Got unexpected refcount %u.\n", refcount); + + block = effect->lpVtbl->EndParameterBlock(effect); + todo_wine ok(!!block, "Got unexpected block %p.\n", block); + + IDirect3DTexture9_AddRef(texture); + refcount = IDirect3DTexture9_Release(texture); + ok(refcount == 2, "Got unexpected refcount %u.\n", refcount); + + hr = effect->lpVtbl->SetTexture(effect, "tex1", NULL); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + + IDirect3DTexture9_AddRef(texture); + refcount = IDirect3DTexture9_Release(texture); + todo_wine ok(refcount == 2, "Got unexpected refcount %u.\n", refcount); + + hr = effect->lpVtbl->SetFloat(effect, "arr2[0]", 0.0f); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + hr = effect->lpVtbl->SetFloat(effect, "arr2[1]", 0.0f); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + hr = effect->lpVtbl->SetFloatArray(effect, "ts1[0].v1", float_array_zero, 3); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + hr = effect->lpVtbl->SetFloat(effect, "ts1[0].fv", 0.0f); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + hr = effect->lpVtbl->SetFloatArray(effect, "ts1[0].v2", float_array_zero, 4); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + memset(&mat, 0, sizeof(mat)); + hr = effect->lpVtbl->SetMatrix(effect, "m3x2row", &mat); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + hr = effect->lpVtbl->SetMatrix(effect, "m3x2column", &mat); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + + + hr = effect->lpVtbl->ApplyParameterBlock(effect, block); + todo_wine ok(hr == D3D_OK, "Got result %#x.\n", hr); + + IDirect3DTexture9_AddRef(texture); + refcount = IDirect3DTexture9_Release(texture); + todo_wine ok(refcount == 3, "Got unexpected refcount %u.\n", refcount); + + hr = effect->lpVtbl->GetFloat(effect, "arr2[0]", &float_value); + todo_wine ok(hr == D3D_OK && float_value == 92.0f, "Got unexpected hr %#x, float_value %g.\n", hr, float_value); + hr = effect->lpVtbl->GetFloat(effect, "arr2[1]", &float_value); + ok(hr == D3D_OK && float_value == 0.0f, "Got unexpected hr %#x, float_value %g.\n", hr, float_value); + hr = effect->lpVtbl->GetFloatArray(effect, "ts1[0].v1", float_array, 3); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + ok(hr == D3D_OK && !memcmp(float_array, float_array_zero, 3 * sizeof(*float_array)), + "Got unexpected hr %#x, ts1[0].v1 (%g, %g, %g).\n", hr, + float_array[0], float_array[1], float_array[2]); + hr = effect->lpVtbl->GetFloat(effect, "ts1[0].fv", &float_value); + hr = effect->lpVtbl->GetMatrix(effect, "m3x2row", &mat); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + todo_wine ok(!memcmp(&mat, &test_mat, sizeof(mat)), "Got unexpected matrix.\n"); + hr = effect->lpVtbl->GetMatrix(effect, "m3x2column", &mat); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + todo_wine ok(!memcmp(&mat, &test_mat, sizeof(mat)), "Got unexpected matrix.\n"); + + todo_wine ok(hr == D3D_OK && float_value == 28.0f, "Got unexpected hr %#x, float_value %g.\n", hr, float_value); + hr = effect->lpVtbl->GetFloatArray(effect, "ts1[0].v2", float_array, 4); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + todo_wine ok(hr == D3D_OK && float_array[0] == -29.0f + && !memcmp(float_array + 1, float_array_zero, 3 * sizeof(*float_array)), + "Got unexpected hr %#x, ts1[0].v2 (%g, %g, %g, %g).\n", hr, + float_array[0], float_array[1], float_array[2], float_array[3]); + + hr = effect->lpVtbl->DeleteParameterBlock(effect, block); + todo_wine ok(hr == D3D_OK, "Got result %#x.\n", hr); + + IDirect3DTexture9_AddRef(texture); + + hr = effect->lpVtbl->SetTexture(effect, "tex1", NULL); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + refcount = IDirect3DTexture9_Release(texture); + ok(refcount == 1, "Got unexpected refcount %u.\n", refcount); + if (refcount) + IDirect3DTexture9_Release(texture); + + refcount = effect->lpVtbl->Release(effect); + ok(!refcount, "Got unexpected refcount %u.\n", refcount); + + refcount = effect2->lpVtbl->Release(effect2); + ok(!refcount, "Got unexpected refcount %u.\n", refcount); + + refcount = pool->lpVtbl->Release(pool); + ok(!refcount, "Got unexpected refcount %u.\n", refcount); + + refcount = IDirect3DDevice9_Release(device); + ok(!refcount, "Device has %u references left.\n", refcount); + IDirect3D9_Release(d3d); + DestroyWindow(window); +} + START_TEST(effect) { IDirect3DDevice9 *device; @@ -8040,4 +8265,5 @@ START_TEST(effect) test_refcount(); test_create_effect_from_file(); test_effect_find_next_valid_technique(); + test_effect_parameter_block(); }
On Thu, Nov 7, 2019 at 10:18 PM Paul Gofman gofmanp@gmail.com wrote:
- /* Child parameters and array members are recorded separately (the whole parameter is not
* updated when parameter block is applied).* Setting the same float value does not affect dirty state, but parameter is still* recorded to parameter block.* Setting shared parameter through effect2 is not recorded to effect parameter block. */- hr = effect->lpVtbl->SetFloat(effect, "arr2[0]", 92.0f);
- ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr);
- hr = effect->lpVtbl->SetFloat(effect, "ts1[0].fv", 28.0f);
- ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr);
- float_array[0] = -29.0f;
- hr = effect->lpVtbl->SetFloatArray(effect, "ts1[0].v2", float_array, 1);
- ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr);
That reminds me of something maybe worth testing: I couldn't manage to have SetArrayRange() ever do anything. Maybe it actually does something with parameter blocks? Not holding my breath for that...
- /* Object reference is not increased when object gets to parameter block
* but is also not lost when object is reset in effect parameter.* Maybe native d3dx is using some copy on write strategy. */- IDirect3DTexture9_AddRef(texture);
- refcount = IDirect3DTexture9_Release(texture);
- ok(refcount == 2, "Got unexpected refcount %u.\n", refcount);
Or stuff might just break if the texture is released while still in a parameter block. It should be possible to test it.
- hr = effect->lpVtbl->SetFloat(effect, "arr2[0]", 0.0f);
- ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr);
- hr = effect->lpVtbl->SetFloat(effect, "arr2[1]", 0.0f);
- ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr);
- hr = effect->lpVtbl->SetFloatArray(effect, "ts1[0].v1", float_array_zero, 3);
- ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr);
- hr = effect->lpVtbl->SetFloat(effect, "ts1[0].fv", 0.0f);
- ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr);
- hr = effect->lpVtbl->SetFloatArray(effect, "ts1[0].v2", float_array_zero, 4);
- ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr);
- memset(&mat, 0, sizeof(mat));
- hr = effect->lpVtbl->SetMatrix(effect, "m3x2row", &mat);
- ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr);
- hr = effect->lpVtbl->SetMatrix(effect, "m3x2column", &mat);
- ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr);
The double blank line here seems unnecessary (but I don't mind).
- hr = effect->lpVtbl->ApplyParameterBlock(effect, block);
- todo_wine ok(hr == D3D_OK, "Got result %#x.\n", hr);
- IDirect3DTexture9_AddRef(texture);
- refcount = IDirect3DTexture9_Release(texture);
- todo_wine ok(refcount == 3, "Got unexpected refcount %u.\n", refcount);
- hr = effect->lpVtbl->GetFloat(effect, "arr2[0]", &float_value);
- todo_wine ok(hr == D3D_OK && float_value == 92.0f, "Got unexpected hr %#x, float_value %g.\n", hr, float_value);
- hr = effect->lpVtbl->GetFloat(effect, "arr2[1]", &float_value);
- ok(hr == D3D_OK && float_value == 0.0f, "Got unexpected hr %#x, float_value %g.\n", hr, float_value);
- hr = effect->lpVtbl->GetFloatArray(effect, "ts1[0].v1", float_array, 3);
- ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr);
- ok(hr == D3D_OK && !memcmp(float_array, float_array_zero, 3 * sizeof(*float_array)),
"Got unexpected hr %#x, ts1[0].v1 (%g, %g, %g).\n", hr,float_array[0], float_array[1], float_array[2]);
Here you're checking hr twice.
- hr = effect->lpVtbl->GetFloat(effect, "ts1[0].fv", &float_value);
- hr = effect->lpVtbl->GetMatrix(effect, "m3x2row", &mat);
- ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr);
I guess one was supposed to go after GetFloat(). There are more of those below.
- todo_wine ok(!memcmp(&mat, &test_mat, sizeof(mat)), "Got unexpected matrix.\n");
- hr = effect->lpVtbl->GetMatrix(effect, "m3x2column", &mat);
- ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr);
- todo_wine ok(!memcmp(&mat, &test_mat, sizeof(mat)), "Got unexpected matrix.\n");
- todo_wine ok(hr == D3D_OK && float_value == 28.0f, "Got unexpected hr %#x, float_value %g.\n", hr, float_value);
- hr = effect->lpVtbl->GetFloatArray(effect, "ts1[0].v2", float_array, 4);
- ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr);
- todo_wine ok(hr == D3D_OK && float_array[0] == -29.0f
&& !memcmp(float_array + 1, float_array_zero, 3 * sizeof(*float_array)),"Got unexpected hr %#x, ts1[0].v2 (%g, %g, %g, %g).\n", hr,float_array[0], float_array[1], float_array[2], float_array[3]);- hr = effect->lpVtbl->DeleteParameterBlock(effect, block);
- todo_wine ok(hr == D3D_OK, "Got result %#x.\n", hr);
- IDirect3DTexture9_AddRef(texture);
- hr = effect->lpVtbl->SetTexture(effect, "tex1", NULL);
- ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr);
- refcount = IDirect3DTexture9_Release(texture);
- ok(refcount == 1, "Got unexpected refcount %u.\n", refcount);
- if (refcount)
IDirect3DTexture9_Release(texture);
Here either a todo_wine is missing or the if is unnecessary.
On 11/12/19 21:50, Matteo Bruni wrote:
On Thu, Nov 7, 2019 at 10:18 PM Paul Gofman gofmanp@gmail.com wrote:
- /* Child parameters and array members are recorded separately (the whole parameter is not
* updated when parameter block is applied).* Setting the same float value does not affect dirty state, but parameter is still* recorded to parameter block.* Setting shared parameter through effect2 is not recorded to effect parameter block. */- hr = effect->lpVtbl->SetFloat(effect, "arr2[0]", 92.0f);
- ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr);
- hr = effect->lpVtbl->SetFloat(effect, "ts1[0].fv", 28.0f);
- ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr);
- float_array[0] = -29.0f;
- hr = effect->lpVtbl->SetFloatArray(effect, "ts1[0].v2", float_array, 1);
- ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr);
That reminds me of something maybe worth testing: I couldn't manage to have SetArrayRange() ever do anything. Maybe it actually does something with parameter blocks? Not holding my breath for that...
SetArrayRange() is currently not implemented, so involving it in parameters block test would make things very complicated for testing parameters blocks. Should not that testing go separately, if we decide to implement SetArrayRange()? I did not encounter an app using it so far.
- /* Object reference is not increased when object gets to parameter block
* but is also not lost when object is reset in effect parameter.* Maybe native d3dx is using some copy on write strategy. */- IDirect3DTexture9_AddRef(texture);
- refcount = IDirect3DTexture9_Release(texture);
- ok(refcount == 2, "Got unexpected refcount %u.\n", refcount);
Or stuff might just break if the texture is released while still in a parameter block. It should be possible to test it.
The test shows that native d3dx is not loosing reference when the texture is left in the parameter block only. Do you mean additionally testing a specific case deleting parameter block with texture still set to effect parameter? In case it will really loose it, I suggest to still keep the reference in our implementation.
On Tue, Nov 12, 2019 at 8:09 PM Paul Gofman gofmanp@gmail.com wrote:
On 11/12/19 21:50, Matteo Bruni wrote:
On Thu, Nov 7, 2019 at 10:18 PM Paul Gofman gofmanp@gmail.com wrote:
- /* Child parameters and array members are recorded separately (the whole parameter is not
* updated when parameter block is applied).* Setting the same float value does not affect dirty state, but parameter is still* recorded to parameter block.* Setting shared parameter through effect2 is not recorded to effect parameter block. */- hr = effect->lpVtbl->SetFloat(effect, "arr2[0]", 92.0f);
- ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr);
- hr = effect->lpVtbl->SetFloat(effect, "ts1[0].fv", 28.0f);
- ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr);
- float_array[0] = -29.0f;
- hr = effect->lpVtbl->SetFloatArray(effect, "ts1[0].v2", float_array, 1);
- ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr);
That reminds me of something maybe worth testing: I couldn't manage to have SetArrayRange() ever do anything. Maybe it actually does something with parameter blocks? Not holding my breath for that...
SetArrayRange() is currently not implemented, so involving it in parameters block test would make things very complicated for testing parameters blocks. Should not that testing go separately, if we decide to implement SetArrayRange()? I did not encounter an app using it so far.
Sure, it's just for my own curiosity. I was unclear: I don't think you should modify the patch for submission, I'm just asking you to check if a call to SetArrayRange() with a reduced range in there makes any difference on native. I could (and should, really) do that myself but I was asking you as a personal favor :)
- /* Object reference is not increased when object gets to parameter block
* but is also not lost when object is reset in effect parameter.* Maybe native d3dx is using some copy on write strategy. */- IDirect3DTexture9_AddRef(texture);
- refcount = IDirect3DTexture9_Release(texture);
- ok(refcount == 2, "Got unexpected refcount %u.\n", refcount);
Or stuff might just break if the texture is released while still in a parameter block. It should be possible to test it.
The test shows that native d3dx is not loosing reference when the texture is left in the parameter block only. Do you mean additionally testing a specific case deleting parameter block with texture still set to effect parameter? In case it will really loose it, I suggest to still keep the reference in our implementation.
Yeah, it's okay to not replicate it (it's definitely an implementation detail unless applications somehow depend on that, which seems very unlikely). I'm just curious about what native is doing, but it's certainly not critical.
On 11/12/19 22:54, Matteo Bruni wrote:
Yeah, it's okay to not replicate it (it's definitely an implementation detail unless applications somehow depend on that, which seems very unlikely). I'm just curious about what native is doing, but it's certainly not critical.
Thanks for pointing me into these details. After testing more I spotted that effect parameter values are not changed at all while parameter block is being recorded. Somehow I completely missed that. It obviously explains the refcounting behaviour and also means that I should change the implementation.
Signed-off-by: Paul Gofman gofmanp@gmail.com --- dlls/d3dx9_36/effect.c | 34 +++++++++++++++++++++++++++++++--- dlls/d3dx9_36/tests/effect.c | 4 ++-- 2 files changed, 33 insertions(+), 5 deletions(-)
diff --git a/dlls/d3dx9_36/effect.c b/dlls/d3dx9_36/effect.c index 980313182a..19c9e552c7 100644 --- a/dlls/d3dx9_36/effect.c +++ b/dlls/d3dx9_36/effect.c @@ -30,6 +30,7 @@ #define INT_FLOAT_MULTI_INVERSE (1/INT_FLOAT_MULTI)
static const char parameter_magic_string[4] = {'@', '!', '#', '\xFF'}; +static const char parameter_block_magic_string[4] = {'@', '!', '#', '\xFE'};
#define PARAMETER_FLAG_SHARED 1
@@ -147,6 +148,11 @@ struct d3dx_technique struct IDirect3DStateBlock9 *saved_state; };
+struct d3dx_parameter_block +{ + char magic_string[ARRAY_SIZE(parameter_block_magic_string)]; +}; + struct d3dx_effect { ID3DXEffect ID3DXEffect_iface; @@ -177,6 +183,8 @@ struct d3dx_effect unsigned int light_updated; D3DMATERIAL9 current_material; BOOL material_updated; + + struct d3dx_parameter_block *current_parameter_block; };
#define INITIAL_SHARED_DATA_SIZE 4 @@ -670,6 +678,14 @@ static void free_technique(struct d3dx_technique *technique) technique->name = NULL; }
+static void free_parameter_block(struct d3dx_parameter_block *block) +{ + if (!block) + return; + + heap_free(block); +} + static void d3dx_effect_cleanup(struct d3dx_effect *effect) { ID3DXEffectPool *pool; @@ -677,6 +693,8 @@ static void d3dx_effect_cleanup(struct d3dx_effect *effect)
TRACE("effect %p.\n", effect);
+ free_parameter_block(effect->current_parameter_block); + heap_free(effect->full_name_tmp);
if (effect->parameters) @@ -4047,11 +4065,21 @@ static HRESULT WINAPI d3dx_effect_GetStateManager(ID3DXEffect *iface, ID3DXEffec
static HRESULT WINAPI d3dx_effect_BeginParameterBlock(ID3DXEffect *iface) { - struct d3dx_effect *This = impl_from_ID3DXEffect(iface); + struct d3dx_effect *effect = impl_from_ID3DXEffect(iface);
- FIXME("(%p)->(): stub\n", This); + TRACE("iface %p.\n", iface);
- return E_NOTIMPL; + if (effect->current_parameter_block) + { + WARN("Parameter block is already started.\n"); + return D3DERR_INVALIDCALL; + } + + effect->current_parameter_block = heap_alloc_zero(sizeof(*effect->current_parameter_block)); + memcpy(effect->current_parameter_block->magic_string, parameter_block_magic_string, + sizeof(parameter_block_magic_string)); + + return D3D_OK; }
static D3DXHANDLE WINAPI d3dx_effect_EndParameterBlock(ID3DXEffect *iface) diff --git a/dlls/d3dx9_36/tests/effect.c b/dlls/d3dx9_36/tests/effect.c index b9e89a6519..f175d84d60 100644 --- a/dlls/d3dx9_36/tests/effect.c +++ b/dlls/d3dx9_36/tests/effect.c @@ -8059,9 +8059,9 @@ static void test_effect_parameter_block(void) ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr);
hr = effect->lpVtbl->BeginParameterBlock(effect); - todo_wine ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); hr = effect->lpVtbl->BeginParameterBlock(effect); - todo_wine ok(hr == D3DERR_INVALIDCALL, "Got unexpected hr %#x.\n", hr); + ok(hr == D3DERR_INVALIDCALL, "Got unexpected hr %#x.\n", hr); block = effect->lpVtbl->EndParameterBlock(effect); todo_wine ok(!!block, "Got unexpected block %p.\n", block); handle = effect->lpVtbl->EndParameterBlock(effect);
Signed-off-by: Paul Gofman gofmanp@gmail.com --- dlls/d3dx9_36/effect.c | 26 +++++++++++++++++++++++--- dlls/d3dx9_36/tests/effect.c | 10 +++++----- 2 files changed, 28 insertions(+), 8 deletions(-)
diff --git a/dlls/d3dx9_36/effect.c b/dlls/d3dx9_36/effect.c index 19c9e552c7..d26e451958 100644 --- a/dlls/d3dx9_36/effect.c +++ b/dlls/d3dx9_36/effect.c @@ -24,6 +24,7 @@ #include "d3dx9_private.h" #include "d3dcompiler.h" #include "winternl.h" +#include "wine/list.h"
/* Constants for special INT/FLOAT conversation */ #define INT_FLOAT_MULTI 255.0f @@ -151,6 +152,7 @@ struct d3dx_technique struct d3dx_parameter_block { char magic_string[ARRAY_SIZE(parameter_block_magic_string)]; + struct list entry; };
struct d3dx_effect @@ -184,6 +186,7 @@ struct d3dx_effect D3DMATERIAL9 current_material; BOOL material_updated;
+ struct list parameter_block_list; struct d3dx_parameter_block *current_parameter_block; };
@@ -688,12 +691,18 @@ static void free_parameter_block(struct d3dx_parameter_block *block)
static void d3dx_effect_cleanup(struct d3dx_effect *effect) { + struct d3dx_parameter_block *block, *cursor; ID3DXEffectPool *pool; unsigned int i;
TRACE("effect %p.\n", effect);
free_parameter_block(effect->current_parameter_block); + LIST_FOR_EACH_ENTRY_SAFE(block, cursor, &effect->parameter_block_list, struct d3dx_parameter_block, entry) + { + list_remove(&block->entry); + free_parameter_block(block); + }
heap_free(effect->full_name_tmp);
@@ -4084,11 +4093,20 @@ static HRESULT WINAPI d3dx_effect_BeginParameterBlock(ID3DXEffect *iface)
static D3DXHANDLE WINAPI d3dx_effect_EndParameterBlock(ID3DXEffect *iface) { - struct d3dx_effect *This = impl_from_ID3DXEffect(iface); + struct d3dx_effect *effect = impl_from_ID3DXEffect(iface); + struct d3dx_parameter_block *ret;
- FIXME("(%p)->(): stub\n", This); + TRACE("iface %p.\n", iface);
- return NULL; + if (!effect->current_parameter_block) + { + WARN("No active parameter block.\n"); + return NULL; + } + ret = effect->current_parameter_block; + effect->current_parameter_block = NULL; + list_add_tail(&effect->parameter_block_list, &ret->entry); + return (D3DXHANDLE)ret; }
static HRESULT WINAPI d3dx_effect_ApplyParameterBlock(ID3DXEffect *iface, D3DXHANDLE parameter_block) @@ -6234,6 +6252,8 @@ static HRESULT d3dx9_effect_init(struct d3dx_effect *effect, struct IDirect3DDev
effect->flags = eflags;
+ list_init(&effect->parameter_block_list); + read_dword(&ptr, &tag); TRACE("Tag: %x\n", tag);
diff --git a/dlls/d3dx9_36/tests/effect.c b/dlls/d3dx9_36/tests/effect.c index f175d84d60..23b4d04487 100644 --- a/dlls/d3dx9_36/tests/effect.c +++ b/dlls/d3dx9_36/tests/effect.c @@ -8063,7 +8063,7 @@ static void test_effect_parameter_block(void) hr = effect->lpVtbl->BeginParameterBlock(effect); ok(hr == D3DERR_INVALIDCALL, "Got unexpected hr %#x.\n", hr); block = effect->lpVtbl->EndParameterBlock(effect); - todo_wine ok(!!block, "Got unexpected block %p.\n", block); + ok(!!block, "Got unexpected block %p.\n", block); handle = effect->lpVtbl->EndParameterBlock(effect); ok(!handle, "Got unexpected handle %p.\n", handle);
@@ -8078,13 +8078,13 @@ static void test_effect_parameter_block(void) todo_wine ok(hr == D3D_OK, "Got result %#x.\n", hr);
hr = effect->lpVtbl->BeginParameterBlock(effect); - todo_wine ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); hr = effect->lpVtbl->SetFloat(effect, "vec3[0]", 1001.0f); ok(hr == D3DERR_INVALIDCALL, "Got unexpected hr %#x.\n", hr); hr = effect->lpVtbl->SetFloat(effect, "arr1[0]", 91.0f); ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); block = effect->lpVtbl->EndParameterBlock(effect); - todo_wine ok(!!block, "Got unexpected block %p.\n", block); + ok(!!block, "Got unexpected block %p.\n", block); hr = effect->lpVtbl->ApplyParameterBlock(effect, block); todo_wine ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr);
@@ -8100,7 +8100,7 @@ static void test_effect_parameter_block(void) ok(hr == D3D_OK, "Got result %#x, expected 0 (D3D_OK).\n", hr);
hr = effect->lpVtbl->BeginParameterBlock(effect); - todo_wine ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr);
hr = effect->lpVtbl->SetTexture(effect, "tex1", (IDirect3DBaseTexture9 *)texture); ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); @@ -8137,7 +8137,7 @@ static void test_effect_parameter_block(void) ok(refcount == 2, "Got unexpected refcount %u.\n", refcount);
block = effect->lpVtbl->EndParameterBlock(effect); - todo_wine ok(!!block, "Got unexpected block %p.\n", block); + ok(!!block, "Got unexpected block %p.\n", block);
IDirect3DTexture9_AddRef(texture); refcount = IDirect3DTexture9_Release(texture);
Signed-off-by: Paul Gofman gofmanp@gmail.com --- dlls/d3dx9_36/effect.c | 30 +++++++++++++++++++++++++++--- dlls/d3dx9_36/tests/effect.c | 8 ++++---- 2 files changed, 31 insertions(+), 7 deletions(-)
diff --git a/dlls/d3dx9_36/effect.c b/dlls/d3dx9_36/effect.c index d26e451958..70c437c44f 100644 --- a/dlls/d3dx9_36/effect.c +++ b/dlls/d3dx9_36/effect.c @@ -509,6 +509,14 @@ static struct d3dx_parameter *get_valid_parameter(struct d3dx_effect *effect, D3 return effect->flags & D3DXFX_LARGEADDRESSAWARE ? NULL : get_parameter_by_name(effect, NULL, parameter); }
+static struct d3dx_parameter_block *get_valid_parameter_block(D3DXHANDLE handle) +{ + struct d3dx_parameter_block *block = (struct d3dx_parameter_block *)handle; + + return block && !strncmp(block->magic_string, parameter_block_magic_string, + sizeof(parameter_block_magic_string)) ? block : NULL; +} + static void free_state(struct d3dx_state *state) { free_parameter(&state->parameter, FALSE, FALSE); @@ -4121,11 +4129,27 @@ static HRESULT WINAPI d3dx_effect_ApplyParameterBlock(ID3DXEffect *iface, D3DXHA #if D3DX_SDK_VERSION >= 26 static HRESULT WINAPI d3dx_effect_DeleteParameterBlock(ID3DXEffect *iface, D3DXHANDLE parameter_block) { - struct d3dx_effect *This = impl_from_ID3DXEffect(iface); + struct d3dx_parameter_block *block = get_valid_parameter_block(parameter_block); + struct d3dx_effect *effect = impl_from_ID3DXEffect(iface); + struct d3dx_parameter_block *b;
- FIXME("(%p)->(%p): stub\n", This, parameter_block); + TRACE("iface %p, parameter_block %p.\n", iface, parameter_block);
- return E_NOTIMPL; + if (!block) + return D3DERR_INVALIDCALL; + + LIST_FOR_EACH_ENTRY(b, &effect->parameter_block_list, struct d3dx_parameter_block, entry) + { + if (b == block) + { + list_remove(&b->entry); + free_parameter_block(b); + return D3D_OK; + } + } + + WARN("Block is not found in issued block list, not freeing memory.\n"); + return D3DERR_INVALIDCALL; } #endif
diff --git a/dlls/d3dx9_36/tests/effect.c b/dlls/d3dx9_36/tests/effect.c index 23b4d04487..1732a77541 100644 --- a/dlls/d3dx9_36/tests/effect.c +++ b/dlls/d3dx9_36/tests/effect.c @@ -8075,7 +8075,7 @@ static void test_effect_parameter_block(void) hr = effect->lpVtbl->ApplyParameterBlock(effect, block); todo_wine ok(hr == D3DERR_INVALIDCALL, "Got unexpected hr %#x.\n", hr); hr = effect->lpVtbl->DeleteParameterBlock(effect, block); - todo_wine ok(hr == D3D_OK, "Got result %#x.\n", hr); + ok(hr == D3D_OK, "Got result %#x.\n", hr);
hr = effect->lpVtbl->BeginParameterBlock(effect); ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); @@ -8089,9 +8089,9 @@ static void test_effect_parameter_block(void) todo_wine ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr);
hr = effect->lpVtbl->DeleteParameterBlock(effect2, block); - todo_wine ok(hr == D3DERR_INVALIDCALL, "Got result %#x.\n", hr); + ok(hr == D3DERR_INVALIDCALL, "Got result %#x.\n", hr); hr = effect->lpVtbl->DeleteParameterBlock(effect, block); - todo_wine ok(hr == D3D_OK, "Got result %#x.\n", hr); + ok(hr == D3D_OK, "Got result %#x.\n", hr);
hr = effect->lpVtbl->ApplyParameterBlock(effect, "parameter_block"); todo_wine ok(hr == D3DERR_INVALIDCALL, "Got unexpected hr %#x.\n", hr); @@ -8200,7 +8200,7 @@ static void test_effect_parameter_block(void) float_array[0], float_array[1], float_array[2], float_array[3]);
hr = effect->lpVtbl->DeleteParameterBlock(effect, block); - todo_wine ok(hr == D3D_OK, "Got result %#x.\n", hr); + ok(hr == D3D_OK, "Got result %#x.\n", hr);
IDirect3DTexture9_AddRef(texture);
On Thu, Nov 7, 2019 at 10:19 PM Paul Gofman gofmanp@gmail.com wrote:
@@ -4121,11 +4129,27 @@ static HRESULT WINAPI d3dx_effect_ApplyParameterBlock(ID3DXEffect *iface, D3DXHA #if D3DX_SDK_VERSION >= 26 static HRESULT WINAPI d3dx_effect_DeleteParameterBlock(ID3DXEffect *iface, D3DXHANDLE parameter_block) {
Any idea what is the behavior with version < 26? Was the allocated memory just dropped on the floor?
- struct d3dx_effect *This = impl_from_ID3DXEffect(iface);
- struct d3dx_parameter_block *block = get_valid_parameter_block(parameter_block);
- struct d3dx_effect *effect = impl_from_ID3DXEffect(iface);
- struct d3dx_parameter_block *b;
- FIXME("(%p)->(%p): stub\n", This, parameter_block);
- TRACE("iface %p, parameter_block %p.\n", iface, parameter_block);
- return E_NOTIMPL;
- if (!block)
return D3DERR_INVALIDCALL;- LIST_FOR_EACH_ENTRY(b, &effect->parameter_block_list, struct d3dx_parameter_block, entry)
- {
if (b == block){list_remove(&b->entry);free_parameter_block(b);return D3D_OK;}- }
- WARN("Block is not found in issued block list, not freeing memory.\n");
- return D3DERR_INVALIDCALL;
} #endif
What happens if the parameter block is currently active?
On 11/12/19 21:51, Matteo Bruni wrote:
On Thu, Nov 7, 2019 at 10:19 PM Paul Gofman gofmanp@gmail.com wrote:
@@ -4121,11 +4129,27 @@ static HRESULT WINAPI d3dx_effect_ApplyParameterBlock(ID3DXEffect *iface, D3DXHA #if D3DX_SDK_VERSION >= 26 static HRESULT WINAPI d3dx_effect_DeleteParameterBlock(ID3DXEffect *iface, D3DXHANDLE parameter_block) {
Any idea what is the behavior with version < 26? Was the allocated memory just dropped on the floor?
As I interpret the test, the parameter block is deleted upon effect delete (I decided so by checking object reference: the texture looses the last effect's reference when the effect is deleted without explicit parameter block deletion). So I guess the earlier versions did not have explicit DeleteParameterBlock(), but all allocated block got deleted on effect release.
- struct d3dx_effect *This = impl_from_ID3DXEffect(iface);
- struct d3dx_parameter_block *block = get_valid_parameter_block(parameter_block);
- struct d3dx_effect *effect = impl_from_ID3DXEffect(iface);
- struct d3dx_parameter_block *b;
- FIXME("(%p)->(%p): stub\n", This, parameter_block);
- TRACE("iface %p, parameter_block %p.\n", iface, parameter_block);
- return E_NOTIMPL;
- if (!block)
return D3DERR_INVALIDCALL;- LIST_FOR_EACH_ENTRY(b, &effect->parameter_block_list, struct d3dx_parameter_block, entry)
- {
if (b == block){list_remove(&b->entry);free_parameter_block(b);return D3D_OK;}- }
- WARN("Block is not found in issued block list, not freeing memory.\n");
- return D3DERR_INVALIDCALL;
} #endif
What happens if the parameter block is currently active?
I am not sure what do you mean by active block? As far as understood, the block does not have active state: applying it is equivalent to separately setting all the parameters it contains, the block is just a collection of parameter values. The block which is currently being recorded cannot be freed here, as the application did not get its handle yet, it will get it only when it calls EndParameterBlock(), and at that moment the block is finalized and the recording of the block stops, it is not a current recording block anymore. If you mean what happens if someone will try to apply parameter block which was previously deleted here, I did not test that, but I don't see yet how this can result in anything but use after free in native d3dx.
On Tue, Nov 12, 2019 at 8:19 PM Paul Gofman gofmanp@gmail.com wrote:
On 11/12/19 21:51, Matteo Bruni wrote:
On Thu, Nov 7, 2019 at 10:19 PM Paul Gofman gofmanp@gmail.com wrote:
@@ -4121,11 +4129,27 @@ static HRESULT WINAPI d3dx_effect_ApplyParameterBlock(ID3DXEffect *iface, D3DXHA #if D3DX_SDK_VERSION >= 26 static HRESULT WINAPI d3dx_effect_DeleteParameterBlock(ID3DXEffect *iface, D3DXHANDLE parameter_block) {
Any idea what is the behavior with version < 26? Was the allocated memory just dropped on the floor?
As I interpret the test, the parameter block is deleted upon effect delete (I decided so by checking object reference: the texture looses the last effect's reference when the effect is deleted without explicit parameter block deletion). So I guess the earlier versions did not have explicit DeleteParameterBlock(), but all allocated block got deleted on effect release.
Right, it makes sense.
- struct d3dx_effect *This = impl_from_ID3DXEffect(iface);
- struct d3dx_parameter_block *block = get_valid_parameter_block(parameter_block);
- struct d3dx_effect *effect = impl_from_ID3DXEffect(iface);
- struct d3dx_parameter_block *b;
- FIXME("(%p)->(%p): stub\n", This, parameter_block);
- TRACE("iface %p, parameter_block %p.\n", iface, parameter_block);
- return E_NOTIMPL;
- if (!block)
return D3DERR_INVALIDCALL;- LIST_FOR_EACH_ENTRY(b, &effect->parameter_block_list, struct d3dx_parameter_block, entry)
- {
if (b == block){list_remove(&b->entry);free_parameter_block(b);return D3D_OK;}- }
- WARN("Block is not found in issued block list, not freeing memory.\n");
- return D3DERR_INVALIDCALL;
} #endif
What happens if the parameter block is currently active?
I am not sure what do you mean by active block? As far as understood, the block does not have active state: applying it is equivalent to separately setting all the parameters it contains, the block is just a collection of parameter values. The block which is currently being recorded cannot be freed here, as the application did not get its handle yet, it will get it only when it calls EndParameterBlock(), and at that moment the block is finalized and the recording of the block stops, it is not a current recording block anymore. If you mean what happens if someone will try to apply parameter block which was previously deleted here, I did not test that, but I don't see yet how this can result in anything but use after free in native d3dx.
Eh, good point. Also you're already freeing the current block if the effect is released while recording so all is fine.
Signed-off-by: Paul Gofman gofmanp@gmail.com --- dlls/d3dx9_36/effect.c | 130 ++++++++++++++++++++++------------------- 1 file changed, 69 insertions(+), 61 deletions(-)
diff --git a/dlls/d3dx9_36/effect.c b/dlls/d3dx9_36/effect.c index 70c437c44f..6e996e5fae 100644 --- a/dlls/d3dx9_36/effect.c +++ b/dlls/d3dx9_36/effect.c @@ -827,6 +827,73 @@ static void set_matrix_transpose(struct d3dx_parameter *param, const D3DXMATRIX } }
+static HRESULT set_string(char **param_data, const char *string) +{ + HeapFree(GetProcessHeap(), 0, *param_data); + *param_data = HeapAlloc(GetProcessHeap(), 0, strlen(string) + 1); + if (!*param_data) + { + ERR("Out of memory.\n"); + return E_OUTOFMEMORY; + } + strcpy(*param_data, string); + return D3D_OK; +} + +static HRESULT set_value(struct d3dx_parameter *param, const void *data, unsigned int bytes) +{ + unsigned int i, count; + + bytes = min(bytes, param->bytes); + count = min(param->element_count ? param->element_count : 1, bytes / sizeof(void *)); + + switch (param->type) + { + case D3DXPT_TEXTURE: + case D3DXPT_TEXTURE1D: + case D3DXPT_TEXTURE2D: + case D3DXPT_TEXTURE3D: + case D3DXPT_TEXTURECUBE: + for (i = 0; i < count; ++i) + { + IUnknown *old_texture = ((IUnknown **)param->data)[i]; + IUnknown *new_texture = ((IUnknown **)data)[i]; + + if (new_texture == old_texture) + continue; + + if (new_texture) + IUnknown_AddRef(new_texture); + if (old_texture) + IUnknown_Release(old_texture); + } + /* fallthrough */ + case D3DXPT_VOID: + case D3DXPT_BOOL: + case D3DXPT_INT: + case D3DXPT_FLOAT: + TRACE("Copy %u bytes.\n", bytes); + memcpy(param->data, data, bytes); + break; + + case D3DXPT_STRING: + { + HRESULT hr; + + for (i = 0; i < count; ++i) + if (FAILED(hr = set_string(&((char **)param->data)[i], ((const char **)data)[i]))) + return hr; + break; + } + + default: + FIXME("Unhandled type %s.\n", debug_d3dxparameter_type(param->type)); + break; + } + + return D3D_OK; +} + static struct d3dx_parameter *get_parameter_element_by_name(struct d3dx_effect *effect, struct d3dx_parameter *parameter, const char *name) { @@ -1176,19 +1243,6 @@ static void set_dirty(struct d3dx_parameter *param) top_param->update_version = new_update_version; }
-static HRESULT set_string(char **param_data, const char *string) -{ - HeapFree(GetProcessHeap(), 0, *param_data); - *param_data = HeapAlloc(GetProcessHeap(), 0, strlen(string) + 1); - if (!*param_data) - { - ERR("Out of memory.\n"); - return E_OUTOFMEMORY; - } - strcpy(*param_data, string); - return D3D_OK; -} - static void d3dx9_set_light_parameter(enum LIGHT_TYPE op, D3DLIGHT9 *light, void *value) { static const struct @@ -2278,7 +2332,6 @@ static HRESULT WINAPI d3dx_effect_SetValue(ID3DXEffect *iface, D3DXHANDLE parame { struct d3dx_effect *effect = impl_from_ID3DXEffect(iface); struct d3dx_parameter *param = get_valid_parameter(effect, parameter); - unsigned int i;
TRACE("iface %p, parameter %p, data %p, bytes %u.\n", iface, parameter, data, bytes);
@@ -2295,53 +2348,8 @@ static HRESULT WINAPI d3dx_effect_SetValue(ID3DXEffect *iface, D3DXHANDLE parame
if (data && param->bytes <= bytes) { - switch (param->type) - { - case D3DXPT_TEXTURE: - case D3DXPT_TEXTURE1D: - case D3DXPT_TEXTURE2D: - case D3DXPT_TEXTURE3D: - case D3DXPT_TEXTURECUBE: - for (i = 0; i < (param->element_count ? param->element_count : 1); ++i) - { - IUnknown *old_texture = ((IUnknown **)param->data)[i]; - IUnknown *new_texture = ((IUnknown **)data)[i]; - - if (new_texture == old_texture) - continue; - - if (new_texture) - IUnknown_AddRef(new_texture); - if (old_texture) - IUnknown_Release(old_texture); - } - /* fallthrough */ - case D3DXPT_VOID: - case D3DXPT_BOOL: - case D3DXPT_INT: - case D3DXPT_FLOAT: - TRACE("Copy %u bytes.\n", param->bytes); - memcpy(param->data, data, param->bytes); - set_dirty(param); - break; - - case D3DXPT_STRING: - { - HRESULT hr; - - set_dirty(param); - for (i = 0; i < (param->element_count ? param->element_count : 1); ++i) - if (FAILED(hr = set_string(&((char **)param->data)[i], ((const char **)data)[i]))) - return hr; - break; - } - - default: - FIXME("Unhandled type %s.\n", debug_d3dxparameter_type(param->type)); - break; - } - - return D3D_OK; + set_dirty(param); + return set_value(param, data, bytes); }
WARN("Invalid argument specified.\n");
On Thu, Nov 7, 2019 at 10:19 PM Paul Gofman gofmanp@gmail.com wrote:
+static HRESULT set_string(char **param_data, const char *string) +{
- HeapFree(GetProcessHeap(), 0, *param_data);
- *param_data = HeapAlloc(GetProcessHeap(), 0, strlen(string) + 1);
- if (!*param_data)
- {
ERR("Out of memory.\n");return E_OUTOFMEMORY;- }
- strcpy(*param_data, string);
- return D3D_OK;
+}
I know that this is not new code but I have a couple of comments. Another option here would be to only free the old memory after the new allocation succeeds. I guess it's not particularly important since the situation must be pretty hopeless if allocating a string fails. Regardless, you should probably make use of the helpers from heap.h.
Signed-off-by: Paul Gofman gofmanp@gmail.com --- dlls/d3dx9_36/effect.c | 81 ++++++++++++++++++++++++------------------ 1 file changed, 46 insertions(+), 35 deletions(-)
diff --git a/dlls/d3dx9_36/effect.c b/dlls/d3dx9_36/effect.c index 6e996e5fae..64106abebb 100644 --- a/dlls/d3dx9_36/effect.c +++ b/dlls/d3dx9_36/effect.c @@ -840,6 +840,51 @@ static HRESULT set_string(char **param_data, const char *string) return D3D_OK; }
+static void get_value(struct d3dx_parameter *param, void *data, const void *input_data, unsigned int bytes) +{ + bytes = min(bytes, param->bytes); + if (!input_data) + input_data = param->data; + + switch (param->type) + { + case D3DXPT_VOID: + case D3DXPT_BOOL: + case D3DXPT_INT: + case D3DXPT_FLOAT: + case D3DXPT_STRING: + break; + + case D3DXPT_VERTEXSHADER: + case D3DXPT_PIXELSHADER: + case D3DXPT_TEXTURE: + case D3DXPT_TEXTURE1D: + case D3DXPT_TEXTURE2D: + case D3DXPT_TEXTURE3D: + case D3DXPT_TEXTURECUBE: + { + unsigned int i, count; + + count = min(param->element_count ? param->element_count : 1, bytes / sizeof(void *)); + + for (i = 0; i < count; ++i) + { + IUnknown *unk = ((IUnknown **)input_data)[i]; + if (unk) + IUnknown_AddRef(unk); + } + break; + } + + default: + FIXME("Unhandled type %s.\n", debug_d3dxparameter_type(param->type)); + break; + } + + TRACE("Copy %u bytes.\n", bytes); + memcpy(data, input_data, bytes); +} + static HRESULT set_value(struct d3dx_parameter *param, const void *data, unsigned int bytes) { unsigned int i, count; @@ -2379,41 +2424,7 @@ static HRESULT WINAPI d3dx_effect_GetValue(ID3DXEffect *iface, D3DXHANDLE parame { TRACE("Type %s.\n", debug_d3dxparameter_type(param->type));
- switch (param->type) - { - case D3DXPT_VOID: - case D3DXPT_BOOL: - case D3DXPT_INT: - case D3DXPT_FLOAT: - case D3DXPT_STRING: - break; - - case D3DXPT_VERTEXSHADER: - case D3DXPT_PIXELSHADER: - case D3DXPT_TEXTURE: - case D3DXPT_TEXTURE1D: - case D3DXPT_TEXTURE2D: - case D3DXPT_TEXTURE3D: - case D3DXPT_TEXTURECUBE: - { - unsigned int i; - - for (i = 0; i < (param->element_count ? param->element_count : 1); ++i) - { - IUnknown *unk = ((IUnknown **)param->data)[i]; - if (unk) - IUnknown_AddRef(unk); - } - break; - } - - default: - FIXME("Unhandled type %s.\n", debug_d3dxparameter_type(param->type)); - break; - } - - TRACE("Copy %u bytes.\n", param->bytes); - memcpy(data, param->data, param->bytes); + get_value(param, data, NULL, bytes); return D3D_OK; }
Signed-off-by: Paul Gofman gofmanp@gmail.com --- dlls/d3dx9_36/effect.c | 29 +++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-)
diff --git a/dlls/d3dx9_36/effect.c b/dlls/d3dx9_36/effect.c index 64106abebb..4739b0cc0e 100644 --- a/dlls/d3dx9_36/effect.c +++ b/dlls/d3dx9_36/effect.c @@ -540,16 +540,21 @@ static void free_sampler(struct d3dx_sampler *sampler)
static void d3dx_pool_release_shared_parameter(struct d3dx_top_level_parameter *param);
-static void free_parameter_data(struct d3dx_parameter *param, BOOL child) +static void free_parameter_object_data(struct d3dx_parameter *param, const void *data, unsigned int bytes) { - if (!param->data) + unsigned int i, count; + + if (param->class != D3DXPC_OBJECT) return; - if (param->class == D3DXPC_OBJECT && !param->element_count) + + count = min(param->element_count ? param->element_count : 1, bytes / sizeof(void *)); + + for (i = 0; i < count; ++i) { switch (param->type) { case D3DXPT_STRING: - HeapFree(GetProcessHeap(), 0, *(char **)param->data); + HeapFree(GetProcessHeap(), 0, ((char **)data)[i]); break;
case D3DXPT_TEXTURE: @@ -559,7 +564,8 @@ static void free_parameter_data(struct d3dx_parameter *param, BOOL child) case D3DXPT_TEXTURECUBE: case D3DXPT_PIXELSHADER: case D3DXPT_VERTEXSHADER: - if (*(IUnknown **)param->data) IUnknown_Release(*(IUnknown **)param->data); + if (*(IUnknown **)data) + IUnknown_Release(((IUnknown **)data)[i]); break;
case D3DXPT_SAMPLER: @@ -567,7 +573,8 @@ static void free_parameter_data(struct d3dx_parameter *param, BOOL child) case D3DXPT_SAMPLER2D: case D3DXPT_SAMPLER3D: case D3DXPT_SAMPLERCUBE: - free_sampler((struct d3dx_sampler *)param->data); + assert(count == 1); + free_sampler((struct d3dx_sampler *)data); return;
default: @@ -575,6 +582,16 @@ static void free_parameter_data(struct d3dx_parameter *param, BOOL child) break; } } +} + +static void free_parameter_data(struct d3dx_parameter *param, BOOL child) +{ + if (!param->data) + return; + + if (!param->element_count) + free_parameter_object_data(param, param->data, param->bytes); + if (!child || is_param_type_sampler(param->type)) HeapFree(GetProcessHeap(), 0, param->data); }
Signed-off-by: Paul Gofman gofmanp@gmail.com --- dlls/d3dx9_36/effect.c | 111 +++++++++++++++++++++++++++++++++++ dlls/d3dx9_36/tests/effect.c | 6 +- 2 files changed, 114 insertions(+), 3 deletions(-)
diff --git a/dlls/d3dx9_36/effect.c b/dlls/d3dx9_36/effect.c index 4739b0cc0e..7a7fc336e1 100644 --- a/dlls/d3dx9_36/effect.c +++ b/dlls/d3dx9_36/effect.c @@ -36,6 +36,7 @@ static const char parameter_block_magic_string[4] = {'@', '!', '#', '\xFE'}; #define PARAMETER_FLAG_SHARED 1
#define INITIAL_POOL_SIZE 16 +#define INITIAL_PARAM_BLOCK_SIZE 1024
WINE_DEFAULT_DEBUG_CHANNEL(d3dx);
@@ -153,6 +154,15 @@ struct d3dx_parameter_block { char magic_string[ARRAY_SIZE(parameter_block_magic_string)]; struct list entry; + size_t buffer_size; + size_t current_size; + BYTE *buffer; +}; + +struct d3dx_recorded_parameter +{ + struct d3dx_parameter *param; + unsigned int bytes; };
struct d3dx_effect @@ -706,11 +716,27 @@ static void free_technique(struct d3dx_technique *technique) technique->name = NULL; }
+static unsigned int get_recorded_parameter_size(const struct d3dx_recorded_parameter *record) +{ + return sizeof(*record) + record->bytes; +} + static void free_parameter_block(struct d3dx_parameter_block *block) { + struct d3dx_recorded_parameter *record; + if (!block) return;
+ record = (struct d3dx_recorded_parameter *)block->buffer; + while ((BYTE *)record < block->buffer + block->current_size) + { + free_parameter_object_data(record->param, record + 1, record->bytes); + record = (struct d3dx_recorded_parameter *)((BYTE *)record + get_recorded_parameter_size(record)); + } + assert((BYTE *)record == block->buffer + block->current_size); + + heap_free(block->buffer); heap_free(block); }
@@ -1293,6 +1319,46 @@ static ULONG64 next_effect_update_version(struct d3dx_effect *effect) return next_update_version(get_version_counter_ptr(effect)); }
+static void record_parameter(struct d3dx_effect *effect, struct d3dx_parameter *param, const void *data, + unsigned int bytes) +{ + struct d3dx_parameter_block *block = effect->current_parameter_block; + struct d3dx_recorded_parameter new_record, *record; + unsigned int new_size, alloc_size; + + if (!block) + return; + + assert(bytes <= param->bytes); + + new_record.param = param; + new_record.bytes = bytes; + new_size = block->current_size + get_recorded_parameter_size(&new_record); + + if (new_size > block->buffer_size) + { + BYTE *new_alloc; + + alloc_size = max(block->buffer_size * 2, max(new_size, INITIAL_PARAM_BLOCK_SIZE)); + if (block->buffer_size) + new_alloc = heap_realloc(block->buffer, alloc_size); + else + new_alloc = heap_alloc(alloc_size); + + if (!new_alloc) + { + ERR("Out of memory.\n"); + return; + } + block->buffer_size = alloc_size; + block->buffer = new_alloc; + } + record = (struct d3dx_recorded_parameter *)(block->buffer + block->current_size); + *record = new_record; + get_value(param, record + 1, data, bytes); + block->current_size = new_size; +} + static void set_dirty(struct d3dx_parameter *param) { struct d3dx_shared_data *shared_data; @@ -2411,6 +2477,7 @@ static HRESULT WINAPI d3dx_effect_SetValue(ID3DXEffect *iface, D3DXHANDLE parame if (data && param->bytes <= bytes) { set_dirty(param); + record_parameter(effect, param, data, param->bytes); return set_value(param, data, bytes); }
@@ -2460,6 +2527,7 @@ static HRESULT WINAPI d3dx_effect_SetBool(ID3DXEffect *iface, D3DXHANDLE paramet if (param && !param->element_count && param->rows == 1 && param->columns == 1) { set_number(param->data, param->type, &b, D3DXPT_BOOL); + record_parameter(effect, param, NULL, sizeof(int)); set_dirty(param); return D3D_OK; } @@ -2511,6 +2579,7 @@ static HRESULT WINAPI d3dx_effect_SetBoolArray(ID3DXEffect *iface, D3DXHANDLE pa /* don't crop the input, use D3DXPT_INT instead of D3DXPT_BOOL */ set_number((DWORD *)param->data + i, param->type, &b[i], D3DXPT_INT); } + record_parameter(effect, param, NULL, count * sizeof(int)); set_dirty(param); return D3D_OK;
@@ -2571,6 +2640,7 @@ static HRESULT WINAPI d3dx_effect_SetInt(ID3DXEffect *iface, D3DXHANDLE paramete set_number(&value, param->type, &n, D3DXPT_INT); if (value != *(DWORD *)param->data) set_dirty(param); + record_parameter(effect, param, &value, sizeof(int)); *(DWORD *)param->data = value; return D3D_OK; } @@ -2587,6 +2657,7 @@ static HRESULT WINAPI d3dx_effect_SetInt(ID3DXEffect *iface, D3DXHANDLE paramete ((float *)param->data)[2] = (n & 0xff) * INT_FLOAT_MULTI_INVERSE; if (param->rows * param->columns > 3) ((float *)param->data)[3] = ((n & 0xff000000) >> 24) * INT_FLOAT_MULTI_INVERSE; + record_parameter(effect, param, NULL, 4 * sizeof(float)); set_dirty(param); return D3D_OK; } @@ -2655,6 +2726,7 @@ static HRESULT WINAPI d3dx_effect_SetIntArray(ID3DXEffect *iface, D3DXHANDLE par case D3DXPC_MATRIX_ROWS: for (i = 0; i < size; ++i) set_number((DWORD *)param->data + i, param->type, &n[i], D3DXPT_INT); + record_parameter(effect, param, NULL, size * sizeof(int)); set_dirty(param); return D3D_OK;
@@ -2712,6 +2784,7 @@ static HRESULT WINAPI d3dx_effect_SetFloat(ID3DXEffect *iface, D3DXHANDLE parame if (value != *(DWORD *)param->data) set_dirty(param); *(DWORD *)param->data = value; + record_parameter(effect, param, NULL, sizeof(float)); return D3D_OK; }
@@ -2760,6 +2833,7 @@ static HRESULT WINAPI d3dx_effect_SetFloatArray(ID3DXEffect *iface, D3DXHANDLE p case D3DXPC_MATRIX_ROWS: for (i = 0; i < size; ++i) set_number((DWORD *)param->data + i, param->type, &f[i], D3DXPT_FLOAT); + record_parameter(effect, param, NULL, size * sizeof(float)); set_dirty(param); return D3D_OK;
@@ -2829,15 +2903,18 @@ static HRESULT WINAPI d3dx_effect_SetVector(ID3DXEffect *iface, D3DXHANDLE param tmp += ((DWORD)(max(min(vector->w, 1.0f), 0.0f) * INT_FLOAT_MULTI)) << 24;
*(int *)param->data = tmp; + record_parameter(effect, param, NULL, sizeof(int)); return D3D_OK; } if (param->type == D3DXPT_FLOAT) { memcpy(param->data, vector, param->columns * sizeof(float)); + record_parameter(effect, param, NULL, param->columns * sizeof(float)); return D3D_OK; }
set_vector(param, vector); + record_parameter(effect, param, NULL, param->columns * sizeof(float)); return D3D_OK;
case D3DXPC_MATRIX_ROWS: @@ -2920,16 +2997,23 @@ static HRESULT WINAPI d3dx_effect_SetVectorArray(ID3DXEffect *iface, D3DXHANDLE if (param->type == D3DXPT_FLOAT) { if (param->columns == 4) + { memcpy(param->data, vector, count * 4 * sizeof(float)); + record_parameter(effect, param, NULL, count * 4 * sizeof(float)); + } else + { for (i = 0; i < count; ++i) memcpy((float *)param->data + param->columns * i, vector + i, param->columns * sizeof(float)); + record_parameter(effect, param, NULL, count * param->columns * sizeof(float)); + } return D3D_OK; }
for (i = 0; i < count; ++i) set_vector(¶m->members[i], &vector[i]); + record_parameter(effect, param, NULL, count * param->columns * sizeof(float)); return D3D_OK;
case D3DXPC_SCALAR: @@ -3005,6 +3089,7 @@ static HRESULT WINAPI d3dx_effect_SetMatrix(ID3DXEffect *iface, D3DXHANDLE param { case D3DXPC_MATRIX_ROWS: set_matrix(param, matrix); + record_parameter(effect, param, NULL, param->rows * param->columns * sizeof(float)); set_dirty(param); return D3D_OK;
@@ -3078,7 +3163,11 @@ static HRESULT WINAPI d3dx_effect_SetMatrixArray(ID3DXEffect *iface, D3DXHANDLE case D3DXPC_MATRIX_ROWS: set_dirty(param); for (i = 0; i < count; ++i) + { set_matrix(¶m->members[i], &matrix[i]); + record_parameter(effect, ¶m->members[i], NULL, param->members[i].rows + * param->members[i].columns * sizeof(float)); + } return D3D_OK;
case D3DXPC_SCALAR: @@ -3156,7 +3245,11 @@ static HRESULT WINAPI d3dx_effect_SetMatrixPointerArray(ID3DXEffect *iface, D3DX case D3DXPC_MATRIX_ROWS: set_dirty(param); for (i = 0; i < count; ++i) + { set_matrix(¶m->members[i], matrix[i]); + record_parameter(effect, ¶m->members[i], NULL, param->members[i].rows + * param->members[i].columns * sizeof(float)); + } return D3D_OK;
case D3DXPC_SCALAR: @@ -3232,6 +3325,7 @@ static HRESULT WINAPI d3dx_effect_SetMatrixTranspose(ID3DXEffect *iface, D3DXHAN case D3DXPC_MATRIX_ROWS: set_dirty(param); set_matrix_transpose(param, matrix); + record_parameter(effect, param, NULL, param->rows * param->columns * sizeof(float)); return D3D_OK;
case D3DXPC_SCALAR: @@ -3308,7 +3402,11 @@ static HRESULT WINAPI d3dx_effect_SetMatrixTransposeArray(ID3DXEffect *iface, D3 case D3DXPC_MATRIX_ROWS: set_dirty(param); for (i = 0; i < count; ++i) + { set_matrix_transpose(¶m->members[i], &matrix[i]); + record_parameter(effect, ¶m->members[i], NULL, param->members[i].rows + * param->members[i].columns * sizeof(float)); + } return D3D_OK;
case D3DXPC_SCALAR: @@ -3386,7 +3484,11 @@ static HRESULT WINAPI d3dx_effect_SetMatrixTransposePointerArray(ID3DXEffect *if case D3DXPC_MATRIX_ROWS: set_dirty(param); for (i = 0; i < count; ++i) + { set_matrix_transpose(¶m->members[i], matrix[i]); + record_parameter(effect, ¶m->members[i], NULL, param->members[i].rows + * param->members[i].columns * sizeof(float)); + } return D3D_OK;
case D3DXPC_SCALAR: @@ -3455,6 +3557,10 @@ static HRESULT WINAPI d3dx_effect_SetString(ID3DXEffect *iface, D3DXHANDLE param if (param && param->type == D3DXPT_STRING) { set_dirty(param); + + if (effect->current_parameter_block) + FIXME("Recording string parameters not supported.\n"); + return set_string(param->data, string); }
@@ -3507,6 +3613,7 @@ static HRESULT WINAPI d3dx_effect_SetTexture(ID3DXEffect *iface, D3DXHANDLE para
*(IDirect3DBaseTexture9 **)param->data = texture; set_dirty(param); + record_parameter(effect, param, NULL, sizeof(void *));
return D3D_OK; } @@ -4148,6 +4255,10 @@ static D3DXHANDLE WINAPI d3dx_effect_EndParameterBlock(ID3DXEffect *iface) return NULL; } ret = effect->current_parameter_block; + + ret->buffer = heap_realloc(ret->buffer, ret->current_size); + ret->buffer_size = ret->current_size; + effect->current_parameter_block = NULL; list_add_tail(&effect->parameter_block_list, &ret->entry); return (D3DXHANDLE)ret; diff --git a/dlls/d3dx9_36/tests/effect.c b/dlls/d3dx9_36/tests/effect.c index 1732a77541..d24e04a753 100644 --- a/dlls/d3dx9_36/tests/effect.c +++ b/dlls/d3dx9_36/tests/effect.c @@ -8134,21 +8134,21 @@ static void test_effect_parameter_block(void) * Maybe native d3dx is using some copy on write strategy. */ IDirect3DTexture9_AddRef(texture); refcount = IDirect3DTexture9_Release(texture); - ok(refcount == 2, "Got unexpected refcount %u.\n", refcount); + todo_wine ok(refcount == 2, "Got unexpected refcount %u.\n", refcount);
block = effect->lpVtbl->EndParameterBlock(effect); ok(!!block, "Got unexpected block %p.\n", block);
IDirect3DTexture9_AddRef(texture); refcount = IDirect3DTexture9_Release(texture); - ok(refcount == 2, "Got unexpected refcount %u.\n", refcount); + todo_wine ok(refcount == 2, "Got unexpected refcount %u.\n", refcount);
hr = effect->lpVtbl->SetTexture(effect, "tex1", NULL); ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr);
IDirect3DTexture9_AddRef(texture); refcount = IDirect3DTexture9_Release(texture); - todo_wine ok(refcount == 2, "Got unexpected refcount %u.\n", refcount); + ok(refcount == 2, "Got unexpected refcount %u.\n", refcount);
hr = effect->lpVtbl->SetFloat(effect, "arr2[0]", 0.0f); ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr);
Signed-off-by: Paul Gofman gofmanp@gmail.com --- dlls/d3dx9_36/effect.c | 18 +++++++++++++++--- dlls/d3dx9_36/tests/effect.c | 20 ++++++++++---------- 2 files changed, 25 insertions(+), 13 deletions(-)
diff --git a/dlls/d3dx9_36/effect.c b/dlls/d3dx9_36/effect.c index 7a7fc336e1..3cf7e39f4a 100644 --- a/dlls/d3dx9_36/effect.c +++ b/dlls/d3dx9_36/effect.c @@ -4266,11 +4266,23 @@ static D3DXHANDLE WINAPI d3dx_effect_EndParameterBlock(ID3DXEffect *iface)
static HRESULT WINAPI d3dx_effect_ApplyParameterBlock(ID3DXEffect *iface, D3DXHANDLE parameter_block) { - struct d3dx_effect *This = impl_from_ID3DXEffect(iface); + struct d3dx_parameter_block *block = get_valid_parameter_block(parameter_block); + struct d3dx_recorded_parameter *record;
- FIXME("(%p)->(%p): stub\n", This, parameter_block); + TRACE("iface %p, paramater_block %p.\n", iface, parameter_block);
- return E_NOTIMPL; + if (!block || !block->current_size) + return D3DERR_INVALIDCALL; + + record = (struct d3dx_recorded_parameter *)block->buffer; + while ((BYTE *)record < block->buffer + block->current_size) + { + set_value(record->param, record + 1, record->bytes); + set_dirty(record->param); + record = (struct d3dx_recorded_parameter *)((BYTE *)record + get_recorded_parameter_size(record)); + } + assert((BYTE *)record == block->buffer + block->current_size); + return D3D_OK; }
#if D3DX_SDK_VERSION >= 26 diff --git a/dlls/d3dx9_36/tests/effect.c b/dlls/d3dx9_36/tests/effect.c index d24e04a753..a61a288420 100644 --- a/dlls/d3dx9_36/tests/effect.c +++ b/dlls/d3dx9_36/tests/effect.c @@ -8073,7 +8073,7 @@ static void test_effect_parameter_block(void) ok(refcount == 1, "Got unexpected refcount %u.\n", refcount);
hr = effect->lpVtbl->ApplyParameterBlock(effect, block); - todo_wine ok(hr == D3DERR_INVALIDCALL, "Got unexpected hr %#x.\n", hr); + ok(hr == D3DERR_INVALIDCALL, "Got unexpected hr %#x.\n", hr); hr = effect->lpVtbl->DeleteParameterBlock(effect, block); ok(hr == D3D_OK, "Got result %#x.\n", hr);
@@ -8086,7 +8086,7 @@ static void test_effect_parameter_block(void) block = effect->lpVtbl->EndParameterBlock(effect); ok(!!block, "Got unexpected block %p.\n", block); hr = effect->lpVtbl->ApplyParameterBlock(effect, block); - todo_wine ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr);
hr = effect->lpVtbl->DeleteParameterBlock(effect2, block); ok(hr == D3DERR_INVALIDCALL, "Got result %#x.\n", hr); @@ -8094,7 +8094,7 @@ static void test_effect_parameter_block(void) ok(hr == D3D_OK, "Got result %#x.\n", hr);
hr = effect->lpVtbl->ApplyParameterBlock(effect, "parameter_block"); - todo_wine ok(hr == D3DERR_INVALIDCALL, "Got unexpected hr %#x.\n", hr); + ok(hr == D3DERR_INVALIDCALL, "Got unexpected hr %#x.\n", hr);
hr = D3DXCreateTexture(device, D3DX_DEFAULT, D3DX_DEFAULT, D3DX_DEFAULT, 0, 0, D3DPOOL_DEFAULT, &texture); ok(hr == D3D_OK, "Got result %#x, expected 0 (D3D_OK).\n", hr); @@ -8168,14 +8168,14 @@ static void test_effect_parameter_block(void)
hr = effect->lpVtbl->ApplyParameterBlock(effect, block); - todo_wine ok(hr == D3D_OK, "Got result %#x.\n", hr); + ok(hr == D3D_OK, "Got result %#x.\n", hr);
IDirect3DTexture9_AddRef(texture); refcount = IDirect3DTexture9_Release(texture); - todo_wine ok(refcount == 3, "Got unexpected refcount %u.\n", refcount); + ok(refcount == 3, "Got unexpected refcount %u.\n", refcount);
hr = effect->lpVtbl->GetFloat(effect, "arr2[0]", &float_value); - todo_wine ok(hr == D3D_OK && float_value == 92.0f, "Got unexpected hr %#x, float_value %g.\n", hr, float_value); + ok(hr == D3D_OK && float_value == 92.0f, "Got unexpected hr %#x, float_value %g.\n", hr, float_value); hr = effect->lpVtbl->GetFloat(effect, "arr2[1]", &float_value); ok(hr == D3D_OK && float_value == 0.0f, "Got unexpected hr %#x, float_value %g.\n", hr, float_value); hr = effect->lpVtbl->GetFloatArray(effect, "ts1[0].v1", float_array, 3); @@ -8186,15 +8186,15 @@ static void test_effect_parameter_block(void) hr = effect->lpVtbl->GetFloat(effect, "ts1[0].fv", &float_value); hr = effect->lpVtbl->GetMatrix(effect, "m3x2row", &mat); ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); - todo_wine ok(!memcmp(&mat, &test_mat, sizeof(mat)), "Got unexpected matrix.\n"); + ok(!memcmp(&mat, &test_mat, sizeof(mat)), "Got unexpected matrix.\n"); hr = effect->lpVtbl->GetMatrix(effect, "m3x2column", &mat); ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); - todo_wine ok(!memcmp(&mat, &test_mat, sizeof(mat)), "Got unexpected matrix.\n"); + ok(!memcmp(&mat, &test_mat, sizeof(mat)), "Got unexpected matrix.\n");
- todo_wine ok(hr == D3D_OK && float_value == 28.0f, "Got unexpected hr %#x, float_value %g.\n", hr, float_value); + ok(hr == D3D_OK && float_value == 28.0f, "Got unexpected hr %#x, float_value %g.\n", hr, float_value); hr = effect->lpVtbl->GetFloatArray(effect, "ts1[0].v2", float_array, 4); ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); - todo_wine ok(hr == D3D_OK && float_array[0] == -29.0f + ok(hr == D3D_OK && float_array[0] == -29.0f && !memcmp(float_array + 1, float_array_zero, 3 * sizeof(*float_array)), "Got unexpected hr %#x, ts1[0].v2 (%g, %g, %g, %g).\n", hr, float_array[0], float_array[1], float_array[2], float_array[3]);
First of all, apologies for not looking into this series in detail yet. I have a bunch of questions though.
On Thu, Nov 7, 2019 at 10:18 PM Paul Gofman gofmanp@gmail.com wrote:
Signed-off-by: Paul Gofman gofmanp@gmail.com
dlls/d3dx9_36/effect.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/dlls/d3dx9_36/effect.c b/dlls/d3dx9_36/effect.c index e69a1fd1f8..980313182a 100644 --- a/dlls/d3dx9_36/effect.c +++ b/dlls/d3dx9_36/effect.c @@ -517,7 +517,6 @@ static void free_sampler(struct d3dx_sampler *sampler) free_state(&sampler->states[i]); } HeapFree(GetProcessHeap(), 0, sampler->states);
- HeapFree(GetProcessHeap(), 0, sampler);
}
static void d3dx_pool_release_shared_parameter(struct d3dx_top_level_parameter *param); @@ -557,7 +556,7 @@ static void free_parameter_data(struct d3dx_parameter *param, BOOL child) break; } }
- if (!child)
- if (!child || is_param_type_sampler(param->type)) HeapFree(GetProcessHeap(), 0, param->data);
}
Seems fine but do D3DXPT_STRING parameters also require the same?
On 11/12/19 21:48, Matteo Bruni wrote:
Signed-off-by: Paul Gofman gofmanp@gmail.com
dlls/d3dx9_36/effect.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/dlls/d3dx9_36/effect.c b/dlls/d3dx9_36/effect.c index e69a1fd1f8..980313182a 100644 --- a/dlls/d3dx9_36/effect.c +++ b/dlls/d3dx9_36/effect.c @@ -517,7 +517,6 @@ static void free_sampler(struct d3dx_sampler *sampler) free_state(&sampler->states[i]); } HeapFree(GetProcessHeap(), 0, sampler->states);
- HeapFree(GetProcessHeap(), 0, sampler);
}
static void d3dx_pool_release_shared_parameter(struct d3dx_top_level_parameter *param); @@ -557,7 +556,7 @@ static void free_parameter_data(struct d3dx_parameter *param, BOOL child) break; } }
- if (!child)
- if (!child || is_param_type_sampler(param->type)) HeapFree(GetProcessHeap(), 0, param->data);
}
Seems fine but do D3DXPT_STRING parameters also require the same?
I thought it doesn't. Unless I am missing something obvious the case which frees the strings does that for the pointers stored at 'data' address, it is just the same what happens with the other objects (if they get freed on _Release). Only sampler's case frees the 'data' pointer itself as it points directly to sampler's structure.
-
Matteo Bruni -
Paul Gofman