2016-03-10 19:50 GMT+01:00 Paul Gofman <gofmanp(a)gmail.com>:
Signed-off-by: Paul Gofman <gofmanp(a)gmail.com>
Looks pretty good to me in general. I also like d3dx_param_eval and related names.
+ for (i = 0; i < param->element_count; i++) + { + if (param->members[i].type != param->type) + { + FIXME("Unexpected member parameter type %u (need %u).\n", param->members[i].type, param->type);
I would use "expected" or something similar instead of "need" in this message.
diff --git a/dlls/d3dx9_36/preshader.c b/dlls/d3dx9_36/preshader.c new file mode 100644 index 0000000..409c00f --- /dev/null +++ b/dlls/d3dx9_36/preshader.c @@ -0,0 +1,688 @@ +/* + * FX preshader parsing & execution, setting shader constants + * from effect parameters. + * + * Copyright 2016 Paul Gofman + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + */ + +#include "config.h" +#include "wine/port.h" + +#include "windef.h" +#include "wingdi.h"
Not your fault but those shouldn't be there. wingdi.h is already included by d3dx9_36_private.h, while windef.h is not but still it's necessary to avoid errors when including d3dx9_36_private.h. I'm sending a patch cleaning up the includes in d3dx9 (and renaming d3dx9_36_private.h to d3dx9_private.h while at it), please update this patch accordingly.
+static const struct +{ + unsigned int value_size; + unsigned int reg_value_cnt;
Just my preference, I would go with component_size and component_count (or maybe reg_component_count).
+ {sizeof(float), 4, PRES_VT_FLOAT } /* PRES_REGTAB_REG */
I think I had tested this in the past and it turned out temporaries are stored as doubles. I'll see if I can find something.
+static const char *table_symbol[] = +{ + "(null)", "imm", "c", "v", "oc", "ob", "oi", "r" +}; + +static const char *xyzw_str = "xyzw";
I haven't checked but maybe you can put those directly in the function using them.
+ +#define OFFSET2REG(table, offset) ((offset) / table_info[table].reg_value_cnt) + +static const enum PRES_REG_TABLES pres_regset2table[] = +{ + PRES_REGTAB_OBCONST, /* D3DXRS_BOOL */ + PRES_REGTAB_OICONST, /* D3DXRS_INT4 */ + PRES_REGTAB_CONST, /* D3DXRS_FLOAT4 */ + PRES_REGTAB_NONE, /* D3DXRS_SAMPLER */ +}; + +static const enum PRES_REG_TABLES shad_regset2table[] = +{ + PRES_REGTAB_OBCONST, /* D3DXRS_BOOL */ + PRES_REGTAB_OICONST, /* D3DXRS_INT4 */ + PRES_REGTAB_OCONST, /* D3DXRS_FLOAT4 */ + PRES_REGTAB_NONE, /* D3DXRS_SAMPLER */ +}; + +struct d3dx_pres_operand +{ + enum PRES_REG_TABLES table; + /* offset is value index, not register index, e. g. + offset for value c3.y is 17 (3 * 4 + 1) */
As I mentioned already, I would refer to "components" instead of "values".
+ unsigned int offset; +}; + +struct d3dx_pres_ins +{ + enum PRES_OPS op; + /* first input argument is scalar, + scalar component is propagated */ + BOOL scalar_op; + unsigned int ncomps; + unsigned int ninp_args;
Something like "component_count" and "operands_count" (or "input_count", or whatever) seems nicer. Similarly for the other counts.
+ +struct d3dx_param_eval +{ + enum param_eval_type peval_type;
I feel like I might have already asked this, sorry if that's the case: do you really need this field? At least for what I can see at the moment you can do without since it's used only in d3dx_create_param_eval(), which is where you set the field, and in d3dx_param_eval_set_shader_constants(), where you don't really need it.
+ D3DXPARAMETER_TYPE param_type;
This one can also probably be avoided (e.g. by passing it down the call chain from the d3dx_parameter struct) although it's probably a bit more useful so I don't mind.
+ + struct d3dx_preshader pres; + struct d3dx_const_tab shader_inputs; +}; + +static HRESULT regstore_alloc_table(struct d3dx_regstore *rs, unsigned int table) +{ + unsigned int sz; + + sz = rs->table_sizes[table] * table_info[table].reg_value_cnt * table_info[table].value_size; + if (sz) + { + rs->tables[table] = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sz); + rs->table_value_set[table] = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, + sizeof(*rs->table_value_set[table]) * + ((rs->table_sizes[table] + PRES_VS_BITS_PER_WORD - 1) / PRES_VS_BITS_PER_WORD)); + if (!rs->tables[table] || !rs->table_value_set[table]) + return E_OUTOFMEMORY; + } + return D3D_OK; +} + +static void regstore_free_tables(struct d3dx_regstore *rs) +{ + unsigned int i; + + for (i = PRES_REGTAB_IMMED; i < PRES_REGTAB_MAX; i++)
Maybe add a couple of enum PRES_REGTAB_FIRST and PRES_REGTAB_LAST and use them here?
+ for (j = 0; j < n; j++) + TRACE("0x%08X,", words[i + j]);
Please use lowercase hexadecimals.
+ if (ninp_args != pres_op_info[i].ninp_args) + { + FIXME("Actual input args %u, expected %u, instruction %s.\n", ninp_args, + pres_op_info[i].ninp_args, pres_op_info[i].mnem); + return NULL; + } + ins->ninp_args = ninp_args;
It shouldn't be necessary to store the number of arguments in the instruction structure at the moment, although maybe there is some variable-arguments opcode?
+ hr = E_FAIL;
This seems unnecessary.
+ --
git am complains about that empty line at the end of the file.