Signed-off-by: Ziqing Hui zhui@codeweavers.com ---
v3: Add a "default_input_count" field to effect_test. Remove off_limit_tests variable. Add winetest_push_context() to loop. Split a loop into two loops. Use individual todo_wine rather than todo_wine block.
dlls/d2d1/tests/d2d1.c | 82 +++++++++++++++++++++++++++++++++++++++--- 1 file changed, 78 insertions(+), 4 deletions(-)
diff --git a/dlls/d2d1/tests/d2d1.c b/dlls/d2d1/tests/d2d1.c index 629bbd65523..146952d7418 100644 --- a/dlls/d2d1/tests/d2d1.c +++ b/dlls/d2d1/tests/d2d1.c @@ -9680,13 +9680,16 @@ static void test_mt_factory(BOOL d3d11)
static void test_effect(BOOL d3d11) { - unsigned int i, min_inputs, max_inputs, str_size; + unsigned int i, j, min_inputs, max_inputs, str_size, input_count; + D2D1_BITMAP_PROPERTIES bitmap_desc; D2D1_BUFFER_PRECISION precision; ID2D1Image *image_a, *image_b; struct d2d1_test_context ctx; ID2D1DeviceContext *context; ID2D1Factory1 *factory; + ID2D1Bitmap *bitmap; ID2D1Effect *effect; + D2D1_SIZE_U size; BYTE buffer[64]; BOOL cached; CLSID clsid; @@ -9695,14 +9698,15 @@ static void test_effect(BOOL d3d11) const struct effect_test { const CLSID *clsid; + UINT32 default_input_count; UINT32 min_inputs; UINT32 max_inputs; } effect_tests[] = { - {&CLSID_D2D12DAffineTransform, 1, 1}, - {&CLSID_D2D13DPerspectiveTransform, 1, 1}, - {&CLSID_D2D1Composite, 1, 0xffffffff}, + {&CLSID_D2D12DAffineTransform, 1, 1, 1}, + {&CLSID_D2D13DPerspectiveTransform, 1, 1, 1}, + {&CLSID_D2D1Composite, 2, 1, 0xffffffff}, };
if (!init_test_context(&ctx, d3d11)) @@ -9797,6 +9801,76 @@ static void test_effect(BOOL d3d11) max_inputs, test->max_inputs); }
+ input_count = ID2D1Effect_GetInputCount(effect); + todo_wine + ok (input_count == test->default_input_count, "Got unexpected input count %u, expected %u.\n", + input_count, test->default_input_count); + + input_count = (test->max_inputs < 16 ? test->max_inputs : 16); + for (j = 0; j < input_count + 4; ++j) + { + winetest_push_context("Input %u", j); + hr = ID2D1Effect_SetInputCount(effect, j); + if (j < test->min_inputs || j > test->max_inputs) + todo_wine + ok(hr == E_INVALIDARG, "Got unexpected hr %#x.\n", hr); + else + todo_wine + ok(hr == S_OK, "Got unexpected hr %#x.\n", hr); + winetest_pop_context(); + } + + input_count = ID2D1Effect_GetInputCount(effect); + for (j = 0; j < input_count + 4; ++j) + { + winetest_push_context("Input %u", j); + ID2D1Effect_GetInput(effect, j, &image_a); + todo_wine + ok(image_a == NULL, "Got unexpected image_a %p.\n", image_a); + winetest_pop_context(); + } + + set_size_u(&size, 1, 1); + bitmap_desc.pixelFormat.format = DXGI_FORMAT_B8G8R8A8_UNORM; + bitmap_desc.pixelFormat.alphaMode = D2D1_ALPHA_MODE_IGNORE; + bitmap_desc.dpiX = 96.0f; + bitmap_desc.dpiY = 96.0f; + hr = ID2D1RenderTarget_CreateBitmap(ctx.rt, size, NULL, 4, &bitmap_desc, &bitmap); + ok(hr == S_OK, "Got unexpected hr %#x.\n", hr); + + ID2D1Effect_SetInput(effect, 0, (ID2D1Image *)bitmap, FALSE); + for (j = 0; j < input_count + 4; ++j) + { + winetest_push_context("Input %u", j); + image_a = (ID2D1Image *)0xdeadbeef; + ID2D1Effect_GetInput(effect, j, &image_a); + if (j == 0) + { + todo_wine + ok(image_a == (ID2D1Image *)bitmap, "Got unexpected image_a %p.\n", image_a); + if (image_a == (ID2D1Image *)bitmap) + ID2D1Image_Release(image_a); + } + else + { + todo_wine + ok(image_a == NULL, "Got unexpected image_a %p.\n", image_a); + } + winetest_pop_context(); + } + + for (j = input_count; j < input_count + 4; ++j) + { + winetest_push_context("Input %u", j); + image_a = (ID2D1Image *)0xdeadbeef; + ID2D1Effect_SetInput(effect, j, (ID2D1Image *)bitmap, FALSE); + ID2D1Effect_GetInput(effect, j, &image_a); + todo_wine + ok(image_a == NULL, "Got unexpected image_a %p.\n", image_a); + winetest_pop_context(); + } + ID2D1Bitmap_Release(bitmap); + ID2D1Effect_Release(effect); winetest_pop_context(); }
Signed-off-by: Ziqing Hui zhui@codeweavers.com ---
v3: Split the large implementation patch into 4 patches. Use d2d_array_reserve(). Add a return value to d2d_effect_init(). Introduce d2d_effect_cleanup().
dlls/d2d1/d2d1_private.h | 5 ++++- dlls/d2d1/device.c | 8 +++++++- dlls/d2d1/effect.c | 29 +++++++++++++++++++++++++---- dlls/d2d1/tests/d2d1.c | 2 +- 4 files changed, 37 insertions(+), 7 deletions(-)
diff --git a/dlls/d2d1/d2d1_private.h b/dlls/d2d1/d2d1_private.h index ce99e7c3432..48d15dfcf22 100644 --- a/dlls/d2d1/d2d1_private.h +++ b/dlls/d2d1/d2d1_private.h @@ -572,9 +572,12 @@ struct d2d_effect LONG refcount;
ID2D1Factory *factory; + ID2D1Image **inputs; + size_t inputs_size; + size_t input_count; };
-void d2d_effect_init(struct d2d_effect *effect, ID2D1Factory *factory) DECLSPEC_HIDDEN; +HRESULT d2d_effect_init(struct d2d_effect *effect, ID2D1Factory *factory) DECLSPEC_HIDDEN;
static inline BOOL d2d_array_reserve(void **elements, size_t *capacity, size_t count, size_t size) { diff --git a/dlls/d2d1/device.c b/dlls/d2d1/device.c index 0647bc57fc3..d8704f1b43b 100644 --- a/dlls/d2d1/device.c +++ b/dlls/d2d1/device.c @@ -1887,13 +1887,19 @@ static HRESULT STDMETHODCALLTYPE d2d_device_context_CreateEffect(ID2D1DeviceCont { struct d2d_device_context *context = impl_from_ID2D1DeviceContext(iface); struct d2d_effect *object; + HRESULT hr;
FIXME("iface %p, effect_id %s, effect %p stub!\n", iface, debugstr_guid(effect_id), effect);
if (!(object = heap_alloc_zero(sizeof(*object)))) return E_OUTOFMEMORY;
- d2d_effect_init(object, context->factory); + if (FAILED(hr = d2d_effect_init(object, context->factory))) + { + WARN("Failed to initialize effect, hr %#x.\n", hr); + heap_free(object); + return hr; + }
TRACE("Created effect %p.\n", object); *effect = &object->ID2D1Effect_iface; diff --git a/dlls/d2d1/effect.c b/dlls/d2d1/effect.c index 40dd2187953..fa616ebe6be 100644 --- a/dlls/d2d1/effect.c +++ b/dlls/d2d1/effect.c @@ -25,6 +25,12 @@ static inline struct d2d_effect *impl_from_ID2D1Effect(ID2D1Effect *iface) return CONTAINING_RECORD(iface, struct d2d_effect, ID2D1Effect_iface); }
+static void d2d_effect_cleanup(struct d2d_effect *effect) +{ + heap_free(effect->inputs); + ID2D1Factory_Release(effect->factory); +} + static HRESULT STDMETHODCALLTYPE d2d_effect_QueryInterface(ID2D1Effect *iface, REFIID iid, void **out) { struct d2d_effect *effect = impl_from_ID2D1Effect(iface); @@ -72,7 +78,7 @@ static ULONG STDMETHODCALLTYPE d2d_effect_Release(ID2D1Effect *iface)
if (!refcount) { - ID2D1Factory_Release(effect->factory); + d2d_effect_cleanup(effect); heap_free(effect); }
@@ -183,9 +189,11 @@ static void STDMETHODCALLTYPE d2d_effect_GetInput(ID2D1Effect *iface, UINT32 ind
static UINT32 STDMETHODCALLTYPE d2d_effect_GetInputCount(ID2D1Effect *iface) { - FIXME("iface %p stub!\n", iface); + struct d2d_effect *effect = impl_from_ID2D1Effect(iface);
- return 0; + TRACE("iface %p.\n", iface); + + return effect->input_count; }
static void STDMETHODCALLTYPE d2d_effect_GetOutput(ID2D1Effect *iface, ID2D1Image **output) @@ -269,10 +277,23 @@ static const ID2D1ImageVtbl d2d_effect_image_vtbl = d2d_effect_image_GetFactory, };
-void d2d_effect_init(struct d2d_effect *effect, ID2D1Factory *factory) +HRESULT d2d_effect_init(struct d2d_effect *effect, ID2D1Factory *factory) { effect->ID2D1Effect_iface.lpVtbl = &d2d_effect_vtbl; effect->ID2D1Image_iface.lpVtbl = &d2d_effect_image_vtbl; effect->refcount = 1; + ID2D1Factory_AddRef(effect->factory = factory); + + effect->input_count = 1; + if (!d2d_array_reserve((void **)&effect->inputs, &effect->inputs_size, + effect->input_count, sizeof(*effect->inputs))) + goto fail; + memset(effect->inputs, 0, sizeof(*effect->inputs) * effect->inputs_size); + + return S_OK; + +fail: + d2d_effect_cleanup(effect); + return E_OUTOFMEMORY; } diff --git a/dlls/d2d1/tests/d2d1.c b/dlls/d2d1/tests/d2d1.c index 146952d7418..64d9765e866 100644 --- a/dlls/d2d1/tests/d2d1.c +++ b/dlls/d2d1/tests/d2d1.c @@ -9802,7 +9802,7 @@ static void test_effect(BOOL d3d11) }
input_count = ID2D1Effect_GetInputCount(effect); - todo_wine + todo_wine_if(test->default_input_count != 1) ok (input_count == test->default_input_count, "Got unexpected input count %u, expected %u.\n", input_count, test->default_input_count);
On Thu, 5 Aug 2021 at 10:06, Ziqing Hui zhui@codeweavers.com wrote:
+static void d2d_effect_cleanup(struct d2d_effect *effect) +{
- heap_free(effect->inputs);
- ID2D1Factory_Release(effect->factory);
+}
[...]
-void d2d_effect_init(struct d2d_effect *effect, ID2D1Factory *factory) +HRESULT d2d_effect_init(struct d2d_effect *effect, ID2D1Factory *factory) { effect->ID2D1Effect_iface.lpVtbl = &d2d_effect_vtbl; effect->ID2D1Image_iface.lpVtbl = &d2d_effect_image_vtbl; effect->refcount = 1;
- ID2D1Factory_AddRef(effect->factory = factory);
- effect->input_count = 1;
- if (!d2d_array_reserve((void **)&effect->inputs, &effect->inputs_size,
effect->input_count, sizeof(*effect->inputs)))goto fail;- memset(effect->inputs, 0, sizeof(*effect->inputs) * effect->inputs_size);
- return S_OK;
+fail:
- d2d_effect_cleanup(effect);
- return E_OUTOFMEMORY;
}
If d2d_array_reserve() were to fail, we'd only need to release the factory reference. (And if we called d2d_array_reserve() before ID2D1Factory_AddRef(), we could avoid that too.) Calling heap_free() on a NULL "effect->inputs" works fine of course, but it becomes more complicated in patch 4/5 of this series. In principle we only ever want to call _cleanup() functions on fully initialised objects.
Note that we only need to memset() "effect->input_count" inputs here; that's what we do for SetInputCount() as well in patch 3/5. And in fact, once we have SetInputCount() in patch 3/5, we can just call it from d2d_effect_init() to set the initial input count.
Signed-off-by: Ziqing Hui zhui@codeweavers.com ---
v3: Return earlier to reduce indentation. Use d2d_array_reserve() to resize the inputs array.
dlls/d2d1/d2d1_private.h | 2 ++ dlls/d2d1/effect.c | 34 ++++++++++++++++++++++++++++++++-- dlls/d2d1/tests/d2d1.c | 3 +-- 3 files changed, 35 insertions(+), 4 deletions(-)
diff --git a/dlls/d2d1/d2d1_private.h b/dlls/d2d1/d2d1_private.h index 48d15dfcf22..10d0598f20e 100644 --- a/dlls/d2d1/d2d1_private.h +++ b/dlls/d2d1/d2d1_private.h @@ -575,6 +575,8 @@ struct d2d_effect ID2D1Image **inputs; size_t inputs_size; size_t input_count; + UINT32 min_inputs; + UINT32 max_inputs; };
HRESULT d2d_effect_init(struct d2d_effect *effect, ID2D1Factory *factory) DECLSPEC_HIDDEN; diff --git a/dlls/d2d1/effect.c b/dlls/d2d1/effect.c index fa616ebe6be..a07ea34c4e2 100644 --- a/dlls/d2d1/effect.c +++ b/dlls/d2d1/effect.c @@ -177,9 +177,37 @@ static void STDMETHODCALLTYPE d2d_effect_SetInput(ID2D1Effect *iface, UINT32 ind
static HRESULT STDMETHODCALLTYPE d2d_effect_SetInputCount(ID2D1Effect *iface, UINT32 count) { - FIXME("iface %p, count %u stub!\n", iface, count); + struct d2d_effect *effect = impl_from_ID2D1Effect(iface); + unsigned int i;
- return E_NOTIMPL; + TRACE("iface %p, count %u.\n", iface, count); + + if (count < effect->min_inputs || count > effect->max_inputs) + return E_INVALIDARG; + if (count == effect->input_count) + return S_OK; + + if (count < effect->input_count) + { + for (i = count; i < effect->input_count; ++i) + { + if (effect->inputs[i]) + ID2D1Image_Release(effect->inputs[i]); + } + } + + if (!d2d_array_reserve((void **)&effect->inputs, &effect->inputs_size, + count, sizeof(*effect->inputs))) + { + ERR("Failed to resize inputs array.\n"); + return E_OUTOFMEMORY; + } + + if (count > effect->input_count) + memset(effect->inputs + effect->input_count, 0, + sizeof(*effect->inputs) * (count - effect->input_count)); + + return S_OK; }
static void STDMETHODCALLTYPE d2d_effect_GetInput(ID2D1Effect *iface, UINT32 index, ID2D1Image **input) @@ -286,6 +314,8 @@ HRESULT d2d_effect_init(struct d2d_effect *effect, ID2D1Factory *factory) ID2D1Factory_AddRef(effect->factory = factory);
effect->input_count = 1; + effect->min_inputs = 1; + effect->max_inputs = 1; if (!d2d_array_reserve((void **)&effect->inputs, &effect->inputs_size, effect->input_count, sizeof(*effect->inputs))) goto fail; diff --git a/dlls/d2d1/tests/d2d1.c b/dlls/d2d1/tests/d2d1.c index 64d9765e866..0a74b6cabdc 100644 --- a/dlls/d2d1/tests/d2d1.c +++ b/dlls/d2d1/tests/d2d1.c @@ -9812,10 +9812,9 @@ static void test_effect(BOOL d3d11) winetest_push_context("Input %u", j); hr = ID2D1Effect_SetInputCount(effect, j); if (j < test->min_inputs || j > test->max_inputs) - todo_wine ok(hr == E_INVALIDARG, "Got unexpected hr %#x.\n", hr); else - todo_wine + todo_wine_if(test->max_inputs > 1 && j > 1) ok(hr == S_OK, "Got unexpected hr %#x.\n", hr); winetest_pop_context(); }
On Thu, 5 Aug 2021 at 10:06, Ziqing Hui zhui@codeweavers.com wrote:
static HRESULT STDMETHODCALLTYPE d2d_effect_SetInputCount(ID2D1Effect *iface, UINT32 count) {
- FIXME("iface %p, count %u stub!\n", iface, count);
- struct d2d_effect *effect = impl_from_ID2D1Effect(iface);
- unsigned int i;
- return E_NOTIMPL;
- TRACE("iface %p, count %u.\n", iface, count);
- if (count < effect->min_inputs || count > effect->max_inputs)
return E_INVALIDARG;- if (count == effect->input_count)
return S_OK;- if (count < effect->input_count)
- {
for (i = count; i < effect->input_count; ++i){if (effect->inputs[i])ID2D1Image_Release(effect->inputs[i]);}- }
In this case we're in fact done after the for-loop and can return.
- if (!d2d_array_reserve((void **)&effect->inputs, &effect->inputs_size,
count, sizeof(*effect->inputs)))- {
We use double (i.e., 8 space) indentation for line continuations.
ERR("Failed to resize inputs array.\n");return E_OUTOFMEMORY;- }
- if (count > effect->input_count)
memset(effect->inputs + effect->input_count, 0,sizeof(*effect->inputs) * (count - effect->input_count));
"effect->inputs + effect->input_count" -> "&effect->inputs[effect->input_count]", by convention.
@@ -286,6 +314,8 @@ HRESULT d2d_effect_init(struct d2d_effect *effect, ID2D1Factory *factory) ID2D1Factory_AddRef(effect->factory = factory);
effect->input_count = 1;
- effect->min_inputs = 1;
- effect->max_inputs = 1;
It seems a little unfortunate to hardcode these to "1" here, since it means we'll never actually grow the inputs array. At the same time, this would seem like a great time to introduce the effect info array containing the minimum, maximum, and initial input counts.
Signed-off-by: Ziqing Hui zhui@codeweavers.com ---
v3: AddRef before Release. Reduce indentation.
dlls/d2d1/effect.c | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-)
diff --git a/dlls/d2d1/effect.c b/dlls/d2d1/effect.c index a07ea34c4e2..768777d3652 100644 --- a/dlls/d2d1/effect.c +++ b/dlls/d2d1/effect.c @@ -27,6 +27,16 @@ static inline struct d2d_effect *impl_from_ID2D1Effect(ID2D1Effect *iface)
static void d2d_effect_cleanup(struct d2d_effect *effect) { + unsigned int i; + + if (effect->inputs) + { + for (i = 0; i < effect->input_count; ++i) + { + if (effect->inputs[i]) + ID2D1Image_Release(effect->inputs[i]); + } + } heap_free(effect->inputs); ID2D1Factory_Release(effect->factory); } @@ -172,7 +182,17 @@ static HRESULT STDMETHODCALLTYPE d2d_effect_GetSubProperties(ID2D1Effect *iface,
static void STDMETHODCALLTYPE d2d_effect_SetInput(ID2D1Effect *iface, UINT32 index, ID2D1Image *input, BOOL invalidate) { - FIXME("iface %p, index %u, input %p, invalidate %d stub!\n", iface, index, input, invalidate); + struct d2d_effect *effect = impl_from_ID2D1Effect(iface); + + TRACE("iface %p, index %u, input %p, invalidate %d.\n", iface, index, input, invalidate); + + if (index >= effect->input_count) + return; + + ID2D1Image_AddRef(input); + if (effect->inputs[index]) + ID2D1Image_Release(effect->inputs[index]); + effect->inputs[index] = input; }
static HRESULT STDMETHODCALLTYPE d2d_effect_SetInputCount(ID2D1Effect *iface, UINT32 count)
On Thu, 5 Aug 2021 at 10:06, Ziqing Hui zhui@codeweavers.com wrote:
static void d2d_effect_cleanup(struct d2d_effect *effect) {
- unsigned int i;
- if (effect->inputs)
- {
for (i = 0; i < effect->input_count; ++i){if (effect->inputs[i])ID2D1Image_Release(effect->inputs[i]);}- }
The "effect->inputs" check should be redundant; if "input_count" is non-zero, "effect->inputs" should be non-NULL.
static void STDMETHODCALLTYPE d2d_effect_SetInput(ID2D1Effect *iface, UINT32 index, ID2D1Image *input, BOOL invalidate) {
- FIXME("iface %p, index %u, input %p, invalidate %d stub!\n", iface, index, input, invalidate);
- struct d2d_effect *effect = impl_from_ID2D1Effect(iface);
- TRACE("iface %p, index %u, input %p, invalidate %d.\n", iface, index, input, invalidate);
This is already present in the original code, but "%#x" for tracing boolean arguments, please.
Signed-off-by: Ziqing Hui zhui@codeweavers.com --- dlls/d2d1/effect.c | 9 ++++++++- dlls/d2d1/tests/d2d1.c | 7 +------ 2 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/dlls/d2d1/effect.c b/dlls/d2d1/effect.c index 768777d3652..ee223c0bd38 100644 --- a/dlls/d2d1/effect.c +++ b/dlls/d2d1/effect.c @@ -232,7 +232,14 @@ static HRESULT STDMETHODCALLTYPE d2d_effect_SetInputCount(ID2D1Effect *iface, UI
static void STDMETHODCALLTYPE d2d_effect_GetInput(ID2D1Effect *iface, UINT32 index, ID2D1Image **input) { - FIXME("iface %p, index %u, input %p stub!\n", iface, index, input); + struct d2d_effect *effect = impl_from_ID2D1Effect(iface); + + TRACE("iface %p, index %u, input %p.\n", iface, index, input); + + if (index < effect->input_count && effect->inputs[index]) + ID2D1Image_AddRef(*input = effect->inputs[index]); + else + *input = NULL; }
static UINT32 STDMETHODCALLTYPE d2d_effect_GetInputCount(ID2D1Effect *iface) diff --git a/dlls/d2d1/tests/d2d1.c b/dlls/d2d1/tests/d2d1.c index 0a74b6cabdc..1951d0966ea 100644 --- a/dlls/d2d1/tests/d2d1.c +++ b/dlls/d2d1/tests/d2d1.c @@ -9824,7 +9824,6 @@ static void test_effect(BOOL d3d11) { winetest_push_context("Input %u", j); ID2D1Effect_GetInput(effect, j, &image_a); - todo_wine ok(image_a == NULL, "Got unexpected image_a %p.\n", image_a); winetest_pop_context(); } @@ -9845,14 +9844,11 @@ static void test_effect(BOOL d3d11) ID2D1Effect_GetInput(effect, j, &image_a); if (j == 0) { - todo_wine ok(image_a == (ID2D1Image *)bitmap, "Got unexpected image_a %p.\n", image_a); - if (image_a == (ID2D1Image *)bitmap) - ID2D1Image_Release(image_a); + ID2D1Image_Release(image_a); } else { - todo_wine ok(image_a == NULL, "Got unexpected image_a %p.\n", image_a); } winetest_pop_context(); @@ -9864,7 +9860,6 @@ static void test_effect(BOOL d3d11) image_a = (ID2D1Image *)0xdeadbeef; ID2D1Effect_SetInput(effect, j, (ID2D1Image *)bitmap, FALSE); ID2D1Effect_GetInput(effect, j, &image_a); - todo_wine ok(image_a == NULL, "Got unexpected image_a %p.\n", image_a); winetest_pop_context(); }
-
Henri Verbeet -
Henri Verbeet -
Ziqing Hui