d3dx9: CloneMesh test and improvements

[Matteo Bruni]

2011/8/8 Michael Mc Donnell <michael at 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.