[PATCH 7/7] d3dx9: Support skip_constants parameter for effect.

[Matteo Bruni]

2017-07-06 13:15 GMT+02:00 Paul Gofman <gofmanp at gmail.com>:

>  HRESULT d3dx_create_param_eval(struct d3dx9_base_effect *base_effect, void *byte_code,
>          unsigned int byte_code_size, D3DXPARAMETER_TYPE type,
> -        struct d3dx_param_eval **peval, ULONG64 *version_counter) DECLSPEC_HIDDEN;
> +        struct d3dx_param_eval **peval, ULONG64 *version_counter,
> +        const char **skip_constants_names, unsigned int skip_constant_name_count) DECLSPEC_HIDDEN;

I think you can get rid of the "_name" part in those parameters, for brevity.

> +static HRESULT parse_skip_constants_string(char *skip_constants, const char ***names,
> +        unsigned int *name_count)

It's a bit a matter of personal taste but triple pointers look quite
ugly to me. In this case you could e.g. return "names" in place of
HRESULT.

> +{
> +    const char *name;
> +    char *s;
> +    unsigned int size = INITIAL_CONST_NAMES_SIZE;
> +
> +    *names = HeapAlloc(GetProcessHeap(), 0, sizeof(*names) * size);
> +    if (!*names)
> +        return E_OUTOFMEMORY;
> +
> +    *name_count = 0;
> +    s = skip_constants;
> +    while ((name = next_valid_constant_name(&s)))
> +    {
> +        if (*name_count == size)
> +        {
> +            size *= 2;
> +            *names = HeapReAlloc(GetProcessHeap(), 0, *names, sizeof(*names) * size);
> +            if (!*names)
> +                return E_OUTOFMEMORY;

I think you're leaking memory on failure here.

> +        }
> +        (*names)[(*name_count)++] = name;
> +    }
> +    *names = HeapReAlloc(GetProcessHeap(), 0, *names, *name_count * sizeof(*names));
> +    if (!*names)
> +        return E_OUTOFMEMORY;

And here.

> +    for (i = 0; i < skip_constants_name_count; ++i)
> +    {
> +        struct d3dx_parameter *param;
> +
> +        param = get_parameter_by_name(base, NULL, skip_constants_names[i]);
> +        if (param)
> +        {
> +            for (j = 0; j < base->technique_count; ++j)
> +            {
> +                if (is_parameter_used(param, &base->techniques[j]))
> +                {
> +                    WARN("Parameter %s found in skip_constants is used in technique %u.\n",
> +                            debugstr_a(skip_constants_names[i]), j);
> +                    HeapFree(GetProcessHeap(), 0, skip_constants_buffer);
> +                    HeapFree(GetProcessHeap(), 0, skip_constants_names);
> +                    d3dx9_base_effect_cleanup(base);
> +                    return D3DERR_INVALIDCALL;
> +                }
> +            }
> +        }
> +        else
> +        {
> +            TRACE("Parameter %s found in skip_constants not found.\n",
> +                    debugstr_a(skip_constants_names[i]));

This one sounds a bit awkward ("found... not found"). Maybe just
rewrite that as "skip_constants parameter %s not found". You could
apply the same trick to the previous message too.

> +        for (j = 0; j < skip_constant_name_count; ++j)
> +        {
> +            if (!strcmp(cdesc[index].Name, skip_constants_names[j]))

I assume the skip_constants thing isn't used often and, when it is,
very few constants (1-3) are in the list. If that's not the case, we
might want to use an rbtree for the constant names.