[v2 6/6] d3dx9: Support parameters sharing in effect.

[Paul Gofman]

On 04/25/2017 07:34 PM, Matteo Bruni wrote:
> 2017-04-25 18:24 GMT+02:00 Paul Gofman <gofmanp at gmail.com>:
>> On 04/25/2017 07:09 PM, Matteo Bruni wrote:
>>>
>>>>>>     static void free_effect_pool(struct d3dx_effect_pool *pool)
>>>>>>     {
>>>>>> +    unsigned int i;
>>>>>> +
>>>>>> +    for (i = 0; i < pool->table_size; ++i)
>>>>>> +    {
>>>>>> +        if (pool->shared_data[i].refcount)
>>>>>> +            ERR("Releasing pool with referenced parameters, shared
>>>>>> data
>>>>>> is not freed.\n");
>>>>>> +    }
>>>>> This can be triggered by an application so it should be a WARN().
>>>>> BTW, this probably needs a test. On pool destruction, the effects
>>>>> still using it might be "unshared" i.e. data from the pool might be
>>>>> moved back to the effects. Or maybe the effects still using the pool
>>>>> are simply broken and accessing parameter data after the pool is
>>>>> destroyed crashes.
>>>>>
>>>>>
>>>> Yes, sure, there is one of the marginal cases to be tested more (probably
>>>> along with more precise understanding on how shared shader parameters
>>>> setting works when sharing non-NULL and NULL shader). Until then maybe we
>>>> could leave something more severe than WARN here, as if we reach this
>>>> point
>>>> it most likely mean that something is wrong and it would be nice to have
>>>> an
>>>> indication right away.
>>> I think we want to know what happens before we go ahead with the code
>>> above.
>>>
>> I just tested, manually releasing pool before effect just crash when I use
>> native d3dx dll. Please see attached patch. I have to do release twice as it
>> is referenced from 2 effects. If I release pool just once, it crashes in
>> freeing effect.
> Excellent. Adding the test in an if (0) {} + a comment would probably
> be nice for documentation purposes. For the implementation I'd go with
> WARN + releasing shared data.
>
Could you please advise how to best add it: just as this patch in if(0) 
{}, or a separate test?