Re: [v5 8/8] d3dx9: Implement setting named shader constants in effect.

[Matteo Bruni]

2016-03-21 14:55 GMT+01:00 Paul Gofman :
> +HRESULT d3dx_set_shader_const_fxlc(struct ID3DXEffectImpl *effect, struct d3dx_pass *pass,
> -
>    struct d3dx_parameter *param, unsigned int vs)

Using a BOOL for the vs flag seems nicer.
> diff --git a/dlls/d3dx9_36/preshader.c b/dlls/d3dx9_36/preshader.c
> index fec9de4..1770046 100644
> --- a/dlls/d3dx9_36/preshader.c
> +++ b/dlls/d3dx9_36/preshader.c
> @@ -242,7 +242,7 @@ static void regstore_set_values(struct d3dx_regstore *rs, unsigned int table, vo
>  static unsigned int regstore_is_val_set_reg(struct d3dx_regstore *rs, unsigned int table, unsigned int reg_idx)
>  {
>      return rs->table_value_set[table][reg_idx / PRES_BITMASK_BLOCK_SIZE] &
> -
>        (1ul << (reg_idx % PRES_BITMASK_BLOCK_SIZE));
> -
>        (1u << (reg_idx % PRES_BITMASK_BLOCK_SIZE));

When you end up changing stuff introduced in a previous patch in the
series you know there is something wrong ;)
I guess it's a rebase glitch (by any chance, was this a remnant of a
try with the uint_ptr idea you mentioned earlier?)
> +HRESULT d3dx_param_eval_set_shader_constants(struct IDirect3DDevice9 *device, struct d3dx_param_eval *peval)
> +{
> - HRESULT hr, res;
> - struct d3dx_preshader *pres = &peval->pres;
> - struct d3dx_regstore *rs = &pres->regs;
> -
> - TRACE("device %p, peval %p, param_type %u.\n", device, peval, peval->param_type);
> -
> - if (FAILED(hr = set_constants(rs, &pres->inputs)))
> -
>    return hr;
> - if (FAILED(hr = execute_preshader(pres)))
> -
>    return hr;
> - if (FAILED(hr = set_constants(rs, &peval->shader_inputs)))
> -
>    return hr;
> -
> - #define SET_TABLE(table, func_suff, type) \
> -
>    { \
> -
>        unsigned int is, n; \
> -
>        \
> -
>        is = 0; \
> -
>        while (is < rs->table_sizes[table]) \
> -
>        {\
> -
>            n = 0; \
> -
>            while (is < rs->table_sizes[table] && !regstore_is_val_set_reg(rs, table, is)) \
> -
>                is++; \
> -
>            while (is + n < rs->table_sizes[table] && regstore_is_val_set_reg(rs, table, is + n)) \
> -
>                n++; \
> -
>            if (!n) \
> -
>                continue; \
> -
>            TRACE("setting %u consts at %u.\n", n, is); \
> -
>            hr = IDirect3DDevice9_##func_suff(device, is, \
> -
>                    (const type *)rs->tables[table] + is * table_info[table].reg_component_count, n); \
> -
>            if (FAILED(hr)) \
> -
>            { \
> -
>                ERR(#func_suff " failed, hr %#x.\n", hr); \
> -
>                res = hr; \
> -
>            } \
> -
>            is += n; \
> -
>        } \
> -
>        regstore_reset_table(rs, table); \
> -
>    }
> - res = D3D_OK;
> - if (peval->param_type == D3DXPT_VERTEXSHADER)
> - {
> -
>    SET_TABLE(PRES_REGTAB_OCONST, SetVertexShaderConstantF, float);
> -
>    SET_TABLE(PRES_REGTAB_OICONST, SetVertexShaderConstantI, int);
> -
>    SET_TABLE(PRES_REGTAB_OBCONST, SetVertexShaderConstantB, BOOL);
> - }
> - else if (peval->param_type == D3DXPT_PIXELSHADER)
> - {
> -
>    SET_TABLE(PRES_REGTAB_OCONST, SetPixelShaderConstantF, float);
> -
>    SET_TABLE(PRES_REGTAB_OICONST, SetPixelShaderConstantI, int);
> -
>    SET_TABLE(PRES_REGTAB_OBCONST, SetPixelShaderConstantB, BOOL);
> - }
> - else
> - {
> -
>    FIXME("Unexpected parameter type %u.\n", peval->param_type);
> -
>    return D3DERR_INVALIDCALL;
> - }
> - #undef SET_TABLE

This is certainly hard to handle nicely because of the multiple
similar d3d9 functions with slightly different signatures but I think
it's better to avoid macros (especially such large ones) as much as
possible.

There are multiple ways to achieve that. Perhaps the easiest one is to
turn SET_TABLE into a function taking an array of enum PRES_REG_TABLES
(which, btw, should be lowercase) and the shader type as additional
parameters, loop over the tables and call the correct d3d9 function by
using two levels of switches (or ifs). The type isn't really
necessary, you can calculate the offset with
table_info[].component_size.