Re: [PATCH 2/3] d2d1: Implement effect creation for registered effect.

[Nikolay Sivov]

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, &reg->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.