On 9 March 2016 at 14:38, Paul Gofman gofmanp@gmail.com wrote:
+static float test_effect_preshader_fconstsv[][4] = +{
- {11.0, 12.0, 13.0, 0.0 },
- {21.0, 22.0, 23.0, 0.0 },
- {31.0, 32.0, 33.0, 0.0 },
- {41.0, 42.0, 43.0, 0.0 },
- {11.0, 21.0, 31.0, 0.0 },
- {12.0, 22.0, 32.0, 0.0 },
- {13.0, 23.0, 33.0, 0.0 },
- {14.0, 24.0, 34.0, 0.0 },
- {11.0, 12.0, 13.0, 14.0},
- {21.0, 22.0, 23.0, 24.0},
- {31.0, 32.0, 33.0, 34.0},
- {11.0, 21.0, 31.0, 41.0},
- {12.0, 22.0, 32.0, 42.0},
- {13.0, 23.0, 33.0, 43.0}
+};
+static float test_effect_preshader_fconstsp[][4] = +{
- {11.0, 21.0, 0.0 , 0.0 },
- {12.0, 22.0, 0.0 , 0.0 },
- {13.0, 23.0, 0.0 , 0.0 },
- {11.0, 12.0, 0.0 , 0.0 },
- {21.0, 22.0, 0.0 , 0.0 },
- {31.0, 32.0, 0.0 , 0.0 },
- {11.0, 12.0, 0.0 , 0.0 },
- {21.0, 22.0, 0.0 , 0.0 },
- {11.0, 21.0, 0.0 , 0.0 },
- {12.0, 22.0, 0.0 , 0.0 },
- {11.0, 12.0, 13.0, 0.0 },
- {21.0, 22.0, 23.0, 0.0 },
- {11.0, 21.0, 31.0, 0.0 },
- {12.0, 22.0, 32.0, 0.0 }
+};
+static BOOL test_effect_preshader_bconsts[] = +{
- 1, 0, 1, 0, 1, 0
+};
+static struct +{
- const char *comment;
- BOOL todo[4];
- DWORD result[4];
+} +test_effect_preshader_op_results[] = +{
- {"1 / op", {1, 1, 1, 1}, {0x7f800000, 0xff800000, 0xbee8ba2e, 0x00200000}},
- {"rsq", {1, 1, 3, 3}, {0x7f800000, 0x7f800000, 0x3f2c985d, 0x1f800000}},
- {"mul", {1, 1, 1, 1}, {0x00000000, 0x80000000, 0x40d33334, 0x7f800000}},
- {"add", {0, 1, 1, 1}, {0x3f800000, 0x40000000, 0xc0a66666, 0x7f7fffff}},
- {"lt", {0, 0, 1, 0}, {0x3f800000, 0x3f800000, 0x00000000, 0x00000000}},
- {"ge", {1, 1, 0, 1}, {0x00000000, 0x00000000, 0x3f800000, 0x3f800000}},
- {"neg", {1, 1, 1, 1}, {0x80000000, 0x00000000, 0x400ccccd, 0xff7fffff}},
- {"rcp", {1, 1, 1, 1}, {0x7f800000, 0xff800000, 0xbee8ba2e, 0x00200000}},
- {"frac", {0, 0, 1, 0}, {0x00000000, 0x00000000, 0x3f4ccccc, 0x00000000}},
- {"min", {0, 1, 1, 1}, {0x00000000, 0x80000000, 0xc0400000, 0x40800000}},
- {"max", {1, 1, 1, 1}, {0x3f800000, 0x40000000, 0xc00ccccd, 0x7f7fffff}},
- {"sin", {0, 1, 1, 3}, {0x00000000, 0x80000000, 0xbf4ef99e, 0xbf0599b3}},
- {"cos", {1, 1, 1, 3}, {0x3f800000, 0x3f800000, 0xbf16a803, 0x3f5a5f96}},
- {"den mul",{1, 1, 1, 1}, {0x7f800000, 0xff800000, 0xbb94f209, 0x000051ec}},
- {"dot", {0, 1, 1, 0}, {0x00000000, 0x7f800000, 0x41f00000, 0x00000000}}
+};
These should all be const. I'd probably use D3DXVECTOR4 for the float constant arrays.
+#define TEST_EFFECT_PRES_NFLOATV (sizeof(test_effect_preshader_fconstsv) / sizeof(*test_effect_preshader_fconstsv)) +#define TEST_EFFECT_PRES_NFLOATP (sizeof(test_effect_preshader_fconstsp) / sizeof(*test_effect_preshader_fconstsp)) +#define TEST_EFFECT_PRES_NFLOATMAX (TEST_EFFECT_PRES_NFLOATV > TEST_EFFECT_PRES_NFLOATP ? \
TEST_EFFECT_PRES_NFLOATV : TEST_EFFECT_PRES_NFLOATP)+#define TEST_EFFECT_PRES_NBOOL (sizeof(test_effect_preshader_bconsts) / sizeof(*test_effect_preshader_bconsts)) +#define TEST_EFFECT_PRES_NOPTESTS (sizeof(test_effect_preshader_op_results) / \
sizeof(*test_effect_preshader_op_results))
I think you should either just write these out, or introduce an ARRAY_SIZE macro.
- const static D3DXVECTOR4 fvect1 = {28.0f, 29.0f, 30.0f, 31.0f};
- const static D3DXVECTOR4 fvect2 = {0.0f, 0.0f, 1.0f, 0.0f};
- const static float fvect_empty[] = {-9999.0f, -9999.0f, -9999.0f, -9999.0f};
This is not wrong, but the usual order is "static const".
todo_wine_if(!!bdata[i] != !!test_effect_preshader_bconsts[i])
You can write that as "!bdata[i] != !test_effect_preshader_bconsts[i]" without changing the meaning.
- todo_wine ok(hr == D3D_OK, "Got result %x, expected 0 (D3D_OK).\n", hr);
%#x to make it clear it's a hexadecimal value. You don't need to print the expected value if it's a constant.
- hr = IDirect3DDevice9_GetVertexShader(device, &vshader);
- ok(hr == D3D_OK, "Got result %x, expected 0 (D3D_OK).\n", hr);
- ok(vshader != NULL, "Got NULL vshader.\n");
- if (vshader)
- {
That kind of thing is pointless, if "vshader" was NULL, the test would have failed.
- ok(vshader == NULL, "Incorrect shader selected.\n");
I would prefer those as "ok(!vshader, ...);", but it's just style.
Thank you for the review. I will fix that, and resubmit probably after I get the comments from Matteo for the other patches in the series. Regarding D3DXVECTOR4, it has floats. I wanted to compare exact values. I can of course convert all these values to text float representation, but this what I intentionally avoided doing not to deal with potential rounding problems when initializing these constants. Maybe there is some better way for exact float results matching?
On 03/09/2016 05:48 PM, Henri Verbeet wrote:
+} +test_effect_preshader_op_results[] = +{
- {"1 / op", {1, 1, 1, 1}, {0x7f800000, 0xff800000, 0xbee8ba2e, 0x00200000}},
- {"rsq", {1, 1, 3, 3}, {0x7f800000, 0x7f800000, 0x3f2c985d, 0x1f800000}},
- {"mul", {1, 1, 1, 1}, {0x00000000, 0x80000000, 0x40d33334, 0x7f800000}},
- {"add", {0, 1, 1, 1}, {0x3f800000, 0x40000000, 0xc0a66666, 0x7f7fffff}},
- {"lt", {0, 0, 1, 0}, {0x3f800000, 0x3f800000, 0x00000000, 0x00000000}},
- {"ge", {1, 1, 0, 1}, {0x00000000, 0x00000000, 0x3f800000, 0x3f800000}},
- {"neg", {1, 1, 1, 1}, {0x80000000, 0x00000000, 0x400ccccd, 0xff7fffff}},
- {"rcp", {1, 1, 1, 1}, {0x7f800000, 0xff800000, 0xbee8ba2e, 0x00200000}},
- {"frac", {0, 0, 1, 0}, {0x00000000, 0x00000000, 0x3f4ccccc, 0x00000000}},
- {"min", {0, 1, 1, 1}, {0x00000000, 0x80000000, 0xc0400000, 0x40800000}},
- {"max", {1, 1, 1, 1}, {0x3f800000, 0x40000000, 0xc00ccccd, 0x7f7fffff}},
- {"sin", {0, 1, 1, 3}, {0x00000000, 0x80000000, 0xbf4ef99e, 0xbf0599b3}},
- {"cos", {1, 1, 1, 3}, {0x3f800000, 0x3f800000, 0xbf16a803, 0x3f5a5f96}},
- {"den mul",{1, 1, 1, 1}, {0x7f800000, 0xff800000, 0xbb94f209, 0x000051ec}},
- {"dot", {0, 1, 1, 0}, {0x00000000, 0x7f800000, 0x41f00000, 0x00000000}}
+}; These should all be const. I'd probably use D3DXVECTOR4 for the float constant arrays.
PS Besides there is an issue with Inf and Nan FP comparison. There are currently no NANs in this table, but there are some INFs. I thought it is safer to compare these as integers, otherwise explicit testing for Nan (and possibly for Inf, though not strictly necessary) will be required.
On 03/09/2016 06:26 PM, Paul Gofman wrote:
Thank you for the review. I will fix that, and resubmit probably after I get the comments from Matteo for the other patches in the series. Regarding D3DXVECTOR4, it has floats. I wanted to compare exact values. I can of course convert all these values to text float representation, but this what I intentionally avoided doing not to deal with potential rounding problems when initializing these constants. Maybe there is some better way for exact float results matching?
On 03/09/2016 05:48 PM, Henri Verbeet wrote:
+} +test_effect_preshader_op_results[] = +{
- {"1 / op", {1, 1, 1, 1}, {0x7f800000, 0xff800000, 0xbee8ba2e, 0x00200000}},
- {"rsq", {1, 1, 3, 3}, {0x7f800000, 0x7f800000, 0x3f2c985d, 0x1f800000}},
- {"mul", {1, 1, 1, 1}, {0x00000000, 0x80000000, 0x40d33334, 0x7f800000}},
- {"add", {0, 1, 1, 1}, {0x3f800000, 0x40000000, 0xc0a66666, 0x7f7fffff}},
- {"lt", {0, 0, 1, 0}, {0x3f800000, 0x3f800000, 0x00000000, 0x00000000}},
- {"ge", {1, 1, 0, 1}, {0x00000000, 0x00000000, 0x3f800000, 0x3f800000}},
- {"neg", {1, 1, 1, 1}, {0x80000000, 0x00000000, 0x400ccccd, 0xff7fffff}},
- {"rcp", {1, 1, 1, 1}, {0x7f800000, 0xff800000, 0xbee8ba2e, 0x00200000}},
- {"frac", {0, 0, 1, 0}, {0x00000000, 0x00000000, 0x3f4ccccc, 0x00000000}},
- {"min", {0, 1, 1, 1}, {0x00000000, 0x80000000, 0xc0400000, 0x40800000}},
- {"max", {1, 1, 1, 1}, {0x3f800000, 0x40000000, 0xc00ccccd, 0x7f7fffff}},
- {"sin", {0, 1, 1, 3}, {0x00000000, 0x80000000, 0xbf4ef99e, 0xbf0599b3}},
- {"cos", {1, 1, 1, 3}, {0x3f800000, 0x3f800000, 0xbf16a803, 0x3f5a5f96}},
- {"den mul",{1, 1, 1, 1}, {0x7f800000, 0xff800000, 0xbb94f209, 0x000051ec}},
- {"dot", {0, 1, 1, 0}, {0x00000000, 0x7f800000, 0x41f00000, 0x00000000}}
+}; These should all be const. I'd probably use D3DXVECTOR4 for the float constant arrays.
On 9 March 2016 at 16:26, Paul Gofman gofmanp@gmail.com wrote:
Thank you for the review. I will fix that, and resubmit probably after I get the comments from Matteo for the other patches in the series. Regarding D3DXVECTOR4, it has floats. I wanted to compare exact values.
You store floats now too, although you initialise them with doubles. (Unless you're talking about the "result" values in test_effect_preshader_op_results[]?)
On 03/09/2016 06:37 PM, Henri Verbeet wrote:
On 9 March 2016 at 16:26, Paul Gofman gofmanp@gmail.com wrote:
Thank you for the review. I will fix that, and resubmit probably after I get the comments from Matteo for the other patches in the series. Regarding D3DXVECTOR4, it has floats. I wanted to compare exact values.
You store floats now too, although you initialise them with doubles. (Unless you're talking about the "result" values in test_effect_preshader_op_results[]?)
Yes, I am talking here solely about "result" values from test_effect_preshader_op_results. These are the only ones which are computed and then compared.
The other float constants which are introduced in natural float representation are either directly copied inside the effect framework and compared with the same table in the test, or converted to int in the effect and used as indices. Initializing them with doubles does not affect the result but it is my inaccuracy and I will fix this either.
On 9 March 2016 at 16:48, Paul Gofman gofmanp@gmail.com wrote:
Yes, I am talking here solely about "result" values from test_effect_preshader_op_results. These are the only ones which are computed and then compared.
Oh, those are fine. I was only referring to using D3DXVECTOR4 instead of float[4] for test_effect_preshader_fconstsv[] and test_effect_preshader_fconstsp.
On 03/09/2016 06:48 PM, Paul Gofman wrote:
The other float constants which are introduced in natural float representation are either directly copied inside the effect framework and compared with the same table in the test, ...
Sorry, I am not quite exact here. Some values are not copied from the test tables initially, they are defined in effect source code independently but using the same input ASCII float representation, so they necessarily get exactly the same float values in effect (since they are small integers double/float difference is not important).
-
Henri Verbeet -
Paul Gofman