The patch subject suggests it shouldn't be too hard to split this patch up. On Sat, 31 Jul 2021 at 06:25, Ziqing Hui <zhui(a)codeweavers.com> wrote:
@@ -572,6 +572,10 @@ struct d2d_effect LONG refcount;
ID2D1Factory *factory; + UINT32 min_inputs; + UINT32 max_inputs; + ID2D1Image **inputs; + UINT32 inputs_count; };
"input_count"
@@ -67,12 +67,19 @@ static ULONG STDMETHODCALLTYPE d2d_effect_Release(ID2D1Effect *iface) { struct d2d_effect *effect = impl_from_ID2D1Effect(iface); ULONG refcount = InterlockedDecrement(&effect->refcount); + unsigned int i;
TRACE("%p decreasing refcount to %u.\n", iface, refcount);
if (!refcount) { ID2D1Factory_Release(effect->factory); + for (i = 0; i < effect->inputs_count; ++i) + { + if (effect->inputs[i]) + ID2D1Image_Release(effect->inputs[i]); + } + heap_free(effect->inputs); heap_free(effect); } We typically clean up in reverse initialisation order, so factory after inputs.
@@ -166,26 +173,64 @@ 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->inputs_count) + { + if (effect->inputs[index]) + ID2D1Image_Release(effect->inputs[index]); + ID2D1Image_AddRef(effect->inputs[index] = input); + } } If we invert the condition above, we can return early and slightly reduce the indentation level. Release() before AddRef() doesn't seem entirely safe above; consider the case that "effect->inputs[index]" holds the only reference to the image, and we're setting the same input image again.
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->inputs_count) + { + if (count < effect->inputs_count) + { + for (i = count; i < effect->inputs_count; ++i) + { + if (effect->inputs[i]) + ID2D1Image_Release(effect->inputs[i]); + } + } + effect->inputs_count = count; + HeapReAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, effect->inputs, sizeof(*effect->inputs) * count); + } + + return S_OK; }
That doesn't generally work; the memory returned by HeapReAlloc() may not be the same as what was originally passed in. More broadly, it's not clear to me that we gain much from avoiding d2d_array_reserve() here.