From: Nikolay Sivov <nsivov@codeweavers.com>
Subject: Re: [PATCH 2/5] d2d1: Partially implement RegisterEffectFromStream().
Message-Id: <8cb3c743-873b-e2ee-de8a-7044892d3669@codeweavers.com>
Date: Tue, 7 Jun 2022 10:28:45 +0300
In-Reply-To: <e1cd45ef-ec32-5e07-7754-f16ebc515a70@codeweavers.com>
References: <20220606073525.1774002-1-zhui@codeweavers.com> <20220606073525.1774002-2-zhui@codeweavers.com> <e6add160-d761-1675-0ac9-5e76ade56cf9@codeweavers.com> <89f35236-b39e-065f-13a6-c30a7baa74cc@codeweavers.com> <e1cd45ef-ec32-5e07-7754-f16ebc515a70@codeweavers.com>



On 6/7/22 06:43, Ziqing Hui wrote:
>>>> +    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>.
>>
> I did some tests today. I didn't find a way to express min/max/default input count in XML.
>
> I tried these ways:
>
> * Property of <Effect>:
>
>      <Effect>
>          ...
>          <Property name='MinInputs' type='uint32' value='1'/>
>      </Effect>
>
> * Sub property of <Inputs>:
>
>      <Effect>
>          ...
>          <Inputs>
>              <Property name='Min' type='uint32' value='1'/>
>          </Inputs>
>      <Effect>
>
> * Attribute of <Inputs>:
>
>      <Effect>
>          ...
>          <Inputs min='1'/>
>      <Effect/>
>
> Unfortunately, none of them work.
>
I see. It's easier to assume it's consistent with returned name, so it 
should be using MinInputs name. If it doesn't work, let's ignore that 
for now.

Structurally, I was thinking that each <Property/> would trigger some 
add_property() helper that would simply append to some array. That could 
use some structure of { index, name, type, value }, for system 
properties name could be empty. It doesn't have to be exactly like that, 
but the point is that it should be easy to use when parsing description, 
and when instantiating effects objects. I haven't looked that closely, 
but I imaging a new instance will use initial values from 'value' 
attributes + set getter/setter functions, which means that instance 
properties are essentially cloned from the description.