Re: [v3 resend 3/6] d3dx9: implement fxlc constants (expressions) in effect.

[Matteo Bruni]

2016-03-10 19:50 GMT+01:00 Paul Gofman :
> Signed-off-by: Paul Gofman
> +static float regstore_get_float(struct d3dx_regstore *rs, unsigned int table, unsigned int offset)
> +{
> - BYTE *p;
> -
> - p = (BYTE *)rs->tables[table] + table_info[table].value_size * offset;
> - switch (table_info[table].type)
> - {
> -
>    case PRES_VT_FLOAT : return *(float *)p;
> -
>    case PRES_VT_DOUBLE: return *(double *)p;
> -
>    case PRES_VT_INT   : return *(int *)p;
> -
>    case PRES_VT_BOOL  : return *(BOOL *)p ? 1.0f : -0.0f;

I guess that -0.0f comes from some test?
> -
>    /* no EOL in WARN to dump arg in the same line */
> -
>    WARN("Using uninitialized input ");
> -
>    dump_arg(rs, opr, comp);
> -
>    TRACE(".\n");
> -
>    dump_ins(rs, ins);

You can probably do without the comment, it's pretty clear anyway.
> +#define ARGS_ARRAY_SIZE 8
> +static HRESULT execute_preshader(struct d3dx_preshader *pres)
> +{
> - unsigned int i, j, k;
> - float args[ARGS_ARRAY_SIZE];
> - float res;
> -
> - for (i = 0; i < pres->nins; i++)
> - {
> -
>    struct d3dx_pres_ins *ins;
> -
>    struct op_info *oi;
> -
> -
>    ins = &pres->ins[i];
> -
>    oi = &pres_op_info[ins->op];
> -
>    if (oi->func_all_comps)
> -
>    {
> -
>        if (ins->ninp_args * ins->ncomps > ARGS_ARRAY_SIZE)
> -
>        {
> -
>            FIXME("Too much arguments (%u) for one instruction.\n", ins->ninp_args * ins->ncomps);

That should be "Too many".
> +HRESULT d3dx_param_eval_set_shader_constants(struct IDirect3DDevice9 *device, struct d3dx_param_eval *peval)

This function is unused in this patch. Please move it to the first
patch requiring it (I think that's patch 6/6). BTW, splitting patch
6/6 would be nice although I don't have any suggestions on how to do
that in a sensible manner...
> +HRESULT d3dx_param_eval_get_shader_parameters(struct d3dx_param_eval *peval, struct d3dx_parameter ***param,
> -
>    unsigned int *nparam, D3DXCONSTANT_DESC **cdesc)
>
> +{
> - *nparam = peval->shader_inputs.ninputs;
> - *param = peval->shader_inputs.inputs_param;
> - *cdesc = peval->shader_inputs.inputs;
> - return D3D_OK;
>
> +}

You don't use this function in this patch either. Also it doesn't seem
useful, it isn't an improvement to just assigning those three pointers
directly in the caller IMO.