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.