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.