[PATCH 1/4] d2d1: Add a properties list for builtin effects.

[Giovanni Mascellani]

Hi,

Il 27/07/21 13:10, Ziqing Hui ha scritto:
> +void d2d_effect_init(struct d2d_effect *effect, ID2D1Factory *factory, const CLSID *effect_id)
> +{
> +    unsigned int i;
> +
>       effect->ID2D1Effect_iface.lpVtbl = &d2d_effect_vtbl;
>       effect->ID2D1Image_iface.lpVtbl = &d2d_effect_image_vtbl;
>       effect->refcount = 1;
>       ID2D1Factory_AddRef(effect->factory = factory);
> +
> +    for (i = 0; i < ARRAY_SIZE(builtin_effects); ++i)
> +    {
> +        if (IsEqualGUID(effect_id, builtin_effects[i].clsid))
> +        {
> +            d2d_effect_init_standard_properties(effect, builtin_effects + i);
> +            return;
> +        }
> +    }
> +
> +    WARN("Unsupported effect clsid %s.\n", debugstr_guid(effect_id));
>   }

My understanding is that you want to implement three different effects 
using the same class, which I suppose will dynamically change its 
behavior depending on the stored clsid. Personally I would rather write 
three different classes, each of which implements one effect (I would 
even put them in three different files, but I guess this is not really 
in Wine's philosophy). They can share code, if needed, but having three 
different classes (with three virtual tables) means that you don't have 
to do some kind of switch on the clsid each time the implementations 
differ. Also, each class can hardcode the values you're putting in 
builtin_effects without having to lookup a shared table, so you can keep 
the implementations more self contained.

Just my two cents, though; given that my Signed-off-by has basically no 
relevance, you might want to check how the big bosses want stuff to be 
done. :-)

Giovanni.