I am also fine with fixing these issues right away, especially because of the ugly typo - the error check for D3DXCreatePolygon() was copied from existing tests, I assumed there is a good reason why D3DXCreate*() could fail on some machines/testbots. Would you mind taking another look please? Thanks for your feedback!
Regards, Sebastian
On 05.11.2014 18:03, Matteo Bruni wrote:
2014-11-04 19:57 GMT+01:00 Sebastian Lackner sebastian@fds-team.de:
Based on a patch by David Adam.
dlls/d3dx9_36/tests/mesh.c | 163 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 163 insertions(+)
Hi Sebastian,
first of all, let me say I'm okay with these patches going in as they are.
IMHO there is still room for improvement though:
+static void test_polygon(IDirect3DDevice9 *device, float length, unsigned int sides) +{
- HRESULT hr;
- ID3DXMesh *polygon;
- struct mesh mesh;
- char name[256];
No need to use such a big array there.
- hr = D3DXCreatePolygon(device, length, sides, &polygon, NULL);
- ok(hr == D3D_OK, "Got result %x, expected 0 (D3D_OK)\n", hr);
- if (hr != D3D_OK)
- {
skip("Couldn't create box\n");
Copy-paste typo. FWIW, we usually prefer a '.' at the end of trace messages.
- adjacency = NULL;
- hr = D3DXCreatePolygon(device, 3.0f, 11, &polygon, &adjacency);
- ok(hr == D3D_OK, "Expected D3D_OK, received %#x\n", hr);
- if (FAILED(hr))
- {
skip("D3DXCreatePolygon failed\n");
goto end;
- }
That D3DXCreatePolygon() call is not expected to ever fail, so no need for the "if".
- if (adjacency)
ID3DXBuffer_Release(adjacency);
If you remove the previous "if" you won't need the "end" label and this one becomes always true. Then I'd put this Release() right after the loop checking the adjacency.
Again, I don't care about those details, this is still good enough for me. It's mostly about any future patch you'd want to send in.