On Wed, Jun 8, 2011 at 12:59 PM, Henri Verbeet <hverb...@gmail.com> wrote:
> On 8 June 2011 11:33, Michael Mc Donnell <mich...@mcdonnell.dk> wrote:
>> Henri, do you have any comments about my UpdateSemantics patches
>> before I send them off to wine-patches?
>>
> Looks fine to me.
Good :-)
> On the subject of style though, two things:
> - I really dislike things like LPDWORD and LPD3DXMESH. For one,
> there's the issue that "const LPDWORD" doesn't do what you want, so
> you'd need something like "LPCDWORD", which doesn't exist. The other
> issue is that unless typedefs are used for some kind of abstraction,
> like e.g. uint32_t, they're just obfuscation. I.e., "typedef type
> *LPTYPE;" just hides that "LPTYPE" is a pointer, and "typedef
> enum/struct x x;" just hides that "x" is a struct or enum.
Thank you for the explanation. I've changed those.
> - "There’s a standard naming scheme for C, and it’s all lower case
> with underscores as separators."
Ah yes, I've been writing too much C++ and C# lately :-) I've changed that too.
I've also moved the declaration of a vertex struct inside the test
function, as it was not needed outside of the test function.
Thank you for your comments.
diff --git a/dlls/d3dx9_36/tests/mesh.c b/dlls/d3dx9_36/tests/mesh.c
index cedef32..ee62d18 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
@@ -61,6 +62,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;
@@ -3305,6 +3393,200 @@ 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;
+ 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_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 = sizeof(faces) / 3;
+ unsigned int num_vertices = ARRAY_SIZE(vertices);
+ int offset = 12;
+ DWORD options = D3DXMESH_32BIT | D3DXMESH_SYSTEMMEM;
+ void *vertex_buffer;
+ void *index_buffer;
+ DWORD *attributes_buffer;
+ D3DVERTEXELEMENT9 declaration[MAX_FVF_DECL_SIZE];
+ D3DVERTEXELEMENT9 *decl_ptr;
+
+ test_context = new_test_context();
+ if (!test_context)
+ {
+ skip("Couldn't create a test_context");
+ 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 mesh declaration %#x\n", hr);
+ goto cleanup;
+ }
+
+ 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 mesh 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);
+ }
+ }
+
+ /* UpdateSemantics does not check for a bigger vertex size */
+ memset(declaration, 0, sizeof(declaration));
+ hr = mesh->lpVtbl->GetDeclaration(mesh, declaration);
+ if (FAILED(hr))
+ {
+ skip("Couldn't get mesh 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 bigger vertex size, "
+ "got %#x expected D3D_OK\n", hr);
+
+ /* Null pointer 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);
+
+ /* Two null pointers. Setting only the mesh to null will result in an
+ * exception on Windows.
+ */
+ hr = mesh->lpVtbl->UpdateSemantics(NULL, NULL);
+ todo_wine ok(hr == D3DERR_INVALIDCALL, "Test UpdateSematics null pointers, "
+ "got %#x expected D3DERR_INVALIDCALL\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 invalid declarations are accepted
+ * with a D3D_OK. An access violations occurs however, if any of the
+ * functions that use the declaration is called, i.e. GetDeclaration and
+ * DrawSubset.
+ */
+
+ /* Double usage (invalid 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);
+
+ /* Set the position to an undefined type (invalid 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);
+
+ /* Use a not 4 byte aligned offset (invalid 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);
+
+cleanup:
+ if (mesh)
+ mesh->lpVtbl->Release(mesh);
+
+ free_test_context(test_context);
+}
+
START_TEST(mesh)
{
D3DXBoundProbeTest();
@@ -3322,4 +3604,5 @@ START_TEST(mesh)
test_get_decl_vertex_size();
test_fvf_decl_conversion();
D3DXGenerateAdjacencyTest();
+ test_update_semantics();
}
diff --git a/dlls/d3dx9_36/mesh.c b/dlls/d3dx9_36/mesh.c
index 48ae20e..7cfb8d4 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
@@ -105,7 +106,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);
@@ -126,6 +128,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);
@@ -193,6 +201,11 @@ static HRESULT WINAPI ID3DXMeshImpl_GetDeclaration(ID3DXMesh *iface, D3DVERTEXEL
TRACE("(%p)\n", This);
if (declaration == NULL) return D3DERR_INVALIDCALL;
+ if (!This->vertex_declaration)
+ {
+ WARN("Can't get vertex declaration because it's invalid.\n");
+ return E_FAIL;
+ }
return IDirect3DVertexDeclaration9_GetDeclaration(This->vertex_declaration,
declaration,
@@ -207,6 +220,12 @@ static DWORD WINAPI ID3DXMeshImpl_GetNumBytesPerVertex(ID3DXMesh *iface)
TRACE("iface (%p)\n", This);
+ if (!This->vertex_declaration)
+ {
+ WARN("Can't get number of bytes per vertex of an invalid vertex declaration.\n");
+ return E_FAIL;
+ }
+
IDirect3DVertexDeclaration9_GetDeclaration(This->vertex_declaration,
declaration,
&numelements);
@@ -602,11 +621,31 @@ cleanup:
static HRESULT WINAPI ID3DXMeshImpl_UpdateSemantics(ID3DXMesh *iface, D3DVERTEXELEMENT9 declaration[MAX_FVF_DECL_SIZE])
{
+ HRESULT hr;
ID3DXMeshImpl *This = impl_from_ID3DXMesh(iface);
+ if (!declaration)
+ return D3DERR_INVALIDCALL;
- FIXME("(%p)->(%p): stub\n", This, declaration);
+ TRACE("(%p)->(%p)\n", This, declaration);
- return E_NOTIMPL;
+ 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 GetDeclaration, GetNumBytesPerVertex, and DrawSubset,
+ * which use the declaration, will fail.
+ */
+ 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 ***/
diff --git a/dlls/d3dx9_36/tests/mesh.c b/dlls/d3dx9_36/tests/mesh.c
index ee62d18..3602357 100644
--- a/dlls/d3dx9_36/tests/mesh.c
+++ b/dlls/d3dx9_36/tests/mesh.c
@@ -3501,7 +3501,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));
@@ -3516,8 +3516,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);
}
}
@@ -3539,25 +3539,25 @@ static void test_update_semantics(void)
}
hr = mesh->lpVtbl->UpdateSemantics(mesh, declaration);
- todo_wine ok(hr == D3D_OK, "Test UpdateSematics for bigger vertex size, "
- "got %#x expected D3D_OK\n", hr);
+ ok(hr == D3D_OK, "Test UpdateSematics for bigger vertex size, "
+ "got %#x expected D3D_OK\n", hr);
/* Null pointer 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);
/* Two null pointers. Setting only the mesh to null will result in an
* exception on Windows.
*/
hr = mesh->lpVtbl->UpdateSemantics(NULL, NULL);
- todo_wine ok(hr == D3DERR_INVALIDCALL, "Test UpdateSematics null pointers, "
- "got %#x expected D3DERR_INVALIDCALL\n", hr);
+ ok(hr == D3DERR_INVALIDCALL, "Test UpdateSematics null pointers, "
+ "got %#x expected D3DERR_INVALIDCALL\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 invalid declarations are accepted
* with a D3D_OK. An access violations occurs however, if any of the
@@ -3567,18 +3567,18 @@ static void test_update_semantics(void)
/* Double usage (invalid 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);
/* Set the position to an undefined type (invalid 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);
/* Use a not 4 byte aligned offset (invalid 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);
+ ok(hr == D3D_OK, "Test UpdateSematics not 4 byte aligned offset, "
+ "got %#x expected D3D_OK\n", hr);
cleanup:
if (mesh)
