Re: [v3 resend 3/6] d3dx9: implement fxlc constants (expressions) in effect.
2016-03-10 19:50 GMT+01:00 Paul Gofman <gofmanp(a)gmail.com>:
Signed-off-by: Paul Gofman <gofmanp(a)gmail.com>
+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.
On 03/15/2016 05:54 AM, Matteo Bruni wrote:
+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, I just saw a lot of constructs in preshader disassembly like "cmp r1.x, c0.x, 1.0, -0.0", somehow they like to put -0.0 for immediate constants for such sort of cmp, and was thinking of this when I was writing it. There is currently no usecase I know for preshader inputting booleans or ints, so I can't test this. So I think this just need to be changed to normal 0.0.
+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.
I would like to remove this function but then I will need to put the param_eval structure into shared header file (now there is only a forward declaration). Is it OK?
2016-03-15 8:33 GMT+01:00 Paul Gofman <gofmanp(a)gmail.com>:
On 03/15/2016 05:54 AM, Matteo Bruni wrote:
+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, I just saw a lot of constructs in preshader disassembly like "cmp r1.x, c0.x, 1.0, -0.0", somehow they like to put -0.0 for immediate constants for such sort of cmp, and was thinking of this when I was writing it. There is currently no usecase I know for preshader inputting booleans or ints, so I can't test this. So I think this just need to be changed to normal 0.0.
+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.
I would like to remove this function but then I will need to put the param_eval structure into shared header file (now there is only a forward declaration). Is it OK?
Sure. Hopefully it doesn't bring too much stuff in from preshader.c (but it's not a problem if it does, we don't need to hide preshader internals from effect.c).
participants (2)
-
Matteo Bruni -
Paul Gofman