[PATCH 3/5] d2d1: Implement property parsing for RegisterEffectFromStream().

[Nikolay Sivov]

On 6/6/22 13:13, Ziqing Hui wrote:
>
> On 6/6/22 4:36 PM, Nikolay Sivov wrote:
>>
>> On 6/6/22 10:35, Ziqing Hui wrote:
>>>    struct d2d_effect_info
>>>    {
>>>        const CLSID *clsid;
>>>        UINT32 default_input_count;
>>>        UINT32 min_inputs;
>>>        UINT32 max_inputs;
>>> +    WCHAR *display_name;
>>> +    WCHAR *author;
>>> +    WCHAR *category;
>>> +    WCHAR *description;
>>>    };
>> Shouldn't these also be expressed as properties?
>>
> Yeah, these are properties in XML.
>
> However, there are two kinds of effect properties, one is system property, the other is custom property.
>
> System property always exists in every effect, not matter if we express them in effect XML or not.
> So my thought is, since they always exists, we don't need to dynamically store them in properties array. We can use one struct to store all system properties.
> We store custom properties as d2d_effect_property, while handle system properties separately.
>
> Currently, we are storing system properties in struct d2d_effect_info. Each field in info is a system property.
> So I just put them there like before.
>
> Also, system properties don't have custom setter and getter, while custom properties have.
>
> Do you think my thought is ok or maybe we should take a different way?

Hard to tell before it's done. Even we don't have to store them along 
with custom properties, it would be better to have some organized way 
for them, and not separate fields. I guess for now we could even ignore 
them to some extent, the only important ones are input/output 
configuration. Effect parameters will be all custom properties, right?

>
>>> +static D2D1_PROPERTY_TYPE str_to_property_type(const WCHAR *str)
>>> +{
>>> +    unsigned int i;
>>> +
>>> +    static const WCHAR *type_str[] =
>>> +    {
>>> +        L"", L"string", L"bool", L"uint32", L"int32", L"float", L"vector2", L"vector3",
>>> +        L"vector4", L"blob", L"iunknown", L"enum", L"array", L"clsid", L"matrix3x2",
>>> +        L"matrix4x3", L"matrix4x4", L"matrix5x4", L"colorcontext",
>>> +    };
>>> +
>>> +    for (i = 1; i < ARRAY_SIZE(type_str); ++i)
>>> +    {
>>> +        if (!wcscmp(str, type_str[i]))
>>> +            return i;
>>> +    }
>>> +
>>> +    return D2D1_PROPERTY_TYPE_UNKNOWN;
>>> +}
>> This uses numeric values of D2D1_PROPERTY_TYPE, that deserves at least a comment.>
> Would it be ok like this: ?
>
> static const WCHAR *type_str[] =
> {
>      L"",       /* D2D1_PROPERTY_TYPE_UNKNOWN */
>      L"string", /* D2D1_PROPERTY_TYPE_STRING */
>      L"bool",   /* D2D1_PROPERTY_TYPE_BOOL */
>      L"uint32", /* D2D1_PROPERTY_TYPE_UINT32 */
>      ...
> };
Yes.