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

[Matteo Bruni]

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.