On Wed, May 25, 2011 at 9:53 AM, Michael Mc Donnell
<mich...@mcdonnell.dk> wrote:
> On Tue, May 24, 2011 at 8:32 PM, Stefan Dösinger <stefandoesin...@gmx.at>
> wrote:
>> On Tuesday 24 May 2011 19:56:06 Michael Mc Donnell wrote:
>>> > *) In your first test you forgot to check the HeapAlloc result.
>>>
>>> Ok, I'll return E_OUTOFMEMORY in that case.
>> I guess the callers will only abort the tests if NewTestContext fails, so I
>> think the NULL / non-NULL return value was better.
I've reverted those changes, so it's back to NULL / non-NULL. I've
also moved it near the top of the file, so that other tests will be
able to re-use the test context creation.
>>> Should I try to make some more invalid
>>> D3DVERTEXELEMENT9 arrays to see if I can provoke an error?
>> I was thinking about something that would make CreateVertexDeclaration fail,
>> e.g. declaring a usage+usage index twice or using an undefined type.
>> dlls/d3d9/vertexdeclaration.c and dlls/wined3d/vertexdeclaration.c check for
>> a
>> few error conditions in their vertexdeclaration_init functions. Just pick one
>> of them(no need to check all of them)
>> But watch out that you don't open a can of worms here. Our
>> CreateVertexDeclaration probably doesn't catch all error conditions. I
>> recommend that you pick one it does catch, otherwise you'll have to fix
>> d3d9.dll and wined3d.dll too for a small test.
>
> Ok thanks I'll do that.
I've added some more tests to see if I could make it fail. Microsoft's
UpdateSemantics is not very picky. I can't get it to return anything
but D3D_OK except for when I pass a null pointer. My implementation
follows this behavior except for two cases: it returns E_FAIL when
passing an undefined type and when the offset is not 4 byte aligned.
This is because the values are checked inside vertexdeclaration_init.
So Wine is stricter. I don't think that it is necessarily a problem
because applications that pass bogus declarations like those will
likely not work anyway. My own tests with a small interactive demo
show that in those cases the application will crash with an access
violation when it tries to re-draw the scene.
diff --git a/dlls/d3dx9_36/tests/mesh.c b/dlls/d3dx9_36/tests/mesh.c
index cedef32..0ca4b25 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 TestContext
+{
+ HWND hwnd;
+ IDirect3D9 *d3d;
+ IDirect3DDevice9 *device;
+};
+
+/*
+ * Initializes a TestContext. Use this to initialize DirectX.
+ *
+ * Returns NULL if an error occured.
+ */
+static struct TestContext* NewTestContext(void)
+{
+ HRESULT hr;
+ HWND hwnd = NULL;
+ IDirect3D9 *d3d = NULL;
+ IDirect3DDevice9 *device = NULL;
+ D3DPRESENT_PARAMETERS d3dpp = {0};
+ struct TestContext *testContext;
+
+ 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;
+ }
+
+ testContext = HeapAlloc(GetProcessHeap(), 0, sizeof(struct TestContext));
+ if (!testContext)
+ {
+ skip("Couldn't allocate memory for TestContext\n");
+ goto error;
+ }
+ testContext->hwnd = hwnd;
+ testContext->d3d = d3d;
+ testContext->device = device;
+
+ return testContext;
+
+error:
+ if (device)
+ IDirect3DDevice9_Release(device);
+
+ if (d3d)
+ IDirect3D9_Release(d3d);
+
+ if (hwnd)
+ DestroyWindow(hwnd);
+
+ return NULL;
+}
+
+static void FreeTestContext(struct TestContext *testContext)
+{
+ if (!testContext)
+ return;
+
+ if (testContext->device)
+ IDirect3DDevice9_Release(testContext->device);
+
+ if (testContext->d3d)
+ IDirect3D9_Release(testContext->d3d);
+
+ if (testContext->hwnd)
+ DestroyWindow(testContext->hwnd);
+
+ HeapFree(GetProcessHeap(), 0, testContext);
+}
+
struct mesh
{
DWORD number_of_vertices;
@@ -3305,6 +3393,200 @@ static void D3DXGenerateAdjacencyTest(void)
if (d3dxmesh) d3dxmesh->lpVtbl->Release(d3dxmesh);
}
+struct VertexTwoPositions
+{
+ float x0, y0, z0; /* Position 0 */
+ float x1, y1, z1; /* Position 1 */
+ float nx, ny, nz; /* Normal */
+ DWORD color;
+};
+
+static void UpdateSemanticsTest(void)
+{
+ HRESULT hr;
+ struct TestContext *tc = NULL;
+ LPD3DXMESH 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_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_wrong_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_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()
+ };
+ /* x0, y0, z0, x1, y1, z1 ,nx, ny, nz, color */
+ struct VertexTwoPositions 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 numFaces = 1;
+ unsigned int numVertices = 3;
+ int offset = 12;
+ DWORD options = D3DXMESH_32BIT | D3DXMESH_SYSTEMMEM;
+ LPVOID pVertices;
+ LPVOID pFaces;
+ LPDWORD pAttributes;
+ D3DVERTEXELEMENT9 declaration[MAX_FVF_DECL_SIZE];
+ D3DVERTEXELEMENT9 *pDecl;
+
+ tc = NewTestContext();
+ if (!tc)
+ {
+ skip("Couldn't create a TestContext");
+ goto cleanup;
+ }
+
+ /* Create the test mesh */
+ hr = D3DXCreateMesh(numFaces, numVertices, options, declaration0, tc->device, &mesh);
+ if (FAILED(hr))
+ {
+ skip("Couldn't create test mesh %#x\n", hr);
+ goto cleanup;
+ }
+
+ mesh->lpVtbl->LockVertexBuffer(mesh, 0, &pVertices);
+ memcpy(pVertices, vertices, sizeof(vertices));
+ mesh->lpVtbl->UnlockVertexBuffer(mesh);
+
+ mesh->lpVtbl->LockIndexBuffer(mesh, 0, &pFaces);
+ memcpy(pFaces, faces, sizeof(faces));
+ mesh->lpVtbl->UnlockIndexBuffer(mesh);
+
+ mesh->lpVtbl->LockAttributeBuffer(mesh, 0, &pAttributes);
+ memcpy(pAttributes, 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 (pDecl = declaration; pDecl->Stream != 0xFF; pDecl++)
+ {
+ if (pDecl->Usage == D3DDECLUSAGE_POSITION)
+ {
+ /* Use second vertex position instead of first */
+ pDecl->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 (pDecl = declaration; pDecl->Stream != 0xFF; pDecl++)
+ {
+ if (pDecl->Usage == D3DDECLUSAGE_POSITION)
+ {
+ todo_wine ok(pDecl->Offset == offset, "Test UpdateSematics, got offset %d expected %d\n",
+ pDecl->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 (pDecl = declaration; pDecl->Stream != 0xFF; pDecl++)
+ {
+ if (pDecl->Type == D3DDECLTYPE_FLOAT3)
+ {
+ pDecl->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);
+
+ /* Double usage is not checked */
+ 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 type to color instead of float3 */
+ hr = mesh->lpVtbl->UpdateSemantics(mesh, declaration_wrong_type_color);
+ todo_wine ok(hr == D3D_OK, "Test UpdateSematics wrong type color, "
+ "got %#x expected D3D_OK\n", hr);
+
+ /* Set the position to an undefined type */
+ 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 */
+ 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);
+
+ FreeTestContext(tc);
+}
+
START_TEST(mesh)
{
D3DXBoundProbeTest();
@@ -3322,4 +3604,5 @@ START_TEST(mesh)
test_get_decl_vertex_size();
test_fvf_decl_conversion();
D3DXGenerateAdjacencyTest();
+ UpdateSemanticsTest();
}
diff --git a/dlls/d3dx9_36/mesh.c b/dlls/d3dx9_36/mesh.c
index c5954b3..74a929f 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
@@ -597,11 +598,30 @@ cleanup:
static HRESULT WINAPI ID3DXMeshImpl_UpdateSemantics(ID3DXMesh *iface, D3DVERTEXELEMENT9 declaration[MAX_FVF_DECL_SIZE])
{
+ HRESULT hr;
ID3DXMeshImpl *This = impl_from_ID3DXMesh(iface);
+ IDirect3DVertexDeclaration9 *new_vertex_declaration;
+ if (declaration == NULL)
+ return D3DERR_INVALIDCALL;
- FIXME("(%p)->(%p): stub\n", This, declaration);
+ TRACE("(%p)->(%p)\n", This, declaration);
- return E_NOTIMPL;
+ /* Create new vertex declaration */
+ hr = IDirect3DDevice9_CreateVertexDeclaration(This->device,
+ declaration,
+ &new_vertex_declaration);
+ if (FAILED(hr))
+ {
+ WARN("Unexpected return value %x from IDirect3DDevice9_CreateVertexDeclaration.\n",hr);
+ return hr;
+ }
+
+ /* Free old vertex declaration */
+ IDirect3DVertexDeclaration9_Release(This->vertex_declaration);
+ /* Use new declaration */
+ This->vertex_declaration = new_vertex_declaration;
+
+ return D3D_OK;
}
/*** ID3DXMesh ***/
diff --git a/dlls/d3dx9_36/tests/mesh.c b/dlls/d3dx9_36/tests/mesh.c
index 0ca4b25..0f5ca27 100644
--- a/dlls/d3dx9_36/tests/mesh.c
+++ b/dlls/d3dx9_36/tests/mesh.c
@@ -3506,7 +3506,7 @@ static void UpdateSemanticsTest(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));
@@ -3521,7 +3521,7 @@ static void UpdateSemanticsTest(void)
{
if (pDecl->Usage == D3DDECLUSAGE_POSITION)
{
- todo_wine ok(pDecl->Offset == offset, "Test UpdateSematics, got offset %d expected %d\n",
+ ok(pDecl->Offset == offset, "Test UpdateSematics, got offset %d expected %d\n",
pDecl->Offset, offset);
}
}
@@ -3544,12 +3544,12 @@ static void UpdateSemanticsTest(void)
}
hr = mesh->lpVtbl->UpdateSemantics(mesh, declaration);
- todo_wine ok(hr == D3D_OK, "Test UpdateSematics for bigger vertex size, "
+ 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, "
+ ok(hr == D3DERR_INVALIDCALL, "Test UpdateSematics null pointer declaration, "
"got %#x expected D3DERR_INVALIDCALL\n", hr);
/*
@@ -3557,17 +3557,17 @@ static void UpdateSemanticsTest(void)
* exception on Windows.
*/
hr = mesh->lpVtbl->UpdateSemantics(NULL, NULL);
- todo_wine ok(hr == D3DERR_INVALIDCALL, "Test UpdateSematics null pointers, "
+ ok(hr == D3DERR_INVALIDCALL, "Test UpdateSematics null pointers, "
"got %#x expected D3DERR_INVALIDCALL\n", hr);
/* Double usage is not checked */
hr = mesh->lpVtbl->UpdateSemantics(mesh, declaration_double_usage);
- todo_wine ok(hr == D3D_OK, "Test UpdateSematics double usage, "
+ ok(hr == D3D_OK, "Test UpdateSematics double usage, "
"got %#x expected D3D_OK\n", hr);
/* Set the position type to color instead of float3 */
hr = mesh->lpVtbl->UpdateSemantics(mesh, declaration_wrong_type_color);
- todo_wine ok(hr == D3D_OK, "Test UpdateSematics wrong type color, "
+ ok(hr == D3D_OK, "Test UpdateSematics wrong type color, "
"got %#x expected D3D_OK\n", hr);
/* Set the position to an undefined type */
