[PATCH 2/5] d2d1: Partially implement RegisterEffectFromStream().

[Ziqing Hui]

On 6/6/22 4:29 PM, Nikolay Sivov wrote:
> 
> 
> On 6/6/22 10:35, Ziqing Hui wrote:
>> +struct d2d_effect_reg
>> +{
>> +    PD2D1_EFFECT_FACTORY factory;
>> +    UINT32 count;
>> +
>> +    struct d2d_effect_info *info;
>> +
>> +    struct list entry;
>> +};
> You don't need to allocate 'info' separately.
> 
>> +    memset(node_name, 0, sizeof(node_name));
>> +    while (IXmlReader_Read(xml_reader, &type) == S_OK)
>> +    {
>> +        if (FAILED(hr = IXmlReader_GetDepth(xml_reader, &depth)))
>> +            goto done;
>> +        if (depth >= 3)
>> +            continue;
>> +        if (FAILED(hr = IXmlReader_GetLocalName(xml_reader, &value, &len)))
>> +            goto done;
>> +        if (node_name[depth] && wcslen(node_name[depth]) <= len)
>> +        {
>> +            wcscpy(node_name[depth], value);
>> +        }
>> +        else
>> +        {
>> +            heap_free(node_name[depth]);
>> +            node_name[depth] = heap_strdupW(value);
>> +        }
>> +
>> +        if (type != XmlNodeType_Element)
>> +            continue;
> 
> Depth should not be used for this. What you need is to first find "Effect" element. After that loop within it, until you run out of nested elements. You can then have helpers to process specific elements like Property or Inputs.
>
>> +                if (!wcscmp(node_name[depth - 1], L"Effect")
>> +                        && !wcscmp(node_name[depth], L"Property"))
> Is it really case-sensitive?

I'm not sure. I'll add a test for it.

> 
>> +    reg->info->default_input_count = input_count;
>> +    reg->info->min_inputs = input_count;
>> +    reg->info->max_inputs = input_count;
> Is this supposed to be subproperties of an Input? Maybe we should have basic property support first, and store it there. Is min/max/default expressible in effect xml? <Input> attributes maybe?

It is likely that they can be expressed in xml as properties of <Effect> like "DisplayName"/"Author", tests are still needed. 
And according to the current test, if I don't explicitly express them in xml, they will be equal to the number of <Input> nodes inside <Inputs>.

>> +    entry->count = 1;
>> +    entry->info->clsid = effect_id;
>> +    entry->factory = effect_factory;
>> +    list_add_tail(&factory->effect_regs, &entry->entry);
> This should store GUID itself, not a pointer.
>