Hotfix for removing scene node

[onurtore]

🦟 Bug fix

Summary

Directly removing scene node from the skeleton creates problem for meshes without childs. This bugfix creates a dummy skeleton for this type of meshes.

Checklist

• Signed all commits for DCO
• Added tests
• Updated documentation (as needed)
• Updated migration guide (as needed)
• Consider updating Python bindings (if the library has them)
• codecheck passed (See contributing)
• All tests passed (See test coverage)
• While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

[onurtore]

Hi @luca-della-vedova,

This PR results with failing MergeBoxWithDoubleSkeleton test.

I am looking for a mesh where there are two children right after Scene Node to represent this test in real life, however I can not find such mesh. Even PatientWalkingCane.dae, which you mentioned have two skeletons, only have one child. So maybe we should delete MergeBoxWithDoubleSkeleton test?

Secondly, this is currently a hotfix, I can spend time to create a better patch, or we can update our code to work with the new skeleton. What do you think is the best way?

[luca-della-vedova]

Do you have any details of which type of meshes exhibit wrong behavior when the scene node is not removed and works when this patch is added?

[luca-della-vedova]

I did a bit more investigation and I guess this goes back to the issues with actor.sdf segfaulting?

I wonder if instead of removing the root and hardcoding a "root is equal to the first child" logic we could just fix the scene node by assigning it a dummy animation that is just an identity (zero translation, unit quaternion) for the whole time, so children are preserved and their animations unchanged.

I'm not familiar enough with actor animations and exporting software to know whether the hardcoding 0 would have any side effects or not but it feels a bit dangerous if a root scene node was to have more than one child for any reason.

[onurtore]

I did a bit more investigation and I guess this goes back to the issues with actor.sdf segfaulting?

I wonder if instead of removing the root and hardcoding a "root is equal to the first child" logic we could just fix the scene node by assigning it a dummy animation that is just an identity (zero translation, unit quaternion) for the whole time, so children are preserved and their animations unchanged.

I'm not familiar enough with actor animations and exporting software to know whether the hardcoding 0 would have any side effects or not but it feels a bit dangerous if a root scene node was to have more than one child for any reason.

Yes it's related to actor.sdf segfault.

This idea might work if this is also merged. Otherwise interpolateX flag for the animation will be always negative with dummy animation. This is the only code that I found checks root node properties for setting the animation properties, however if there is more, then this approach create problem. But I think it is bad to set the whole animation properties by just looking root node? And if such thing exist we should remove them?

[onurtore]

Hi @luca-della-vedova,
Friendly ping.

Do you have any details of which type of meshes exhibit wrong behavior when the scene node is not removed and works when this patch is added?

Actor.sdf

We could just fix the scene node by assigning it a dummy animation that is just an identity (zero translation, unit quaternion) for the whole time, so children are preserved and their animations unchanged.

Here, however I dont think thats a good idea.

What is the best way to proceed with this, we still cant load actors with assimp.

[azeey]

@luca-della-vedova can we close this PR? There hasn't been any progress in a long time.