#22 DeboneProcess minus "unit test"

[Mick P.]

This removes bones which have no actual affect upon presentation. It splits out the faces in question, and supplants them directly under the node that would've driven the removed bone.

I wrapped this up a long time ago but put off trying to formulate an appropriate unit test. I've given up on bothering with the unit test. There's a skeleton there, but the body is not implemented. The "SplitByBoneCount" process probably would've been a good template for a unit test, but it doesn't seem to have a unit test either. So go figure. I think in general requiring unit tests accompany a patch seriously hampers progress. I have other patches in the same straits. Enjoy.

PS: There's a generic mesh splitting routine in here as well. The bone weights process might benefit from sharing it. The inline documentation in the aiPostProcess.h file could probably use some filling out. Will provide a test model if necessary.

[Mick P.]

[Alexander Gessler]

It would be great to have a test model to see the step actually work. Even greater, but not strictly required, if we had permission to add the file to the respository.

I've so far merged and adapted DeBones with my working copy and I'm not experiencing any problems so far. I added support for the new step in all relevant sites (i.e. assimp_cmd).

Besides, many steps could use the mesh splitting routine, but I fear rewriting all of them bears many risks. Still, I'll put it on the TODO list.

Bye, Alex

[Alexander Gessler]

Oh, I forgot: under which Name do you wish to appear in the CREDITS?

Bye, Alex

[Mick P.]

I somehow missed the below comments. I apologize. Just noticed them today in my mail. About credits, I'm cool with being unaccredited unless the credits are contributions specific. Mick P. is fine, though I'd prefer not to be given general credit until I'm permitted write access to the SCM repository.

I forgot btw to include the CMAKE file. I'm trying to make a point to do more with CMAKE myself.

I will try to obtain a good model. I don't know if I have an ideal one on hand or not. I may have to ask for some outside help in that.

On the mesh splitting. I forgot to incorporate the Vertex Animation structures. I did my best actually to just poach code from various places in the file tree so that it would at least blend in some stylistically (one thing I've never been able to keep track of is the one or two letter prefixes on the camel case identifiers)

PS: I prefer Debone over DeBone if I have any say in that :)

[Alexander Gessler]

I called it Debone in my working copy. I too prefer it over DeBone :-)

Apart from that, only waiting for a test case.

Bye, Alex

[Mick P.]

I've asked someone to track down an emblematic model. I think though (I could be wrong) the dwarf.x file you guys have might technically be a suitable test....

I just tried the dwarf, and some bones appear to be removed. I came across a bug too :D

In MakeSubMesh there needs to be a condition on the bone loop...

for(size_t a=0;a<pMesh->mNumBones;++a)
{
+ if(subBones[a]==0) continue;

My head is no longer in the game, but that looks like the right fix. Seems to work. Good thing I threw in the "paranoia" assert. Helps to be paranoid.

I guess I never tested a file that wasn't completely deboned; woops.

Hmmm, there seems to be yet another bug with dwarf.x...

// mesh was split
if(!newMeshes.empty())
{
unsigned int out = 0, in = srcMesh->mNumBones;

// store new meshes and indices of the new meshes
for(size_t b=0;b<newMeshes.size();b++)
{
-const aiString &find = newMeshes[b].second->mName;
+const aiString *find = +newMeshes[b].second?&newMeshes[b].second->mName:0;
-aiNode *theNode = pScene->mRootNode->FindNode(find);
+aiNode *theNode = find?pScene->mRootNode->FindNode(*find):0;

This seemed to go thru. Two more bones are eventually removed. However dwarf.x does not look righ in Assimp_view with Debone built in.

This is going to take some more work it seems... I will have to look at it later when I can get a chance :/

Anyway, maybe dwarf.x is a good test after all.

[Mick P.]

Here are the last two fixes. @Aramis, I reckon it's easier for you to just slip the fixes in than deal with a new patch so...

-for(size_t srcIndex = 0; srcIndex < numSubVerts; ++srcIndex )
+for(size_t srcIndex = 0; srcIndex < pMesh->mNumVertices; ++srcIndex )
{
- unsigned int nvi = vMap[srcIndex];
+ unsigned int nvi = vMap[srcIndex]; if(nvi==UINT_MAX) continue;

I also put some conditions around the vertex/face allocation. I'm not sure if it's safe anyway if one or the other is zero anyway. I reckon an empty submesh is conceivable??

This is a new section in SplitMesh

// invalidate any "cojoined" faces
for(unsigned int i=0;i<pMesh->mNumFaces;i++)
if(faceBones[i]<pMesh->mNumBones&&isBoneNecessary[faceBones[i]])
{
ai_assert(facesPerBone[faceBones[i]]>0);

facesPerBone[faceBones[i]]--; nFacesUnowned++;

faceBones[i] = cUnowned;
}

Which comes before...

if(nFacesUnowned)
{
std::vector<unsigned int> subFaces;

for(unsigned int i=0;i<pMesh->mNumFaces;i++)
{
if(faceBones[i]==cUnowned) subFaces.push_back(i);
}

aiMesh *baseMesh = Assimp_x::MakeSubmesh(pMesh,subFaces,0);

std::pair<aiMesh*,const aiBone*> push_pair(baseMesh,0);

poNewMeshes.push_back(push_pair);
}

It may be worth uncommenting the Coowned flag stuff and forcing the interstitial counting loop to ignore coowned faces. That would add some additional stability if the weights are not normalized.

I put this process before the other bone steps in importer.cpp so to save them the trouble of the removed bones. However if there is a processes that somehow insures correct normalization of weights (if that's even doable) it would want to come first.

I'm a little embarrassed I'd not thought to test this with a partially deboned file. I think maybe I lost interest prematurely and forgot that I had not been more thorough. The code itself all seemed very cut and dry. Anyway, apologies. Please try for yourself with dwarf.x after these changes...

It should debone his axe and shoulder pads.

[Mick P.]

PS: I Think the aiPostProcess.h (or whatever filename) documentation should make it clear that the goal is to remove the bones... versus (as in not) pulling out submeshes that are static. I think you could do both with a config option, but it would require some extra logic I think, since there'd no longer be a 1:1 bone to submesh ratio. Probably not worth it.

[Alexander Gessler]

As of r950, Debone is integrated into Assimp.

I found that we actually have code to 'produce' great test models in the core library: the SkeletonMeshBuilder constructs artificial meshes with dummy bones and is used whenever an input file contains only an animation skeleton but no mesh data (i.e. a single MD5ANIM file). With those and the other animated files from the repository, I've tested the new code and found no issues. After all, in most models at least two bones got removed, so I think it's a very welcome addition.

I did use the occassion to refactor ProcessHelper.h a bit. MakeSubmesh() is there as well.

Please take the time to check if I didn't break anything and if I applied all your patch instructions correctly.

Bye & thanks, Alex