Re: [PATCH 1/3] d2d1: Support registering builtin effect.

[Nikolay Sivov]

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