Signed-off-by: Nikolay Sivov nsivov@codeweavers.com --- dlls/d3d10/d3d10_private.h | 1 + dlls/d3d10/effect.c | 35 +++++++++++++++++ dlls/d3d10/tests/effect.c | 78 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 114 insertions(+)
diff --git a/dlls/d3d10/d3d10_private.h b/dlls/d3d10/d3d10_private.h index 7f83fb1182a..b35711b55e5 100644 --- a/dlls/d3d10/d3d10_private.h +++ b/dlls/d3d10/d3d10_private.h @@ -52,6 +52,7 @@ enum d3d10_effect_object_operation D3D10_EOO_CONST_INDEX = 3, D3D10_EOO_VAR_INDEX = 4, D3D10_EOO_INDEX_EXPRESSION = 5, + D3D10_EOO_VALUE_EXPRESSION = 6, D3D10_EOO_ANONYMOUS_SHADER = 7, };
diff --git a/dlls/d3d10/effect.c b/dlls/d3d10/effect.c index 3f35e9cc28a..9c2012768dd 100644 --- a/dlls/d3d10/effect.c +++ b/dlls/d3d10/effect.c @@ -289,6 +289,10 @@ struct d3d10_effect_prop_dependency struct d3d10_effect_variable *v; struct d3d10_effect_preshader index; } index_expr; + struct + { + struct d3d10_effect_preshader value; + } value_expr; }; };
@@ -392,6 +396,9 @@ static void d3d10_effect_clear_prop_dependencies(struct d3d10_effect_prop_depend case D3D10_EOO_INDEX_EXPRESSION: d3d10_effect_preshader_clear(&dep->index_expr.index); break; + case D3D10_EOO_VALUE_EXPRESSION: + d3d10_effect_preshader_clear(&dep->value_expr.value); + break; } } heap_free(d->entries); @@ -2632,6 +2639,34 @@ static HRESULT parse_fx10_property_assignment(const char *data, size_t data_size
return d3d10_effect_add_prop_dependency(d, &dep);
+ case D3D10_EOO_VALUE_EXPRESSION: + + if (value_offset >= data_size || !require_space(value_offset, 1, sizeof(uint16_t), data_size)) + { + WARN("Invalid offset %#x (data size %#lx).\n", value_offset, (long)data_size); + return E_FAIL; + } + + data_ptr = data + value_offset; + read_dword(&data_ptr, &blob_size); + + if (!require_space(value_offset, 1, sizeof(uint16_t) + blob_size, data_size)) + { + WARN("Invalid offset %#x (data size %#lx).\n", value_offset, (long)data_size); + return E_FAIL; + } + + dep.id = id; + dep.idx = idx; + dep.operation = operation; + if (FAILED(hr = parse_fx10_preshader(data_ptr, blob_size, effect, &dep.value_expr.value))) + { + WARN("Failed to parse preshader, hr %#x.\n", hr); + return hr; + } + + return d3d10_effect_add_prop_dependency(d, &dep); + case D3D10_EOO_ANONYMOUS_SHADER:
/* Anonymous shader */ diff --git a/dlls/d3d10/tests/effect.c b/dlls/d3d10/tests/effect.c index 7c2bba6d818..eae0036c0b6 100644 --- a/dlls/d3d10/tests/effect.c +++ b/dlls/d3d10/tests/effect.c @@ -7947,6 +7947,83 @@ static void test_effect_index_expression(void) ok(!refcount, "Device has %u references left.\n", refcount); }
+#if 0 +float4 g_var; +float4 g_var2; +technique10 tech +{ + pass p0 + { + SetBlendState( NULL, g_var + 0.1f + g_var2, 0 ); + } + pass p1 + { + SetBlendState( NULL, g_var.x + g_var2 + g_var.y, 0 ); + } +} +#endif +static DWORD fx_test_value_expression[] = +{ + 0x43425844, 0xe517f17c, 0xe44eaede, 0xe4fd1240, 0xc67d5084, 0x00000001, 0x0000047c, 0x00000001, + 0x00000024, 0x30315846, 0x00000450, 0xfeff1001, 0x00000001, 0x00000002, 0x00000000, 0x00000000, + 0x00000000, 0x00000000, 0x00000001, 0x00000330, 0x00000000, 0x00000000, 0x00000000, 0x00000000, + 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x6f6c4724, + 0x736c6162, 0x6f6c6600, 0x00347461, 0x0000000d, 0x00000001, 0x00000000, 0x00000010, 0x00000010, + 0x00000010, 0x0000210a, 0x61765f67, 0x5f670072, 0x32726176, 0x63657400, 0x30700068, 0x00016000, + 0x42584400, 0x312ce543, 0xc3ebc897, 0xea47c0a2, 0x8a710323, 0x000001d2, 0x00016000, 0x00000300, + 0x00002c00, 0x0000d400, 0x0000f000, 0x41544300, 0x0000a042, 0x00001c00, 0x00007700, 0x58040000, + 0x00000246, 0x00001c00, 0x00010000, 0x00007400, 0x00004400, 0x00000200, 0x00000100, 0x00004c00, + 0x00000000, 0x00005c00, 0x01000200, 0x00000100, 0x00006400, 0x00000000, 0x765f6700, 0xab007261, + 0x030001ab, 0x04000100, 0x00000100, 0x00000000, 0x765f6700, 0x00327261, 0x030001ab, 0x04000100, + 0x00000100, 0x00000000, 0x00787400, 0x7263694d, 0x666f736f, 0x52282074, 0x4c482029, 0x53204c53, + 0x65646168, 0x6f432072, 0x6c69706d, 0x31207265, 0x00312e30, 0x494c43ab, 0x00001434, 0x00000400, + 0xcccccd00, 0x0000003d, 0x00000000, 0x00000000, 0x4c584600, 0x00006843, 0x00000200, 0x40000400, + 0x00000220, 0x00000000, 0x00000200, 0x00000000, 0x00000000, 0x00000200, 0x00000400, 0x00000000, + 0x00000700, 0x00000000, 0x40000400, 0x000002a0, 0x00000000, 0x00000100, 0x00000000, 0x00000000, + 0x00000700, 0x00000000, 0x00000000, 0x00000400, 0x00000000, 0xf0f0f000, 0x0f0f0ff0, 0x00ffff0f, + 0x00000100, 0x00000200, 0x00000000, 0x00000100, 0x00000200, 0x00000000, 0x00317000, 0x00000150, + 0x43425844, 0xc6a29e4c, 0x6292ed35, 0xd90bb8cb, 0x50dcd25f, 0x00000001, 0x00000150, 0x00000003, + 0x0000002c, 0x000000d4, 0x000000e0, 0x42415443, 0x000000a0, 0x0000001c, 0x00000077, 0x46580400, + 0x00000002, 0x0000001c, 0x00000100, 0x00000074, 0x00000044, 0x00000002, 0x00000001, 0x0000004c, + 0x00000000, 0x0000005c, 0x00010002, 0x00000001, 0x00000064, 0x00000000, 0x61765f67, 0xabab0072, + 0x00030001, 0x00040001, 0x00000001, 0x00000000, 0x61765f67, 0xab003272, 0x00030001, 0x00040001, + 0x00000001, 0x00000000, 0x4d007874, 0x6f726369, 0x74666f73, 0x29522820, 0x534c4820, 0x6853204c, + 0x72656461, 0x6d6f4320, 0x656c6970, 0x30312072, 0xab00312e, 0x34494c43, 0x00000004, 0x00000000, + 0x434c5846, 0x00000068, 0x00000002, 0xa0400004, 0x00000002, 0x00000000, 0x00000002, 0x00000000, + 0x00000000, 0x00000002, 0x00000004, 0x00000000, 0x00000007, 0x00000000, 0xa0400004, 0x00000002, + 0x00000000, 0x00000002, 0x00000001, 0x00000000, 0x00000007, 0x00000000, 0x00000000, 0x00000004, + 0x00000000, 0xf0f0f0f0, 0x0f0f0f0f, 0x0000ffff, 0x00000001, 0x00000002, 0x00000000, 0x00000001, + 0x00000002, 0x00000000, 0x00000004, 0x00000020, 0x00000000, 0x00000002, 0xffffffff, 0x00000000, + 0x00000030, 0x00000014, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000036, + 0x00000014, 0x00000000, 0x00000010, 0x00000000, 0x00000000, 0x00000000, 0x0000003d, 0x00000002, + 0x00000000, 0x00000042, 0x00000003, 0x00000000, 0x0000000a, 0x00000000, 0x00000006, 0x00000045, + 0x0000000b, 0x00000000, 0x00000001, 0x000001a9, 0x00000002, 0x00000000, 0x00000001, 0x000001b5, + 0x000001c1, 0x00000003, 0x00000000, 0x0000000a, 0x00000000, 0x00000006, 0x000001c4, 0x0000000b, + 0x00000000, 0x00000001, 0x00000318, 0x00000002, 0x00000000, 0x00000001, 0x00000324, +}; + +static void test_effect_value_expression(void) +{ + ID3D10Effect *effect; + ID3D10Device *device; + ULONG refcount; + HRESULT hr; + + if (!(device = create_device())) + { + skip("Failed to create device, skipping tests.\n"); + return; + } + + hr = create_effect(fx_test_value_expression, 0, device, NULL, &effect); + ok(SUCCEEDED(hr), "Failed to create an effect, hr %#x.\n", hr); + + effect->lpVtbl->Release(effect); + + refcount = ID3D10Device_Release(device); + ok(!refcount, "Device has %u references left.\n", refcount); +} + START_TEST(effect) { test_effect_constant_buffer_type(); @@ -7972,4 +8049,5 @@ START_TEST(effect) test_effect_raw_value(); test_effect_dynamic_numeric_field(); test_effect_index_expression(); + test_effect_value_expression(); }
Signed-off-by: Nikolay Sivov nsivov@codeweavers.com --- dlls/d3d10/effect.c | 39 +++++++++++++++++++-------------------- 1 file changed, 19 insertions(+), 20 deletions(-)
diff --git a/dlls/d3d10/effect.c b/dlls/d3d10/effect.c index 9c2012768dd..74ee4bf468c 100644 --- a/dlls/d3d10/effect.c +++ b/dlls/d3d10/effect.c @@ -191,17 +191,21 @@ struct preshader_instr unsigned int scalar : 1; };
-typedef float (*pres_op_func)(float **args, unsigned int n); +typedef void (*pres_op_func)(float **args, unsigned int n, const struct preshader_instr *instr);
-static float pres_ftou(float **args, unsigned int n) +static void pres_ftou(float **args, unsigned int n, const struct preshader_instr *instr) { unsigned int u = *args[0]; - return *(float *)&u; + *args[1] = *(float *)&u; }
-static float pres_add(float **args, unsigned int n) +static void pres_add(float **args, unsigned int n, const struct preshader_instr *instr) { - return *args[0] + *args[1]; + float *retval = args[2]; + unsigned int i; + + for (i = 0; i < instr->comp_count; ++i) + retval[i] = args[0][instr->scalar ? 0 : i] + args[1][i]; }
struct preshader_op_info @@ -333,10 +337,10 @@ static float * d3d10_effect_preshader_get_reg_ptr(const struct d3d10_effect_pres
static HRESULT d3d10_effect_preshader_eval(struct d3d10_effect_preshader *p) { - unsigned int i, j, regt, offset, instr_count, input_count; + unsigned int i, j, regt, offset, instr_count, arg_count; const DWORD *ip = ID3D10Blob_GetBufferPointer(p->code); - float *dst, *args[4], *retval; struct preshader_instr ins; + float *dst, *args[4];
dst = d3d10_effect_preshader_get_reg_ptr(p, D3D10_REG_TABLE_RESULT, 0); memset(dst, 0, sizeof(float) * p->reg_tables[D3D10_REG_TABLE_RESULT].count); @@ -355,16 +359,15 @@ static HRESULT d3d10_effect_preshader_eval(struct d3d10_effect_preshader *p) for (i = 0; i < instr_count; ++i) { *(DWORD *)&ins = *ip++; - input_count = *ip++; + arg_count = 1 + *ip++;
- if (input_count > ARRAY_SIZE(args)) + if (arg_count > ARRAY_SIZE(args)) { - FIXME("Unexpected argument count %u.\n", input_count); + FIXME("Unexpected argument count %u.\n", arg_count); return E_FAIL; }
- /* Arguments */ - for (j = 0; j < input_count; ++j) + for (j = 0; j < arg_count; ++j) { ip++; /* TODO: argument register flags are currently ignored */ regt = *ip++; @@ -373,12 +376,7 @@ static HRESULT d3d10_effect_preshader_eval(struct d3d10_effect_preshader *p) args[j] = d3d10_effect_preshader_get_reg_ptr(p, regt, offset); }
- ip++; /* TODO: result register flags are currently ignored */ - regt = *ip++; - offset = *ip++; - retval = d3d10_effect_preshader_get_reg_ptr(p, regt, offset); - - *retval = d3d10_effect_get_op_info(ins.opcode)->func(args, input_count); + d3d10_effect_get_op_info(ins.opcode)->func(args, arg_count, &ins); }
return S_OK; @@ -2100,7 +2098,7 @@ static HRESULT parse_fx10_preshader_instr(struct d3d10_preshader_parse_context * unsigned int *offset, const char **ptr, unsigned int data_size) { const struct preshader_op_info *op_info; - unsigned int i, param_count; + unsigned int i, param_count, size; struct preshader_instr ins; uint32_t input_count;
@@ -2152,7 +2150,8 @@ static HRESULT parse_fx10_preshader_instr(struct d3d10_preshader_parse_context * case D3D10_REG_TABLE_CB: case D3D10_REG_TABLE_RESULT: case D3D10_REG_TABLE_TEMP: - context->table_sizes[regt] = max(context->table_sizes[regt], param_offset + 1); + size = param_offset + (ins.scalar && i == 0 ? 1 : ins.comp_count); + context->table_sizes[regt] = max(context->table_sizes[regt], size); break; default: FIXME("Unexpected register table index %u.\n", regt);
On Tue, Nov 16, 2021 at 6:15 PM Nikolay Sivov nsivov@codeweavers.com wrote:
Signed-off-by: Nikolay Sivov nsivov@codeweavers.com
dlls/d3d10/effect.c | 39 +++++++++++++++++++-------------------- 1 file changed, 19 insertions(+), 20 deletions(-)
diff --git a/dlls/d3d10/effect.c b/dlls/d3d10/effect.c index 9c2012768dd..74ee4bf468c 100644 --- a/dlls/d3d10/effect.c +++ b/dlls/d3d10/effect.c @@ -191,17 +191,21 @@ struct preshader_instr unsigned int scalar : 1; };
-typedef float (*pres_op_func)(float **args, unsigned int n); +typedef void (*pres_op_func)(float **args, unsigned int n, const struct preshader_instr *instr);
-static float pres_ftou(float **args, unsigned int n) +static void pres_ftou(float **args, unsigned int n, const struct preshader_instr *instr) { unsigned int u = *args[0];
- return *(float *)&u;
- *args[1] = *(float *)&u;
}
Is ftou always scalar? Not that I'm quite sure how to test it i.e. what would be a case where a vector ftou could be in effect?
What I'm getting at, I think it would be safer to handle vector ftou, just in case that's a thing.
-static float pres_add(float **args, unsigned int n) +static void pres_add(float **args, unsigned int n, const struct preshader_instr *instr) {
- return *args[0] + *args[1];
- float *retval = args[2];
- unsigned int i;
- for (i = 0; i < instr->comp_count; ++i)
retval[i] = args[0][instr->scalar ? 0 : i] + args[1][i];}
struct preshader_op_info @@ -333,10 +337,10 @@ static float * d3d10_effect_preshader_get_reg_ptr(const struct d3d10_effect_pres
static HRESULT d3d10_effect_preshader_eval(struct d3d10_effect_preshader *p) {
- unsigned int i, j, regt, offset, instr_count, input_count;
- unsigned int i, j, regt, offset, instr_count, arg_count; const DWORD *ip = ID3D10Blob_GetBufferPointer(p->code);
- float *dst, *args[4], *retval; struct preshader_instr ins;
float *dst, *args[4];
dst = d3d10_effect_preshader_get_reg_ptr(p, D3D10_REG_TABLE_RESULT, 0); memset(dst, 0, sizeof(float) * p->reg_tables[D3D10_REG_TABLE_RESULT].count);
@@ -355,16 +359,15 @@ static HRESULT d3d10_effect_preshader_eval(struct d3d10_effect_preshader *p) for (i = 0; i < instr_count; ++i) { *(DWORD *)&ins = *ip++;
input_count = *ip++;
arg_count = 1 + *ip++;
if (input_count > ARRAY_SIZE(args))
if (arg_count > ARRAY_SIZE(args)) {
FIXME("Unexpected argument count %u.\n", input_count);
FIXME("Unexpected argument count %u.\n", arg_count); return E_FAIL; }
/* Arguments */for (j = 0; j < input_count; ++j)
for (j = 0; j < arg_count; ++j) { ip++; /* TODO: argument register flags are currently ignored */ regt = *ip++;@@ -373,12 +376,7 @@ static HRESULT d3d10_effect_preshader_eval(struct d3d10_effect_preshader *p) args[j] = d3d10_effect_preshader_get_reg_ptr(p, regt, offset); }
ip++; /* TODO: result register flags are currently ignored */regt = *ip++;offset = *ip++;retval = d3d10_effect_preshader_get_reg_ptr(p, regt, offset);*retval = d3d10_effect_get_op_info(ins.opcode)->func(args, input_count);
d3d10_effect_get_op_info(ins.opcode)->func(args, arg_count, &ins);
I guess a small comment mentioning that the last argument is (usually?) the return register could be useful.
} return S_OK;@@ -2100,7 +2098,7 @@ static HRESULT parse_fx10_preshader_instr(struct d3d10_preshader_parse_context * unsigned int *offset, const char **ptr, unsigned int data_size) { const struct preshader_op_info *op_info;
- unsigned int i, param_count;
- unsigned int i, param_count, size; struct preshader_instr ins; uint32_t input_count;
@@ -2152,7 +2150,8 @@ static HRESULT parse_fx10_preshader_instr(struct d3d10_preshader_parse_context * case D3D10_REG_TABLE_CB: case D3D10_REG_TABLE_RESULT: case D3D10_REG_TABLE_TEMP:
context->table_sizes[regt] = max(context->table_sizes[regt], param_offset + 1);
size = param_offset + (ins.scalar && i == 0 ? 1 : ins.comp_count);context->table_sizes[regt] = max(context->table_sizes[regt], size); break; default: FIXME("Unexpected register table index %u.\n", regt);
On 11/19/21 4:41 PM, Matteo Bruni wrote:
On Tue, Nov 16, 2021 at 6:15 PM Nikolay Sivov nsivov@codeweavers.com wrote:
Signed-off-by: Nikolay Sivov nsivov@codeweavers.com
dlls/d3d10/effect.c | 39 +++++++++++++++++++-------------------- 1 file changed, 19 insertions(+), 20 deletions(-)
diff --git a/dlls/d3d10/effect.c b/dlls/d3d10/effect.c index 9c2012768dd..74ee4bf468c 100644 --- a/dlls/d3d10/effect.c +++ b/dlls/d3d10/effect.c @@ -191,17 +191,21 @@ struct preshader_instr unsigned int scalar : 1; };
-typedef float (*pres_op_func)(float **args, unsigned int n); +typedef void (*pres_op_func)(float **args, unsigned int n, const struct preshader_instr *instr);
-static float pres_ftou(float **args, unsigned int n) +static void pres_ftou(float **args, unsigned int n, const struct preshader_instr *instr) { unsigned int u = *args[0];
- return *(float *)&u;
- *args[1] = *(float *)&u;
}
Is ftou always scalar? Not that I'm quite sure how to test it i.e. what would be a case where a vector ftou could be in effect?
What I'm getting at, I think it would be safer to handle vector ftou, just in case that's a thing.
Yes, I guess it makes sense. I can't think of a case where this could happen in effects, I've only seen ftou() as a conversion to index integer, and since index is scalar, it always generated ftou() on a single component.
-static float pres_add(float **args, unsigned int n) +static void pres_add(float **args, unsigned int n, const struct preshader_instr *instr) {
- return *args[0] + *args[1];
- float *retval = args[2];
- unsigned int i;
- for (i = 0; i < instr->comp_count; ++i)
retval[i] = args[0][instr->scalar ? 0 : i] + args[1][i];}
struct preshader_op_info @@ -333,10 +337,10 @@ static float * d3d10_effect_preshader_get_reg_ptr(const struct d3d10_effect_pres
static HRESULT d3d10_effect_preshader_eval(struct d3d10_effect_preshader *p) {
- unsigned int i, j, regt, offset, instr_count, input_count;
- unsigned int i, j, regt, offset, instr_count, arg_count; const DWORD *ip = ID3D10Blob_GetBufferPointer(p->code);
- float *dst, *args[4], *retval; struct preshader_instr ins;
float *dst, *args[4];
dst = d3d10_effect_preshader_get_reg_ptr(p, D3D10_REG_TABLE_RESULT, 0); memset(dst, 0, sizeof(float) * p->reg_tables[D3D10_REG_TABLE_RESULT].count);
@@ -355,16 +359,15 @@ static HRESULT d3d10_effect_preshader_eval(struct d3d10_effect_preshader *p) for (i = 0; i < instr_count; ++i) { *(DWORD *)&ins = *ip++;
input_count = *ip++;
arg_count = 1 + *ip++;
if (input_count > ARRAY_SIZE(args))
if (arg_count > ARRAY_SIZE(args)) {
FIXME("Unexpected argument count %u.\n", input_count);
FIXME("Unexpected argument count %u.\n", arg_count); return E_FAIL; }
/* Arguments */for (j = 0; j < input_count; ++j)
for (j = 0; j < arg_count; ++j) { ip++; /* TODO: argument register flags are currently ignored */ regt = *ip++;@@ -373,12 +376,7 @@ static HRESULT d3d10_effect_preshader_eval(struct d3d10_effect_preshader *p) args[j] = d3d10_effect_preshader_get_reg_ptr(p, regt, offset); }
ip++; /* TODO: result register flags are currently ignored */regt = *ip++;offset = *ip++;retval = d3d10_effect_preshader_get_reg_ptr(p, regt, offset);*retval = d3d10_effect_get_op_info(ins.opcode)->func(args, input_count);
d3d10_effect_get_op_info(ins.opcode)->func(args, arg_count, &ins);I guess a small comment mentioning that the last argument is (usually?) the return register could be useful.
Ok, I think it's always present.
} return S_OK;@@ -2100,7 +2098,7 @@ static HRESULT parse_fx10_preshader_instr(struct d3d10_preshader_parse_context * unsigned int *offset, const char **ptr, unsigned int data_size) { const struct preshader_op_info *op_info;
- unsigned int i, param_count;
- unsigned int i, param_count, size; struct preshader_instr ins; uint32_t input_count;
@@ -2152,7 +2150,8 @@ static HRESULT parse_fx10_preshader_instr(struct d3d10_preshader_parse_context * case D3D10_REG_TABLE_CB: case D3D10_REG_TABLE_RESULT: case D3D10_REG_TABLE_TEMP:
context->table_sizes[regt] = max(context->table_sizes[regt], param_offset + 1);
size = param_offset + (ins.scalar && i == 0 ? 1 : ins.comp_count);context->table_sizes[regt] = max(context->table_sizes[regt], size); break; default: FIXME("Unexpected register table index %u.\n", regt);
On Fri, Nov 19, 2021 at 3:13 PM Nikolay Sivov nsivov@codeweavers.com wrote:
On 11/19/21 4:41 PM, Matteo Bruni wrote:
On Tue, Nov 16, 2021 at 6:15 PM Nikolay Sivov nsivov@codeweavers.com wrote:
Signed-off-by: Nikolay Sivov nsivov@codeweavers.com
dlls/d3d10/effect.c | 39 +++++++++++++++++++-------------------- 1 file changed, 19 insertions(+), 20 deletions(-)
diff --git a/dlls/d3d10/effect.c b/dlls/d3d10/effect.c index 9c2012768dd..74ee4bf468c 100644 --- a/dlls/d3d10/effect.c +++ b/dlls/d3d10/effect.c @@ -191,17 +191,21 @@ struct preshader_instr unsigned int scalar : 1; };
-typedef float (*pres_op_func)(float **args, unsigned int n); +typedef void (*pres_op_func)(float **args, unsigned int n, const struct preshader_instr *instr);
-static float pres_ftou(float **args, unsigned int n) +static void pres_ftou(float **args, unsigned int n, const struct preshader_instr *instr) { unsigned int u = *args[0];
- return *(float *)&u;
- *args[1] = *(float *)&u;
}
Is ftou always scalar? Not that I'm quite sure how to test it i.e. what would be a case where a vector ftou could be in effect?
What I'm getting at, I think it would be safer to handle vector ftou, just in case that's a thing.
Yes, I guess it makes sense. I can't think of a case where this could happen in effects, I've only seen ftou() as a conversion to index integer, and since index is scalar, it always generated ftou() on a single component.
Yeah, it's a bit paranoid on my part, but we might as well...
The only case where it would come up that I can think of is with preshaders used to "preprocess" uniform data i.e. the compiler figures out that some intermediate shader data only depends on uniform values and pulls the relevant computation out of the shader proper into a preshader. That's a thing in d3dx9 but I don't think we encountered it with d3d10 yet.
On 11/19/21 5:22 PM, Matteo Bruni wrote:
On Fri, Nov 19, 2021 at 3:13 PM Nikolay Sivov nsivov@codeweavers.com wrote:
On 11/19/21 4:41 PM, Matteo Bruni wrote:
On Tue, Nov 16, 2021 at 6:15 PM Nikolay Sivov nsivov@codeweavers.com wrote:
Signed-off-by: Nikolay Sivov nsivov@codeweavers.com
dlls/d3d10/effect.c | 39 +++++++++++++++++++-------------------- 1 file changed, 19 insertions(+), 20 deletions(-)
diff --git a/dlls/d3d10/effect.c b/dlls/d3d10/effect.c index 9c2012768dd..74ee4bf468c 100644 --- a/dlls/d3d10/effect.c +++ b/dlls/d3d10/effect.c @@ -191,17 +191,21 @@ struct preshader_instr unsigned int scalar : 1; };
-typedef float (*pres_op_func)(float **args, unsigned int n); +typedef void (*pres_op_func)(float **args, unsigned int n, const struct preshader_instr *instr);
-static float pres_ftou(float **args, unsigned int n) +static void pres_ftou(float **args, unsigned int n, const struct preshader_instr *instr) { unsigned int u = *args[0];
- return *(float *)&u;
- *args[1] = *(float *)&u;
}
Is ftou always scalar? Not that I'm quite sure how to test it i.e. what would be a case where a vector ftou could be in effect?
What I'm getting at, I think it would be safer to handle vector ftou, just in case that's a thing.
Yes, I guess it makes sense. I can't think of a case where this could happen in effects, I've only seen ftou() as a conversion to index integer, and since index is scalar, it always generated ftou() on a single component.
Yeah, it's a bit paranoid on my part, but we might as well...
The only case where it would come up that I can think of is with preshaders used to "preprocess" uniform data i.e. the compiler figures out that some intermediate shader data only depends on uniform values and pulls the relevant computation out of the shader proper into a preshader. That's a thing in d3dx9 but I don't think we encountered it with d3d10 yet.
I see. Is that an equivalent of some constant buffer variables depending on other constant buffer variables?
On Fri, Nov 19, 2021 at 3:50 PM Nikolay Sivov nsivov@codeweavers.com wrote:
On 11/19/21 5:22 PM, Matteo Bruni wrote:
On Fri, Nov 19, 2021 at 3:13 PM Nikolay Sivov nsivov@codeweavers.com wrote:
On 11/19/21 4:41 PM, Matteo Bruni wrote:
On Tue, Nov 16, 2021 at 6:15 PM Nikolay Sivov nsivov@codeweavers.com wrote:
Signed-off-by: Nikolay Sivov nsivov@codeweavers.com
dlls/d3d10/effect.c | 39 +++++++++++++++++++-------------------- 1 file changed, 19 insertions(+), 20 deletions(-)
diff --git a/dlls/d3d10/effect.c b/dlls/d3d10/effect.c index 9c2012768dd..74ee4bf468c 100644 --- a/dlls/d3d10/effect.c +++ b/dlls/d3d10/effect.c @@ -191,17 +191,21 @@ struct preshader_instr unsigned int scalar : 1; };
-typedef float (*pres_op_func)(float **args, unsigned int n); +typedef void (*pres_op_func)(float **args, unsigned int n, const struct preshader_instr *instr);
-static float pres_ftou(float **args, unsigned int n) +static void pres_ftou(float **args, unsigned int n, const struct preshader_instr *instr) { unsigned int u = *args[0];
- return *(float *)&u;
- *args[1] = *(float *)&u;
}
Is ftou always scalar? Not that I'm quite sure how to test it i.e. what would be a case where a vector ftou could be in effect?
What I'm getting at, I think it would be safer to handle vector ftou, just in case that's a thing.
Yes, I guess it makes sense. I can't think of a case where this could happen in effects, I've only seen ftou() as a conversion to index integer, and since index is scalar, it always generated ftou() on a single component.
Yeah, it's a bit paranoid on my part, but we might as well...
The only case where it would come up that I can think of is with preshaders used to "preprocess" uniform data i.e. the compiler figures out that some intermediate shader data only depends on uniform values and pulls the relevant computation out of the shader proper into a preshader. That's a thing in d3dx9 but I don't think we encountered it with d3d10 yet.
I see. Is that an equivalent of some constant buffer variables depending on other constant buffer variables?
Yes, pretty much. For d3dx9 the preshader would compute a bunch of "anonymous" constants only depending on the value of effect parameters (aka d3d10's effect variables). The effect parameters would only be part of the actual shader interface if they still ended up being used in the simplified shader proper.
On Tue, Nov 16, 2021 at 6:15 PM Nikolay Sivov nsivov@codeweavers.com wrote:
Signed-off-by: Nikolay Sivov nsivov@codeweavers.com
dlls/d3d10/d3d10_private.h | 1 + dlls/d3d10/effect.c | 35 +++++++++++++++++ dlls/d3d10/tests/effect.c | 78 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 114 insertions(+)
Sorry for the delay.
diff --git a/dlls/d3d10/d3d10_private.h b/dlls/d3d10/d3d10_private.h index 7f83fb1182a..b35711b55e5 100644 --- a/dlls/d3d10/d3d10_private.h +++ b/dlls/d3d10/d3d10_private.h @@ -52,6 +52,7 @@ enum d3d10_effect_object_operation D3D10_EOO_CONST_INDEX = 3, D3D10_EOO_VAR_INDEX = 4, D3D10_EOO_INDEX_EXPRESSION = 5,
- D3D10_EOO_VALUE_EXPRESSION = 6, D3D10_EOO_ANONYMOUS_SHADER = 7,
};
diff --git a/dlls/d3d10/effect.c b/dlls/d3d10/effect.c index 3f35e9cc28a..9c2012768dd 100644 --- a/dlls/d3d10/effect.c +++ b/dlls/d3d10/effect.c @@ -289,6 +289,10 @@ struct d3d10_effect_prop_dependency struct d3d10_effect_variable *v; struct d3d10_effect_preshader index; } index_expr;
struct{struct d3d10_effect_preshader value;} value_expr;};
@@ -392,6 +396,9 @@ static void d3d10_effect_clear_prop_dependencies(struct d3d10_effect_prop_depend case D3D10_EOO_INDEX_EXPRESSION: d3d10_effect_preshader_clear(&dep->index_expr.index); break;
case D3D10_EOO_VALUE_EXPRESSION:d3d10_effect_preshader_clear(&dep->value_expr.value);break; }@@ -2632,6 +2639,34 @@ static HRESULT parse_fx10_property_assignment(const char *data, size_t data_size
return d3d10_effect_add_prop_dependency(d, &dep);
case D3D10_EOO_VALUE_EXPRESSION:if (value_offset >= data_size || !require_space(value_offset, 1, sizeof(uint16_t), data_size)){WARN("Invalid offset %#x (data size %#lx).\n", value_offset, (long)data_size);return E_FAIL;}
Pretty sure you want sizeof(DWORD) here.
data_ptr = data + value_offset;read_dword(&data_ptr, &blob_size);if (!require_space(value_offset, 1, sizeof(uint16_t) + blob_size, data_size)){WARN("Invalid offset %#x (data size %#lx).\n", value_offset, (long)data_size);return E_FAIL;}
Not sure about this sizeof(uint16_t) either, what's the idea?
BTW, we probably want this kind of require_space(..., blob_size, ...) in the D3D10_EOO_INDEX_EXPRESSION case too.
On 11/19/21 4:41 PM, Matteo Bruni wrote:
On Tue, Nov 16, 2021 at 6:15 PM Nikolay Sivov nsivov@codeweavers.com wrote:
Signed-off-by: Nikolay Sivov nsivov@codeweavers.com
dlls/d3d10/d3d10_private.h | 1 + dlls/d3d10/effect.c | 35 +++++++++++++++++ dlls/d3d10/tests/effect.c | 78 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 114 insertions(+)
Sorry for the delay.
diff --git a/dlls/d3d10/d3d10_private.h b/dlls/d3d10/d3d10_private.h index 7f83fb1182a..b35711b55e5 100644 --- a/dlls/d3d10/d3d10_private.h +++ b/dlls/d3d10/d3d10_private.h @@ -52,6 +52,7 @@ enum d3d10_effect_object_operation D3D10_EOO_CONST_INDEX = 3, D3D10_EOO_VAR_INDEX = 4, D3D10_EOO_INDEX_EXPRESSION = 5,
- D3D10_EOO_VALUE_EXPRESSION = 6, D3D10_EOO_ANONYMOUS_SHADER = 7,
};
diff --git a/dlls/d3d10/effect.c b/dlls/d3d10/effect.c index 3f35e9cc28a..9c2012768dd 100644 --- a/dlls/d3d10/effect.c +++ b/dlls/d3d10/effect.c @@ -289,6 +289,10 @@ struct d3d10_effect_prop_dependency struct d3d10_effect_variable *v; struct d3d10_effect_preshader index; } index_expr;
struct{struct d3d10_effect_preshader value;} value_expr;};
@@ -392,6 +396,9 @@ static void d3d10_effect_clear_prop_dependencies(struct d3d10_effect_prop_depend case D3D10_EOO_INDEX_EXPRESSION: d3d10_effect_preshader_clear(&dep->index_expr.index); break;
case D3D10_EOO_VALUE_EXPRESSION:d3d10_effect_preshader_clear(&dep->value_expr.value);break; }@@ -2632,6 +2639,34 @@ static HRESULT parse_fx10_property_assignment(const char *data, size_t data_size
return d3d10_effect_add_prop_dependency(d, &dep);
case D3D10_EOO_VALUE_EXPRESSION:if (value_offset >= data_size || !require_space(value_offset, 1, sizeof(uint16_t), data_size)){WARN("Invalid offset %#x (data size %#lx).\n", value_offset, (long)data_size);return E_FAIL;}Pretty sure you want sizeof(DWORD) here.
data_ptr = data + value_offset;read_dword(&data_ptr, &blob_size);if (!require_space(value_offset, 1, sizeof(uint16_t) + blob_size, data_size)){WARN("Invalid offset %#x (data size %#lx).\n", value_offset, (long)data_size);return E_FAIL;}Not sure about this sizeof(uint16_t) either, what's the idea?
BTW, we probably want this kind of require_space(..., blob_size, ...) in the D3D10_EOO_INDEX_EXPRESSION case too.
Yes, it should have been uint32_t in both places.
First check is to read 4 bytes at "value_offset" offset, which would give blob_size. Additional size in second check is to account for blob_size that's been read already. Alternatively this could do value_offset += 4.
On Fri, Nov 19, 2021 at 2:47 PM Nikolay Sivov nsivov@codeweavers.com wrote:
On 11/19/21 4:41 PM, Matteo Bruni wrote:
On Tue, Nov 16, 2021 at 6:15 PM Nikolay Sivov nsivov@codeweavers.com wrote:
Signed-off-by: Nikolay Sivov nsivov@codeweavers.com
dlls/d3d10/d3d10_private.h | 1 + dlls/d3d10/effect.c | 35 +++++++++++++++++ dlls/d3d10/tests/effect.c | 78 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 114 insertions(+)
Sorry for the delay.
diff --git a/dlls/d3d10/d3d10_private.h b/dlls/d3d10/d3d10_private.h index 7f83fb1182a..b35711b55e5 100644 --- a/dlls/d3d10/d3d10_private.h +++ b/dlls/d3d10/d3d10_private.h @@ -52,6 +52,7 @@ enum d3d10_effect_object_operation D3D10_EOO_CONST_INDEX = 3, D3D10_EOO_VAR_INDEX = 4, D3D10_EOO_INDEX_EXPRESSION = 5,
- D3D10_EOO_VALUE_EXPRESSION = 6, D3D10_EOO_ANONYMOUS_SHADER = 7,
};
diff --git a/dlls/d3d10/effect.c b/dlls/d3d10/effect.c index 3f35e9cc28a..9c2012768dd 100644 --- a/dlls/d3d10/effect.c +++ b/dlls/d3d10/effect.c @@ -289,6 +289,10 @@ struct d3d10_effect_prop_dependency struct d3d10_effect_variable *v; struct d3d10_effect_preshader index; } index_expr;
struct{struct d3d10_effect_preshader value;} value_expr;};
@@ -392,6 +396,9 @@ static void d3d10_effect_clear_prop_dependencies(struct d3d10_effect_prop_depend case D3D10_EOO_INDEX_EXPRESSION: d3d10_effect_preshader_clear(&dep->index_expr.index); break;
case D3D10_EOO_VALUE_EXPRESSION:d3d10_effect_preshader_clear(&dep->value_expr.value);break; }@@ -2632,6 +2639,34 @@ static HRESULT parse_fx10_property_assignment(const char *data, size_t data_size
return d3d10_effect_add_prop_dependency(d, &dep);
case D3D10_EOO_VALUE_EXPRESSION:if (value_offset >= data_size || !require_space(value_offset, 1, sizeof(uint16_t), data_size)){WARN("Invalid offset %#x (data size %#lx).\n", value_offset, (long)data_size);return E_FAIL;}Pretty sure you want sizeof(DWORD) here.
data_ptr = data + value_offset;read_dword(&data_ptr, &blob_size);if (!require_space(value_offset, 1, sizeof(uint16_t) + blob_size, data_size)){WARN("Invalid offset %#x (data size %#lx).\n", value_offset, (long)data_size);return E_FAIL;}Not sure about this sizeof(uint16_t) either, what's the idea?
BTW, we probably want this kind of require_space(..., blob_size, ...) in the D3D10_EOO_INDEX_EXPRESSION case too.
Yes, it should have been uint32_t in both places.
First check is to read 4 bytes at "value_offset" offset, which would give blob_size. Additional size in second check is to account for blob_size that's been read already. Alternatively this could do value_offset += 4.
Ah yes, that seems right. It's okay as it is (with the fixed sizeof() of course).
-
Matteo Bruni -
Nikolay Sivov