Re: [v2 3/5] d3dx9: Support skip_constants parameter for effect.

[Matteo Bruni]

2017-07-11 13:26 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>:
> @@ -6397,24 +6464,75 @@ static HRESULT d3dx9_base_effect_init(struct d3dx9_base_effect *base,
>          TRACE("Tag: %x\n", tag);
>      }
>
> +    if (skip_constants_string)
> +    {
> +        skip_constants_buffer = HeapAlloc(GetProcessHeap(), 0,
> +                sizeof(*skip_constants_buffer) * (strlen(skip_constants_string) + 1));

I think you can get rid of the sizeof() in this case. If
sizeof(*skip_constants_buffer) is not 1 we have bigger problems
anyway...
> +        if (!skip_constants_buffer)
> +        {
> +            if (bytecode)
> +                ID3D10Blob_Release(bytecode);
> +            return E_OUTOFMEMORY;
> +        }
> +        strcpy(skip_constants_buffer, skip_constants_string);

Ideally this one should be a memcpy(), reusing the string length computed above.
> @@ -588,7 +588,8 @@ static unsigned int *parse_pres_ins(unsigned int *ptr, unsigned int count, struc
>      return ptr;
>  }
>
> -static HRESULT get_ctab_constant_desc(ID3DXConstantTable *ctab, D3DXHANDLE hc, D3DXCONSTANT_DESC *desc)
> +static HRESULT get_ctab_constant_desc(ID3DXConstantTable *ctab, D3DXHANDLE hc, D3DXCONSTANT_DESC *desc,
> +        WORD *constantinfo_reserved)
>  {
>      const struct ctab_constant *constant = d3dx_shader_get_ctab_constant(ctab, hc);
>
> @@ -598,10 +599,13 @@ static HRESULT get_ctab_constant_desc(ID3DXConstantTable *ctab, D3DXHANDLE hc, D
>          return D3DERR_INVALIDCALL;
>      }
>      *desc = constant->desc;
> +    if (constantinfo_reserved)
> +        *constantinfo_reserved = constant->constantinfo_reserved;
>      return D3D_OK;
>  }

For future reference: these (and related) changes should probably have
been in a separate patch, or even part of the previous patch in the
series.

Also, as it becomes more obvious here, one could go one step further
with this and directly return struct ctab_constant to the caller. That
would essentially make the get_ctab_constant_desc() helper redundant
so it could be merged into the callers and removed entirely.
> +        for (j = 0; j < skip_constants_count; ++j)
> +        {
> +            if (!strcmp(cdesc[index].Name, skip_constants[j]))
> +            {
> +                if (!constantinfo_reserved)
> +                {
> +                    WARN("skip_constants parameter %s is not register bound.\n",
> +                            cdesc[index].Name);
> +                    hr = D3DERR_INVALIDCALL;
> +                    goto cleanup;
> +                }
> +                TRACE("Skipping constant %s.\n", cdesc[index].Name);
> +                break;
> +            }
> +        }
> +        if (j < skip_constants_count)
> +            continue;
> +        ++out->input_count;
> +        if (inputs_param[index]->class == D3DXPC_OBJECT)
> +            continue;
> +        if (FAILED(hr = init_set_constants_param(out, ctab, hc, inputs_param[index])))
> +            goto cleanup;
>      }
> -    out->input_count = desc.Constants;
>      if (out->const_set_count)
>      {
>          out->const_set = HeapReAlloc(GetProcessHeap(), 0, out->const_set,

Not particularly important but we could shrink out->inputs and
out->inputs_param to out->input_count if any parameter was skipped.