On Tue, Jun 14, 2011 at 1:43 PM, Stefan Dösinger <stefandoesin...@gmx.at> wrote:
> On Tuesday 14 June 2011 12:57:35 Michael Mc Donnell wrote:
>> I cache the vertex declaration (D3DVERTEXELEMENT9 array) and the size
>> of the vertex declaration, so that they can be used by
>> GetNumBytesPerVertex and GetDeclaration when an invalid declaration
>> has been passed.
>
> In GetDeclaration:
>
>> +    memcpy(declaration, This->cached_declaration, sizeof(This-
>>cached_declaration));
> You should probably test if native really writes the full MAX_FVF_DECL_SIZE
> declaration elements, or only up to the D3DDECL_END() marker in the real
> declaration. A similar consideration applies to UpdateSemantics, but that is
> harder to test. E.g.:
>
> +    D3DVERTEXELEMENT9 declaration[MAX_FVF_DECL_SIZE];
> +    D3DVERTEXELEMENT9 declaration0[] =
> +    {
> ...
>
> memset(declaration, 0xaa, sizeof(declaration));
> memcpy(declaration, declaration0, sizeof(declaration0));
> UpdateSemantics(declaration)
> memset(declaration, 0xbb, sizeof(declaration));
> GetDeclaration(declaration);
>
> Now look at the bytes in 'declaration' after the D3DDECL_END() marker up to
> the end of the array. They could be 0xaa(UpdateSemantics reads everything,
> GetDeclaration writes everything), 0xbb(UpdateSemantics is unknown,
> GetDeclaration writes only the defined part), or something else(e.g.
> UpdateSemantics and/or GetDeclaration clear set the extra bytes to 0).

I've added the test you outlined. It shows you're correct that
GetDeclaration only writes up to D3DDECL_END().  I've changed the
implementation so that it caches the number of elements and only
writes up to D3DDECL_END().

> In the way your implementation and tests are currently set up UpdateSemantics
> will read undefined memory because your test declarations like declaration0
> don't have the full MAX_FVF_DECL_SIZE array size. Otoh I am not quite sure
> about the semantics of passing an array on the stack like this. Maybe the
> compiler allocates a new array with the full size and copies the content
> around.

You're right it could potentially read undefined memory depending on
the compiler semantics. I think the safest thing is just to read up to
D3DDECL_END() in the passed in declaration, then it will never read
undefined memory (except if the programmer makes a mistake). I've
changed the implementation to count the number of elements in the new
declaration, cache the number, and then copy the contents of new
declaration into the cached declaration. Counting the elements doesn't
add any extra overhead as I also needed it to check for non-zero
stream values.

> Also I think this method has been pretty thoroughly tested and debated by now.
> If we implement all the functions this way we'd probably be pretty bug-free
> :-)

Yeah it turned out to be a lot harder than I had expected to get all
the details correct. I have also added a check for non-zero stream
values that Dylan Smith wanted me to add.

> Also, there's the obligatory style nitpick:
>> +    IDirect3DDevice9 *device;
>> ...
>> +static struct test_context* new_test_context(void)
> For consistency this should probably be static struct test_context
> *new_test_context(void).

Check :-)
From c1c2bc576a5ba3758c15d65d4b21404e80a3c5be Mon Sep 17 00:00:00 2001
From: Michael Mc Donnell <mich...@mcdonnell.dk>
Date: Tue, 24 May 2011 19:43:47 +0200
Subject: d3dx9/test: Add UpdateSemantics test.

Removed a superfluous test.
Added NULL check before releasing mesh.
Changed struct declaration style.
Changed the struct declaration style again.
Added GetNumBytesPerVertex test
Added GetDeclaration tests
Added equals check
Renamed larger declaration test to overlapping declaration test.
Added smaller and larger declarations.
Added D3DECL_END() marker check
Moved pointer * to new_test_context (nitpick)
Added multiple streams test
---
 dlls/d3dx9_36/tests/mesh.c |  418 ++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 418 insertions(+), 0 deletions(-)

diff --git a/dlls/d3dx9_36/tests/mesh.c b/dlls/d3dx9_36/tests/mesh.c
index 668097f..f4b1bbf 100644
--- a/dlls/d3dx9_36/tests/mesh.c
+++ b/dlls/d3dx9_36/tests/mesh.c
@@ -2,6 +2,7 @@
  * Copyright 2008 David Adam
  * Copyright 2008 Luis Busquets
  * Copyright 2009 Henri Verbeet for CodeWeavers
+ * Copyright 2011 Michael Mc Donnell
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -89,6 +90,93 @@ static BOOL compare_face(face a, face b)
     return (a[0]==b[0] && a[1] == b[1] && a[2] == b[2]);
 }
 
