2016-04-21 10:56 GMT+02:00 Paul Gofman gofmanp@gmail.com:
@@ -43,6 +43,7 @@ enum pres_ops PRESHADER_OP_SIN, PRESHADER_OP_COS, PRESHADER_OP_RSQ,
- PRESHADER_OP_EXP,
};
#define PRES_OPCODE_MASK 0x7ff00000 #define PRES_OPCODE_SHIFT 20 @@ -119,6 +121,7 @@ static const struct op_info pres_op_info[] = {0x108, "sin", 1, 0, pres_sin}, /* PRESHADER_OP_SIN */ {0x109, "cos", 1, 0, pres_cos}, /* PRESHADER_OP_COS */ {0x107, "rsq", 1, 0, pres_rsq}, /* PRESHADER_OP_RSQ */
- {0x105, "exp", 1, 0, pres_exp}, /* PRESHADER_OP_EXP */
};
It might make sense to keep the array sorted by opcode (which requires either adding the enum into the table or, most likely, reordering the enum too). That way you can binary search into it in the future, if necessary. No particular hurry for that though.
+static void test_preshader_op(IDirect3DDevice9 *device, const char *op_mnem,
unsigned int opcode, unsigned int args_count, unsigned int *expected_result,D3DXVECTOR4 *fvect1, D3DXVECTOR4 *fvect2, unsigned int ulps, BOOL *todo)+{
- DWORD test_effect_blob[ARRAY_SIZE(test_effect_preshader_effect_blob)];
It's not a huge array but it seems better to avoid putting it on the stack nonetheless - i.e. please allocate it dynamically with HeapAlloc().
- HRESULT hr;
- ID3DXEffect *effect;
- D3DLIGHT9 light;
- unsigned int i, npasses;
passes_count?
- float *v;
- unsigned int op_pos, op_step, result_index;
- D3DXHANDLE param;
- if (args_count == 1)
- {
op_pos = TEST_EFFECT_PRESHADER_OP0_POS;op_step = TEST_EFFECT_PRESHADER_OP0_INS_SIZE;result_index = 0;- }
- else if (args_count == 2)
- {
op_pos = TEST_EFFECT_PRESHADER_OPMUL_POS;op_step = TEST_EFFECT_PRESHADER_OPMUL_INS_SIZE;result_index = 2;- }
- else
- {
return;- }
That doesn't seem particularly nice, especially WRT extending this test later on. It seems better to have the "original" bytecode and these variables you compute here passed in from the caller (maybe stored in a struct together with the opcode parameters). test_effect_preshader_ops() should probably be table-based. At that point the *_POS and *_SIZE defines might become unnecessary; if they stay they should probably be named after the argument count (which is the actually interesting part, the original operation is going to change for the test and can be figured out from the light index) instead.
- memcpy(test_effect_blob, test_effect_preshader_effect_blob, sizeof test_effect_blob);
- for (i = 0; i < 4; i++)
++i, for consistency.
On 04/22/2016 05:15 PM, Matteo Bruni wrote:
- float *v;
- unsigned int op_pos, op_step, result_index;
- D3DXHANDLE param;
- if (args_count == 1)
- {
op_pos = TEST_EFFECT_PRESHADER_OP0_POS;op_step = TEST_EFFECT_PRESHADER_OP0_INS_SIZE;result_index = 0;- }
- else if (args_count == 2)
- {
op_pos = TEST_EFFECT_PRESHADER_OPMUL_POS;op_step = TEST_EFFECT_PRESHADER_OPMUL_INS_SIZE;result_index = 2;- }
- else
- {
return;- }
That doesn't seem particularly nice, especially WRT extending this test later on. It seems better to have the "original" bytecode and these variables you compute here passed in from the caller (maybe stored in a struct together with the opcode parameters).
I am not sure I understand how do you see this original bytecode: is it a pointer inside the effect blob to the code to be altered, the whole effect blob pointer or something else?
test_effect_preshader_ops() should probably be table-based. At that point the *_POS and *_SIZE defines might become unnecessary; if they stay they should probably be named after the argument count (which is the actually interesting part, the original operation is going to change for the test and can be figured out from the light index) instead.
I can put it in the array but do we need to make this too generic? I. e. support other argument counts besides 1 and 2? I know just one opcode which has more opcodes and we are unlikely to encounter much more. I was planning to test d3dts_dotswiz separately outside of this infrastructure. I can get rid of _SIZE define (as actually it can be computed from arguments count), but how do you see getting rid of _POS?
2016-04-22 17:59 GMT+02:00 Paul Gofman gofmanp@gmail.com:
On 04/22/2016 05:15 PM, Matteo Bruni wrote:
- float *v;
- unsigned int op_pos, op_step, result_index;
- D3DXHANDLE param;
- if (args_count == 1)
- {
op_pos = TEST_EFFECT_PRESHADER_OP0_POS;op_step = TEST_EFFECT_PRESHADER_OP0_INS_SIZE;result_index = 0;- }
- else if (args_count == 2)
- {
op_pos = TEST_EFFECT_PRESHADER_OPMUL_POS;op_step = TEST_EFFECT_PRESHADER_OPMUL_INS_SIZE;result_index = 2;- }
- else
- {
return;- }
That doesn't seem particularly nice, especially WRT extending this test later on. It seems better to have the "original" bytecode and these variables you compute here passed in from the caller (maybe stored in a struct together with the opcode parameters).
I am not sure I understand how do you see this original bytecode: is it a pointer inside the effect blob to the code to be altered, the whole effect blob pointer or something else?
I imagined passing a pointer to the whole bytecode, which you then copy and modify here. I.e. pretty much what you did in v2.
test_effect_preshader_ops() should probably be table-based. At that point the *_POS and *_SIZE defines might become unnecessary; if they stay they should probably be named after the argument count (which is the actually interesting part, the original operation is going to change for the test and can be figured out from the light index) instead.
I can put it in the array but do we need to make this too generic? I. e. support other argument counts besides 1 and 2? I know just one opcode which has more opcodes and we are unlikely to encounter much more. I was planning to test d3dts_dotswiz separately outside of this infrastructure. I can get rid of _SIZE define (as actually it can be computed from arguments count), but how do you see getting rid of _POS?
Just putting it in the table without a define, I don't think it would look too bad. Unless you'll need to use the same values in some other test.
-
Matteo Bruni -
Paul Gofman