Signed-off-by: Ziqing Hui zhui@codeweavers.com --- dlls/d2d1/d2d1_private.h | 44 +++++++++++++++++++++++-- dlls/d2d1/effect.c | 69 ++++++++++++++++++++++++++++++---------- dlls/d2d1/factory.c | 44 ++----------------------- 3 files changed, 96 insertions(+), 61 deletions(-)
diff --git a/dlls/d2d1/d2d1_private.h b/dlls/d2d1/d2d1_private.h index 9be553c72d8..75a9c4f9366 100644 --- a/dlls/d2d1/d2d1_private.h +++ b/dlls/d2d1/d2d1_private.h @@ -20,6 +20,7 @@ #define __WINE_D2D1_PRIVATE_H
#include "wine/debug.h" +#include "wine/list.h"
#include <assert.h> #include <limits.h> @@ -61,6 +62,22 @@ struct d2d_settings }; extern struct d2d_settings d2d_settings DECLSPEC_HIDDEN;
+struct d2d_factory +{ + ID2D1Factory3 ID2D1Factory3_iface; + ID2D1Multithread ID2D1Multithread_iface; + LONG refcount; + + ID3D10Device1 *device; + + float dpi_x; + float dpi_y; + + struct list effects; + + CRITICAL_SECTION cs; +}; + struct d2d_clip_stack { D2D1_RECT_F *stack; @@ -602,12 +619,30 @@ struct d2d_effect_context void d2d_effect_context_init(struct d2d_effect_context *effect_context, struct d2d_device_context *device_context) DECLSPEC_HIDDEN;
-struct d2d_effect_info +struct d2d_effect_property { - const CLSID *clsid; + WCHAR *name; + D2D1_PROPERTY_TYPE type; + BYTE *value; + PD2D1_PROPERTY_SET_FUNCTION set_function; + PD2D1_PROPERTY_GET_FUNCTION get_function; +}; + +struct d2d_effect_registration +{ + struct list entry; + BOOL is_builtin; + PD2D1_EFFECT_FACTORY factory; + UINT32 registration_count; + CLSID id; + UINT32 default_input_count; UINT32 min_inputs; UINT32 max_inputs; + + struct d2d_effect_property *properties; + size_t property_size; + size_t property_count; };
struct d2d_effect @@ -616,7 +651,9 @@ struct d2d_effect ID2D1Image ID2D1Image_iface; LONG refcount;
- const struct d2d_effect_info *info; + CLSID id; + UINT32 min_inputs; + UINT32 max_inputs;
struct d2d_effect_context *effect_context; ID2D1Image **inputs; @@ -626,6 +663,7 @@ struct d2d_effect
HRESULT d2d_effect_init(struct d2d_effect *effect, struct d2d_effect_context *effect_context, const CLSID *effect_id) DECLSPEC_HIDDEN; +HRESULT d2d_register_builtin_effects(struct d2d_factory *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/effect.c b/dlls/d2d1/effect.c index f5d5494c67a..a87fedd0742 100644 --- a/dlls/d2d1/effect.c +++ b/dlls/d2d1/effect.c @@ -20,14 +20,23 @@
WINE_DEFAULT_DEBUG_CHANNEL(d2d);
-static const struct d2d_effect_info builtin_effects[] = +struct d2d_builtin_effect_registration { - {&CLSID_D2D12DAffineTransform, 1, 1, 1}, - {&CLSID_D2D13DPerspectiveTransform, 1, 1, 1}, - {&CLSID_D2D1Composite, 2, 1, 0xffffffff}, - {&CLSID_D2D1Crop, 1, 1, 1}, - {&CLSID_D2D1Shadow, 1, 1, 1}, - {&CLSID_D2D1Grayscale, 1, 1, 1}, + const CLSID *id; + PD2D1_EFFECT_FACTORY factory; + UINT32 default_input_count; + UINT32 min_inputs; + UINT32 max_inputs; +}; + +static const struct d2d_builtin_effect_registration builtin_effects[] = +{ + {&CLSID_D2D12DAffineTransform, NULL, 1, 1, 1}, + {&CLSID_D2D13DPerspectiveTransform, NULL, 1, 1, 1}, + {&CLSID_D2D1Composite, NULL, 2, 1, 0xffffffff}, + {&CLSID_D2D1Crop, NULL, 1, 1, 1}, + {&CLSID_D2D1Shadow, NULL, 1, 1, 1}, + {&CLSID_D2D1Grayscale, NULL, 1, 1, 1}, };
static inline struct d2d_effect_context *impl_from_ID2D1EffectContext(ID2D1EffectContext *iface) @@ -554,21 +563,21 @@ static HRESULT STDMETHODCALLTYPE d2d_effect_GetValue(ID2D1Effect *iface, UINT32 { case D2D1_PROPERTY_CLSID: if ((type != D2D1_PROPERTY_TYPE_UNKNOWN && type != D2D1_PROPERTY_TYPE_CLSID) - || value_size != sizeof(*effect->info->clsid)) + || value_size != sizeof(effect->id)) return E_INVALIDARG; - src = effect->info->clsid; + src = &effect->id; break; case D2D1_PROPERTY_MIN_INPUTS: if ((type != D2D1_PROPERTY_TYPE_UNKNOWN && type != D2D1_PROPERTY_TYPE_UINT32) - || value_size != sizeof(effect->info->min_inputs)) + || value_size != sizeof(effect->min_inputs)) return E_INVALIDARG; - src = &effect->info->min_inputs; + src = &effect->min_inputs; break; case D2D1_PROPERTY_MAX_INPUTS: if ((type != D2D1_PROPERTY_TYPE_UNKNOWN && type != D2D1_PROPERTY_TYPE_UINT32) - || value_size != sizeof(effect->info->max_inputs)) + || value_size != sizeof(effect->max_inputs)) return E_INVALIDARG; - src = &effect->info->max_inputs; + src = &effect->max_inputs; break; default: if (index < D2D1_PROPERTY_CLSID) @@ -619,7 +628,7 @@ static HRESULT STDMETHODCALLTYPE d2d_effect_SetInputCount(ID2D1Effect *iface, UI
TRACE("iface %p, count %u.\n", iface, count);
- if (count < effect->info->min_inputs || count > effect->info->max_inputs) + if (count < effect->min_inputs || count > effect->max_inputs) return E_INVALIDARG; if (count == effect->input_count) return S_OK; @@ -760,10 +769,11 @@ HRESULT d2d_effect_init(struct d2d_effect *effect, struct d2d_effect_context *ef
for (i = 0; i < ARRAY_SIZE(builtin_effects); ++i) { - if (IsEqualGUID(effect_id, builtin_effects[i].clsid)) + if (IsEqualGUID(effect_id, &builtin_effects[i].id)) { - effect->info = &builtin_effects[i]; - d2d_effect_SetInputCount(&effect->ID2D1Effect_iface, effect->info->default_input_count); + effect->min_inputs = builtin_effects[i].min_inputs; + effect->max_inputs = builtin_effects[i].max_inputs; + d2d_effect_SetInputCount(&effect->ID2D1Effect_iface, builtin_effects[i].default_input_count); effect->effect_context = effect_context; ID2D1EffectContext_AddRef(&effect_context->ID2D1EffectContext_iface); return S_OK; @@ -773,3 +783,28 @@ HRESULT d2d_effect_init(struct d2d_effect *effect, struct d2d_effect_context *ef WARN("Unsupported effect clsid %s.\n", debugstr_guid(effect_id)); return E_FAIL; } + +HRESULT d2d_register_builtin_effects(struct d2d_factory *factory) +{ + struct d2d_effect_registration *reg; + unsigned int i; + + for (i = 0; i < ARRAY_SIZE(builtin_effects); ++i) + { + const struct d2d_builtin_effect_registration *builtin_reg = &builtin_effects[i]; + + if (!(reg = calloc(1, sizeof(*reg)))) + return E_OUTOFMEMORY; + + reg->is_builtin = TRUE; + reg->factory = builtin_reg->factory; + reg->registration_count = 1; + reg->id = *builtin_reg->id; + reg->default_input_count = builtin_reg->default_input_count; + reg->min_inputs = builtin_reg->min_inputs; + reg->max_inputs = builtin_reg->max_inputs; + list_add_tail(&factory->effects, ®->entry); + } + + return S_OK; +} diff --git a/dlls/d2d1/factory.c b/dlls/d2d1/factory.c index 5c79d1791ff..7eec52c01a1 100644 --- a/dlls/d2d1/factory.c +++ b/dlls/d2d1/factory.c @@ -20,7 +20,6 @@ #include "d2d1_private.h"
#include "xmllite.h" -#include "wine/list.h"
WINE_DECLARE_DEBUG_CHANNEL(winediag); WINE_DEFAULT_DEBUG_CHANNEL(d2d); @@ -30,29 +29,6 @@ struct d2d_settings d2d_settings = ~0u, /* No ID2D1Factory version limit by default. */ };
-struct d2d_effect_property -{ - WCHAR *name; - D2D1_PROPERTY_TYPE type; - BYTE *value; - PD2D1_PROPERTY_SET_FUNCTION set_function; - PD2D1_PROPERTY_GET_FUNCTION get_function; -}; - -struct d2d_effect_registration -{ - struct list entry; - PD2D1_EFFECT_FACTORY factory; - UINT32 registration_count; - CLSID id; - - UINT32 input_count; - - struct d2d_effect_property *properties; - size_t property_size; - size_t property_count; -}; - static void d2d_effect_registration_cleanup(struct d2d_effect_registration *reg) { size_t i; @@ -66,22 +42,6 @@ static void d2d_effect_registration_cleanup(struct d2d_effect_registration *reg) free(reg); }
-struct d2d_factory -{ - ID2D1Factory3 ID2D1Factory3_iface; - ID2D1Multithread ID2D1Multithread_iface; - LONG refcount; - - ID3D10Device1 *device; - - float dpi_x; - float dpi_y; - - struct list effects; - - CRITICAL_SECTION cs; -}; - static inline struct d2d_factory *impl_from_ID2D1Factory3(ID2D1Factory3 *iface) { return CONTAINING_RECORD(iface, struct d2d_factory, ID2D1Factory3_iface); @@ -916,6 +876,7 @@ static HRESULT STDMETHODCALLTYPE d2d_factory_RegisterEffectFromStream(ID2D1Facto property->set_function = bindings[i].setFunction; }
+ effect->is_builtin = FALSE; effect->registration_count = 1; effect->id = *effect_id; effect->factory = effect_factory; @@ -960,7 +921,7 @@ static HRESULT STDMETHODCALLTYPE d2d_factory_UnregisterEffect(ID2D1Factory3 *ifa
LIST_FOR_EACH_ENTRY(effect, &factory->effects, struct d2d_effect_registration, entry) { - if (IsEqualGUID(effect_id, &effect->id)) + if (!effect->is_builtin && IsEqualGUID(effect_id, &effect->id)) { if (!--effect->registration_count) { @@ -1126,6 +1087,7 @@ static void d2d_factory_init(struct d2d_factory *factory, D2D1_FACTORY_TYPE fact factory->refcount = 1; d2d_factory_reload_sysmetrics(factory); list_init(&factory->effects); + d2d_register_builtin_effects(factory); InitializeCriticalSection(&factory->cs); }
Signed-off-by: Ziqing Hui zhui@codeweavers.com --- dlls/d2d1/d2d1_private.h | 3 + dlls/d2d1/effect.c | 127 ++++++++++++++++++++++--- dlls/d2d1/factory.c | 8 ++ dlls/d2d1/tests/d2d1.c | 196 +++++++++++++++++---------------------- 4 files changed, 213 insertions(+), 121 deletions(-)
diff --git a/dlls/d2d1/d2d1_private.h b/dlls/d2d1/d2d1_private.h index 75a9c4f9366..ffe1cd9e1dc 100644 --- a/dlls/d2d1/d2d1_private.h +++ b/dlls/d2d1/d2d1_private.h @@ -78,6 +78,8 @@ struct d2d_factory CRITICAL_SECTION cs; };
+struct d2d_factory *unsafe_impl_from_ID2D1Factory3(ID2D1Factory3 *iface) DECLSPEC_HIDDEN; + struct d2d_clip_stack { D2D1_RECT_F *stack; @@ -655,6 +657,7 @@ struct d2d_effect UINT32 min_inputs; UINT32 max_inputs;
+ ID2D1EffectImpl *impl; struct d2d_effect_context *effect_context; ID2D1Image **inputs; size_t inputs_size; diff --git a/dlls/d2d1/effect.c b/dlls/d2d1/effect.c index a87fedd0742..c1ef67c89aa 100644 --- a/dlls/d2d1/effect.c +++ b/dlls/d2d1/effect.c @@ -20,6 +20,12 @@
WINE_DEFAULT_DEBUG_CHANNEL(d2d);
+struct d2d_effect_impl +{ + ID2D1EffectImpl ID2D1EffectImpl_iface; + LONG refcount; +}; + struct d2d_builtin_effect_registration { const CLSID *id; @@ -29,14 +35,98 @@ struct d2d_builtin_effect_registration UINT32 max_inputs; };
+static inline struct d2d_effect_impl *impl_from_ID2D1EffectImpl(ID2D1EffectImpl *iface) +{ + return CONTAINING_RECORD(iface, struct d2d_effect_impl, ID2D1EffectImpl_iface); +} + +static HRESULT STDMETHODCALLTYPE d2d_effect_impl_QueryInterface(ID2D1EffectImpl *iface, REFIID iid, void **out) +{ + TRACE("iface %p, iid %s, out %p.\n", iface, debugstr_guid(iid), out); + + if (IsEqualGUID(iid, &IID_ID2D1EffectImpl) + || IsEqualGUID(iid, &IID_IUnknown)) + { + ID2D1EffectImpl_AddRef(iface); + *out = iface; + return S_OK; + } + + *out = NULL; + return E_NOINTERFACE; +} + +static ULONG STDMETHODCALLTYPE d2d_effect_impl_AddRef(ID2D1EffectImpl *iface) +{ + struct d2d_effect_impl *effect_impl =impl_from_ID2D1EffectImpl(iface); + ULONG refcount = InterlockedIncrement(&effect_impl->refcount); + + TRACE("%p increasing refcount to %lu.\n", iface, refcount); + + return refcount; +} + +static ULONG STDMETHODCALLTYPE d2d_effect_impl_Release(ID2D1EffectImpl *iface) +{ + struct d2d_effect_impl *effect_impl = impl_from_ID2D1EffectImpl(iface); + ULONG refcount = InterlockedDecrement(&effect_impl->refcount); + + TRACE("%p decreasing refcount to %lu.\n", iface, refcount); + + if (!refcount) + free(effect_impl); + + return refcount; +} + +static HRESULT STDMETHODCALLTYPE d2d_effect_impl_Initialize(ID2D1EffectImpl *iface, + ID2D1EffectContext *context, ID2D1TransformGraph *graph) +{ + return S_OK; +} + +static HRESULT STDMETHODCALLTYPE d2d_effect_impl_PrepareForRender(ID2D1EffectImpl *iface, D2D1_CHANGE_TYPE type) +{ + return S_OK; +} + +static HRESULT STDMETHODCALLTYPE d2d_effect_impl_SetGraph(ID2D1EffectImpl *iface, ID2D1TransformGraph *graph) +{ + return E_NOTIMPL; +} + +static const ID2D1EffectImplVtbl d2d_effect_impl_vtbl = +{ + d2d_effect_impl_QueryInterface, + d2d_effect_impl_AddRef, + d2d_effect_impl_Release, + d2d_effect_impl_Initialize, + d2d_effect_impl_PrepareForRender, + d2d_effect_impl_SetGraph, +}; + +static HRESULT STDMETHODCALLTYPE d2d_effect_impl_create(IUnknown **effect_impl) +{ + struct d2d_effect_impl *object; + + if (!(object = calloc(1, sizeof(*object)))) + return E_OUTOFMEMORY; + + object->ID2D1EffectImpl_iface.lpVtbl = &d2d_effect_impl_vtbl; + object->refcount = 1; + + *effect_impl = (IUnknown *)&object->ID2D1EffectImpl_iface; + return S_OK; +} + static const struct d2d_builtin_effect_registration builtin_effects[] = { - {&CLSID_D2D12DAffineTransform, NULL, 1, 1, 1}, - {&CLSID_D2D13DPerspectiveTransform, NULL, 1, 1, 1}, - {&CLSID_D2D1Composite, NULL, 2, 1, 0xffffffff}, - {&CLSID_D2D1Crop, NULL, 1, 1, 1}, - {&CLSID_D2D1Shadow, NULL, 1, 1, 1}, - {&CLSID_D2D1Grayscale, NULL, 1, 1, 1}, + {&CLSID_D2D12DAffineTransform, d2d_effect_impl_create, 1, 1, 1}, + {&CLSID_D2D13DPerspectiveTransform, d2d_effect_impl_create, 1, 1, 1}, + {&CLSID_D2D1Composite, d2d_effect_impl_create, 2, 1, 0xffffffff}, + {&CLSID_D2D1Crop, d2d_effect_impl_create, 1, 1, 1}, + {&CLSID_D2D1Shadow, d2d_effect_impl_create, 1, 1, 1}, + {&CLSID_D2D1Grayscale, d2d_effect_impl_create, 1, 1, 1}, };
static inline struct d2d_effect_context *impl_from_ID2D1EffectContext(ID2D1EffectContext *iface) @@ -433,6 +523,7 @@ static void d2d_effect_cleanup(struct d2d_effect *effect) } free(effect->inputs); ID2D1EffectContext_Release(&effect->effect_context->ID2D1EffectContext_iface); + ID2D1EffectImpl_Release(effect->impl); }
static HRESULT STDMETHODCALLTYPE d2d_effect_QueryInterface(ID2D1Effect *iface, REFIID iid, void **out) @@ -761,21 +852,33 @@ static const ID2D1ImageVtbl d2d_effect_image_vtbl =
HRESULT d2d_effect_init(struct d2d_effect *effect, struct d2d_effect_context *effect_context, const CLSID *effect_id) { - unsigned int i; + struct d2d_effect_registration *reg; + struct d2d_factory *factory; + HRESULT hr;
effect->ID2D1Effect_iface.lpVtbl = &d2d_effect_vtbl; effect->ID2D1Image_iface.lpVtbl = &d2d_effect_image_vtbl; effect->refcount = 1;
- for (i = 0; i < ARRAY_SIZE(builtin_effects); ++i) + factory = unsafe_impl_from_ID2D1Factory3((ID2D1Factory3 *)effect_context->device_context->factory); + LIST_FOR_EACH_ENTRY(reg, &factory->effects, struct d2d_effect_registration, entry) { - if (IsEqualGUID(effect_id, &builtin_effects[i].id)) + if (IsEqualGUID(effect_id, ®->id)) { - effect->min_inputs = builtin_effects[i].min_inputs; - effect->max_inputs = builtin_effects[i].max_inputs; - d2d_effect_SetInputCount(&effect->ID2D1Effect_iface, builtin_effects[i].default_input_count); + if (FAILED(hr = reg->factory((IUnknown **)&effect->impl))) + return hr; + if (FAILED(hr = ID2D1EffectImpl_Initialize(effect->impl, &effect_context->ID2D1EffectContext_iface, NULL))) + { + ID2D1EffectImpl_Release(effect->impl); + return hr; + } + effect->id = *effect_id; + effect->min_inputs = reg->min_inputs; + effect->max_inputs = reg->max_inputs; + d2d_effect_SetInputCount(&effect->ID2D1Effect_iface, reg->default_input_count); effect->effect_context = effect_context; ID2D1EffectContext_AddRef(&effect_context->ID2D1EffectContext_iface); + /* FIXME: Properties are ignored. */ return S_OK; } } diff --git a/dlls/d2d1/factory.c b/dlls/d2d1/factory.c index 7eec52c01a1..9f458b07fd9 100644 --- a/dlls/d2d1/factory.c +++ b/dlls/d2d1/factory.c @@ -1091,6 +1091,14 @@ static void d2d_factory_init(struct d2d_factory *factory, D2D1_FACTORY_TYPE fact InitializeCriticalSection(&factory->cs); }
+struct d2d_factory *unsafe_impl_from_ID2D1Factory3(ID2D1Factory3 *iface) +{ + if (!iface) + return NULL; + assert(iface->lpVtbl == &d2d_factory_vtbl); + return CONTAINING_RECORD(iface, struct d2d_factory, ID2D1Factory3_iface); +} + HRESULT WINAPI D2D1CreateFactory(D2D1_FACTORY_TYPE factory_type, REFIID iid, const D2D1_FACTORY_OPTIONS *factory_options, void **factory) { diff --git a/dlls/d2d1/tests/d2d1.c b/dlls/d2d1/tests/d2d1.c index f1cec1d7a0b..d749bc63b20 100644 --- a/dlls/d2d1/tests/d2d1.c +++ b/dlls/d2d1/tests/d2d1.c @@ -10719,15 +10719,13 @@ static void test_effect_register(BOOL d3d11) ok(hr == test->hr, "Got unexpected hr %#lx, expected %#lx.\n", hr, test->hr); if (hr == S_OK) { - effect = NULL; hr = ID2D1DeviceContext_CreateEffect(device_context, &CLSID_TestEffect, &effect); - todo_wine ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); hr = ID2D1Factory1_UnregisterEffect(factory, &CLSID_TestEffect); ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); hr = ID2D1Factory1_UnregisterEffect(factory, &CLSID_TestEffect); ok(hr == D2DERR_EFFECT_IS_NOT_REGISTERED, "Got unexpected hr %#lx.\n", hr); - if (effect) - ID2D1Effect_Release(effect); + ID2D1Effect_Release(effect); }
winetest_pop_context(); @@ -10762,29 +10760,23 @@ static void test_effect_register(BOOL d3d11) effect_xml_a, NULL, 0, effect_impl_create); ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); hr = ID2D1DeviceContext_CreateEffect(device_context, &CLSID_TestEffect, &effect); + ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + hr = ID2D1Effect_GetValue(effect, D2D1_PROPERTY_DISPLAYNAME, + D2D1_PROPERTY_TYPE_STRING, (BYTE *)display_name, sizeof(display_name)); todo_wine ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); - if (hr == S_OK) - { - hr = ID2D1Effect_GetValue(effect, D2D1_PROPERTY_DISPLAYNAME, - D2D1_PROPERTY_TYPE_STRING, (BYTE *)display_name, sizeof(display_name)); - ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); - ok(!wcscmp(display_name, L"TestEffectA"), "Got unexpected display name %s.\n", debugstr_w(display_name)); - ID2D1Effect_Release(effect); - } + todo_wine ok(!wcscmp(display_name, L"TestEffectA"), "Got unexpected display name %s.\n", debugstr_w(display_name)); + ID2D1Effect_Release(effect);
hr = ID2D1Factory1_RegisterEffectFromString(factory, &CLSID_TestEffect, effect_xml_b, NULL, 0, effect_impl_create); ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); hr = ID2D1DeviceContext_CreateEffect(device_context, &CLSID_TestEffect, &effect); + ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + hr = ID2D1Effect_GetValue(effect, D2D1_PROPERTY_DISPLAYNAME, + D2D1_PROPERTY_TYPE_STRING, (BYTE *)display_name, sizeof(display_name)); todo_wine ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); - if (hr == S_OK) - { - hr = ID2D1Effect_GetValue(effect, D2D1_PROPERTY_DISPLAYNAME, - D2D1_PROPERTY_TYPE_STRING, (BYTE *)display_name, sizeof(display_name)); - ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); - ok(!wcscmp(display_name, L"TestEffectA"), "Got unexpected display name %s.\n", debugstr_w(display_name)); - ID2D1Effect_Release(effect); - } + todo_wine ok(!wcscmp(display_name, L"TestEffectA"), "Got unexpected display name %s.\n", debugstr_w(display_name)); + ID2D1Effect_Release(effect);
hr = ID2D1Factory1_UnregisterEffect(factory, &CLSID_TestEffect); ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); @@ -10810,41 +10802,35 @@ static void test_effect_register(BOOL d3d11) effect_xml_c, binding, 1, effect_impl_create); ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); hr = ID2D1DeviceContext_CreateEffect(device_context, &CLSID_TestEffect, &effect); + ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + integer = 0xdeadbeef; + effect_context = (ID2D1EffectContext *)0xdeadbeef; + hr = ID2D1Effect_GetValueByName(effect, L"Integer", + D2D1_PROPERTY_TYPE_UINT32, (BYTE *)&integer, sizeof(integer)); todo_wine ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); - if (hr == S_OK) - { - integer = 0xdeadbeef; - effect_context = (ID2D1EffectContext *)0xdeadbeef; - hr = ID2D1Effect_GetValueByName(effect, L"Integer", - D2D1_PROPERTY_TYPE_UINT32, (BYTE *)&integer, sizeof(integer)); - ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); - hr = ID2D1Effect_GetValueByName(effect, L"Context", - D2D1_PROPERTY_TYPE_IUNKNOWN, (BYTE *)&effect_context, sizeof(effect_context)); - ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); - ok(integer == 10, "Got unexpected integer %u.\n", integer); - ok(effect_context == NULL, "Got unexpected effect context %p.\n", effect_context); - ID2D1Effect_Release(effect); - } + hr = ID2D1Effect_GetValueByName(effect, L"Context", + D2D1_PROPERTY_TYPE_IUNKNOWN, (BYTE *)&effect_context, sizeof(effect_context)); + todo_wine ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + todo_wine ok(integer == 10, "Got unexpected integer %u.\n", integer); + todo_wine ok(effect_context == NULL, "Got unexpected effect context %p.\n", effect_context); + ID2D1Effect_Release(effect);
hr = ID2D1Factory1_RegisterEffectFromString(factory, &CLSID_TestEffect, effect_xml_c, binding + 1, 1, effect_impl_create); ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); hr = ID2D1DeviceContext_CreateEffect(device_context, &CLSID_TestEffect, &effect); + ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + integer = 0xdeadbeef; + effect_context = (ID2D1EffectContext *)0xdeadbeef; + hr = ID2D1Effect_GetValueByName(effect, L"Integer", + D2D1_PROPERTY_TYPE_UINT32, (BYTE *)&integer, sizeof(integer)); todo_wine ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); - if (hr == S_OK) - { - integer = 0xdeadbeef; - effect_context = (ID2D1EffectContext *)0xdeadbeef; - hr = ID2D1Effect_GetValueByName(effect, L"Integer", - D2D1_PROPERTY_TYPE_UINT32, (BYTE *)&integer, sizeof(integer)); - ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); - hr = ID2D1Effect_GetValueByName(effect, L"Context", - D2D1_PROPERTY_TYPE_IUNKNOWN, (BYTE *)&effect_context, sizeof(effect_context)); - ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); - ok(integer == 10, "Got unexpected integer %u.\n", integer); - ok(effect_context == NULL, "Got unexpected effect context %p.\n", effect_context); - ID2D1Effect_Release(effect); - } + hr = ID2D1Effect_GetValueByName(effect, L"Context", + D2D1_PROPERTY_TYPE_IUNKNOWN, (BYTE *)&effect_context, sizeof(effect_context)); + todo_wine ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + todo_wine ok(integer == 10, "Got unexpected integer %u.\n", integer); + todo_wine ok(effect_context == NULL, "Got unexpected effect context %p.\n", effect_context); + ID2D1Effect_Release(effect);
hr = ID2D1Factory1_UnregisterEffect(factory, &CLSID_TestEffect); ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); @@ -10863,8 +10849,8 @@ static void test_effect_context(BOOL d3d11) ID2D1EffectContext *effect_context; D2D1_PROPERTY_BINDING binding; struct d2d1_test_context ctx; - ID2D1Effect *effect = NULL; ID2D1Factory1 *factory; + ID2D1Effect *effect; BOOL loaded; HRESULT hr;
@@ -10887,12 +10873,10 @@ static void test_effect_context(BOOL d3d11) ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr);
hr = ID2D1DeviceContext_CreateEffect(ctx.context, &CLSID_TestEffect, &effect); - todo_wine ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); - if (hr != S_OK) - goto done; + ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); hr = ID2D1Effect_GetValueByName(effect, L"Context", D2D1_PROPERTY_TYPE_IUNKNOWN, (BYTE *)&effect_context, sizeof(effect_context)); - ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + todo_wine ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); if (hr != S_OK) goto done;
@@ -10928,8 +10912,7 @@ static void test_effect_context(BOOL d3d11) ok(loaded, "Shader is not loaded.\n");
done: - if (effect) - ID2D1Effect_Release(effect); + ID2D1Effect_Release(effect); hr = ID2D1Factory1_UnregisterEffect(factory, &CLSID_TestEffect); ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); release_test_context(&ctx); @@ -10997,11 +10980,8 @@ static void test_effect_properties(BOOL d3d11) hr = ID2D1Factory1_RegisterEffectFromString(factory, &CLSID_TestEffect, test->xml, NULL, 0, effect_impl_create); ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr);
- effect = NULL; hr = ID2D1DeviceContext_CreateEffect(ctx.context, &CLSID_TestEffect, &effect); - todo_wine ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); - if (hr != S_OK) - goto next; + ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr);
hr = ID2D1Effect_GetValue(effect, D2D1_PROPERTY_CLSID, D2D1_PROPERTY_TYPE_CLSID, (BYTE *)&clsid, sizeof(clsid)); @@ -11009,67 +10989,73 @@ static void test_effect_properties(BOOL d3d11) ok(IsEqualGUID(&clsid, &CLSID_TestEffect), "Got unexpected clsid %s, expected %s.\n", debugstr_guid(&clsid), debugstr_guid(&CLSID_TestEffect));
+ wcscpy(buffer, L"DeadBeef"); hr = ID2D1Effect_GetValue(effect, D2D1_PROPERTY_DISPLAYNAME, D2D1_PROPERTY_TYPE_STRING, (BYTE *)buffer, sizeof(buffer)); - ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); - ok(!wcscmp(buffer, test->display_name), "Got unexpected display name %s, expected %s.\n", + todo_wine ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + todo_wine ok(!wcscmp(buffer, test->display_name), "Got unexpected display name %s, expected %s.\n", debugstr_w(buffer), debugstr_w(test->display_name));
+ wcscpy(buffer, L"DeadBeef"); hr = ID2D1Effect_GetValue(effect, D2D1_PROPERTY_AUTHOR, D2D1_PROPERTY_TYPE_STRING, (BYTE *)buffer, sizeof(buffer)); - ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); - ok(!wcscmp(buffer, test->author), "Got unexpected author %s, expected %s.\n", + todo_wine ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + todo_wine ok(!wcscmp(buffer, test->author), "Got unexpected author %s, expected %s.\n", debugstr_w(buffer), debugstr_w(test->author));
+ wcscpy(buffer, L"DeadBeef"); hr = ID2D1Effect_GetValue(effect, D2D1_PROPERTY_CATEGORY, D2D1_PROPERTY_TYPE_STRING, (BYTE *)buffer, sizeof(buffer)); - ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); - ok(!wcscmp(buffer, test->category), "Got unexpected category %s, expected %s.\n", + todo_wine ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + todo_wine ok(!wcscmp(buffer, test->category), "Got unexpected category %s, expected %s.\n", debugstr_w(buffer), debugstr_w(test->category));
+ wcscpy(buffer, L"DeadBeef"); hr = ID2D1Effect_GetValue(effect, D2D1_PROPERTY_DESCRIPTION, D2D1_PROPERTY_TYPE_STRING, (BYTE *)buffer, sizeof(buffer)); - ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); - ok(!wcscmp(buffer, test->description), "Got unexpected description %s, expected %s.\n", + todo_wine ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + todo_wine ok(!wcscmp(buffer, test->description), "Got unexpected description %s, expected %s.\n", debugstr_w(buffer), debugstr_w(test->description));
hr = ID2D1Effect_GetValue(effect, D2D1_PROPERTY_CACHED, D2D1_PROPERTY_TYPE_BOOL, (BYTE *)&cached, sizeof(cached)); - ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); - ok(cached == FALSE, "Got unexpected cached %d.\n", cached); + todo_wine ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + todo_wine ok(cached == FALSE, "Got unexpected cached %d.\n", cached);
hr = ID2D1Effect_GetValue(effect, D2D1_PROPERTY_PRECISION, D2D1_PROPERTY_TYPE_ENUM, (BYTE *)&precision, sizeof(precision)); - ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); - ok(precision == D2D1_BUFFER_PRECISION_UNKNOWN, "Got unexpected precision %#x.\n", precision); + todo_wine ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + todo_wine ok(precision == D2D1_BUFFER_PRECISION_UNKNOWN, "Got unexpected precision %#x.\n", precision);
hr = ID2D1Effect_GetValue(effect, D2D1_PROPERTY_MIN_INPUTS, D2D1_PROPERTY_TYPE_UINT32, (BYTE *)&min_inputs, sizeof(min_inputs)); ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + todo_wine_if(test->max_inputs != 0) ok(min_inputs == test->min_inputs, "Got unexpected min inputs %u, expected %u.\n", min_inputs, test->min_inputs);
hr = ID2D1Effect_GetValue(effect, D2D1_PROPERTY_MAX_INPUTS, D2D1_PROPERTY_TYPE_UINT32, (BYTE *)&max_inputs, sizeof(max_inputs)); ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + todo_wine_if(test->max_inputs != 0) ok(max_inputs == test->max_inputs, "Got unexpected max inputs %u, expected %u.\n", max_inputs, test->max_inputs);
hr = ID2D1Effect_SetValue(effect, D2D1_PROPERTY_CLSID, D2D1_PROPERTY_TYPE_CLSID, (BYTE *)&clsid, sizeof(clsid)); - ok(hr == E_INVALIDARG || broken(hr == S_OK) /* win8 */, "Got unexpected hr %#lx.\n", hr); + todo_wine ok(hr == E_INVALIDARG || broken(hr == S_OK) /* win8 */, "Got unexpected hr %#lx.\n", hr); hr = ID2D1Effect_SetValue(effect, D2D1_PROPERTY_DISPLAYNAME, D2D1_PROPERTY_TYPE_STRING, (BYTE *)buffer, sizeof(buffer)); - ok(hr == E_INVALIDARG, "Got unexpected hr %#lx.\n", hr); + todo_wine ok(hr == E_INVALIDARG, "Got unexpected hr %#lx.\n", hr); hr = ID2D1Effect_SetValue(effect, D2D1_PROPERTY_AUTHOR, D2D1_PROPERTY_TYPE_STRING, (BYTE *)buffer, sizeof(buffer)); - ok(hr == E_INVALIDARG, "Got unexpected hr %#lx.\n", hr); + todo_wine ok(hr == E_INVALIDARG, "Got unexpected hr %#lx.\n", hr); hr = ID2D1Effect_SetValue(effect, D2D1_PROPERTY_CATEGORY, D2D1_PROPERTY_TYPE_STRING, (BYTE *)buffer, sizeof(buffer)); - ok(hr == E_INVALIDARG, "Got unexpected hr %#lx.\n", hr); + todo_wine ok(hr == E_INVALIDARG, "Got unexpected hr %#lx.\n", hr); hr = ID2D1Effect_SetValue(effect, D2D1_PROPERTY_DISPLAYNAME, D2D1_PROPERTY_TYPE_STRING, (BYTE *)buffer, sizeof(buffer)); - ok(hr == E_INVALIDARG, "Got unexpected hr %#lx.\n", hr); + todo_wine ok(hr == E_INVALIDARG, "Got unexpected hr %#lx.\n", hr); hr = ID2D1Effect_SetValue(effect, D2D1_PROPERTY_CACHED, D2D1_PROPERTY_TYPE_BOOL, (BYTE *)&cached, sizeof(cached)); ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); @@ -11078,14 +11064,12 @@ static void test_effect_properties(BOOL d3d11) ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); hr = ID2D1Effect_SetValue(effect, D2D1_PROPERTY_MIN_INPUTS, D2D1_PROPERTY_TYPE_UINT32, (BYTE *)&min_inputs, sizeof(min_inputs)); - ok(hr == E_INVALIDARG || broken(hr == S_OK) /* win8 */, "Got unexpected hr %#lx.\n", hr); + todo_wine ok(hr == E_INVALIDARG || broken(hr == S_OK) /* win8 */, "Got unexpected hr %#lx.\n", hr); hr = ID2D1Effect_SetValue(effect, D2D1_PROPERTY_MAX_INPUTS, D2D1_PROPERTY_TYPE_UINT32, (BYTE *)&max_inputs, sizeof(max_inputs)); - ok(hr == E_INVALIDARG || broken(hr == S_OK) /* win8 */, "Got unexpected hr %#lx.\n", hr); + todo_wine ok(hr == E_INVALIDARG || broken(hr == S_OK) /* win8 */, "Got unexpected hr %#lx.\n", hr);
- next: - if (effect) - ID2D1Effect_Release(effect); + ID2D1Effect_Release(effect); hr = ID2D1Factory1_UnregisterEffect(factory, &CLSID_TestEffect); ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); winetest_pop_context(); @@ -11097,50 +11081,47 @@ static void test_effect_properties(BOOL d3d11) effect_xml_c, binding, 2, effect_impl_create); ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr);
- effect = NULL; hr = ID2D1DeviceContext_CreateEffect(ctx.context, &CLSID_TestEffect, &effect); - todo_wine ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); - if (hr != S_OK) - goto done; + ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr);
index = ID2D1Effect_GetPropertyIndex(effect, L"Context"); ok(index == 0, "Got unexpected index %u.\n", index); index = ID2D1Effect_GetPropertyIndex(effect, L"Integer"); - ok(index == 1, "Got unexpected index %u.\n", index); + todo_wine ok(index == 1, "Got unexpected index %u.\n", index);
effect_context = (ID2D1EffectContext *)0xdeadbeef; hr = ID2D1Effect_GetValueByName(effect, L"Context", D2D1_PROPERTY_TYPE_IUNKNOWN, (BYTE *)&effect_context, sizeof(effect_context)); - ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); - ok(effect_context != NULL && effect_context != (ID2D1EffectContext *)0xdeadbeef, + todo_wine ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + todo_wine ok(effect_context != NULL && effect_context != (ID2D1EffectContext *)0xdeadbeef, "Got unexpected effect context %p.\n", effect_context);
effect_context = (ID2D1EffectContext *)0xdeadbeef; hr = ID2D1Effect_GetValue(effect, 0, D2D1_PROPERTY_TYPE_IUNKNOWN, (BYTE *)&effect_context, sizeof(effect_context)); - ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); - ok(effect_context != NULL && effect_context != (ID2D1EffectContext *)0xdeadbeef, + todo_wine ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + todo_wine ok(effect_context != NULL && effect_context != (ID2D1EffectContext *)0xdeadbeef, "Got unexpected effect context %p.\n", effect_context);
hr = ID2D1Effect_SetValue(effect, 0, D2D1_PROPERTY_TYPE_IUNKNOWN, (BYTE *)&effect_context, sizeof(effect_context)); - ok(hr == E_INVALIDARG, "Got unexpected hr %#lx.\n", hr); + todo_wine ok(hr == E_INVALIDARG, "Got unexpected hr %#lx.\n", hr);
integer = 0xdeadbeef; hr = ID2D1Effect_GetValueByName(effect, L"Integer", D2D1_PROPERTY_TYPE_UINT32, (BYTE *)&integer, sizeof(integer)); - ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); - ok(integer == 10, "Got unexpected integer %u.", integer); + todo_wine ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + todo_wine ok(integer == 10, "Got unexpected integer %u.", integer);
integer = 0xdeadbeef; hr = ID2D1Effect_GetValue(effect, 1, D2D1_PROPERTY_TYPE_UINT32, (BYTE *)&integer, sizeof(integer)); - ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); - ok(integer == 10, "Got unexpected integer %u.", integer); + todo_wine ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + todo_wine ok(integer == 10, "Got unexpected integer %u.", integer);
integer = 20; hr = ID2D1Effect_SetValue(effect, 1, D2D1_PROPERTY_TYPE_UINT32, (BYTE *)&integer, sizeof(integer)); ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); integer = 0xdeadbeef; hr = ID2D1Effect_GetValue(effect, 1, D2D1_PROPERTY_TYPE_UINT32, (BYTE *)&integer, sizeof(integer)); - ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); - ok(integer == 20, "Got unexpected integer %u.", integer); + todo_wine ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + todo_wine ok(integer == 20, "Got unexpected integer %u.", integer);
ID2D1Effect_Release(effect); hr = ID2D1Factory1_UnregisterEffect(factory, &CLSID_TestEffect); @@ -11156,32 +11137,29 @@ static void test_effect_properties(BOOL d3d11) index = ID2D1Effect_GetPropertyIndex(effect, L"Context"); ok(index == 0, "Got unexpected index %u.\n", index); index = ID2D1Effect_GetPropertyIndex(effect, L"Integer"); - ok(index == 1, "Got unexpected index %u.\n", index); + todo_wine ok(index == 1, "Got unexpected index %u.\n", index);
hr = ID2D1Effect_GetValueByName(effect, L"DeadBeef", D2D1_PROPERTY_TYPE_UINT32, (BYTE *)&integer, sizeof(integer)); - ok(hr == D2DERR_INVALID_PROPERTY, "Got unexpected hr %#lx.\n", hr); + todo_wine ok(hr == D2DERR_INVALID_PROPERTY, "Got unexpected hr %#lx.\n", hr);
effect_context = (ID2D1EffectContext *)0xdeadbeef; hr = ID2D1Effect_GetValue(effect, 0, D2D1_PROPERTY_TYPE_IUNKNOWN, (BYTE *)&effect_context, sizeof(effect_context)); - ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); - ok(effect_context == NULL, "Got unexpected effect context %p.\n", effect_context); + todo_wine ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + todo_wine ok(effect_context == NULL, "Got unexpected effect context %p.\n", effect_context);
integer = 0xdeadbeef; hr = ID2D1Effect_GetValue(effect, 1, D2D1_PROPERTY_TYPE_UINT32, (BYTE *)&integer, sizeof(integer)); - ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); - ok(integer == 0, "Got unexpected integer %u.", integer); + todo_wine ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + todo_wine ok(integer == 0, "Got unexpected integer %u.", integer);
hr = ID2D1Effect_SetValue(effect, 0, D2D1_PROPERTY_TYPE_IUNKNOWN, (BYTE *)&effect_context, sizeof(effect_context)); - ok(hr == E_INVALIDARG, "Got unexpected hr %#lx.\n", hr); + todo_wine ok(hr == E_INVALIDARG, "Got unexpected hr %#lx.\n", hr); hr = ID2D1Effect_SetValue(effect, 1, D2D1_PROPERTY_TYPE_UINT32, (BYTE *)&integer, sizeof(integer)); - ok(hr == E_INVALIDARG || broken(hr == S_OK) /* win8 */, "Got unexpected hr %#lx.\n", hr); + todo_wine ok(hr == E_INVALIDARG || broken(hr == S_OK) /* win8 */, "Got unexpected hr %#lx.\n", hr);
-done: - if (effect) - ID2D1Effect_Release(effect); + ID2D1Effect_Release(effect); hr = ID2D1Factory1_UnregisterEffect(factory, &CLSID_TestEffect); ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); - release_test_context(&ctx); }
Hi,
It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated.
The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details:
The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=117348
Your paranoid android.
=== debian11 (64 bit WoW report) ===
d2d1: d2d1.c:11023: Test succeeded inside todo block: Test 0: Got unexpected cached 0. d2d1.c:11023: Test succeeded inside todo block: Test 1: Got unexpected cached 0. d2d1.c:11023: Test succeeded inside todo block: Test 2: Got unexpected cached 0. d2d1.c:11023: Test succeeded inside todo block: Test 3: Got unexpected cached 0. d2d1.c:11023: Test succeeded inside todo block: Test 0: Got unexpected cached 0. d2d1.c:11023: Test succeeded inside todo block: Test 1: Got unexpected cached 0. d2d1.c:11023: Test succeeded inside todo block: Test 2: Got unexpected cached 0. d2d1.c:11023: Test succeeded inside todo block: Test 3: Got unexpected cached 0.
On 6/21/22 06:17, Ziqing Hui wrote:
HRESULT d2d_effect_init(struct d2d_effect *effect, struct d2d_effect_context *effect_context, const CLSID *effect_id) {
- unsigned int i;
struct d2d_effect_registration *reg;
struct d2d_factory *factory;
HRESULT hr;
effect->ID2D1Effect_iface.lpVtbl = &d2d_effect_vtbl; effect->ID2D1Image_iface.lpVtbl = &d2d_effect_image_vtbl; effect->refcount = 1;
- for (i = 0; i < ARRAY_SIZE(builtin_effects); ++i)
- factory = unsafe_impl_from_ID2D1Factory3((ID2D1Factory3 *)effect_context->device_context->factory);
- LIST_FOR_EACH_ENTRY(reg, &factory->effects, struct d2d_effect_registration, entry) {
if (IsEqualGUID(effect_id, &builtin_effects[i].id))
if (IsEqualGUID(effect_id, ®->id)) {
effect->min_inputs = builtin_effects[i].min_inputs;effect->max_inputs = builtin_effects[i].max_inputs;d2d_effect_SetInputCount(&effect->ID2D1Effect_iface, builtin_effects[i].default_input_count);
if (FAILED(hr = reg->factory((IUnknown **)&effect->impl)))return hr;if (FAILED(hr = ID2D1EffectImpl_Initialize(effect->impl, &effect_context->ID2D1EffectContext_iface, NULL))){ID2D1EffectImpl_Release(effect->impl);return hr;}effect->id = *effect_id;effect->min_inputs = reg->min_inputs;effect->max_inputs = reg->max_inputs;d2d_effect_SetInputCount(&effect->ID2D1Effect_iface, reg->default_input_count); effect->effect_context = effect_context; ID2D1EffectContext_AddRef(&effect_context->ID2D1EffectContext_iface);/* FIXME: Properties are ignored. */ return S_OK; } }
CreateEffect() is a single place where this would be used, so I'd suggest to pick correct d2d_effect_registration there, and simply pass it to this helper.
Using unsafe_* is normally reserved for externally provided pointers, that's not the case here.
static const struct d2d_builtin_effect_registration builtin_effects[] = {
- {&CLSID_D2D12DAffineTransform, NULL, 1, 1, 1},
- {&CLSID_D2D13DPerspectiveTransform, NULL, 1, 1, 1},
- {&CLSID_D2D1Composite, NULL, 2, 1, 0xffffffff},
- {&CLSID_D2D1Crop, NULL, 1, 1, 1},
- {&CLSID_D2D1Shadow, NULL, 1, 1, 1},
- {&CLSID_D2D1Grayscale, NULL, 1, 1, 1},
- {&CLSID_D2D12DAffineTransform, d2d_effect_impl_create, 1, 1, 1},
- {&CLSID_D2D13DPerspectiveTransform, d2d_effect_impl_create, 1, 1, 1},
- {&CLSID_D2D1Composite, d2d_effect_impl_create, 2, 1, 0xffffffff},
- {&CLSID_D2D1Crop, d2d_effect_impl_create, 1, 1, 1},
- {&CLSID_D2D1Shadow, d2d_effect_impl_create, 1, 1, 1},
- {&CLSID_D2D1Grayscale, d2d_effect_impl_create, 1, 1, 1}, };
I don't know how extensible this is, every _create would be different.
On 6/21/22 4:45 PM, Nikolay Sivov wrote:
On 6/21/22 06:17, Ziqing Hui wrote:
HRESULT d2d_effect_init(struct d2d_effect *effect, struct d2d_effect_context *effect_context, const CLSID *effect_id) { - unsigned int i; + struct d2d_effect_registration *reg; + struct d2d_factory *factory; + HRESULT hr; effect->ID2D1Effect_iface.lpVtbl = &d2d_effect_vtbl; effect->ID2D1Image_iface.lpVtbl = &d2d_effect_image_vtbl; effect->refcount = 1; - for (i = 0; i < ARRAY_SIZE(builtin_effects); ++i) + factory = unsafe_impl_from_ID2D1Factory3((ID2D1Factory3 *)effect_context->device_context->factory); + LIST_FOR_EACH_ENTRY(reg, &factory->effects, struct d2d_effect_registration, entry) { - if (IsEqualGUID(effect_id, &builtin_effects[i].id)) + if (IsEqualGUID(effect_id, ®->id)) { - effect->min_inputs = builtin_effects[i].min_inputs; - effect->max_inputs = builtin_effects[i].max_inputs; - d2d_effect_SetInputCount(&effect->ID2D1Effect_iface, builtin_effects[i].default_input_count); + if (FAILED(hr = reg->factory((IUnknown **)&effect->impl))) + return hr; + if (FAILED(hr = ID2D1EffectImpl_Initialize(effect->impl, &effect_context->ID2D1EffectContext_iface, NULL))) + { + ID2D1EffectImpl_Release(effect->impl); + return hr; + } + effect->id = *effect_id; + effect->min_inputs = reg->min_inputs; + effect->max_inputs = reg->max_inputs; + d2d_effect_SetInputCount(&effect->ID2D1Effect_iface, reg->default_input_count); effect->effect_context = effect_context; ID2D1EffectContext_AddRef(&effect_context->ID2D1EffectContext_iface); + /* FIXME: Properties are ignored. */ return S_OK; } }
CreateEffect() is a single place where this would be used, so I'd suggest to pick correct d2d_effect_registration there, and simply pass it to this helper.
Using unsafe_* is normally reserved for externally provided pointers, that's not the case here.
OK, I'll do that.
static const struct d2d_builtin_effect_registration builtin_effects[] = { - {&CLSID_D2D12DAffineTransform, NULL, 1, 1, 1}, - {&CLSID_D2D13DPerspectiveTransform, NULL, 1, 1, 1}, - {&CLSID_D2D1Composite, NULL, 2, 1, 0xffffffff}, - {&CLSID_D2D1Crop, NULL, 1, 1, 1}, - {&CLSID_D2D1Shadow, NULL, 1, 1, 1}, - {&CLSID_D2D1Grayscale, NULL, 1, 1, 1}, + {&CLSID_D2D12DAffineTransform, d2d_effect_impl_create, 1, 1, 1}, + {&CLSID_D2D13DPerspectiveTransform, d2d_effect_impl_create, 1, 1, 1}, + {&CLSID_D2D1Composite, d2d_effect_impl_create, 2, 1, 0xffffffff}, + {&CLSID_D2D1Crop, d2d_effect_impl_create, 1, 1, 1}, + {&CLSID_D2D1Shadow, d2d_effect_impl_create, 1, 1, 1}, + {&CLSID_D2D1Grayscale, d2d_effect_impl_create, 1, 1, 1}, };
I don't know how extensible this is, every _create would be different.
Yeah, each builtin effect would have a different _create. So the _create here in this patch is just a placeholder, which makes sure every builtin effect in the list can be at least successfully created for now.
My plan is something like, we take 2DAffineTransform as an example: we define a new struct for each effect:
struct d2d_2d_affine_transform { struct d2d_effect_impl effect_impl;
... its own fileds ... }
d2d_effect_impl will be included in the beginning of each effect struct. The benefit of this is that we can reuse one QueryInterface, AddRef, Release, we don't have to define a new one for each new effect.
And of course, _create will be different for each effect.
On 6/21/22 12:34, Ziqing Hui wrote:
On 6/21/22 4:45 PM, Nikolay Sivov wrote:
On 6/21/22 06:17, Ziqing Hui wrote:
HRESULT d2d_effect_init(struct d2d_effect *effect, struct d2d_effect_context *effect_context, const CLSID *effect_id) { - unsigned int i; + struct d2d_effect_registration *reg; + struct d2d_factory *factory; + HRESULT hr; effect->ID2D1Effect_iface.lpVtbl = &d2d_effect_vtbl; effect->ID2D1Image_iface.lpVtbl = &d2d_effect_image_vtbl; effect->refcount = 1; - for (i = 0; i < ARRAY_SIZE(builtin_effects); ++i) + factory = unsafe_impl_from_ID2D1Factory3((ID2D1Factory3 *)effect_context->device_context->factory); + LIST_FOR_EACH_ENTRY(reg, &factory->effects, struct d2d_effect_registration, entry) { - if (IsEqualGUID(effect_id, &builtin_effects[i].id)) + if (IsEqualGUID(effect_id, ®->id)) { - effect->min_inputs = builtin_effects[i].min_inputs; - effect->max_inputs = builtin_effects[i].max_inputs; - d2d_effect_SetInputCount(&effect->ID2D1Effect_iface, builtin_effects[i].default_input_count); + if (FAILED(hr = reg->factory((IUnknown **)&effect->impl))) + return hr; + if (FAILED(hr = ID2D1EffectImpl_Initialize(effect->impl, &effect_context->ID2D1EffectContext_iface, NULL))) + { + ID2D1EffectImpl_Release(effect->impl); + return hr; + } + effect->id = *effect_id; + effect->min_inputs = reg->min_inputs; + effect->max_inputs = reg->max_inputs; + d2d_effect_SetInputCount(&effect->ID2D1Effect_iface, reg->default_input_count); effect->effect_context = effect_context; ID2D1EffectContext_AddRef(&effect_context->ID2D1EffectContext_iface); + /* FIXME: Properties are ignored. */ return S_OK; } }
CreateEffect() is a single place where this would be used, so I'd suggest to pick correct d2d_effect_registration there, and simply pass it to this helper.
Using unsafe_* is normally reserved for externally provided pointers, that's not the case here.
OK, I'll do that.
static const struct d2d_builtin_effect_registration builtin_effects[] = { - {&CLSID_D2D12DAffineTransform, NULL, 1, 1, 1}, - {&CLSID_D2D13DPerspectiveTransform, NULL, 1, 1, 1}, - {&CLSID_D2D1Composite, NULL, 2, 1, 0xffffffff}, - {&CLSID_D2D1Crop, NULL, 1, 1, 1}, - {&CLSID_D2D1Shadow, NULL, 1, 1, 1}, - {&CLSID_D2D1Grayscale, NULL, 1, 1, 1}, + {&CLSID_D2D12DAffineTransform, d2d_effect_impl_create, 1, 1, 1}, + {&CLSID_D2D13DPerspectiveTransform, d2d_effect_impl_create, 1, 1, 1}, + {&CLSID_D2D1Composite, d2d_effect_impl_create, 2, 1, 0xffffffff}, + {&CLSID_D2D1Crop, d2d_effect_impl_create, 1, 1, 1}, + {&CLSID_D2D1Shadow, d2d_effect_impl_create, 1, 1, 1}, + {&CLSID_D2D1Grayscale, d2d_effect_impl_create, 1, 1, 1}, };
I don't know how extensible this is, every _create would be different.
Yeah, each builtin effect would have a different _create. So the _create here in this patch is just a placeholder, which makes sure every builtin effect in the list can be at least successfully created for now.
My plan is something like, we take 2DAffineTransform as an example: we define a new struct for each effect:
struct d2d_2d_affine_transform { struct d2d_effect_impl effect_impl;
... its own fileds ...}
d2d_effect_impl will be included in the beginning of each effect struct. The benefit of this is that we can reuse one QueryInterface, AddRef, Release, we don't have to define a new one for each new effect.
And of course, _create will be different for each effect.
Single stub is fine. Regarding nesting structures, I don't think it buys a lot in this case. My understanding is that structures for specific effects like d2d_2d_affine_transform will have to hold only current property values, updated through setter function. So yes, you will be able to reuse QI, AddRef, and maybe Release, unless some effects will have properties that need to be released. The rest of the methods will be very different between effects. Maybe it's easier to pick some simple effect and make that work first, without adding stubs that will have to be removed? From existing list Grayscale one is probably the easiest.
Signed-off-by: Ziqing Hui zhui@codeweavers.com --- dlls/d2d1/d2d1_private.h | 1 + dlls/d2d1/effect.c | 161 +++++++++++++++++++++++++++++++++++++-- 2 files changed, 156 insertions(+), 6 deletions(-)
diff --git a/dlls/d2d1/d2d1_private.h b/dlls/d2d1/d2d1_private.h index ffe1cd9e1dc..a42bf367916 100644 --- a/dlls/d2d1/d2d1_private.h +++ b/dlls/d2d1/d2d1_private.h @@ -657,6 +657,7 @@ struct d2d_effect UINT32 min_inputs; UINT32 max_inputs;
+ ID2D1TransformGraph *transform_graph; ID2D1EffectImpl *impl; struct d2d_effect_context *effect_context; ID2D1Image **inputs; diff --git a/dlls/d2d1/effect.c b/dlls/d2d1/effect.c index c1ef67c89aa..23f117c85bf 100644 --- a/dlls/d2d1/effect.c +++ b/dlls/d2d1/effect.c @@ -20,6 +20,12 @@
WINE_DEFAULT_DEBUG_CHANNEL(d2d);
+struct d2d_transform_graph +{ + ID2D1TransformGraph ID2D1TransformGraph_iface; + LONG refcount; +}; + struct d2d_effect_impl { ID2D1EffectImpl ID2D1EffectImpl_iface; @@ -35,6 +41,136 @@ struct d2d_builtin_effect_registration UINT32 max_inputs; };
+static inline struct d2d_transform_graph *impl_from_ID2D1TransformGraph(ID2D1TransformGraph *iface) +{ + return CONTAINING_RECORD(iface, struct d2d_transform_graph, ID2D1TransformGraph_iface); +} + +static HRESULT STDMETHODCALLTYPE d2d_transform_graph_QueryInterface(ID2D1TransformGraph *iface, REFIID iid, void **out) +{ + TRACE("iface %p, iid %s, out %p.\n", iface, debugstr_guid(iid), out); + + if (IsEqualGUID(iid, &IID_ID2D1TransformGraph) + || IsEqualGUID(iid, &IID_IUnknown)) + { + ID2D1TransformGraph_AddRef(iface); + *out = iface; + return S_OK; + } + + *out = NULL; + return E_NOINTERFACE; +} + +static ULONG STDMETHODCALLTYPE d2d_transform_graph_AddRef(ID2D1TransformGraph *iface) +{ + struct d2d_transform_graph *graph =impl_from_ID2D1TransformGraph(iface); + ULONG refcount = InterlockedIncrement(&graph->refcount); + + TRACE("%p increasing refcount to %lu.\n", iface, refcount); + + return refcount; +} + +static ULONG STDMETHODCALLTYPE d2d_transform_graph_Release(ID2D1TransformGraph *iface) +{ + struct d2d_transform_graph *graph = impl_from_ID2D1TransformGraph(iface); + ULONG refcount = InterlockedDecrement(&graph->refcount); + + TRACE("%p decreasing refcount to %lu.\n", iface, refcount); + + if (!refcount) + free(graph); + + return refcount; +} + +static UINT32 STDMETHODCALLTYPE d2d_transform_graph_GetInputCount(ID2D1TransformGraph *iface) +{ + FIXME("iface %p stub!\n", iface); + + return 0; +} + +static HRESULT STDMETHODCALLTYPE d2d_transform_graph_SetSingleTransformNode(ID2D1TransformGraph *iface, + ID2D1TransformNode *node) +{ + FIXME("iface %p, node %p stub!\n", iface, node); + + return E_NOTIMPL; +} + +static HRESULT STDMETHODCALLTYPE d2d_transform_graph_AddNode(ID2D1TransformGraph *iface, ID2D1TransformNode *node) +{ + FIXME("iface %p, node %p stub!\n", iface, node); + + return E_NOTIMPL; +} + +static HRESULT STDMETHODCALLTYPE d2d_transform_graph_RemoveNode(ID2D1TransformGraph *iface, ID2D1TransformNode *node) +{ + FIXME("iface %p, node %p stub!\n", iface, node); + + return E_NOTIMPL; +} + +static HRESULT STDMETHODCALLTYPE d2d_transform_graph_SetOutputNode(ID2D1TransformGraph *iface, ID2D1TransformNode *node) +{ + FIXME("iface %p, node %p stub!\n", iface, node); + + return E_NOTIMPL; +} + +static HRESULT STDMETHODCALLTYPE d2d_transform_graph_ConnectNode(ID2D1TransformGraph *iface, + ID2D1TransformNode *from_node, ID2D1TransformNode *to_node, UINT32 index) +{ + FIXME("iface %p, from_node %p, to_node %p, index %u stub!\n", iface, from_node, to_node, index); + + return E_NOTIMPL; +} + +static HRESULT STDMETHODCALLTYPE d2d_transform_graph_ConnectToEffectInput(ID2D1TransformGraph *iface, + UINT32 input_index, ID2D1TransformNode *node, UINT32 node_index) +{ + FIXME("iface %p, input_index %u, node %p, node_index %u stub!\n", iface, input_index, node, node_index); + + return E_NOTIMPL; +} + +static void STDMETHODCALLTYPE d2d_transform_graph_Clear(ID2D1TransformGraph *iface) +{ + FIXME("iface %p stub!\n", iface); +} + +static HRESULT STDMETHODCALLTYPE d2d_transform_graph_SetPassthroughGraph(ID2D1TransformGraph *iface, UINT32 index) +{ + FIXME("iface %p, index %u stub!\n", iface, index); + + return E_NOTIMPL; +} + +static const ID2D1TransformGraphVtbl d2d_transform_graph_vtbl = +{ + d2d_transform_graph_QueryInterface, + d2d_transform_graph_AddRef, + d2d_transform_graph_Release, + d2d_transform_graph_GetInputCount, + d2d_transform_graph_SetSingleTransformNode, + d2d_transform_graph_AddNode, + d2d_transform_graph_RemoveNode, + d2d_transform_graph_SetOutputNode, + d2d_transform_graph_ConnectNode, + d2d_transform_graph_ConnectToEffectInput, + d2d_transform_graph_Clear, + d2d_transform_graph_SetPassthroughGraph, +}; + +static void d2d_transform_graph_init(struct d2d_transform_graph *graph) +{ + graph->ID2D1TransformGraph_iface.lpVtbl = &d2d_transform_graph_vtbl; + graph->refcount = 1; +} + static inline struct d2d_effect_impl *impl_from_ID2D1EffectImpl(ID2D1EffectImpl *iface) { return CONTAINING_RECORD(iface, struct d2d_effect_impl, ID2D1EffectImpl_iface); @@ -524,6 +660,7 @@ static void d2d_effect_cleanup(struct d2d_effect *effect) free(effect->inputs); ID2D1EffectContext_Release(&effect->effect_context->ID2D1EffectContext_iface); ID2D1EffectImpl_Release(effect->impl); + ID2D1TransformGraph_Release(effect->transform_graph); }
static HRESULT STDMETHODCALLTYPE d2d_effect_QueryInterface(ID2D1Effect *iface, REFIID iid, void **out) @@ -852,6 +989,7 @@ static const ID2D1ImageVtbl d2d_effect_image_vtbl =
HRESULT d2d_effect_init(struct d2d_effect *effect, struct d2d_effect_context *effect_context, const CLSID *effect_id) { + struct d2d_transform_graph *transform_graph = NULL; struct d2d_effect_registration *reg; struct d2d_factory *factory; HRESULT hr; @@ -865,13 +1003,18 @@ HRESULT d2d_effect_init(struct d2d_effect *effect, struct d2d_effect_context *ef { if (IsEqualGUID(effect_id, ®->id)) { + if (!(transform_graph = calloc(1, sizeof(*transform_graph)))) + return E_OUTOFMEMORY; + d2d_transform_graph_init(transform_graph); + if (FAILED(hr = reg->factory((IUnknown **)&effect->impl))) - return hr; - if (FAILED(hr = ID2D1EffectImpl_Initialize(effect->impl, &effect_context->ID2D1EffectContext_iface, NULL))) - { - ID2D1EffectImpl_Release(effect->impl); - return hr; - } + goto err; + effect->transform_graph = &transform_graph->ID2D1TransformGraph_iface; + + if (FAILED(hr = ID2D1EffectImpl_Initialize(effect->impl, + &effect_context->ID2D1EffectContext_iface, effect->transform_graph))) + goto err; + effect->id = *effect_id; effect->min_inputs = reg->min_inputs; effect->max_inputs = reg->max_inputs; @@ -885,6 +1028,12 @@ HRESULT d2d_effect_init(struct d2d_effect *effect, struct d2d_effect_context *ef
WARN("Unsupported effect clsid %s.\n", debugstr_guid(effect_id)); return E_FAIL; + +err: + if (effect->impl) + ID2D1EffectImpl_Release(effect->impl); + free(transform_graph); + return hr; }
HRESULT d2d_register_builtin_effects(struct d2d_factory *factory)
Hi,
It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated.
The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details:
The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=117349
Your paranoid android.
=== debian11 (64 bit WoW report) ===
d2d1: d2d1.c:11023: Test succeeded inside todo block: Test 0: Got unexpected cached 0. d2d1.c:11023: Test succeeded inside todo block: Test 1: Got unexpected cached 0. d2d1.c:11023: Test succeeded inside todo block: Test 2: Got unexpected cached 0. d2d1.c:11023: Test succeeded inside todo block: Test 3: Got unexpected cached 0. d2d1.c:11023: Test succeeded inside todo block: Test 0: Got unexpected cached 0. d2d1.c:11023: Test succeeded inside todo block: Test 1: Got unexpected cached 0. d2d1.c:11023: Test succeeded inside todo block: Test 2: Got unexpected cached 0. d2d1.c:11023: Test succeeded inside todo block: Test 3: Got unexpected cached 0.
On 6/21/22 06:17, Ziqing Hui wrote:
+err:
- if (effect->impl)
ID2D1EffectImpl_Release(effect->impl);- free(transform_graph);
- return hr;
Graph object freeing does not look right here.
Hi,
It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated.
The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details:
The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=117347
Your paranoid android.
=== debian11 (64 bit WoW report) ===
d2d1: d2d1.c:10373: Test failed: Test 0: Got unexpected hr 0x80004005. Unhandled exception: page fault on read access to 0xffffffffffffffff in 64-bit code (0x0000000041bacd).
On 6/21/22 06:17, Ziqing Hui wrote:
@@ -616,7 +651,9 @@ struct d2d_effect ID2D1Image ID2D1Image_iface; LONG refcount;
- const struct d2d_effect_info *info;
CLSID id;
UINT32 min_inputs;
UINT32 max_inputs;
struct d2d_effect_context *effect_context; ID2D1Image **inputs;
This should be a part of property system, not exposed like that.
@@ -554,21 +563,21 @@ static HRESULT STDMETHODCALLTYPE d2d_effect_GetValue(ID2D1Effect *iface, UINT32 { case D2D1_PROPERTY_CLSID: if ((type != D2D1_PROPERTY_TYPE_UNKNOWN && type != D2D1_PROPERTY_TYPE_CLSID)
|| value_size != sizeof(*effect->info->clsid))
|| value_size != sizeof(effect->id)) return E_INVALIDARG;
src = effect->info->clsid;
src = &effect->id; break; case D2D1_PROPERTY_MIN_INPUTS: if ((type != D2D1_PROPERTY_TYPE_UNKNOWN && type != D2D1_PROPERTY_TYPE_UINT32)
|| value_size != sizeof(effect->info->min_inputs))
|| value_size != sizeof(effect->min_inputs)) return E_INVALIDARG;
src = &effect->info->min_inputs;
src = &effect->min_inputs; break; case D2D1_PROPERTY_MAX_INPUTS: if ((type != D2D1_PROPERTY_TYPE_UNKNOWN && type != D2D1_PROPERTY_TYPE_UINT32)
|| value_size != sizeof(effect->info->max_inputs))
|| value_size != sizeof(effect->max_inputs)) return E_INVALIDARG;
src = &effect->info->max_inputs;
src = &effect->max_inputs; break; default: if (index < D2D1_PROPERTY_CLSID)
Similarly, this needs rework.
-static const struct d2d_effect_info builtin_effects[] = +struct d2d_builtin_effect_registration {
- {&CLSID_D2D12DAffineTransform, 1, 1, 1},
- {&CLSID_D2D13DPerspectiveTransform, 1, 1, 1},
- {&CLSID_D2D1Composite, 2, 1, 0xffffffff},
- {&CLSID_D2D1Crop, 1, 1, 1},
- {&CLSID_D2D1Shadow, 1, 1, 1},
- {&CLSID_D2D1Grayscale, 1, 1, 1},
- const CLSID *id;
- PD2D1_EFFECT_FACTORY factory;
- UINT32 default_input_count;
- UINT32 min_inputs;
- UINT32 max_inputs;
+};
It should be possible to reuse same structure for builtin effects.
+HRESULT d2d_register_builtin_effects(struct d2d_factory *factory) +{
- struct d2d_effect_registration *reg;
- unsigned int i;
- for (i = 0; i < ARRAY_SIZE(builtin_effects); ++i)
- {
const struct d2d_builtin_effect_registration *builtin_reg = &builtin_effects[i];if (!(reg = calloc(1, sizeof(*reg))))return E_OUTOFMEMORY;reg->is_builtin = TRUE;reg->factory = builtin_reg->factory;reg->registration_count = 1;reg->id = *builtin_reg->id;reg->default_input_count = builtin_reg->default_input_count;reg->min_inputs = builtin_reg->min_inputs;reg->max_inputs = builtin_reg->max_inputs;list_add_tail(&factory->effects, ®->entry);- }
- return S_OK;
+}
I'm not sure if we want that. It should be enough to check if CLSID is for builtin in Register* call, and redirect to builtin data in CreateEffect().
On 6/21/22 4:35 PM, Nikolay Sivov wrote:
On 6/21/22 06:17, Ziqing Hui wrote:
@@ -616,7 +651,9 @@ struct d2d_effect ID2D1Image ID2D1Image_iface; LONG refcount; - const struct d2d_effect_info *info; + CLSID id; + UINT32 min_inputs; + UINT32 max_inputs; struct d2d_effect_context *effect_context; ID2D1Image **inputs;
This should be a part of property system, not exposed like that.
@@ -554,21 +563,21 @@ static HRESULT STDMETHODCALLTYPE d2d_effect_GetValue(ID2D1Effect *iface, UINT32 { case D2D1_PROPERTY_CLSID: if ((type != D2D1_PROPERTY_TYPE_UNKNOWN && type != D2D1_PROPERTY_TYPE_CLSID) - || value_size != sizeof(*effect->info->clsid)) + || value_size != sizeof(effect->id)) return E_INVALIDARG; - src = effect->info->clsid; + src = &effect->id; break; case D2D1_PROPERTY_MIN_INPUTS: if ((type != D2D1_PROPERTY_TYPE_UNKNOWN && type != D2D1_PROPERTY_TYPE_UINT32) - || value_size != sizeof(effect->info->min_inputs)) + || value_size != sizeof(effect->min_inputs)) return E_INVALIDARG; - src = &effect->info->min_inputs; + src = &effect->min_inputs; break; case D2D1_PROPERTY_MAX_INPUTS: if ((type != D2D1_PROPERTY_TYPE_UNKNOWN && type != D2D1_PROPERTY_TYPE_UINT32) - || value_size != sizeof(effect->info->max_inputs)) + || value_size != sizeof(effect->max_inputs)) return E_INVALIDARG; - src = &effect->info->max_inputs; + src = &effect->max_inputs; break; default: if (index < D2D1_PROPERTY_CLSID)
Similarly, this needs rework.
OK, I'll rework the property system first.
-static const struct d2d_effect_info builtin_effects[] = +struct d2d_builtin_effect_registration { - {&CLSID_D2D12DAffineTransform, 1, 1, 1}, - {&CLSID_D2D13DPerspectiveTransform, 1, 1, 1}, - {&CLSID_D2D1Composite, 2, 1, 0xffffffff}, - {&CLSID_D2D1Crop, 1, 1, 1}, - {&CLSID_D2D1Shadow, 1, 1, 1}, - {&CLSID_D2D1Grayscale, 1, 1, 1}, + const CLSID *id; + PD2D1_EFFECT_FACTORY factory; + UINT32 default_input_count; + UINT32 min_inputs; + UINT32 max_inputs; +};
It should be possible to reuse same structure for builtin effects.
Does it means that we should reuse d2d_effect_registration for the builtin effect data here? Or we keep d2d_effect_info and ignore factory field in this patch?
If use d2d_effect_registration, one problem is that CLSID is stored by itself in d2d_effect_registration, not a const pointer. So we are not able to set it statically. Then we can have something like:
struct d2d_builtin_effects { const CLSID *id; struct d2d_effect_registration reg; } builtin_effects[] = {....};
Does it work?
+HRESULT d2d_register_builtin_effects(struct d2d_factory *factory) +{ + struct d2d_effect_registration *reg; + unsigned int i;
+ for (i = 0; i < ARRAY_SIZE(builtin_effects); ++i) + { + const struct d2d_builtin_effect_registration *builtin_reg = &builtin_effects[i];
+ if (!(reg = calloc(1, sizeof(*reg)))) + return E_OUTOFMEMORY;
+ reg->is_builtin = TRUE; + reg->factory = builtin_reg->factory; + reg->registration_count = 1; + reg->id = *builtin_reg->id; + reg->default_input_count = builtin_reg->default_input_count; + reg->min_inputs = builtin_reg->min_inputs; + reg->max_inputs = builtin_reg->max_inputs; + list_add_tail(&factory->effects, ®->entry); + }
+ return S_OK; +}
I'm not sure if we want that. It should be enough to check if CLSID is for builtin in Register* call, and redirect to builtin data in CreateEffect().
It means that, we don't register builtin effect to factory's registered effects list, and access builtin data directly in CreateEffect()?
On 6/21/22 12:24, Ziqing Hui wrote:
On 6/21/22 4:35 PM, Nikolay Sivov wrote:
On 6/21/22 06:17, Ziqing Hui wrote:
@@ -616,7 +651,9 @@ struct d2d_effect ID2D1Image ID2D1Image_iface; LONG refcount; - const struct d2d_effect_info *info; + CLSID id; + UINT32 min_inputs; + UINT32 max_inputs; struct d2d_effect_context *effect_context; ID2D1Image **inputs;
This should be a part of property system, not exposed like that.
@@ -554,21 +563,21 @@ static HRESULT STDMETHODCALLTYPE d2d_effect_GetValue(ID2D1Effect *iface, UINT32 { case D2D1_PROPERTY_CLSID: if ((type != D2D1_PROPERTY_TYPE_UNKNOWN && type != D2D1_PROPERTY_TYPE_CLSID) - || value_size != sizeof(*effect->info->clsid)) + || value_size != sizeof(effect->id)) return E_INVALIDARG; - src = effect->info->clsid; + src = &effect->id; break; case D2D1_PROPERTY_MIN_INPUTS: if ((type != D2D1_PROPERTY_TYPE_UNKNOWN && type != D2D1_PROPERTY_TYPE_UINT32) - || value_size != sizeof(effect->info->min_inputs)) + || value_size != sizeof(effect->min_inputs)) return E_INVALIDARG; - src = &effect->info->min_inputs; + src = &effect->min_inputs; break; case D2D1_PROPERTY_MAX_INPUTS: if ((type != D2D1_PROPERTY_TYPE_UNKNOWN && type != D2D1_PROPERTY_TYPE_UINT32) - || value_size != sizeof(effect->info->max_inputs)) + || value_size != sizeof(effect->max_inputs)) return E_INVALIDARG; - src = &effect->info->max_inputs; + src = &effect->max_inputs; break; default: if (index < D2D1_PROPERTY_CLSID)
Similarly, this needs rework.
OK, I'll rework the property system first.
-static const struct d2d_effect_info builtin_effects[] = +struct d2d_builtin_effect_registration { - {&CLSID_D2D12DAffineTransform, 1, 1, 1}, - {&CLSID_D2D13DPerspectiveTransform, 1, 1, 1}, - {&CLSID_D2D1Composite, 2, 1, 0xffffffff}, - {&CLSID_D2D1Crop, 1, 1, 1}, - {&CLSID_D2D1Shadow, 1, 1, 1}, - {&CLSID_D2D1Grayscale, 1, 1, 1}, + const CLSID *id; + PD2D1_EFFECT_FACTORY factory; + UINT32 default_input_count; + UINT32 min_inputs; + UINT32 max_inputs; +};
It should be possible to reuse same structure for builtin effects.
Does it means that we should reuse d2d_effect_registration for the builtin effect data here? Or we keep d2d_effect_info and ignore factory field in this patch?
I think single structure is better, but there are options of course. You could have some lighter array of essential configuration for builtin effects, and later turn that into _effect_registration. But later that will need to have properties for builtin effects too, so it won't stay that light.
If use d2d_effect_registration, one problem is that CLSID is stored by itself in d2d_effect_registration, not a const pointer. So we are not able to set it statically. Then we can have something like:
struct d2d_builtin_effects { const CLSID *id; struct d2d_effect_registration reg; } builtin_effects[] = {....};
Does it work?
Or you can initialize it once dynamically, keeping same structure.
+HRESULT d2d_register_builtin_effects(struct d2d_factory *factory) +{ + struct d2d_effect_registration *reg; + unsigned int i;
+ for (i = 0; i < ARRAY_SIZE(builtin_effects); ++i) + { + const struct d2d_builtin_effect_registration *builtin_reg = &builtin_effects[i];
+ if (!(reg = calloc(1, sizeof(*reg)))) + return E_OUTOFMEMORY;
+ reg->is_builtin = TRUE; + reg->factory = builtin_reg->factory; + reg->registration_count = 1; + reg->id = *builtin_reg->id; + reg->default_input_count = builtin_reg->default_input_count; + reg->min_inputs = builtin_reg->min_inputs; + reg->max_inputs = builtin_reg->max_inputs; + list_add_tail(&factory->effects, ®->entry); + }
+ return S_OK; +}
I'm not sure if we want that. It should be enough to check if CLSID is for builtin in Register* call, and redirect to builtin data in CreateEffect().
It means that, we don't register builtin effect to factory's registered effects list, and access builtin data directly in CreateEffect()?
Yes, the only value of getting it registered is for CreateEffect() to pick them up. But, first you can't unregister builtin ones, and can't register them manually, it's an error, not a refcount increase like for custom effects.
So you'll need some helper to get builtin effect config structure for CreateEffect(), and you can use same helper for RegisterEffect, to return error.
-
Marvin -
Nikolay Sivov -
Ziqing Hui