Re: [PATCH 3/3] d3drm/tests: Some tests for mesh builder and faces

[Henri Verbeet]

On 9 June 2017 at 01:36, Nikolay Sivov <nsivov(a)codeweavers.com> wrote:
> +#define test_vector_eq(a, b, c, d) test_vector_eq_(__LINE__, a, b, c, d)
> +static void test_vector_eq_(unsigned int line, D3DVECTOR *v, float x, float y, float z)
> "v" should probably be const. More generally, with the scheme we've
> been using for the other D3D tests, this would look like the
> following:

static void check_vector_(unsigned int line, const D3DVECTOR *v,
float x, float y, float x, unsigned int ulps)

and print the ok_() as something like "Got unexpected vector {%.8e,
%.8e, %.8e}, expected {%.8e, %.8e, %.8e}.\n". Note that the %.8e is
because %f is lossy. See e.g. compare_vec4() in the ddraw and d3d11
tests.
> +    memset(v, 0, sizeof(v));
> +    hr = IDirect3DRMFace_GetVertex(face1, 0, v, n);
> +    ok(SUCCEEDED(hr), "Failed to get vertex data, hr %#x.\n", hr);
> +    ok(U1(v[0]).x == 1.0f, "Wrong component v[0].x = %f (expected %f)\n", U1(v[0]).x, 1.0f);
> +    ok(U2(v[0]).y == 2.0f, "Wrong component v[0].y = %f (expected %f)\n", U2(v[0]).y, 2.0f);
> +    ok(U3(v[0]).z == 3.0f, "Wrong component v[0].z = %f (expected %f)\n", U3(v[0]).z, 3.0f);
> +
> That should probably use check_vector(), and I think there are more
> places in the d3drm tests that would benefit from using
> check_vector().
> +    color = IDirect3DRMFace_GetColor(face1);
> +todo_wine
> +    ok(color == 0xff00ff00, "Got wrong color, %#x.\n", color);
> %#x is a little awkward for colours, 0x%08x would be more appropriate.
> I'd argue for "unexpected" over "wrong".
> +    hr = IDirect3DRMMeshBuilder3_GetFace(mesh_builder3, 0, &face2);
> +    ok(SUCCEEDED(hr), "Failed to delete a face, %#x.\n", hr);
> s/delete/retrieve/
> I think that should be "the" instead of "a", but I think simply
> omitting the article would also be fine.
> -#define IDirect3DRMFace_GetColor(p,a)                     (p)->lpVtbl->GetColor(p,a)
> +#define IDirect3DRMFace_GetColor(p)                       (p)->lpVtbl->GetColor(p)
> IDirect3DRMFace2 seems to have the same issue.