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

[Matteo Bruni]

2017-04-24 13:36 GMT+02:00 Paul Gofman <gofmanp at gmail.com>:

> +struct d3dx_shared_data
> +{
> +    void *data;
> +    LONG refcount;
> +    struct d3dx_parameter **parameters;
> +    unsigned int table_size;
> +};

A bit nitpicky but generally for those dynamic arrays we call
"refcount" "count" and "table_size" "size".

> +    new_refcount = InterlockedIncrement(&pool->shared_data[i].refcount);

I think you don't need the Interlocked functions for this count given
that adding / removing multiple effects to a pool is expected to be
racy.

> +    if (new_refcount >= pool->shared_data[i].table_size)
> +    {
> +        if (!pool->shared_data[i].table_size)
> +        {
> +            new_size = INITIAL_SHARED_DATA_TABLE_SIZE;
> +            pool->shared_data[i].parameters = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY,
> +                    sizeof(*pool->shared_data[i].parameters) * INITIAL_SHARED_DATA_TABLE_SIZE);
> +        }
> +        else
> +        {
> +            new_size = pool->shared_data[i].table_size * 2;
> +            pool->shared_data[i].parameters = HeapReAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY,
> +                    pool->shared_data[i].parameters,
> +                    sizeof(*pool->shared_data[i].parameters) * new_size);
> +        }
> +        pool->shared_data[i].table_size = new_size;
> +    }
> +
> +    param->shared_data = &pool->shared_data[i];
> +    pool->shared_data[i].parameters[new_refcount - 1] = param;
> +
> +    TRACE("name %s, parameter idx %u, new refcount %u.\n", param->name, i,
> +            new_refcount);
> +    return;
> +}

Unnecessary return.

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

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