2016-03-09 14:38 GMT+01:00 Paul Gofman gofmanp@gmail.com:
Signed-off-by: Paul Gofman gofmanp@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.
On 03/10/2016 03:12 AM, Matteo Bruni wrote:
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.
I will rename what is currently called 'pres' in d3dx_parameter to param_eval, and create_preshader to create_param_eval. I hope 'parameter evaluator' name can suit both for evaluating fxlc state constants and evaluating shader inputs (parameters), both precomputed and passed directly.
+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).
Will be enum param_eval_type, with PEVAL_... constants.
+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?
There is the first word in operand spec byte code that specifies register type. I probed the full range of possible values by putting values in this type and seeing shader decomp results by native D3DXDecompileShader. Value of 3 gives vX (v0, v1, v2) registers that (as I can guess) stay for vertex, though apparently it is not applicable to preshaders. In the parsing code below there is an explicit check for this value and it is recognized as invalid. Here I am keeping this constant just for completeness and to enable direct indexing of register tables in the code (and maybe to be able to decompile the given byte code exactly as native decomp does, though currently parser will just abort on seeing this constant).
+struct d3dx_preshader and this one d3dx_shader or something.
I suggest d3dx_param_eval, as above.
- {
TRACE("Could not get CTAB data, hr %#x.\n", hr);/* returnin OK is not a typo */But that is a typo ;)
Oh...
-
Matteo Bruni -
Paul Gofman