+struct test_context
+{
+    HWND hwnd;
+    IDirect3D9 *d3d;
+    IDirect3DDevice9 *device;
+};
+
+/* Initializes a test context struct. Use it to initialize DirectX.
+ *
+ * Returns NULL if an error occured.
+ */
+static struct test_context *new_test_context(void)
+{
+    HRESULT hr;
+    HWND hwnd = NULL;
+    IDirect3D9 *d3d = NULL;
+    IDirect3DDevice9 *device = NULL;
+    D3DPRESENT_PARAMETERS d3dpp = {0};
+    struct test_context *test_context;
+
+    hwnd = CreateWindow("static", "d3dx9_test", 0, 0, 0, 0, 0, NULL, NULL, NULL, NULL);
+    if (!hwnd)
+    {
+        skip("Couldn't create application window\n");
+        goto error;
+    }
+
+    d3d = Direct3DCreate9(D3D_SDK_VERSION);
+    if (!d3d)
+    {
+        skip("Couldn't create IDirect3D9 object\n");
+        goto error;
+    }
+
+    memset(&d3dpp, 0, sizeof(d3dpp));
+    d3dpp.Windowed = TRUE;
+    d3dpp.SwapEffect = D3DSWAPEFFECT_DISCARD;
+    hr = IDirect3D9_CreateDevice(d3d, D3DADAPTER_DEFAULT, D3DDEVTYPE_HAL, hwnd,
+                                 D3DCREATE_MIXED_VERTEXPROCESSING, &d3dpp, &device);
+    if (FAILED(hr))
+    {
+        skip("Couldn't create IDirect3DDevice9 object %#x\n", hr);
+        goto error;
+    }
+
+    test_context = HeapAlloc(GetProcessHeap(), 0, sizeof(*test_context));
+    if (!test_context)
+    {
+        skip("Couldn't allocate memory for test_context\n");
+        goto error;
+    }
+    test_context->hwnd = hwnd;
+    test_context->d3d = d3d;
+    test_context->device = device;
+
+    return test_context;
+
+error:
+    if (device)
+        IDirect3DDevice9_Release(device);
+
+    if (d3d)
+        IDirect3D9_Release(d3d);
+
+    if (hwnd)
+        DestroyWindow(hwnd);
+
+    return NULL;
+}
+
+static void free_test_context(struct test_context *test_context)
+{
+    if (!test_context)
+        return;
+
+    if (test_context->device)
+        IDirect3DDevice9_Release(test_context->device);
+
+    if (test_context->d3d)
+        IDirect3D9_Release(test_context->d3d);
+
+    if (test_context->hwnd)
+        DestroyWindow(test_context->hwnd);
+
+    HeapFree(GetProcessHeap(), 0, test_context);
+}
+
 struct mesh
 {
     DWORD number_of_vertices;
@@ -4259,6 +4347,335 @@ static void D3DXGenerateAdjacencyTest(void)
     if (d3dxmesh) d3dxmesh->lpVtbl->Release(d3dxmesh);
 }
 
