[2/2] d3dx9_36/tests: Add tests for D3DXCreatePolygon.

[Sebastian Lackner]

Thanks for the feedback.

I am fine with changing what you suggested, but please note that most of these "issues" are also present in existing code, which I used as a template for implementing the new function. To point out a few examples:

+    TRACE("(%p, %f, %u, %p, %p)\n", device, length, sides, mesh, adjacency);

Thats also the way its implemented for D3DXCreateSphere, D3DXCreateCylinder, D3DXCreateTeapot, D3DXCreateTextW, ...

+    for (i = 0; i < sides; i++)

"i++" has 95 matches in mesh.c, "++i" only 4 ...

Setting *mesh = NULL on error is only used at one place in the whole file. If it is necessary to match native behaviour, it is basically missing for all functions.

+static BOOL compute_polygon(struct mesh *mesh, float length, UINT sides)

See compute_sphere, compute_cylinder, compute_torus, ...

+    IDirect3D9* d3d;
+    IDirect3DDevice9* device;
+    D3DPRESENT_PARAMETERS d3dpp;
+    ID3DXMesh* polygon;
+    ID3DXBuffer* ppBuffer;

My fault, copy & paste from existing tests.

Regards,
Sebastian

On 03.11.2014 18:57, Matteo Bruni wrote:
> 2014-11-03 6:35 GMT+01:00 Sebastian Lackner <sebastian at fds-team.de>:
>> Based on a patch by David Adam.
>>
>> ---
>>  dlls/d3dx9_36/tests/mesh.c | 154 +++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 154 insertions(+)
> 
> More nitpicks...
> 
> +static BOOL compute_polygon(struct mesh *mesh, float length, UINT sides)
> 
> I'd prefer a plain unsigned int there.
> 
> The various comments I made for the implementation patch obviously
> apply to the body of compute_polygon() too.
> 
> +    IDirect3D9* d3d;
> +    IDirect3DDevice9* device;
> +    D3DPRESENT_PARAMETERS d3dpp;
> +    ID3DXMesh* polygon;
> +    ID3DXBuffer* ppBuffer;
> 
> Watch out for the '*' placement. Also, no hungarian notation please.
> 
> +    hr = IDirect3D9_CreateDevice(d3d, D3DADAPTER_DEFAULT,
> D3DDEVTYPE_HAL, wnd, D3DCREATE_MIXED_VERTEXPROCESSING, &d3dpp,
> &device);
> 
> Not sure we care but the other tests in mesh.c use
> D3DCREATE_SOFTWARE_VERTEXPROCESSING.
> 
> +    ok(buffer_size == 33 * sizeof(DWORD), "expected size %d, received %d\n",
> +       33 * sizeof(DWORD), buffer_size);
> 
> I think it's usually better not to print sizeof() results.
> 
> +        ok(buffer[i][1] == -1, "wrong adjacency[%d][1] = %d\n", i,
> buffer[i][1]);
> 
> I'd prefer ~0 here. Also, unsigned int should be printed with "%u".
> 
> +    if (ppBuffer) ID3DXBuffer_Release(ppBuffer);
> 
> Please put the "if" body on a separate line.
> 
> I'd like to see an additional test to check whether the ID3DXMesh
> pointer is set to NULL on D3DXCreatePolygon() failure. Probably you
> could simply modify (at least) one of the failure tests at the
> beginning for that.
>