Re: d3dx9: CloneMesh test and improvements
On Thu, Aug 18, 2011 at 8:05 PM, Stefan Dösinger stefandoesin...@gmx.at wrote:
Hi,
I noticed one small issue:
On Saturday 13 August 2011 12:22:03 Michael Mc Donnell wrote:
+static HRESULT convert_vertex_buffer(ID3DXMesh *mesh_dst, ID3DXMesh
*mesh_src)
...
+ hr = mesh_dst-lpVtbl-LockVertexBuffer(mesh_dst, D3DLOCK_DISCARD,
(void**)vb_dst);
On paper, D3DLOCK_DISCARD is only valid on D3DUSAGE_DYNAMIC buffers. Wine and
Windows = WinVista don't enforce this. Win7 does, and it broke some apps that
way.
Ok I see that in dlls/d3d9/tests/buffers.c. So it will fail to lock on
Win7 if D3DLOCK_DISCARD is specified and the vertex buffer wasn't
created with D3DUSAGE_DYNAMIC?
Am I correct in D3DLOCK_DISCARD makes a copy of the vertex buffer so
that the video card can keep reading the old one, and the old vertex
buffer memory is released after the video card releases its lock [1]?
In that case it does not make sense to use D3DLOCK_DISCARD at all in
CloneMesh, as the video card has not begun to use the new vertex
buffer, and it will just add memory overhead. The easiest fix would be
to just pass 0 instead of D3DLOCK_DISCARD. I've done that in the
updated patch.
I noticed a few more D3DLOCK_DISCARD locks in the mesh.c code.
I guess the other methods should check the mesh options for
D3DXMESH_DYNAMIC, D3DXMESH_VB_DYNAMIC, and D3DXMESH_IB_DYNAMIC [2]
before using D3DLOCK_DISCARD? I'll write patches for those.
[1]
http://msdn.microsoft.com/en-us/library/bb147263(v=vs.85).aspx#Using_Dynamic_Vertex_and_Index_Buffers
[2] http://msdn.microsoft.com/en-us/library/bb205370(v=vs.85).aspx
From e52cb6d31e7c8fbb3dd7e93fbf64adf90fad807e Mon Sep 17 00:00:00 2001
From: Michael Mc Donnell mich...@mcdonnell.dk
Date: Wed, 17 Aug 2011 12:50:09 +0200
Subject: d3dx9: Implemented non-equal declaration support in CloneMesh.
Don't use D3DLOCK_DISCARD it only adds memory overhead in CloneMesh.
---
dlls/d3dx9_36/mesh.c | 457 ++
1 files changed, 425 insertions(+), 32 deletions(-)
diff --git a/dlls/d3dx9_36/mesh.c b/dlls/d3dx9_36/mesh.c
index 9a0fd03..26c726f 100644
--- a/dlls/d3dx9_36/mesh.c
+++ b/dlls/d3dx9_36/mesh.c
@@ -65,6 +65,27 @@ typedef struct ID3DXMeshImpl
D3DXATTRIBUTERANGE *attrib_table;
} ID3DXMeshImpl;
+const UINT d3dx_decltype_size[] =
+{
+ /* D3DDECLTYPE_FLOAT1*/ sizeof(FLOAT),
+ /* D3DDECLTYPE_FLOAT2*/ sizeof(D3DXVECTOR2),
+ /* D3DDECLTYPE_FLOAT3*/ sizeof(D3DXVECTOR3),
+ /* D3DDECLTYPE_FLOAT4*/ sizeof(D3DXVECTOR4),
+ /* D3DDECLTYPE_D3DCOLOR */ sizeof(D3DCOLOR),
+ /* D3DDECLTYPE_UBYTE4*/ 4 * sizeof(BYTE),
+ /* D3DDECLTYPE_SHORT2*/ 2 * sizeof(SHORT),
+ /* D3DDECLTYPE_SHORT4*/ 4 * sizeof(SHORT),
+ /* D3DDECLTYPE_UBYTE4N */ 4 * sizeof(BYTE),
+ /* D3DDECLTYPE_SHORT2N */ 2 * sizeof(SHORT),
+ /* D3DDECLTYPE_SHORT4N */ 4 * sizeof(SHORT),
+ /* D3DDECLTYPE_USHORT2N */ 2 * sizeof(USHORT),
+ /* D3DDECLTYPE_USHORT4N */ 4 * sizeof(USHORT),
+ /* D3DDECLTYPE_UDEC3 */ 4, /* 3 * 10 bits + 2 padding */
+ /* D3DDECLTYPE_DEC3N */ 4,
+ /* D3DDECLTYPE_FLOAT16_2 */ 2 * sizeof(D3DXFLOAT16),
+ /* D3DDECLTYPE_FLOAT16_4 */ 4 * sizeof(D3DXFLOAT16),
+};
+
static inline ID3DXMeshImpl *impl_from_ID3DXMesh(ID3DXMesh *iface)
{
return CONTAINING_RECORD(iface, ID3DXMeshImpl, ID3DXMesh_iface);
@@ -260,6 +281,401 @@ static HRESULT WINAPI ID3DXMeshImpl_CloneMeshFVF(ID3DXMesh *iface, DWORD options
return iface-lpVtbl-CloneMesh(iface, options, declaration, device, clone_mesh);
}
+static FLOAT scale_clamp_ubyten(FLOAT value)
+{
+value = value * UCHAR_MAX;
+
+if (value 0.0f)
+{
+return 0.0f;
+}
+else
+{
+if (value UCHAR_MAX) /* Clamp at 255 */
+return UCHAR_MAX;
+else
+return value;
+}
+}
+
+static FLOAT scale_clamp_shortn(FLOAT value)
+{
+value = value * SHRT_MAX;
+
+/* The tests show that the range is SHRT_MIN + 1 to SHRT_MAX. */
+if (value = SHRT_MIN)
+{
+return SHRT_MIN + 1;
+}
+else if (value SHRT_MAX)
+{
+ return SHRT_MAX;
+}
+else
+{
+return value;
+}
+}
+
+static FLOAT scale_clamp_ushortn(FLOAT value)
+{
+value = value * USHRT_MAX;
+
+if (value 0.0f)
+{
+return 0.0f;
+}
+else
+{
+if (value USHRT_MAX) /* Clamp at 65535 */
+return USHRT_MAX;
+else
+return value;
+}
+}
+
+static INT simple_round(FLOAT value)
+{
+int res = (INT)(value + 0.5f);
+
+return res;
+}
+
+static void convert_float4(BYTE *dst, CONST D3DXVECTOR4 *src, D3DDECLTYPE type_dst)
+{
+BOOL fixme_once = FALSE;
+
+switch (type_dst)
+{
+case D3DDECLTYPE_FLOAT1:
+{
+FLOAT *dst_ptr = (FLOAT*)dst;
+*dst_ptr = src-x;
+break;
+}
+case D3DDECLTYPE_FLOAT2:
+{
+D3DXVECTOR2 *dst_ptr = (D3DXVECTOR2*)dst;
+
Re: d3dx9: CloneMesh test and improvements
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Am 19.08.2011 um 14:04 schrieb Michael Mc Donnell: Ok I see that in dlls/d3d9/tests/buffers.c. So it will fail to lock on Win7 if D3DLOCK_DISCARD is specified and the vertex buffer wasn't created with D3DUSAGE_DYNAMIC? Yes, unless Microsoft changed this again. Am I correct in D3DLOCK_DISCARD makes a copy of the vertex buffer so that the video card can keep reading the old one, and the old vertex buffer memory is released after the video card releases its lock [1]? Sort of. It doesn't copy the old data, it just gives you a fresh, uninitialized block of memory. So if you read from the pointer you get from a DISCARD lock you'll get garbage. In that case it does not make sense to use D3DLOCK_DISCARD at all in CloneMesh, as the video card has not begun to use the new vertex buffer, and it will just add memory overhead. The easiest fix would be to just pass 0 instead of D3DLOCK_DISCARD. I've done that in the updated patch. Right, the buffer is newly created in CloneMesh. Just not using DISCARD is the best fix then. I noticed a few more D3DLOCK_DISCARD locks in the mesh.c code. I guess the other methods should check the mesh options for D3DXMESH_DYNAMIC, D3DXMESH_VB_DYNAMIC, and D3DXMESH_IB_DYNAMIC [2] before using D3DLOCK_DISCARD? I'll write patches for those. Most likely yes. I haven't looked at the details. If the buffer is new in those situations as well just don't pass any flags, otherwise pass DISCARD if you lock a dynamic buffer and are going to rewrite the entire buffer contents. -BEGIN PGP SIGNATURE- Version: GnuPG/MacGPG2 v2.0.17 (Darwin) iQIcBAEBAgAGBQJOTlUYAAoJEN0/YqbEcdMwi3kP/AwlIBwuD8XpvAnpVJMmFyM8 lZCJ5zrEbBypYPSin3lEv7yiA+tw2YQwD4ODoZHRZCLKi7Gy07EI2FLAX34KAhxa ESNXanQuMWaglkj2QfV7aJan8plHdKhTA6HZl3VFCGSLHNjbGWzDXpO60TKTJU3F jU0Iqnyxq2KSdDU0R6gMsIHKF6wLBkVMOheGTXO2Mc2VwBMt85js+T9+tSUKvKTR ncVhvnygGLhyB0QfprFvN70If+BKQIicseJm6zO9nIt8/qqmtd6t9QYneEY+HIJI rZMpHCwxRKmGnCJuJfXnZDtRqUTsT2S7iZuIDUi7xH+It79uWvFEP+Cs/bB2ssqF kyw1kmTVSi99vbmiQRn4wBGv9C32L9C64SUwNH/nQg7E0iO9zdQxLE3oU64v4lVs OaTYuIghLxrDaR9+28yLjt97oBvxPLvrhuve2ASpevz+6Fa3n40o1OTBi3ch+Q70 xOBe1Qh9lX40C56b2XVx+j5XfqrNFl1BS/Tluvcs66mgcdTKeZp4ATjx5Js9c1eT /JsfBiARrGEZ1pVYLRbUY1oky+sq64/sSFsjs8QMerfhms1kTk8oziTTvSw0uT5H UCF3MadXFwrFnif/kPQ4hfBKr29E28lB/JSp/tb7Rxtim7Se2F8a6I5kmrIwjRoQ nWko6eM0FzSMR+OiZbsz =SQxW -END PGP SIGNATURE-
Only use D3DLOCK_DISCARD if dynamic memory (was Re: d3dx9: CloneMesh test and improvements)
On Fri, Aug 19, 2011 at 2:20 PM, Stefan Dösinger stefandoesin...@gmx.at wrote:
Am 19.08.2011 um 14:04 schrieb Michael Mc Donnell:
Ok I see that in dlls/d3d9/tests/buffers.c. So it will fail to lock on
Win7 if D3DLOCK_DISCARD is specified and the vertex buffer wasn't
created with D3DUSAGE_DYNAMIC?
Yes, unless Microsoft changed this again.
Am I correct in D3DLOCK_DISCARD makes a copy of the vertex buffer so
that the video card can keep reading the old one, and the old vertex
buffer memory is released after the video card releases its lock [1]?
Sort of. It doesn't copy the old data, it just gives you a fresh,
uninitialized block of memory. So if you read from the pointer you get from a
DISCARD lock you'll get garbage.
In that case it does not make sense to use D3DLOCK_DISCARD at all in
CloneMesh, as the video card has not begun to use the new vertex
buffer, and it will just add memory overhead. The easiest fix would be
to just pass 0 instead of D3DLOCK_DISCARD. I've done that in the
updated patch.
Right, the buffer is newly created in CloneMesh. Just not using DISCARD is
the best fix then.
I noticed a few more D3DLOCK_DISCARD locks in the mesh.c code.
I guess the other methods should check the mesh options for
D3DXMESH_DYNAMIC, D3DXMESH_VB_DYNAMIC, and D3DXMESH_IB_DYNAMIC [2]
before using D3DLOCK_DISCARD? I'll write patches for those.
Most likely yes. I haven't looked at the details. If the buffer is new in
those situations as well just don't pass any flags, otherwise pass DISCARD if
you lock a dynamic buffer and are going to rewrite the entire buffer contents.
All of the places with D3DLOCK_DISCARD in mesh.c were working on newly
created meshes or vertex buffers, so they should all pass 0. I've
attached patches for it.
From 161a8d36437b2b950642f8cca4012b2792d4b385 Mon Sep 17 00:00:00 2001
From: Michael Mc Donnell mich...@mcdonnell.dk
Date: Fri, 19 Aug 2011 16:48:18 +0200
Subject: d3dx9: Use 0 instead of D3DLOCK_DISCARD in OptimizeInPlace
It does not make sense to use D3DLOCK_DISCARD for locking a newly created
vertex buffer because it will be allocated, freed, and then allocated again.
Furthermore, D3DLOCK_DISCARD should only be used in conjuction with dynamic
memory, which the current implementation does not check for. Using the flag
D3DLOCK_DISCARD with static memory causes failure on Windows7 (see dlls/d3d9/
tests/buffer.c).
---
dlls/d3dx9_36/mesh.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/dlls/d3dx9_36/mesh.c b/dlls/d3dx9_36/mesh.c
index 9a0fd03..ed42883 100644
--- a/dlls/d3dx9_36/mesh.c
+++ b/dlls/d3dx9_36/mesh.c
@@ -1293,7 +1293,7 @@ static HRESULT WINAPI ID3DXMeshImpl_OptimizeInplace(ID3DXMesh *iface, DWORD flag
hr = IDirect3DVertexBuffer9_Lock(This-vertex_buffer, 0, 0, (void**)orig_vertices, D3DLOCK_READONLY);
if (FAILED(hr)) goto cleanup;
-hr = IDirect3DVertexBuffer9_Lock(vertex_buffer, 0, 0, (void**)new_vertices, D3DLOCK_DISCARD);
+hr = IDirect3DVertexBuffer9_Lock(vertex_buffer, 0, 0, (void**)new_vertices, 0);
if (FAILED(hr)) {
IDirect3DVertexBuffer9_Unlock(This-vertex_buffer);
goto cleanup;
--
1.7.6
From a3d069449d7b4d54b2847b8de6af447f65e709c5 Mon Sep 17 00:00:00 2001
From: Michael Mc Donnell mich...@mcdonnell.dk
Date: Fri, 19 Aug 2011 17:21:30 +0200
Subject: d3dx9: Use 0 instead of D3DLOCK_DISCARD in load_skin_mesh_from_xof
It does not make sense to use D3DLOCK_DISCARD for locking newly created
buffers because they will be allocated, freed, and then allocated again.
---
dlls/d3dx9_36/mesh.c |6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/dlls/d3dx9_36/mesh.c b/dlls/d3dx9_36/mesh.c
index ed42883..2462422 100644
--- a/dlls/d3dx9_36/mesh.c
+++ b/dlls/d3dx9_36/mesh.c
@@ -2966,7 +2966,7 @@ static HRESULT load_skin_mesh_from_xof(IDirectXFileData *filedata,
hr = D3DXCreateMeshFVF(mesh_data.num_tri_faces, total_vertices, options, mesh_data.fvf, device, d3dxmesh);
if (FAILED(hr)) goto cleanup;
-hr = d3dxmesh-lpVtbl-LockVertexBuffer(d3dxmesh, D3DLOCK_DISCARD, vertices);
+hr = d3dxmesh-lpVtbl-LockVertexBuffer(d3dxmesh, 0, vertices);
if (FAILED(hr)) goto cleanup;
out_ptr = vertices;
@@ -3008,7 +3008,7 @@ static HRESULT load_skin_mesh_from_xof(IDirectXFileData *filedata,
}
d3dxmesh-lpVtbl-UnlockVertexBuffer(d3dxmesh);
-hr = d3dxmesh-lpVtbl-LockIndexBuffer(d3dxmesh, D3DLOCK_DISCARD, (void**)indices);
+hr = d3dxmesh-lpVtbl-LockIndexBuffer(d3dxmesh, 0, (void**)indices);
if (FAILED(hr)) goto cleanup;
index_in_ptr = mesh_data.indices;
@@ -3037,7 +3037,7 @@ static HRESULT load_skin_mesh_from_xof(IDirectXFileData *filedata,
if (mesh_data.material_indices) {
DWORD *attrib_buffer = NULL;
-hr = d3dxmesh-lpVtbl-LockAttributeBuffer(d3dxmesh, D3DLOCK_DISCARD, attrib_buffer);
+hr = d3dxmesh-lpVtbl-LockAttributeBuffer(d3dxmesh, 0, attrib_buffer);
Re: d3dx9: CloneMesh test and improvements
Hi, I noticed one small issue: On Saturday 13 August 2011 12:22:03 Michael Mc Donnell wrote: +static HRESULT convert_vertex_buffer(ID3DXMesh *mesh_dst, ID3DXMesh *mesh_src) ... +hr = mesh_dst-lpVtbl-LockVertexBuffer(mesh_dst, D3DLOCK_DISCARD, (void**)vb_dst); On paper, D3DLOCK_DISCARD is only valid on D3DUSAGE_DYNAMIC buffers. Wine and Windows = WinVista don't enforce this. Win7 does, and it broke some apps that way. I noticed a few more D3DLOCK_DISCARD locks in the mesh.c code. signature.asc Description: This is a digitally signed message part.
Re: d3dx9: CloneMesh test and improvements
On Thu, Aug 11, 2011 at 4:07 PM, Michael Mc Donnell mich...@mcdonnell.dk wrote: On Thu, Aug 11, 2011 at 3:01 PM, Matteo Bruni matteo.myst...@gmail.com wrote: 2011/8/11 Michael Mc Donnell mich...@mcdonnell.dk: + dst_ptr[0] = src-x 0.0f ? (SHORT)ceilf(src-x * SHRT_MAX + 0.5f) :(SHORT)floorf(src-x * SHRT_MAX + 0.5f); You can use roundf() instead. Actually, notice that maybe what you actually need for correct rounding is rintf() (which essentially matches what D3DXFloat32To16Array does). You should probably test this (even on your own, not necessarily adding those to the testsuite, I'd say). Ok I'll look into that. I also noticed that I had a bug in the UBYTE4 conversion. I'll modify the tests to catch those cases. Is it ok to use roundf and rintf? They're both C99 functions [1]. [1] http://en.wikipedia.org/wiki/Math.h
Re: d3dx9: CloneMesh test and improvements
On Fri, Aug 12, 2011 at 12:59 PM, Michael Mc Donnell mich...@mcdonnell.dkwrote: Is it ok to use roundf and rintf? They're both C99 functions. Hello, As far as I know C99 is not allowed. However, you can emulate round by doing: floorf(val + 0.5f) According to [1] floorf is C99 (only floor is C89), but including math.h in wine actually gives you msvcrt's math.h which is a bit different and doesn't even have round nor rint. Gdiplus uses floorf [2] so it should be OK. Octavian [1] http://linux.die.net/man/3/floorf [2] http://source.winehq.org/git/wine.git/?a=searchh=HEADst=greps=floorf
Re: d3dx9: CloneMesh test and improvements
On Fri, Aug 12, 2011 at 1:06 PM, Octavian Voicu octavian.vo...@gmail.com wrote: On Fri, Aug 12, 2011 at 12:59 PM, Michael Mc Donnell mich...@mcdonnell.dk wrote: Is it ok to use roundf and rintf? They're both C99 functions. Hello, As far as I know C99 is not allowed. However, you can emulate round by doing: floorf(val + 0.5f) According to [1] floorf is C99 (only floor is C89), but including math.h in wine actually gives you msvcrt's math.h which is a bit different and doesn't even have round nor rint. Gdiplus uses floorf [2] so it should be OK. Octavian [1] http://linux.die.net/man/3/floorf [2] http://source.winehq.org/git/wine.git/?a=searchh=HEADst=greps=floorf Thanks Octavian. I was already using floorf(val + 0.5f), so I'll keep using that.
Re: d3dx9: CloneMesh test and improvements
2011/8/12 Octavian Voicu octavian.vo...@gmail.com: On Fri, Aug 12, 2011 at 12:59 PM, Michael Mc Donnell mich...@mcdonnell.dk wrote: Is it ok to use roundf and rintf? They're both C99 functions. Hello, As far as I know C99 is not allowed. However, you can emulate round by doing: floorf(val + 0.5f) According to [1] floorf is C99 (only floor is C89), but including math.h in wine actually gives you msvcrt's math.h which is a bit different and doesn't even have round nor rint. Gdiplus uses floorf [2] so it should be OK. No, we are still using native math library in wine dlls (actually, linking wine dlls to msvcrt is usually frowned upon), so those floorf calls go directly to libm. Anyway, if you look into wine msvcrt's code, floorf doesn't do anything else than directly calling native floorf (and without any #ifdef guard, as far as I can see), so it wouldn't really make a difference. That said, I don't know what is allowed and what's not. Requiring rintf or roundf support doesn't seem really asking more than what is needed now, but you could argue that we shouldn't use floorf in wine either. BTW, as you are actually rounding to convert a float to an integer type (as opposed to needing a rounded value still in a floating point variable), you can also avoid using floorf() altogether. So, e.g., instead of +dst_ptr[0] = src-x 0.0f ? (SHORT)ceilf(src-x * SHRT_MAX + 0.5f) :(SHORT)floorf(src-x * SHRT_MAX + 0.5f); something like +dst_ptr[0] = src-x 0.0f ? (SHORT)(src-x * SHRT_MAX - 0.5f) :(SHORT)(src-x * SHRT_MAX + 0.5f); still works, since C floating point to integer conversion rules mandate for truncation. Octavian [1] http://linux.die.net/man/3/floorf [2] http://source.winehq.org/git/wine.git/?a=searchh=HEADst=greps=floorf
Re: d3dx9: CloneMesh test and improvements
On 12 August 2011 13:06, Octavian Voicu octavian.vo...@gmail.com wrote: As far as I know C99 is not allowed. However, you can emulate round by doing: floorf(val + 0.5f) It probably doesn't matter here, but note that that isn't the same as roundf() for values below zero.
Re: d3dx9: CloneMesh test and improvements
2011/8/11 Michael Mc Donnell mich...@mcdonnell.dk: +dst_ptr[0] = src-x 0.0f ? (SHORT)ceilf(src-x * SHRT_MAX + 0.5f) :(SHORT)floorf(src-x * SHRT_MAX + 0.5f); You can use roundf() instead. Actually, notice that maybe what you actually need for correct rounding is rintf() (which essentially matches what D3DXFloat32To16Array does). You should probably test this (even on your own, not necessarily adding those to the testsuite, I'd say). Still on the conversion: going on with this strategy is going to require a ton of code to handle every src-dst format combination. You may look into doing that in two steps: first converting src to float4, then float4 into dst. I haven't looked into it, so there may be some detail making this impractical, but still it's worth a try, I guess.
Re: d3dx9: CloneMesh test and improvements
On Thu, Aug 11, 2011 at 3:01 PM, Matteo Bruni matteo.myst...@gmail.com wrote: 2011/8/11 Michael Mc Donnell mich...@mcdonnell.dk: + dst_ptr[0] = src-x 0.0f ? (SHORT)ceilf(src-x * SHRT_MAX + 0.5f) :(SHORT)floorf(src-x * SHRT_MAX + 0.5f); You can use roundf() instead. Actually, notice that maybe what you actually need for correct rounding is rintf() (which essentially matches what D3DXFloat32To16Array does). You should probably test this (even on your own, not necessarily adding those to the testsuite, I'd say). Ok I'll look into that. I also noticed that I had a bug in the UBYTE4 conversion. I'll modify the tests to catch those cases. Still on the conversion: going on with this strategy is going to require a ton of code to handle every src-dst format combination. You may look into doing that in two steps: first converting src to float4, then float4 into dst. I haven't looked into it, so there may be some detail making this impractical, but still it's worth a try, I guess. That's a good idea. I had started doing the other conversions, but noticed that it was a lot of code. I'll try your idea. Thanks!
Re: d3dx9: CloneMesh test and improvements
2011/8/8 Michael Mc Donnell mich...@mcdonnell.dk:
Hi
I've been working on a test and improvements for CloneMesh as part of
GSoC 2011. I would be grateful if anyone could comment on my work.
There are five test cases in the attached patch:
0. Basic mesh cloning. Declaration has position and normal, and the
new declaration is the same as the original.
1. Same as test 0, but with 16-bit indices.
2. Shows that the vertex buffer can be widened so that it has room
for a new vertex component. The declaration has position and normal,
and the new declaration has texture coordinates after position and
normal.
3. Same as test 2, but the new declaration has the texture
coordinates in between position and normal.
4. Shows that a vertex component can be converted into another type
if the new declaration specifies another type. The declaration has
position and normal. The normal is a FLOAT2 and in the new declaration
it is instead FLOAT16_2.
Test 0 and 1 tests what is already known to work in the current
implementation. Test 2, 3 and 4 do not work in the current
implementation because the original and new declarations are not the
same.
I've also improved the current CloneMesh implementation so that it
also passes test 2, 3, and 4. The patch is not complete because it
does not support converting all the possible types. I expect to
implement conversion for the remaining types during this week.
Again any comments or ideas are welcome.
Thanks,
Michael Mc Donnell
Hello Michael,
I have some comments:
+const UINT d3dx_decltype_size[D3DDECLTYPE_UNUSED] =
+{
I don't really like sizing the vector like that. Just remove the
D3DDECLTYPE_UNUSED between the square brackets, I'd say. Otherwise,
you could use the D3DMAXDECLTYPE define.
+ /* D3DDECLTYPE_FLOAT1*/ 1 * 4,
...
You may want to replace the 4 with a sizeof(float) for better clarity.
Same for the others.
+if (orig_declaration.Method == declaration[i].Method
+ orig_declaration.Usage == declaration[i].Usage
+ orig_declaration.UsageIndex == declaration[i].UsageIndex)
Does the Method field really need to be compared? Maybe add a test if
that is the case.
You may also e.g. test if in a POSITION, NORMAL, TEXCOORD(0) -
POSITION, NORMAL, TEXCOORD(1) CloneMesh (or even the other way around)
the texture coordinates are actually lost or not.
+for (i = 0; orig_declaration[i].Stream != 0xff; i++) {
+if (memcmp(orig_declaration[i], declaration[i],
sizeof(*declaration))) {
+same_declaration = FALSE;
+break;
+}
+}
+
+if (vertex_size != orig_vertex_size)
+same_declaration = FALSE;
Not really a big thing, but you could do the vertex size check first
and then compare the fields only if same_declaration is still TRUE.
The tests look good to me, except some nits like the ok() in
check_vertex_components in the D3DDECLTYPE_FLOAT1 case, splitting long
lines and such.
d3dx9: CloneMesh test and improvements
Hi
I've been working on a test and improvements for CloneMesh as part of
GSoC 2011. I would be grateful if anyone could comment on my work.
There are five test cases in the attached patch:
0. Basic mesh cloning. Declaration has position and normal, and the
new declaration is the same as the original.
1. Same as test 0, but with 16-bit indices.
2. Shows that the vertex buffer can be widened so that it has room
for a new vertex component. The declaration has position and normal,
and the new declaration has texture coordinates after position and
normal.
3. Same as test 2, but the new declaration has the texture
coordinates in between position and normal.
4. Shows that a vertex component can be converted into another type
if the new declaration specifies another type. The declaration has
position and normal. The normal is a FLOAT2 and in the new declaration
it is instead FLOAT16_2.
Test 0 and 1 tests what is already known to work in the current
implementation. Test 2, 3 and 4 do not work in the current
implementation because the original and new declarations are not the
same.
I've also improved the current CloneMesh implementation so that it
also passes test 2, 3, and 4. The patch is not complete because it
does not support converting all the possible types. I expect to
implement conversion for the remaining types during this week.
Again any comments or ideas are welcome.
Thanks,
Michael Mc Donnell
From caac5a9e2f6bbba3d05e52cf952f65db86b15099 Mon Sep 17 00:00:00 2001
From: Michael Mc Donnell mich...@mcdonnell.dk
Date: Sun, 24 Jul 2011 17:18:05 +0200
Subject: d3dx9: Implemented CloneMesh conversion and test.
---
dlls/d3dx9_36/mesh.c | 165 ++---
dlls/d3dx9_36/tests/mesh.c | 614
2 files changed, 749 insertions(+), 30 deletions(-)
diff --git a/dlls/d3dx9_36/mesh.c b/dlls/d3dx9_36/mesh.c
index 9a0fd03..b6c3792 100644
--- a/dlls/d3dx9_36/mesh.c
+++ b/dlls/d3dx9_36/mesh.c
@@ -65,6 +65,27 @@ typedef struct ID3DXMeshImpl
D3DXATTRIBUTERANGE *attrib_table;
} ID3DXMeshImpl;
+const UINT d3dx_decltype_size[D3DDECLTYPE_UNUSED] =
+{
+ /* D3DDECLTYPE_FLOAT1*/ 1 * 4,
+ /* D3DDECLTYPE_FLOAT2*/ 2 * 4,
+ /* D3DDECLTYPE_FLOAT3*/ 3 * 4,
+ /* D3DDECLTYPE_FLOAT4*/ 4 * 4,
+ /* D3DDECLTYPE_D3DCOLOR */ 4 * 1,
+ /* D3DDECLTYPE_UBYTE4*/ 4 * 1,
+ /* D3DDECLTYPE_SHORT2*/ 2 * 2,
+ /* D3DDECLTYPE_SHORT4*/ 4 * 2,
+ /* D3DDECLTYPE_UBYTE4N */ 4 * 1,
+ /* D3DDECLTYPE_SHORT2N */ 2 * 2,
+ /* D3DDECLTYPE_SHORT4N */ 4 * 2,
+ /* D3DDECLTYPE_USHORT2N */ 2 * 2,
+ /* D3DDECLTYPE_USHORT4N */ 4 * 2,
+ /* D3DDECLTYPE_UDEC3 */ 4, /* 3 * 10 bits + 2 padding */
+ /* D3DDECLTYPE_DEC3N */ 4,
+ /* D3DDECLTYPE_FLOAT16_2 */ 2 * 2,
+ /* D3DDECLTYPE_FLOAT16_4 */ 4 * 2,
+};
+
static inline ID3DXMeshImpl *impl_from_ID3DXMesh(ID3DXMesh *iface)
{
return CONTAINING_RECORD(iface, ID3DXMeshImpl, ID3DXMesh_iface);
@@ -260,6 +281,102 @@ static HRESULT WINAPI ID3DXMeshImpl_CloneMeshFVF(ID3DXMesh *iface, DWORD options
return iface-lpVtbl-CloneMesh(iface, options, declaration, device, clone_mesh);
}
+static INT get_equivalent_declaration_index(D3DVERTEXELEMENT9 orig_declaration, D3DVERTEXELEMENT9 *declaration)
+{
+INT i;
+
+for (i = 0; declaration[i].Stream != 0xff; i++)
+{
+if (orig_declaration.Method == declaration[i].Method
+ orig_declaration.Usage == declaration[i].Usage
+ orig_declaration.UsageIndex == declaration[i].UsageIndex)
+{
+return i;
+}
+}
+
+return -1;
+}
+
+static void convert_component(BYTE *dst, BYTE *src, D3DDECLTYPE type_dst, D3DDECLTYPE type_src)
+{
+BOOL fixme_once = FALSE;
+
+switch (type_src)
+{
+case D3DDECLTYPE_FLOAT2:
+{
+switch (type_dst)
+{
+case D3DDECLTYPE_FLOAT16_2:
+D3DXFloat32To16Array((D3DXFLOAT16*)dst, (FLOAT*)src, 2);
+break;
+default:
+break;
+}
+break;
+}
+default:
+if (!fixme_once++)
+FIXME(Conversion of D3DDECLTYPE %d to %d not implemented.\n, type_src, type_dst);
+break;
+}
+}
+
+static HRESULT convert_vertex_buffer(ID3DXMesh *mesh_dst, ID3DXMesh *mesh_src)
+{
+HRESULT hr;
+D3DVERTEXELEMENT9 orig_declaration[MAX_FVF_DECL_SIZE] = {D3DDECL_END()};
+D3DVERTEXELEMENT9 declaration[MAX_FVF_DECL_SIZE] = {D3DDECL_END()};
+BYTE *vb_dst = NULL;
+BYTE *vb_src = NULL;
+UINT i;
+UINT num_vertices = mesh_src-lpVtbl-GetNumVertices(mesh_src);
+UINT dst_vertex_size = mesh_dst-lpVtbl-GetNumBytesPerVertex(mesh_dst);
+UINT src_vertex_size = mesh_src-lpVtbl-GetNumBytesPerVertex(mesh_src);
+
+hr = mesh_src-lpVtbl-GetDeclaration(mesh_src, orig_declaration);
+if (FAILED(hr)) return hr;
+hr =