+static void test_update_semantics(void)
+{
+    HRESULT hr;
+    struct test_context *test_context = NULL;
+    ID3DXMesh *mesh = NULL;
+    D3DVERTEXELEMENT9 declaration0[] =
+    {
+         {0, 0, D3DDECLTYPE_FLOAT3, D3DDECLMETHOD_DEFAULT, D3DDECLUSAGE_POSITION, 0},
+         {0, 24, D3DDECLTYPE_FLOAT3, D3DDECLMETHOD_DEFAULT, D3DDECLUSAGE_NORMAL, 0},
+         {0, 36, D3DDECLTYPE_D3DCOLOR, D3DDECLMETHOD_DEFAULT, D3DDECLUSAGE_COLOR, 0},
+         D3DDECL_END()
+    };
+    D3DVERTEXELEMENT9 declaration_pos_type_color[] =
+    {
+         {0, 0, D3DDECLTYPE_D3DCOLOR, D3DDECLMETHOD_DEFAULT, D3DDECLUSAGE_POSITION, 0},
+         {0, 24, D3DDECLTYPE_FLOAT3, D3DDECLMETHOD_DEFAULT, D3DDECLUSAGE_NORMAL, 0},
+         {0, 36, D3DDECLTYPE_D3DCOLOR, D3DDECLMETHOD_DEFAULT, D3DDECLUSAGE_COLOR, 0},
+         D3DDECL_END()
+    };
+    D3DVERTEXELEMENT9 declaration_smaller[] =
+    {
+         {0, 0, D3DDECLTYPE_FLOAT3, D3DDECLMETHOD_DEFAULT, D3DDECLUSAGE_POSITION, 0},
+         {0, 24, D3DDECLTYPE_FLOAT3, D3DDECLMETHOD_DEFAULT, D3DDECLUSAGE_NORMAL, 0},
+         D3DDECL_END()
+    };
+    D3DVERTEXELEMENT9 declaration_larger[] =
+    {
+         {0, 0, D3DDECLTYPE_FLOAT3, D3DDECLMETHOD_DEFAULT, D3DDECLUSAGE_POSITION, 0},
+         {0, 24, D3DDECLTYPE_FLOAT3, D3DDECLMETHOD_DEFAULT, D3DDECLUSAGE_NORMAL, 0},
+         {0, 36, D3DDECLTYPE_D3DCOLOR, D3DDECLMETHOD_DEFAULT, D3DDECLUSAGE_COLOR, 0},
+         {0, 40, D3DDECLTYPE_FLOAT3, D3DDECLMETHOD_DEFAULT, D3DDECLUSAGE_TANGENT, 0},
+         D3DDECL_END()
+    };
+    D3DVERTEXELEMENT9 declaration_multiple_streams[] =
+    {
+         {0, 0, D3DDECLTYPE_FLOAT3, D3DDECLMETHOD_DEFAULT, D3DDECLUSAGE_POSITION, 0},
+         {1, 12, D3DDECLTYPE_FLOAT3, D3DDECLMETHOD_DEFAULT, D3DDECLUSAGE_TANGENT, 0},
+         {0, 24, D3DDECLTYPE_FLOAT3, D3DDECLMETHOD_DEFAULT, D3DDECLUSAGE_NORMAL, 0},
+         {0, 36, D3DDECLTYPE_D3DCOLOR, D3DDECLMETHOD_DEFAULT, D3DDECLUSAGE_COLOR, 0},
+
+         D3DDECL_END()
+    };
+    D3DVERTEXELEMENT9 declaration_double_usage[] =
+    {
+         {0, 0, D3DDECLTYPE_FLOAT3, D3DDECLMETHOD_DEFAULT, D3DDECLUSAGE_POSITION, 0},
+         {0, 12, D3DDECLTYPE_FLOAT3, D3DDECLMETHOD_DEFAULT, D3DDECLUSAGE_POSITION, 0},
+         {0, 24, D3DDECLTYPE_FLOAT3, D3DDECLMETHOD_DEFAULT, D3DDECLUSAGE_NORMAL, 0},
+         {0, 36, D3DDECLTYPE_D3DCOLOR, D3DDECLMETHOD_DEFAULT, D3DDECLUSAGE_COLOR, 0},
+         D3DDECL_END()
+    };
+    D3DVERTEXELEMENT9 declaration_undefined_type[] =
+    {
+         {0, 0, D3DDECLTYPE_UNUSED+1, D3DDECLMETHOD_DEFAULT, D3DDECLUSAGE_POSITION, 0},
+         {0, 24, D3DDECLTYPE_FLOAT3, D3DDECLMETHOD_DEFAULT, D3DDECLUSAGE_NORMAL, 0},
+         {0, 36, D3DDECLTYPE_D3DCOLOR, D3DDECLMETHOD_DEFAULT, D3DDECLUSAGE_COLOR, 0},
+         D3DDECL_END()
+    };
+    D3DVERTEXELEMENT9 declaration_not_4_byte_aligned_offset[] =
+    {
+         {0, 3, D3DDECLTYPE_FLOAT3, D3DDECLMETHOD_DEFAULT, D3DDECLUSAGE_POSITION, 0},
+         {0, 24, D3DDECLTYPE_FLOAT3, D3DDECLMETHOD_DEFAULT, D3DDECLUSAGE_NORMAL, 0},
+         {0, 36, D3DDECLTYPE_D3DCOLOR, D3DDECLMETHOD_DEFAULT, D3DDECLUSAGE_COLOR, 0},
+         D3DDECL_END()
+    };
+    struct
+    {
+        D3DXVECTOR3 position0;
+        D3DXVECTOR3 position1;
+        D3DXVECTOR3 normal;
+        DWORD color;
+    }
+    vertices[] =
+    {
+        { { 0.0f,  1.0f,  0.f}, { 1.0f,  0.0f,  0.f}, {0.0f, 0.0f, 1.0f}, 0xffff0000 },
+        { { 1.0f, -1.0f,  0.f}, {-1.0f, -1.0f,  0.f}, {0.0f, 0.0f, 1.0f}, 0xff00ff00 },
+        { {-1.0f, -1.0f,  0.f}, {-1.0f,  1.0f,  0.f}, {0.0f, 0.0f, 1.0f}, 0xff0000ff },
+    };
+    unsigned int faces[] = {0, 1, 2};
+    unsigned int attributes[] = {0};
+    unsigned int num_faces = ARRAY_SIZE(faces) / 3;
+    unsigned int num_vertices = ARRAY_SIZE(vertices);
+    int offset = sizeof(D3DXVECTOR3);
+    DWORD options = D3DXMESH_32BIT | D3DXMESH_SYSTEMMEM;
+    void *vertex_buffer;
+    void *index_buffer;
+    DWORD *attributes_buffer;
+    D3DVERTEXELEMENT9 declaration[MAX_FVF_DECL_SIZE];
+    D3DVERTEXELEMENT9 *decl_ptr;
+    DWORD exp_vertex_size = sizeof(*vertices);
+    DWORD vertex_size = 0;
+    int equal;
+    int i = 0;
+    int *decl_mem;
+    int filler_a = 0xaaaaaaaa;
+    int filler_b = 0xbbbbbbbb;
+
+    test_context = new_test_context();
+    if (!test_context)
+    {
+        skip("Couldn't create a test_context\n");
+        goto cleanup;
+    }
+
+    hr = D3DXCreateMesh(num_faces, num_vertices, options, declaration0,
+                        test_context->device, &mesh);
+    if (FAILED(hr))
+    {
+        skip("Couldn't create test mesh %#x\n", hr);
+        goto cleanup;
+    }
+
+    mesh->lpVtbl->LockVertexBuffer(mesh, 0, &vertex_buffer);
+    memcpy(vertex_buffer, vertices, sizeof(vertices));
+    mesh->lpVtbl->UnlockVertexBuffer(mesh);
+
+    mesh->lpVtbl->LockIndexBuffer(mesh, 0, &index_buffer);
+    memcpy(index_buffer, faces, sizeof(faces));
+    mesh->lpVtbl->UnlockIndexBuffer(mesh);
+
+    mesh->lpVtbl->LockAttributeBuffer(mesh, 0, &attributes_buffer);
+    memcpy(attributes_buffer, attributes, sizeof(attributes));
+    mesh->lpVtbl->UnlockAttributeBuffer(mesh);
+
+    /* Get the declaration and try to change it */
+    hr = mesh->lpVtbl->GetDeclaration(mesh, declaration);
+    if (FAILED(hr))
+    {
+        skip("Couldn't get vertex declaration %#x\n", hr);
+        goto cleanup;
+    }
+    equal = memcmp(declaration, declaration0, sizeof(declaration0));
+    ok(equal == 0, "Vertex declarations were not equal\n");
+
+    for (decl_ptr = declaration; decl_ptr->Stream != 0xFF; decl_ptr++)
+    {
+        if (decl_ptr->Usage == D3DDECLUSAGE_POSITION)
+        {
+            /* Use second vertex position instead of first */
+            decl_ptr->Offset = offset;
+        }
+    }
+
+    hr = mesh->lpVtbl->UpdateSemantics(mesh, declaration);
+    todo_wine ok(hr == D3D_OK, "Test UpdateSematics, got %#x expected %#x\n", hr, D3D_OK);
+
+    /* Check that declaration was written by getting it again */
+    memset(declaration, 0, sizeof(declaration));
+    hr = mesh->lpVtbl->GetDeclaration(mesh, declaration);
+    if (FAILED(hr))
+    {
+        skip("Couldn't get vertex declaration %#x\n", hr);
+        goto cleanup;
+    }
+
+    for (decl_ptr = declaration; decl_ptr->Stream != 0xFF; decl_ptr++)
+    {
+        if (decl_ptr->Usage == D3DDECLUSAGE_POSITION)
+        {
+            todo_wine ok(decl_ptr->Offset == offset, "Test UpdateSematics, got offset %d expected %d\n",
+                         decl_ptr->Offset, offset);
+        }
+    }
+
+    /* Check that GetDeclaration only writes up to the D3DDECL_END() marker and
+     * not the full MAX_FVF_DECL_SIZE elements.
+     */
+    memset(declaration, filler_a, sizeof(declaration));
+    memcpy(declaration, declaration0, sizeof(declaration0));
+    hr = mesh->lpVtbl->UpdateSemantics(mesh, declaration);
+    todo_wine ok(hr == D3D_OK, "Test UpdateSematics, "
+       "got %#x expected D3D_OK\n", hr);
+    memset(declaration, filler_b, sizeof(declaration));
+    hr = mesh->lpVtbl->GetDeclaration(mesh, declaration);
+    ok(hr == D3D_OK, "Couldn't get vertex declaration. Got %#x, expected D3D_OK\n", hr);
+    decl_mem = (int*)declaration;
+    for (i = sizeof(declaration0)/sizeof(*decl_mem); i < sizeof(declaration)/sizeof(*decl_mem); i++)
+    {
+        equal = memcmp(&decl_mem[i], &filler_b, sizeof(filler_b));
+        ok(equal == 0,
+           "GetDeclaration wrote past the D3DDECL_END() marker. "
+           "Got %#x, expected  %#x\n", decl_mem[i], filler_b);
+        if (equal != 0) break;
+    }
+
+    /* UpdateSemantics does not check for overlapping fields */
+    memset(declaration, 0, sizeof(declaration));
+    hr = mesh->lpVtbl->GetDeclaration(mesh, declaration);
+    if (FAILED(hr))
+    {
+        skip("Couldn't get vertex declaration %#x\n", hr);
+        goto cleanup;
+    }
+
+    for (decl_ptr = declaration; decl_ptr->Stream != 0xFF; decl_ptr++)
+    {
+        if (decl_ptr->Type == D3DDECLTYPE_FLOAT3)
+        {
+            decl_ptr->Type = D3DDECLTYPE_FLOAT4;
+        }
+    }
+
+    hr = mesh->lpVtbl->UpdateSemantics(mesh, declaration);
+    todo_wine ok(hr == D3D_OK, "Test UpdateSematics for overlapping fields, "
+                 "got %#x expected D3D_OK\n", hr);
+
+    /* Set the position type to color instead of float3 */
+    hr = mesh->lpVtbl->UpdateSemantics(mesh, declaration_pos_type_color);
+    todo_wine ok(hr == D3D_OK, "Test UpdateSematics position type color, "
+                 "got %#x expected D3D_OK\n", hr);
+
+    /* The following test cases show that NULL, smaller or larger declarations,
+     * and declarations with non-zero Stream values are not accepted.
+     * UpdateSemantics returns D3DERR_INVALIDCALL and the previously set
+     * declaration will be used by DrawSubset, GetNumBytesPerVertex, and
+     * GetDeclaration.
+     */
+
+    /* Null declaration (invalid declaration) */
+    mesh->lpVtbl->UpdateSemantics(mesh, declaration0); /* Set a valid declaration */
+    hr = mesh->lpVtbl->UpdateSemantics(mesh, NULL);
+    todo_wine ok(hr == D3DERR_INVALIDCALL, "Test UpdateSematics null pointer declaration, "
+                 "got %#x expected D3DERR_INVALIDCALL\n", hr);
+    vertex_size = mesh->lpVtbl->GetNumBytesPerVertex(mesh);
+    ok(vertex_size == exp_vertex_size, "Got vertex declaration size %u, expected %u\n",
+       vertex_size, exp_vertex_size);
+    memset(declaration, 0, sizeof(declaration));
+    hr = mesh->lpVtbl->GetDeclaration(mesh, declaration);
+    ok(hr == D3D_OK, "Couldn't get vertex declaration. Got %#x, expected D3D_OK\n", hr);
+    equal = memcmp(declaration, declaration0, sizeof(declaration0));
+    ok(equal == 0, "Vertex declarations were not equal\n");
+
+    /* Smaller vertex declaration (invalid declaration) */
+    mesh->lpVtbl->UpdateSemantics(mesh, declaration0); /* Set a valid declaration */
+    hr = mesh->lpVtbl->UpdateSemantics(mesh, declaration_smaller);
+    todo_wine ok(hr == D3DERR_INVALIDCALL, "Test UpdateSematics for smaller vertex declaration, "
+                 "got %#x expected D3DERR_INVALIDCALL\n", hr);
+    vertex_size = mesh->lpVtbl->GetNumBytesPerVertex(mesh);
+    ok(vertex_size == exp_vertex_size, "Got vertex declaration size %u, expected %u\n",
+       vertex_size, exp_vertex_size);
+    memset(declaration, 0, sizeof(declaration));
+    hr = mesh->lpVtbl->GetDeclaration(mesh, declaration);
+    ok(hr == D3D_OK, "Couldn't get vertex declaration. Got %#x, expected D3D_OK\n", hr);
+    equal = memcmp(declaration, declaration0, sizeof(declaration0));
+    ok(equal == 0, "Vertex declarations were not equal\n");
+
+    /* Larger vertex declaration (invalid declaration) */
+    mesh->lpVtbl->UpdateSemantics(mesh, declaration0); /* Set a valid declaration */
+    hr = mesh->lpVtbl->UpdateSemantics(mesh, declaration_larger);
+    todo_wine ok(hr == D3DERR_INVALIDCALL, "Test UpdateSematics for larger vertex declaration, "
+                 "got %#x expected D3DERR_INVALIDCALL\n", hr);
+    vertex_size = mesh->lpVtbl->GetNumBytesPerVertex(mesh);
+    ok(vertex_size == exp_vertex_size, "Got vertex declaration size %u, expected %u\n",
+       vertex_size, exp_vertex_size);
+    memset(declaration, 0, sizeof(declaration));
+    hr = mesh->lpVtbl->GetDeclaration(mesh, declaration);
+    ok(hr == D3D_OK, "Couldn't get vertex declaration. Got %#x, expected D3D_OK\n", hr);
+    equal = memcmp(declaration, declaration0, sizeof(declaration0));
+    ok(equal == 0, "Vertex declarations were not equal\n");
+
+    /* Use multiple streams and keep the same vertex size (invalid declaration) */
+    mesh->lpVtbl->UpdateSemantics(mesh, declaration0); /* Set a valid declaration */
+    hr = mesh->lpVtbl->UpdateSemantics(mesh, declaration_multiple_streams);
+    todo_wine ok(hr == D3DERR_INVALIDCALL, "Test UpdateSematics using multiple streams, "
+                 "got %#x expected D3DERR_INVALIDCALL\n", hr);
+    vertex_size = mesh->lpVtbl->GetNumBytesPerVertex(mesh);
+    ok(vertex_size == exp_vertex_size, "Got vertex declaration size %u, expected %u\n",
+       vertex_size, exp_vertex_size);
+    memset(declaration, 0, sizeof(declaration));
+    hr = mesh->lpVtbl->GetDeclaration(mesh, declaration);
+    ok(hr == D3D_OK, "Couldn't get vertex declaration. Got %#x, expected D3D_OK\n", hr);
+    equal = memcmp(declaration, declaration0, sizeof(declaration0));
+    ok(equal == 0, "Vertex declarations were not equal\n");
+
+    /* The next following test cases show that some invalid declarations are
+     * accepted with a D3D_OK. An access violation is thrown on Windows if
+     * DrawSubset is called. The methods GetNumBytesPerVertex and GetDeclaration
+     * are not affected, which indicates that the declaration is cached.
+     */
+
+    /* Double usage (invalid declaration) */
+    mesh->lpVtbl->UpdateSemantics(mesh, declaration0); /* Set a valid declaration */
+    hr = mesh->lpVtbl->UpdateSemantics(mesh, declaration_double_usage);
+    todo_wine ok(hr == D3D_OK, "Test UpdateSematics double usage, "
+                 "got %#x expected D3D_OK\n", hr);
+    vertex_size = mesh->lpVtbl->GetNumBytesPerVertex(mesh);
+    ok(vertex_size == exp_vertex_size, "Got vertex declaration size %u, expected %u\n",
+       vertex_size, exp_vertex_size);
+    memset(declaration, 0, sizeof(declaration));
+    hr = mesh->lpVtbl->GetDeclaration(mesh, declaration);
+    ok(hr == D3D_OK, "Couldn't get vertex declaration. Got %#x, expected D3D_OK\n", hr);
+    equal = memcmp(declaration, declaration_double_usage, sizeof(declaration_double_usage));
+    todo_wine ok(equal == 0, "Vertex declarations were not equal\n");
+
+    /* Set the position to an undefined type (invalid declaration) */
+    mesh->lpVtbl->UpdateSemantics(mesh, declaration0); /* Set a valid declaration */
+    hr = mesh->lpVtbl->UpdateSemantics(mesh, declaration_undefined_type);
+    todo_wine ok(hr == D3D_OK, "Test UpdateSematics undefined type, "
+                 "got %#x expected D3D_OK\n", hr);
+    vertex_size = mesh->lpVtbl->GetNumBytesPerVertex(mesh);
+    ok(vertex_size == exp_vertex_size, "Got vertex declaration size %u, expected %u\n",
+       vertex_size, exp_vertex_size);
+    memset(declaration, 0, sizeof(declaration));
+    hr = mesh->lpVtbl->GetDeclaration(mesh, declaration);
+    ok(hr == D3D_OK, "Couldn't get vertex declaration. Got %#x, expected D3D_OK\n", hr);
+    equal = memcmp(declaration, declaration_undefined_type, sizeof(declaration_undefined_type));
+    todo_wine ok(equal == 0, "Vertex declarations were not equal\n");
+
+    /* Use a not 4 byte aligned offset (invalid declaration) */
+    mesh->lpVtbl->UpdateSemantics(mesh, declaration0); /* Set a valid declaration */
+    hr = mesh->lpVtbl->UpdateSemantics(mesh, declaration_not_4_byte_aligned_offset);
+    todo_wine ok(hr == D3D_OK, "Test UpdateSematics not 4 byte aligned offset, "
+       "got %#x expected D3D_OK\n", hr);
+    vertex_size = mesh->lpVtbl->GetNumBytesPerVertex(mesh);
+    ok(vertex_size == exp_vertex_size, "Got vertex declaration size %u, expected %u\n",
+       vertex_size, exp_vertex_size);
+    memset(declaration, 0, sizeof(declaration));
+    hr = mesh->lpVtbl->GetDeclaration(mesh, declaration);
+    ok(hr == D3D_OK, "Couldn't get vertex declaration. Got %#x, expected D3D_OK\n", hr);
+    equal = memcmp(declaration, declaration_not_4_byte_aligned_offset,
+                   sizeof(declaration_not_4_byte_aligned_offset));
+    todo_wine ok(equal == 0, "Vertex declarations were not equal\n");
+
+cleanup:
+    if (mesh)
+        mesh->lpVtbl->Release(mesh);
+
+    free_test_context(test_context);
+}
+
 START_TEST(mesh)
 {
     D3DXBoundProbeTest();
@@ -4277,4 +4694,5 @@ START_TEST(mesh)
     test_get_decl_vertex_size();
     test_fvf_decl_conversion();
     D3DXGenerateAdjacencyTest();
+    test_update_semantics();
 }
