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

[Matteo Bruni]

2017-04-25 18:03 GMT+02:00 Paul Gofman <gofmanp at gmail.com>:
> On 04/25/2017 06:35 PM, Matteo Bruni wrote:
>>
>>
>>>         +static BOOL param_zero_data_func(void *dummy, struct
>>> d3dx_parameter *param)
>>> +{
>>> +    param->data = NULL;
>>> +    return FALSE;
>>> +}
>>> +
>>> +static void d3dx_pool_release_shared_parameter(struct d3dx_parameter
>>> *param)
>>> +{
>>> +    LONG new_refcount;
>>> +
>>> +    if (!(param->flags & PARAMETER_FLAG_SHARED) || !param->shared_data)
>>> +        return;
>>
>> No need to check for the PARAMETER_FLAG_SHARED flag. Maybe it makes
>> sense to assert that the flag is there if param->shared_data is not
>> NULL.
>
> I need to check this flag because shared_data pointer is "unioned" with
> reference_param, and removing the check should break freeing of non-shared
> parameters with non-zero referenced_param. Checking just a flag won't do
> either, as actually the parameter marked as shared could have nothing to be
> shared with.

Good point.

>>>
>>>   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.