This is my first test, so I think it's better to ask for feedback on wine-devel, first.
There are some points, where I wasn't sure what to do: line 26: Is using inline okay here? IIRC that's C++ only, but other tests do it the same way... line 82: Is it legal to demand A8R8G8B8 support here? Otherwise, how would I do this without making the code overcomplicated? For what other formats should I check then? line 120-123: I hope it's okay to use hardcoded matrix values here, as the point of that test is only to make sure that the same matrix gets returned... line 215-253: 1) Is it okay to initialize D3D that way? 2) Can I simplify the initialization even further?
Also, I'd like to know wether the code in general is clean enough. Any other feedback is welcome, too. Best regards, Tony
(Note1: This test only finishes successfully if my recent ID3DXSprite patch series is applied, but I already had all of them pass/todo'ed. Note2: visual tests will follow later, this is just for the basic functionality)
From 02587ae3cedb0b68fba1c360f63d2571c175bf07 Mon Sep 17 00:00:00 2001
From: Tony Wasserka tony.wasserka@freenet.de Date: Sun, 16 Nov 2008 12:26:06 +0100 Subject: [PATCH] d3dx9: Add ID3DXSprite tests
--- dlls/d3dx9_36/tests/Makefile.in | 3 +- dlls/d3dx9_36/tests/core.c | 252 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 254 insertions(+), 1 deletions(-) create mode 100644 dlls/d3dx9_36/tests/core.c
diff --git a/dlls/d3dx9_36/tests/Makefile.in b/dlls/d3dx9_36/tests/Makefile.in index 40500e2..1ac8b7e 100644 --- a/dlls/d3dx9_36/tests/Makefile.in +++ b/dlls/d3dx9_36/tests/Makefile.in @@ -3,9 +3,10 @@ TOPOBJDIR = ../../.. SRCDIR = @srcdir@ VPATH = @srcdir@ TESTDLL = d3dx9_36.dll -IMPORTS = d3dx9 kernel32 +IMPORTS = d3dx9 d3d9 user32 kernel32
CTESTS = \ + core.c \ math.c \ shader.c
diff --git a/dlls/d3dx9_36/tests/core.c b/dlls/d3dx9_36/tests/core.c new file mode 100644 index 0000000..2d15859 --- /dev/null +++ b/dlls/d3dx9_36/tests/core.c @@ -0,0 +1,252 @@ +/* + * Tests for the D3DX9 core interfaces + * + * Copyright 2008 Tony Wasserka + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + */ + +#define COBJMACROS +#include "wine/test.h" +#include <dxerr9.h> +#include "d3dx9core.h" + +static inline int get_ref(IUnknown *obj) +{ + if(!obj) return 0; + IUnknown_AddRef(obj); + return IUnknown_Release(obj); +} + +#define CHECK_REF(obj, exp) \ + ok(exp==get_ref((IUnknown*)(obj)), "Invalid refcount. Expected %d, got %d\n", exp, get_ref((IUnknown*)(obj))); + +#define CHECK_RELEASE(obj, exp) \ + { \ + if(obj) { \ + int ref=IUnknown_Release((IUnknown*)(obj)); \ + ok(ref==exp, "Invalid refcount. Expected %d, got %d\n", exp, ref); \ + } else skip("Object was NULL before Release\n"); \ + } + +#define CHECK_CALL(call, name, exp) \ + hr=(call); \ + ok(hr==(exp), "%s returned %#x, expected %#x\n", (name), hr, (exp)); + +#define admitted_error 0.0001f +static inline void check_mat(D3DXMATRIX got, D3DXMATRIX exp) +{ + int i, j, equal=1; + for (i=0; i<4; i++) + for (j=0; j<4; j++) + if (fabs(U(exp).m[i][j]-U(got).m[i][j]) > admitted_error) + equal=0; + + ok(equal, "Got matrix\n\t(%f,%f,%f,%f\n\t %f,%f,%f,%f\n\t %f,%f,%f,%f\n\t %f,%f,%f,%f)\n" + "Expected matrix=\n\t(%f,%f,%f,%f\n\t %f,%f,%f,%f\n\t %f,%f,%f,%f\n\t %f,%f,%f,%f)\n", + U(got).m[0][0],U(got).m[0][1],U(got).m[0][2],U(got).m[0][3], + U(got).m[1][0],U(got).m[1][1],U(got).m[1][2],U(got).m[1][3], + U(got).m[2][0],U(got).m[2][1],U(got).m[2][2],U(got).m[2][3], + U(got).m[3][0],U(got).m[3][1],U(got).m[3][2],U(got).m[3][3], + U(exp).m[0][0],U(exp).m[0][1],U(exp).m[0][2],U(exp).m[0][3], + U(exp).m[1][0],U(exp).m[1][1],U(exp).m[1][2],U(exp).m[1][3], + U(exp).m[2][0],U(exp).m[2][1],U(exp).m[2][2],U(exp).m[2][3], + U(exp).m[3][0],U(exp).m[3][1],U(exp).m[3][2],U(exp).m[3][3]); +} + +static void test_ID3DXSprite(IDirect3DDevice9 *device) +{ + ID3DXSprite *sprite=NULL; + IDirect3D9 *d3d; + IDirect3DDevice9 *cmpdev=NULL; + IDirect3DTexture9 *tex1=NULL, *tex2=NULL; + D3DXMATRIX mat, cmpmat; + D3DVIEWPORT9 vp; + RECT rect; + D3DXVECTOR3 pos, center; + HRESULT hr; + + IDirect3DDevice9_GetDirect3D(device, &d3d); + if(FAILED(IDirect3D9_CheckDeviceFormat(d3d, 0, D3DDEVTYPE_HAL, D3DFMT_UNKNOWN, 0, D3DRTYPE_TEXTURE, D3DFMT_A8R8G8B8))) { + skip("D3DFMT_A8R8B8G8 not supported\n"); + return; + } + IDirect3D9_Release(d3d); + + if(FAILED(hr=IDirect3DDevice9_CreateTexture(device, 64, 64, 1, 0, D3DFMT_A8R8G8B8, D3DPOOL_DEFAULT, &tex1, NULL))) { + skip("Couldn't create first texture, CreateTexture returned %#x\n", hr); + return; + } + if(FAILED(hr=IDirect3DDevice9_CreateTexture(device, 32, 32, 1, 0, D3DFMT_A8R8G8B8, D3DPOOL_DEFAULT, &tex2, NULL))) { + skip("Couldn't create second texture, CreateTexture returned %#x\n", hr); + IDirect3DTexture9_Release(tex1); + return; + } + /* Test D3DXCreateSprite */ + CHECK_CALL(D3DXCreateSprite(device, NULL), "D3DXCreateSprite", D3DERR_INVALIDCALL); + CHECK_CALL(D3DXCreateSprite(NULL, &sprite), "D3DXCreateSprite", D3DERR_INVALIDCALL); + CHECK_CALL(D3DXCreateSprite(device, &sprite), "D3DXCreateSprite", D3D_OK); + + /* Test ID3DXSprite_GetDevice */ + CHECK_CALL(ID3DXSprite_GetDevice(sprite, NULL), "GetDevice", D3DERR_INVALIDCALL); + CHECK_CALL(ID3DXSprite_GetDevice(sprite, &cmpdev), "GetDevice", D3D_OK); /* cmpdev=NULL */ + CHECK_CALL(ID3DXSprite_GetDevice(sprite, &cmpdev), "GetDevice", D3D_OK); /* cmpdev!=NULL */ + IDirect3DDevice9_Release(device); + IDirect3DDevice9_Release(device); + + /* Test ID3DXSprite_GetTransform */ + CHECK_CALL(ID3DXSprite_GetTransform(sprite, NULL), "GetTransform", D3DERR_INVALIDCALL); + CHECK_CALL(ID3DXSprite_GetTransform(sprite, &mat), "GetTransform", D3D_OK); + if(SUCCEEDED(hr)) { + D3DXMATRIX identity; + D3DXMatrixIdentity(&identity); + check_mat(mat, identity); + } else skip("No matrix to test\n"); + + /* Test ID3DXSprite_SetTransform */ + /* Set a transform and test if it gets returned correctly */ + U(mat).m[0][0]=2.1f; U(mat).m[0][1]=6.5f; U(mat).m[0][2]=-9.6f; U(mat).m[0][3]=1.7f; + U(mat).m[1][0]=4.2f; U(mat).m[1][1]=-2.5f; U(mat).m[1][2]=2.1f; U(mat).m[1][3]=5.5f; + U(mat).m[2][0]=-2.6f; U(mat).m[2][1]=0.3f; U(mat).m[2][2]=8.6f; U(mat).m[2][3]=8.4f; + U(mat).m[3][0]=6.7f; U(mat).m[3][1]=-5.1f; U(mat).m[3][2]=6.1f; U(mat).m[3][3]=2.2f; + CHECK_CALL(ID3DXSprite_SetTransform(sprite, NULL), "SetTransform", D3DERR_INVALIDCALL); + CHECK_CALL(ID3DXSprite_SetTransform(sprite, &mat), "SetTransform", D3D_OK); + if(SUCCEEDED(hr)) { + hr=ID3DXSprite_GetTransform(sprite, &cmpmat); + if(SUCCEEDED(hr)) check_mat(cmpmat, mat); + else skip("GetTransform returned %#x\n", hr); + } else skip("No matrix to test\n"); + + /* Test ID3DXSprite_SetWorldViewLH/RH */ + todo_wine { + CHECK_CALL(ID3DXSprite_SetWorldViewLH(sprite, &mat, &mat), "SetWorldViewLH", D3D_OK); + CHECK_CALL(ID3DXSprite_SetWorldViewLH(sprite, NULL, &mat), "SetWorldViewLH", D3D_OK); + CHECK_CALL(ID3DXSprite_SetWorldViewLH(sprite, &mat, NULL), "SetWorldViewLH", D3D_OK); + CHECK_CALL(ID3DXSprite_SetWorldViewLH(sprite, NULL, NULL), "SetWorldViewLH", D3D_OK); + CHECK_CALL(ID3DXSprite_SetWorldViewRH(sprite, &mat, &mat), "SetWorldViewRH", D3D_OK); + CHECK_CALL(ID3DXSprite_SetWorldViewRH(sprite, NULL, &mat), "SetWorldViewRH", D3D_OK); + CHECK_CALL(ID3DXSprite_SetWorldViewRH(sprite, &mat, NULL), "SetWorldViewRH", D3D_OK); + CHECK_CALL(ID3DXSprite_SetWorldViewRH(sprite, NULL, NULL), "SetWorldViewRH", D3D_OK); + } + IDirect3DDevice9_BeginScene(device); + + /* Test ID3DXSprite_Begin*/ + CHECK_CALL(ID3DXSprite_Begin(sprite, 0), "Begin", D3D_OK); + IDirect3DDevice9_GetTransform(device, D3DTS_WORLD, &mat); + D3DXMatrixIdentity(&cmpmat); + check_mat(mat, cmpmat); + IDirect3DDevice9_GetTransform(device, D3DTS_VIEW, &mat); + check_mat(mat, cmpmat); + IDirect3DDevice9_GetTransform(device, D3DTS_PROJECTION, &mat); + IDirect3DDevice9_GetViewport(device, &vp); + D3DXMatrixOrthoOffCenterLH(&cmpmat, vp.X+0.5f, (float)vp.Width+vp.X+0.5f, (float)vp.Height+vp.Y+0.5f, vp.Y+0.5f, vp.MinZ, vp.MaxZ); + check_mat(mat, cmpmat); + + /* Test ID3DXSprite_Flush and ID3DXSprite_End */ + CHECK_CALL(ID3DXSprite_Flush(sprite), "Flush", D3D_OK); + CHECK_CALL(ID3DXSprite_End(sprite), "End", D3D_OK); + CHECK_CALL(ID3DXSprite_Flush(sprite), "Flush", D3DERR_INVALIDCALL); /* May not be called before next Begin */ + CHECK_CALL(ID3DXSprite_End(sprite), "End", D3DERR_INVALIDCALL); + + /* Test ID3DXSprite_Draw */ + CHECK_CALL(ID3DXSprite_Begin(sprite, 0), "Begin", D3D_OK); + if(FAILED(hr)) skip("Couldn't ID3DXSprite_Begin, can't test ID3DXSprite_Draw\n"); + else { /* Feed the sprite batch */ + int texref1, texref2; + + SetRect(&rect, 0, 0, 192, 128); + pos.x=0.f; pos.y=10.f; pos.z=0.f; + center.x=192.f; center.y=0.f; center.z=0.f; + + texref1=get_ref((IUnknown*)tex1); + texref2=get_ref((IUnknown*)tex2); + CHECK_CALL(ID3DXSprite_Draw(sprite, NULL, &rect, ¢er, &pos, D3DCOLOR_XRGB(255, 255, 255)), "Draw", D3DERR_INVALIDCALL); + CHECK_CALL(ID3DXSprite_Draw(sprite, tex1, &rect, ¢er, &pos, D3DCOLOR_XRGB(255, 255, 255)), "Draw", D3D_OK); + CHECK_CALL(ID3DXSprite_Draw(sprite, tex2, &rect, ¢er, &pos, D3DCOLOR_XRGB( 3, 45, 66)), "Draw", D3D_OK); + CHECK_REF(tex1, texref1+1); CHECK_REF(tex2, texref2+1); + CHECK_CALL(ID3DXSprite_Flush(sprite), "Flush", D3D_OK); + CHECK_REF(tex1, texref1); CHECK_REF(tex2, texref2); + + CHECK_CALL(ID3DXSprite_Draw(sprite, tex1, NULL, ¢er, &pos, D3DCOLOR_XRGB(255, 255, 255)), "Draw", D3D_OK); + CHECK_CALL(ID3DXSprite_Draw(sprite, tex1, &rect, NULL, &pos, D3DCOLOR_XRGB(255, 255, 255)), "Draw", D3D_OK); + CHECK_CALL(ID3DXSprite_Draw(sprite, tex1, &rect, ¢er, NULL, D3DCOLOR_XRGB(255, 255, 255)), "Draw", D3D_OK); + CHECK_CALL(ID3DXSprite_Draw(sprite, tex1, NULL, NULL, NULL, 0), "Draw", D3D_OK); + CHECK_CALL(ID3DXSprite_Flush(sprite), "Flush", D3D_OK); + CHECK_CALL(ID3DXSprite_Flush(sprite), "Flush", D3D_OK); /* Flushing twice should work */ + CHECK_CALL(ID3DXSprite_End(sprite), "End", D3D_OK); + } + + /* Test ID3DXSprite_OnLostDevice and ID3DXSprite_OnResetDevice */ + /* Both can be called twice */ + CHECK_CALL(ID3DXSprite_OnLostDevice(sprite), "OnLostDevice", D3D_OK); + CHECK_CALL(ID3DXSprite_OnLostDevice(sprite), "OnLostDevice", D3D_OK); + CHECK_CALL(ID3DXSprite_OnResetDevice(sprite), "OnResetDevice", D3D_OK); + CHECK_CALL(ID3DXSprite_OnResetDevice(sprite), "OnResetDevice", D3D_OK); + + /* Make sure everything works like before */ + CHECK_CALL(ID3DXSprite_Begin(sprite, 0), "Begin", D3D_OK); + CHECK_CALL(ID3DXSprite_Draw(sprite, tex2, &rect, ¢er, &pos, D3DCOLOR_XRGB(255, 255, 255)), "Draw", D3D_OK); + CHECK_CALL(ID3DXSprite_Flush(sprite), "Flush", D3D_OK); + CHECK_CALL(ID3DXSprite_End(sprite), "End", D3D_OK); + + /* OnResetDevice makes the interface "forget" the Begin call */ + CHECK_CALL(ID3DXSprite_Begin(sprite, 0), "Begin", D3D_OK); + CHECK_CALL(ID3DXSprite_OnResetDevice(sprite), "OnResetDevice", D3D_OK); + CHECK_CALL(ID3DXSprite_End(sprite), "End", D3DERR_INVALIDCALL); + + IDirect3DDevice9_EndScene(device); + CHECK_RELEASE(sprite, 0); + CHECK_RELEASE(tex2, 0); + CHECK_RELEASE(tex1, 0); +} + +START_TEST(core) +{ + HWND wnd=NULL; + IDirect3D9 *d3d = NULL; + IDirect3DDevice9 *device=NULL; + D3DPRESENT_PARAMETERS d3dpp; + HRESULT hr; + + wnd=CreateWindow("static", "d3dx9_test", 0, 0, 0, 0, 0, NULL, NULL, NULL, NULL); + d3d=Direct3DCreate9(D3D_SDK_VERSION); + if(!wnd) { + skip("Couldn't create application window\n"); + return; + } + if(!d3d) { + skip("Couldn't create Direct3D9 object\n"); + DestroyWindow(wnd); + return; + } + + ZeroMemory(&d3dpp, sizeof(d3dpp)); + d3dpp.Windowed = TRUE; + d3dpp.SwapEffect = D3DSWAPEFFECT_DISCARD; + hr=IDirect3D9_CreateDevice(d3d, D3DADAPTER_DEFAULT, D3DDEVTYPE_HAL, wnd, + D3DCREATE_MIXED_VERTEXPROCESSING, &d3dpp, &device); + if(FAILED(hr)) { + skip("Failed to create Direct3DDevice9 object %#x\n", hr); + IDirect3D9_Release(d3d); + DestroyWindow(wnd); + return; + } + + test_ID3DXSprite(device); + + CHECK_RELEASE(device, 0); + CHECK_RELEASE(d3d, 0); + if(wnd) DestroyWindow(wnd); +}
Tony Wasserka wrote:
This is my first test, so I think it's better to ask for feedback on wine-devel, first.
In all of your patch:
- if(!obj) return 0;
Please add space after "if" to make it consistent and more readable: if (!obj) return 0;
hr=IDirect3DDevice9_CreateTexture
Please add spaces to the both sides of "=".
+#define CHECK_REF(obj, exp) \
ok(exp==get_ref((IUnknown*)(obj)), "Invalid refcount. Expected %d, got %d\n", exp, get_ref((IUnknown*)(obj)));
You should make it an inine perhaps to not call get_ref twice on the same object.
+#define CHECK_RELEASE(obj, exp) \
- { \
if(obj) { \
int ref=IUnknown_Release((IUnknown*)(obj)); \
ok(ref==exp, "Invalid refcount. Expected %d, got %d\n", exp, ref); \
} else skip("Object was NULL before Release\n"); \
- }
This also should be a function. And please drop skip - it should only be used to skip parts of the test. Here you just skipping one check which should always be valid.
+#define CHECK_CALL(call, name, exp) \
- hr=(call); \
- ok(hr==(exp), "%s returned %#x, expected %#x\n", (name), hr, (exp));
Don't use macros like this. Put it all in the code. It's bad thing to have macro modify some variables (hr) you don't explicitly specify as params or return.
- ID3DXSprite *sprite=NULL;
- IDirect3DDevice9 *cmpdev=NULL;
- IDirect3DTexture9 *tex1=NULL, *tex2=NULL;
No need to initialize variables that you always assign to.
- if(FAILED(hr=IDirect3DDevice9_CreateTexture(device, 64, 64, 1, 0, D3DFMT_A8R8G8B8, D3DPOOL_DEFAULT, &tex1, NULL))) {
skip("Couldn't create first texture, CreateTexture returned %#x\n", hr);
return;
- }
Failure to create a texture should be considered a test - use ok(). And please put hr=IDirect3DDevice9_CreateTexture() on the separate line.
- if(SUCCEEDED(hr)) {
D3DXMATRIX identity;
D3DXMatrixIdentity(&identity);
check_mat(mat, identity);
- } else skip("No matrix to test\n");
No need to use skip() here.
Vitaliy.
Tony Wasserka wrote:
This is my first test, so I think it's better to ask for feedback on wine-devel, first.
And please fix your e-mail server to properly accept e-mails if you asking some one to comment on them.
Vitaliy.
"Vitaliy Margolen" wine-devel@kievinfo.com wrote:
In all of your patch:
- if(!obj) return 0;
Please add space after "if" to make it consistent and more readable: if (!obj) return 0;
Actually it would be better to either completely remove that line, or even do an assert(obj != NULL) instead IMO.