-- 
1.7.5.4

From 04c8edde5a140f97852741932890690b51d1ceb8 Mon Sep 17 00:00:00 2001
From: Michael Mc Donnell <mich...@mcdonnell.dk>
Date: Tue, 24 May 2011 19:44:48 +0200
Subject: d3dx9: Implement UpdateSemantics mesh method.

Added a chached vertex declaration and a declaration size field.
Changed copy method to not write past D3DECL_END() marker.
Handle non-zero stream values.
---
 dlls/d3dx9_36/mesh.c       |   84 ++++++++++++++++++++++++++++++++++++-------
 dlls/d3dx9_36/tests/mesh.c |   46 ++++++++++++------------
 2 files changed, 93 insertions(+), 37 deletions(-)

diff --git a/dlls/d3dx9_36/mesh.c b/dlls/d3dx9_36/mesh.c
index cfadb75..dca92c1 100644
--- a/dlls/d3dx9_36/mesh.c
+++ b/dlls/d3dx9_36/mesh.c
@@ -6,6 +6,7 @@
  * Copyright (C) 2009 David Adam
  * Copyright (C) 2010 Tony Wasserka
  * Copyright (C) 2011 Dylan Smith
+ * Copyright (C) 2011 Michael Mc Donnell
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -52,7 +53,10 @@ typedef struct ID3DXMeshImpl
     DWORD options;
     DWORD fvf;
     IDirect3DDevice9 *device;
