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

[Ziqing Hui]

Hi Giovanni,

On 7/27/21 9:55 PM, Giovanni Mascellani wrote:
> 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. 

Thanks for your reply. My plan is to add something like " struct d2d_effect_ops", which is a vtable. And each effect class needs to implement this vtable. It may look like this:

    struct d2d_effect_ops

    {

        void (*op1)();

        void (*op2)();

        ....

    }


    struct d2d_effect

    {

        ....

        struct d2d_effect_ops *ops;

    }


Assuming that we are trying to implement 2D Affine Effect:

    void 2d_affine_op1(){ ....}

        void 2d_affine_op2(){ ....}

        static struct d2d_effect_ops 2d_affine_ops =

        {

            2d_affine_op1,

            2d_affine_op2,

             ....

        }

       

        And this op table will be initialized in d2d_effect_init() according to clsid.