Re: [v11 1/4] d3dx9: Implement fxlc constants (expressions) in effect.

[Matteo Bruni]

2016-04-12 11:34 GMT+02:00 Paul Gofman :
> @@ -369,12 +410,17 @@ static HRESULT get_constants_desc(unsigned int *byte_code, struct d3dx_const_tab
>              goto err_out;
>          }
>          count = 1;
> -
>    hr = ID3DXConstantTable_GetConstantDesc(ctab, hc, &cdesc[i], &count);
> -
>    if (FAILED(hr))
> -
>    if (FAILED(hr = ID3DXConstantTable_GetConstantDesc(ctab, hc, &cdesc[i], &count)))
>    {
>        FIXME("Could not get constant desc, hr %#x.\n", hr);
>        goto err_out;
>    }
> -
>    if (count != 1)
> -
>    {
> -
>        FIXME("Unexpected constant descriptors count %u.\n", count);
> -
>        hr = D3DERR_INVALIDCALL;
> -
>        goto err_out;
> -
>    }

Notice that, since you set count to 1 before calling
GetConstantDesc(), the only possible results are 0 or 1 and the 0 case
should be already handled by the "if (FAILED())" branch.
You might use a temporary array of 3 elements and pass 3 as count,
then only copy the first element to the destination so the rest of the
code is unchanged while still giving a chance for the FIXME to
trigger. Not sure it's worth it though, maybe just adding support for
multiple descriptors (even only in struct d3dx_const_tab and the
immediately related code) might be a better plan.
> +static HRESULT set_constants_param(struct d3dx_regstore *rs, struct d3dx_const_tab *const_tab,
> -
>    D3DXHANDLE hc, struct d3dx_parameter *param)
>
> +{
> - ID3DXConstantTable *ctab = const_tab->ctab;
> - D3DXCONSTANT_DESC desc;
> - unsigned int const_count, param_count, i, j, n, table, start_offset;
> - unsigned int minor, major, major_stride, param_offset;
> - BOOL transpose, get_element;
> - unsigned int desc_count;
> -
> - desc_count = 1;
> - ID3DXConstantTable_GetConstantDesc(ctab, hc, &desc, &desc_count);
> - if (desc_count != 1)
> - {
> -
>    FIXME("Unexpected constant descriptors count %u.\n", desc_count);
> -
>    return D3DERR_INVALIDCALL;
> - }

Same here, in this case the descriptors are only used locally so both
options above should be easier to do.
> - if (const_count != param_count)
> - {
> -
>    FIXME("Number of elements or struct members differs between parameter and constant.\n");
> -
>    return D3DERR_INVALIDCALL;
> - }

You could print the two counts here. Not a big deal though.

Looks good otherwise.