+    D3DVERTEXELEMENT9 cached_declaration[MAX_FVF_DECL_SIZE];
     IDirect3DVertexDeclaration9 *vertex_declaration;
+    UINT vertex_declaration_size;
+    UINT num_elem;
     IDirect3DVertexBuffer9 *vertex_buffer;
     IDirect3DIndexBuffer9 *index_buffer;
     DWORD *attrib_buffer;
@@ -106,7 +110,8 @@ static ULONG WINAPI ID3DXMeshImpl_Release(ID3DXMesh *iface)
     {
         IDirect3DIndexBuffer9_Release(This->index_buffer);
         IDirect3DVertexBuffer9_Release(This->vertex_buffer);
-        IDirect3DVertexDeclaration9_Release(This->vertex_declaration);
+        if (This->vertex_declaration)
+            IDirect3DVertexDeclaration9_Release(This->vertex_declaration);
         IDirect3DDevice9_Release(This->device);
         HeapFree(GetProcessHeap(), 0, This->attrib_buffer);
         HeapFree(GetProcessHeap(), 0, This->attrib_table);
@@ -127,6 +132,12 @@ static HRESULT WINAPI ID3DXMeshImpl_DrawSubset(ID3DXMesh *iface, DWORD attrib_id
 
     TRACE("(%p)->(%u)\n", This, attrib_id);
 
+    if (!This->vertex_declaration)
+    {
+        WARN("Can't draw a mesh with an invalid vertex declaration.\n");
+        return E_FAIL;
+    }
+
     vertex_size = iface->lpVtbl->GetNumBytesPerVertex(iface);
 
     hr = IDirect3DDevice9_SetVertexDeclaration(This->device, This->vertex_declaration);
@@ -186,32 +197,31 @@ static DWORD WINAPI ID3DXMeshImpl_GetFVF(ID3DXMesh *iface)
     return This->fvf;
 }
 
+static void copy_declaration(D3DVERTEXELEMENT9 *dst, const D3DVERTEXELEMENT9 *src, UINT num_elem)
+{
+    memcpy(dst, src, num_elem * sizeof(*src));
+}
+
 static HRESULT WINAPI ID3DXMeshImpl_GetDeclaration(ID3DXMesh *iface, D3DVERTEXELEMENT9 declaration[MAX_FVF_DECL_SIZE])
 {
     ID3DXMeshImpl *This = impl_from_ID3DXMesh(iface);
-    UINT numelements;
 
     TRACE("(%p)\n", This);
 
     if (declaration == NULL) return D3DERR_INVALIDCALL;
 
-    return IDirect3DVertexDeclaration9_GetDeclaration(This->vertex_declaration,
-                                                      declaration,
-                                                      &numelements);
+    copy_declaration(declaration, This->cached_declaration, This->num_elem);
+
+    return D3D_OK;
 }
 
 static DWORD WINAPI ID3DXMeshImpl_GetNumBytesPerVertex(ID3DXMesh *iface)
 {
     ID3DXMeshImpl *This = impl_from_ID3DXMesh(iface);
-    UINT numelements;
-    D3DVERTEXELEMENT9 declaration[MAX_FVF_DECL_SIZE] = { D3DDECL_END() };
 
     TRACE("iface (%p)\n", This);
 
-    IDirect3DVertexDeclaration9_GetDeclaration(This->vertex_declaration,
-                                               declaration,
-                                               &numelements);
-    return D3DXGetDeclVertexSize(declaration, 0);
+    return This->vertex_declaration_size;
 }
 
 static DWORD WINAPI ID3DXMeshImpl_GetOptions(ID3DXMesh *iface)
@@ -603,11 +613,50 @@ cleanup:
 
 static HRESULT WINAPI ID3DXMeshImpl_UpdateSemantics(ID3DXMesh *iface, D3DVERTEXELEMENT9 declaration[MAX_FVF_DECL_SIZE])
 {
+    HRESULT hr;
     ID3DXMeshImpl *This = impl_from_ID3DXMesh(iface);
+    UINT vertex_declaration_size;
+    int i;
 
-    FIXME("(%p)->(%p): stub\n", This, declaration);
+    TRACE("(%p)->(%p)\n", This, declaration);
 
-    return E_NOTIMPL;
+    if (!declaration)
+        return D3DERR_INVALIDCALL;
+
+    /* New declaration must be same size as original */
+    vertex_declaration_size = D3DXGetDeclVertexSize(declaration, declaration[0].Stream);
+    if (vertex_declaration_size != This->vertex_declaration_size)
+        return D3DERR_INVALIDCALL;
+
+    /* New declaration must not contain non-zero Stream value  */
+    for (i = 0; declaration[i].Stream != 0xff; i++)
+        if (declaration[i].Stream != 0)
+            return D3DERR_INVALIDCALL;
+
+    This->num_elem = i + 1;
+    copy_declaration(This->cached_declaration, declaration, This->num_elem);
+
+    if (This->vertex_declaration)
+        IDirect3DVertexDeclaration9_Release(This->vertex_declaration);
+
+    /* An application can pass an invalid declaration to UpdateSemantics and
+     * still expect D3D_OK (see tests). If the declaration is invalid, then
+     * subsequent calls to DrawSubset will fail. This is handled by setting the
+     * vertex declaration to NULL.
+     *     GetDeclaration, GetNumBytesPerVertex must, however, use the new
+     * invalid declaration. This is handled by them using the cached vertex
+     * declaration instead of the actual vertex declaration.
+     */
+    hr = IDirect3DDevice9_CreateVertexDeclaration(This->device,
+                                                  declaration,
+                                                  &This->vertex_declaration);
+    if (FAILED(hr))
+    {
+        WARN("Invalid declaration passed to UpdateSemantics.\n");
+        This->vertex_declaration = NULL;
+    }
+
+    return D3D_OK;
 }
 
 /*** ID3DXMesh ***/
@@ -1652,6 +1701,8 @@ HRESULT WINAPI D3DXCreateMesh(DWORD numfaces, DWORD numvertices, DWORD options,
     HRESULT hr;
     DWORD fvf;
     IDirect3DVertexDeclaration9 *vertex_declaration;
+    UINT vertex_declaration_size;
+    UINT num_elem;
     IDirect3DVertexBuffer9 *vertex_buffer;
     IDirect3DIndexBuffer9 *index_buffer;
     DWORD *attrib_buffer;
@@ -1674,6 +1725,7 @@ HRESULT WINAPI D3DXCreateMesh(DWORD numfaces, DWORD numvertices, DWORD options,
     for (i = 0; declaration[i].Stream != 0xff; i++)
         if (declaration[i].Stream != 0)
             return D3DERR_INVALIDCALL;
+    num_elem = i + 1;
 
     if (options & D3DXMESH_32BIT)
         index_format = D3DFMT_INDEX32;
@@ -1734,10 +1786,11 @@ HRESULT WINAPI D3DXCreateMesh(DWORD numfaces, DWORD numvertices, DWORD options,
         WARN("Unexpected return value %x from IDirect3DDevice9_CreateVertexDeclaration.\n",hr);
         return hr;
     }
+    vertex_declaration_size = D3DXGetDeclVertexSize(declaration, declaration[0].Stream);
 
     /* Create vertex buffer */
     hr = IDirect3DDevice9_CreateVertexBuffer(device,
-                                             numvertices * D3DXGetDeclVertexSize(declaration, declaration[0].Stream),
+                                             numvertices * vertex_declaration_size,
                                              vertex_usage,
                                              fvf,
                                              vertex_pool,
@@ -1787,7 +1840,10 @@ HRESULT WINAPI D3DXCreateMesh(DWORD numfaces, DWORD numvertices, DWORD options,
     object->device = device;
     IDirect3DDevice9_AddRef(device);
 
+    copy_declaration(object->cached_declaration, declaration, num_elem);
     object->vertex_declaration = vertex_declaration;
+    object->vertex_declaration_size = vertex_declaration_size;
+    object->num_elem = num_elem;
     object->vertex_buffer = vertex_buffer;
     object->index_buffer = index_buffer;
     object->attrib_buffer = attrib_buffer;
diff --git a/dlls/d3dx9_36/tests/mesh.c b/dlls/d3dx9_36/tests/mesh.c
index f4b1bbf..2fe9692 100644
--- a/dlls/d3dx9_36/tests/mesh.c
+++ b/dlls/d3dx9_36/tests/mesh.c
@@ -4490,7 +4490,7 @@ static void test_update_semantics(void)
     }
 
     hr = mesh->lpVtbl->UpdateSemantics(mesh, declaration);
-    todo_wine ok(hr == D3D_OK, "Test UpdateSematics, got %#x expected %#x\n", hr, D3D_OK);
+    ok(hr == D3D_OK, "Test UpdateSematics, got %#x expected %#x\n", hr, D3D_OK);
 
     /* Check that declaration was written by getting it again */
     memset(declaration, 0, sizeof(declaration));
@@ -4505,8 +4505,8 @@ static void test_update_semantics(void)
     {
         if (decl_ptr->Usage == D3DDECLUSAGE_POSITION)
         {
-            todo_wine ok(decl_ptr->Offset == offset, "Test UpdateSematics, got offset %d expected %d\n",
-                         decl_ptr->Offset, offset);
+            ok(decl_ptr->Offset == offset, "Test UpdateSematics, got offset %d expected %d\n",
+               decl_ptr->Offset, offset);
         }
     }
 
@@ -4516,7 +4516,7 @@ static void test_update_semantics(void)
     memset(declaration, filler_a, sizeof(declaration));
     memcpy(declaration, declaration0, sizeof(declaration0));
     hr = mesh->lpVtbl->UpdateSemantics(mesh, declaration);
-    todo_wine ok(hr == D3D_OK, "Test UpdateSematics, "
+    ok(hr == D3D_OK, "Test UpdateSematics, "
        "got %#x expected D3D_OK\n", hr);
     memset(declaration, filler_b, sizeof(declaration));
     hr = mesh->lpVtbl->GetDeclaration(mesh, declaration);
@@ -4549,13 +4549,13 @@ static void test_update_semantics(void)
     }
 
     hr = mesh->lpVtbl->UpdateSemantics(mesh, declaration);
-    todo_wine ok(hr == D3D_OK, "Test UpdateSematics for overlapping fields, "
-                 "got %#x expected D3D_OK\n", hr);
+    ok(hr == D3D_OK, "Test UpdateSematics for overlapping fields, "
+       "got %#x expected D3D_OK\n", hr);
 
     /* Set the position type to color instead of float3 */
     hr = mesh->lpVtbl->UpdateSemantics(mesh, declaration_pos_type_color);
-    todo_wine ok(hr == D3D_OK, "Test UpdateSematics position type color, "
-                 "got %#x expected D3D_OK\n", hr);
+    ok(hr == D3D_OK, "Test UpdateSematics position type color, "
+       "got %#x expected D3D_OK\n", hr);
 
     /* The following test cases show that NULL, smaller or larger declarations,
      * and declarations with non-zero Stream values are not accepted.
@@ -4567,8 +4567,8 @@ static void test_update_semantics(void)
     /* Null declaration (invalid declaration) */
     mesh->lpVtbl->UpdateSemantics(mesh, declaration0); /* Set a valid declaration */
     hr = mesh->lpVtbl->UpdateSemantics(mesh, NULL);
-    todo_wine ok(hr == D3DERR_INVALIDCALL, "Test UpdateSematics null pointer declaration, "
-                 "got %#x expected D3DERR_INVALIDCALL\n", hr);
+    ok(hr == D3DERR_INVALIDCALL, "Test UpdateSematics null pointer declaration, "
+       "got %#x expected D3DERR_INVALIDCALL\n", hr);
     vertex_size = mesh->lpVtbl->GetNumBytesPerVertex(mesh);
     ok(vertex_size == exp_vertex_size, "Got vertex declaration size %u, expected %u\n",
        vertex_size, exp_vertex_size);
@@ -4581,8 +4581,8 @@ static void test_update_semantics(void)
     /* Smaller vertex declaration (invalid declaration) */
     mesh->lpVtbl->UpdateSemantics(mesh, declaration0); /* Set a valid declaration */
     hr = mesh->lpVtbl->UpdateSemantics(mesh, declaration_smaller);
-    todo_wine ok(hr == D3DERR_INVALIDCALL, "Test UpdateSematics for smaller vertex declaration, "
-                 "got %#x expected D3DERR_INVALIDCALL\n", hr);
+    ok(hr == D3DERR_INVALIDCALL, "Test UpdateSematics for smaller vertex declaration, "
+       "got %#x expected D3DERR_INVALIDCALL\n", hr);
     vertex_size = mesh->lpVtbl->GetNumBytesPerVertex(mesh);
     ok(vertex_size == exp_vertex_size, "Got vertex declaration size %u, expected %u\n",
        vertex_size, exp_vertex_size);
@@ -4595,8 +4595,8 @@ static void test_update_semantics(void)
     /* Larger vertex declaration (invalid declaration) */
     mesh->lpVtbl->UpdateSemantics(mesh, declaration0); /* Set a valid declaration */
     hr = mesh->lpVtbl->UpdateSemantics(mesh, declaration_larger);
-    todo_wine ok(hr == D3DERR_INVALIDCALL, "Test UpdateSematics for larger vertex declaration, "
-                 "got %#x expected D3DERR_INVALIDCALL\n", hr);
+    ok(hr == D3DERR_INVALIDCALL, "Test UpdateSematics for larger vertex declaration, "
+       "got %#x expected D3DERR_INVALIDCALL\n", hr);
     vertex_size = mesh->lpVtbl->GetNumBytesPerVertex(mesh);
     ok(vertex_size == exp_vertex_size, "Got vertex declaration size %u, expected %u\n",
        vertex_size, exp_vertex_size);
@@ -4609,7 +4609,7 @@ static void test_update_semantics(void)
     /* Use multiple streams and keep the same vertex size (invalid declaration) */
     mesh->lpVtbl->UpdateSemantics(mesh, declaration0); /* Set a valid declaration */
     hr = mesh->lpVtbl->UpdateSemantics(mesh, declaration_multiple_streams);
-    todo_wine ok(hr == D3DERR_INVALIDCALL, "Test UpdateSematics using multiple streams, "
+    ok(hr == D3DERR_INVALIDCALL, "Test UpdateSematics using multiple streams, "
                  "got %#x expected D3DERR_INVALIDCALL\n", hr);
     vertex_size = mesh->lpVtbl->GetNumBytesPerVertex(mesh);
     ok(vertex_size == exp_vertex_size, "Got vertex declaration size %u, expected %u\n",
@@ -4629,8 +4629,8 @@ static void test_update_semantics(void)
     /* Double usage (invalid declaration) */
     mesh->lpVtbl->UpdateSemantics(mesh, declaration0); /* Set a valid declaration */
     hr = mesh->lpVtbl->UpdateSemantics(mesh, declaration_double_usage);
-    todo_wine ok(hr == D3D_OK, "Test UpdateSematics double usage, "
-                 "got %#x expected D3D_OK\n", hr);
+    ok(hr == D3D_OK, "Test UpdateSematics double usage, "
+       "got %#x expected D3D_OK\n", hr);
     vertex_size = mesh->lpVtbl->GetNumBytesPerVertex(mesh);
     ok(vertex_size == exp_vertex_size, "Got vertex declaration size %u, expected %u\n",
        vertex_size, exp_vertex_size);
@@ -4638,13 +4638,13 @@ static void test_update_semantics(void)
     hr = mesh->lpVtbl->GetDeclaration(mesh, declaration);
     ok(hr == D3D_OK, "Couldn't get vertex declaration. Got %#x, expected D3D_OK\n", hr);
     equal = memcmp(declaration, declaration_double_usage, sizeof(declaration_double_usage));
-    todo_wine ok(equal == 0, "Vertex declarations were not equal\n");
+    ok(equal == 0, "Vertex declarations were not equal\n");
 
     /* Set the position to an undefined type (invalid declaration) */
     mesh->lpVtbl->UpdateSemantics(mesh, declaration0); /* Set a valid declaration */
     hr = mesh->lpVtbl->UpdateSemantics(mesh, declaration_undefined_type);
-    todo_wine ok(hr == D3D_OK, "Test UpdateSematics undefined type, "
-                 "got %#x expected D3D_OK\n", hr);
+    ok(hr == D3D_OK, "Test UpdateSematics undefined type, "
+       "got %#x expected D3D_OK\n", hr);
     vertex_size = mesh->lpVtbl->GetNumBytesPerVertex(mesh);
     ok(vertex_size == exp_vertex_size, "Got vertex declaration size %u, expected %u\n",
        vertex_size, exp_vertex_size);
@@ -4652,12 +4652,12 @@ static void test_update_semantics(void)
     hr = mesh->lpVtbl->GetDeclaration(mesh, declaration);
     ok(hr == D3D_OK, "Couldn't get vertex declaration. Got %#x, expected D3D_OK\n", hr);
     equal = memcmp(declaration, declaration_undefined_type, sizeof(declaration_undefined_type));
-    todo_wine ok(equal == 0, "Vertex declarations were not equal\n");
+    ok(equal == 0, "Vertex declarations were not equal\n");
 
     /* Use a not 4 byte aligned offset (invalid declaration) */
     mesh->lpVtbl->UpdateSemantics(mesh, declaration0); /* Set a valid declaration */
     hr = mesh->lpVtbl->UpdateSemantics(mesh, declaration_not_4_byte_aligned_offset);
-    todo_wine ok(hr == D3D_OK, "Test UpdateSematics not 4 byte aligned offset, "
+    ok(hr == D3D_OK, "Test UpdateSematics not 4 byte aligned offset, "
        "got %#x expected D3D_OK\n", hr);
     vertex_size = mesh->lpVtbl->GetNumBytesPerVertex(mesh);
     ok(vertex_size == exp_vertex_size, "Got vertex declaration size %u, expected %u\n",
@@ -4667,7 +4667,7 @@ static void test_update_semantics(void)
     ok(hr == D3D_OK, "Couldn't get vertex declaration. Got %#x, expected D3D_OK\n", hr);
     equal = memcmp(declaration, declaration_not_4_byte_aligned_offset,
                    sizeof(declaration_not_4_byte_aligned_offset));
-    todo_wine ok(equal == 0, "Vertex declarations were not equal\n");
+    ok(equal == 0, "Vertex declarations were not equal\n");
 
 cleanup:
     if (mesh)
-- 
1.7.5.4



Reply via email to