2016-03-10 19:50 GMT+01:00 Paul Gofman gofmanp@gmail.com:
Signed-off-by: Paul Gofman gofmanp@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.