[PATCH 2/3] d2d1: Implement effect creation for registered effect.
Ziqing Hui
zhui at codeweavers.com
Tue Jun 21 04:34:01 CDT 2022
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.
More information about the wine-devel
mailing list