2016-03-09 14:38 GMT+01:00 Paul Gofman <gofmanp(a)gmail.com>:
Signed-off-by: Paul Gofman <gofmanp(a)gmail.com> --- dlls/d3dx9_36/Makefile.in | 3 +- dlls/d3dx9_36/d3dx9_36_private.h | 36 +++ dlls/d3dx9_36/effect.c | 80 +++-- dlls/d3dx9_36/preshader.c | 671 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 757 insertions(+), 33 deletions(-) create mode 100644 dlls/d3dx9_36/preshader.c
diff --git a/dlls/d3dx9_36/Makefile.in b/dlls/d3dx9_36/Makefile.in index 95e3045..b71b38e 100644 --- a/dlls/d3dx9_36/Makefile.in +++ b/dlls/d3dx9_36/Makefile.in @@ -19,6 +19,7 @@ C_SRCS = \ texture.c \ util.c \ volume.c \ - xfile.c + xfile.c \ + preshader.c
Please keep this in alphabetic order.
@@ -5427,6 +5407,7 @@ static HRESULT d3dx9_parse_array_selector(struct d3dx9_base_effect *base, struct DWORD string_size; struct d3dx_object *object = &base->objects[param->object_id]; char *ptr = object->data; + HRESULT ret;
TRACE("Parsing array entry selection state for parameter %p.\n", param);
@@ -5443,9 +5424,30 @@ static HRESULT d3dx9_parse_array_selector(struct d3dx9_base_effect *base, struct } TRACE("Unknown DWORD: 0x%.8x.\n", *(DWORD *)(ptr + string_size));
- FIXME("Parse preshader.\n"); + d3dx_create_preshader(base, (DWORD *)(ptr + string_size) + 1, object->size - (string_size + 1), + D3DXPT_INT, ¶m->pres); + ret = D3D_OK; + param = param->referenced_param; + if (param->type == D3DXPT_VERTEXSHADER || param->type == D3DXPT_PIXELSHADER) + { + UINT i;
- return D3D_OK; + 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); + return D3DXERR_INVALIDDATA; + } + if (!param->members[i].pres) + { + TRACE("Creating preshader for object %u.\n", param->members[i].object_id); + object = &base->objects[param->members[i].object_id]; + d3dx_create_preshader(base, object->data, object->size, param->type, ¶m->members[i].pres); + } + } + } + return ret;
We discussed about this a bit already and arguably it's a matter of conventions but: a shader in an effect might have a preshader (PRES comment with its various subparts) or not. Calling the "plain" CTAB constants a "preshader" doesn't seem right to me.
+#define MIN(a, b) ((a) < (b) ? (a) : (b)) +#define MAX(a, b) ((a) > (b) ? (a) : (b))
min() and max() are already defined and should be usable.
+ +enum preshader_type +{ + PRES_TYPE_FXLC, + PRES_TYPE_SHADER +};
You should rename this to something that doesn't imply that "plain shader constants" are a preshader type or drop it entirely (depending on how you separate the preshader from the plain constants an explicit type field might not be necessary).
+ +enum PRES_OPS +{ + PO_NOP, + PO_MOV, + PO_OP_MAX +};
I would use somewhat longer and more descriptive names (e.g. PRESHADER_OP_NOP).
+ +typedef float (*pres_op_func)(float *args, int ncomp); + +static float pres_mov(float *args, int ncomp) {return args[0];} + +#define PRES_OPCODE_MASK 0x7FF00000 +#define PRES_OPCODE_SHIFT 20 +#define PRES_SCALAR_FLAG 0x80000000 +#define PRES_NCOMP_MASK 0x0000FFFF + +#define FOURCC_PRES 0x53455250 +#define FOURCC_CLIT 0x54494C43 +#define FOURCC_FXLC 0x434C5846 +#define FOURCC_PRSI 0x49535250 +#define PRES_SIGN 0x46580000
Please use lowercase hexadecimal constants.
+struct op_info +{ + DWORD opcode; + char mnem[8]; + unsigned int ninp_args; + BOOL func_all_comps; + pres_op_func func; +}; + +static struct op_info pres_op_info[] = +{ + {0x000, "nop", 0, 0, NULL }, /* PO_NOP */ + {0x100, "mov", 1, 0, pres_mov}, /* PO_MOV */ +}; + +enum PRES_VALUE_TYPE +{ + PRES_VT_FLOAT, + PRES_VT_DOUBLE, + PRES_VT_INT, + PRES_VT_BOOL +}; + +enum PRES_TABLES +{ + PRES_TAB_NONE, + PRES_TAB_IMMED, /* immediate double constants from CLIT */ + PRES_TAB_CONST, /* preshader input float constants */ + PRES_TAB_VERTEX, /* not used */ + PRES_TAB_OCONST, /* shader input float constants */ + PRES_TAB_OBCONST, /* shader input bool constants */ + PRES_TAB_OICONST, /* shader input int constants */ + PRES_TAB_REG, /* registers */ + PRES_TAB_MAX +};
Same here, with more expressive names you might not need comments at all (although they seem pretty clear already). BTW, what is that PRES_TAB_VERTEX entry?
+ +static const struct +{ + unsigned int value_size; + unsigned int reg_value_cnt; + enum PRES_VALUE_TYPE type; +} +table_info[] = +{ + {0, 0}, + {sizeof(double), 1, PRES_VT_DOUBLE}, /* PRES_TAB_IMMED */ + {sizeof(float), 4, PRES_VT_FLOAT }, /* PRES_TAB_CONST */ + {sizeof(float), 4, PRES_VT_FLOAT }, /* PRES_TAB_VERTEX */ + {sizeof(float), 4, PRES_VT_FLOAT }, /* PRES_TAB_OCONST */ + {sizeof(BOOL), 1, PRES_VT_BOOL }, /* PRES_TAB_OBCONST */ + {sizeof(int), 4, PRES_VT_INT, }, /* PRES_TAB_OICONST */ + {sizeof(float), 4, PRES_VT_FLOAT } /* PRES_TAB_REG */ +}; + +static const char *table_symbol[] = +{ + "(null)", "imm", "c", "v", "oc", "ob", "oi", "r" +}; + +static const char *xyzw_str = "xyzw"; + +#define OFFSET2REG(table, offset) ((offset) / table_info[table].reg_value_cnt) + +static const enum PRES_TABLES pres_regset2table[] = +{ + PRES_TAB_OBCONST, /* D3DXRS_BOOL */ + PRES_TAB_OICONST, /* D3DXRS_INT4 */ + PRES_TAB_CONST, /* D3DXRS_FLOAT4 */ + PRES_TAB_NONE, /* D3DXRS_SAMPLER */ +}; + +static const enum PRES_TABLES shad_regset2table[] = +{ + PRES_TAB_OBCONST, /* D3DXRS_BOOL */ + PRES_TAB_OICONST, /* D3DXRS_INT4 */ + PRES_TAB_OCONST, /* D3DXRS_FLOAT4 */ + PRES_TAB_NONE, /* D3DXRS_SAMPLER */ +}; + +struct d3dx_pres_operand +{ + enum PRES_TABLES table; + unsigned int offset; /* value index, not register index */
That probably isn't very clear. Maybe call it "component index" instead of "value index" or write a longer comment.
+}; + +struct d3dx_pres_ins +{ + enum PRES_OPS op; + BOOL scalar_op; /* first input argument is scalar, + first scalar component is propagated */
Maybe put the comment above the field instead.
+ unsigned int ncomps; + unsigned int ninp_args; + struct d3dx_pres_operand inputs[3]; + struct d3dx_pres_operand output; +}; + +#define PRES_VS_BITS_PER_WORD (sizeof(DWORD)*8) + +struct d3dx_const_tab +{ + unsigned int ninputs; + D3DXCONSTANT_DESC *inputs; + struct d3dx_parameter **inputs_param; + ID3DXConstantTable *ctab; + /* TODO: do not keep input constant structure + (use it only at the parse stage) */ + const enum PRES_TABLES *regset2table; +}; + +struct d3dx_fxlc
I would call this d3dx_preshader,
+{ + void *tables[PRES_TAB_MAX]; + unsigned int table_sizes[PRES_TAB_MAX]; /* registers count */ + DWORD *table_value_set[PRES_TAB_MAX]; + + unsigned int nins; + struct d3dx_pres_ins *ins; + + struct d3dx_const_tab inputs; +}; + +struct d3dx_preshader
and this one d3dx_shader or something.
+{ + enum preshader_type pres_type; + D3DXPARAMETER_TYPE param_type; + + struct d3dx_fxlc fxlc; + struct d3dx_const_tab shader_inputs; +}; + +static void val_set_reg(struct d3dx_fxlc *fxlc, unsigned int table, unsigned int reg_idx) +{ + fxlc->table_value_set[table][reg_idx / PRES_VS_BITS_PER_WORD] |= + 1 << (reg_idx % PRES_VS_BITS_PER_WORD); +} + +static void dump_shader_bytecode(void *data, unsigned int size) +{ + DWORD *words = (DWORD *)data; + unsigned int i, j, n; + + size /= 4; + i = 0; + while (i < size) + { + n = MIN(size - i, 15); + for (j = 0; j < n; j++) + MESSAGE("0x%08X,", words[i + j]); + i += n; + MESSAGE("\n"); + } +}
I would just use TRACE instead. 15 seems a bit too much, I'd stick to 8 per line or so. Also please use lowercase with hex.
+ +static DWORD *find_bytecode_comment(DWORD *ptr, DWORD fourcc) +{ + while ((*ptr & 0xFFFF) == 0xFFFE) + { + if (*(ptr + 1) == fourcc) + return ptr + 2; + ptr += 1 + (*ptr >> 16); + } + return NULL; +} + +static DWORD *parse_pres_arg(DWORD *ptr, struct d3dx_pres_operand *opr) +{ + if (*ptr != 0) + { + FIXME("Relative addressing not supported yet, word %#x.\n", *ptr); + return NULL; + } + ptr++; + + if (*ptr < 1 || *ptr >= PRES_TAB_MAX || *ptr == PRES_TAB_VERTEX)
It's probably better to use the proper enum value instead of '1'.
+static HRESULT get_constants_desc(DWORD *byte_code, struct d3dx_const_tab *out, struct d3dx9_base_effect *base) +{ + ID3DXConstantTable *ctab; + D3DXCONSTANT_DESC *cdesc; + struct d3dx_parameter **inputs_param; + D3DXCONSTANTTABLE_DESC desc; + HRESULT hr; + D3DXHANDLE hc; + unsigned int i; + unsigned int cnt; + + out->inputs = cdesc = NULL; + out->ctab = NULL; + out->inputs_param = NULL; + out->ninputs = 0; + inputs_param = NULL; + hr = D3DXGetShaderConstantTable(byte_code, &ctab); + if (FAILED(hr) || !ctab) + { + TRACE("Could not get CTAB data, hr %#x.\n", hr); + /* returnin OK is not a typo */
But that is a typo ;)
+ return D3D_OK; + } + hr = ID3DXConstantTable_GetDesc(ctab, &desc); + if (FAILED(hr)) + { + FIXME("Could not get CTAB desc, hr %#x.\n", hr); + goto err_out; + } + + cdesc = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(D3DXCONSTANT_DESC) * desc.Constants); + inputs_param = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(struct d3dx_parameter **) * desc.Constants);
Here you can use sizeof on the variable, as with your other patch, e.g.: cdesc = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(*cdesc) * desc.Constants); inputs_param = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(*inputs_param) * desc.Constants); Notice that the second allocation was wrong. I haven't looked in detail at the rest of the patch, although I spotted a number of formatting issues.