Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Analysis of external mesh loading libraries and comparison with the existing approach #344

Open
onurtore opened this issue Apr 16, 2022 · 20 comments · Fixed by #393
Open
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@onurtore
Copy link
Contributor

onurtore commented Apr 16, 2022

Desired behavior

Be able to load glTF2 and FBX mesh files into Gazebo.

Alternatives considered

Only alternative is on the user side, which creates additional step for converting mesh files into Ignition Gazebo readable format.

Implementation suggestion

There are two ways to extend the capabilities of the Gazebo's mesh loading system; (A) Using an external open-source library or (B) Writing glTF2 and FBX mesh loaders in a consistent way with the current approach.

Analysis is required to decide which approach is better. I am planning to generate such analysis in the next couple of days, so any idea is welcome. Currently gathering ideas around the internet to make a suitable comparision.

I created an additional document for this analysis in here. I will start my analysis with assimp.

Additional context

  1. Currently working on a creating a connection between the assimp library and Ignition Gazebo.
  2. Possible related issues Support loading meshes in glTF format #173,
@chapulina
Copy link
Contributor

Thank you for the ticket and for looking into it, @onurtore . It would be great to add this capability to Gazebo. Looking forward to seeing the results of your analysis.

@chapulina chapulina added the help wanted Extra attention is needed label Apr 18, 2022
@onurtore
Copy link
Contributor Author

small update: Very basic transformation algorithm written (Just considers, vertices,indices and normals like the STLLoader.), now going to integrate Assimp to Gazebo to create speed and size metrics. If possible I will write this as plugin.

@luca-della-vedova
Copy link
Member

luca-della-vedova commented May 17, 2022

Hi! I thought it would be interesting to try out assimp and am in the process of writing an assimp loader class following the collada / obj / stl loader classes to load assets through assimp.
My hope is to be able to both parse all the asset formats supported by assimp (including but not limited to fbx and gltf) and reduce the size of the gz-common codebase by removing all the custom loaders.
It's been going reasonably smoothly so far, keeping a checklist of tasks and things remaining below:

  • Meshes (vertices, normals, faces)
  • Materials - external textures
  • Materials - embedded textures (i.e. GLB)
  • Skeletons
  • Animations - Some segfaults with some models at the ign-gazebo level

For now I'll be focusing on parsing obj, dae and glb since those are the formats I personally use the most, I'll also try to unify all the unit tests in the collada / stl / obj loader into a single unit test file for assimp to try keep coverage high.
Work is currently taking place in the luca/assimp_sandbox branch.

@luca-della-vedova
Copy link
Member

I ran into a critical issue with the assimp library included in Ubuntu 20.04 (v5.0.1) that is unable to parse DAE assets with empty tags (i.e. empty author tag) which sadly makes up a majority of the assets in Fuel.
For this reason I'll be porting the code to work with the assimp version in Ubuntu 22.04, which seems to involve quite a few big changes @onurtore, you might want to build assimp from source in version 5.2.2 for work going further.

@onurtore
Copy link
Contributor Author

onurtore commented May 23, 2022

@luca-della-vedova
Can you share some mesh files that creates problems? Can you mention some of the big changes ? So your setup includes 5.2.2 and Ubuntu 22.04, I didnt quite catch the reason to change the OS can you explain it? (Is it related to Fuel?)

Thanks.

@onurtore
Copy link
Contributor Author

@luca-della-vedova
Can you share the models with seg-faults?

@luca-della-vedova
Copy link
Member

The main change I had to deal with is that assets with empty author tags, there is a lot of them in the Ignition Edifice world, for example if you try to run:

ign gazebo -v3 "https://fuel.gazebosim.org/1.0/OpenRobotics/worlds/Edifice demo"

With a 20.04 binary it will fail and crash since the empty tag XML, for example in the Crates.dae file, will make assimp fail to import the asset. If you build assimp from source on latest master it's better.
A world that fails for me now is the actor.sdf world, so if you run:

ign gazebo actor.sdf

You should see a segmentation fault, still looking into it.

@onurtore
Copy link
Contributor Author

onurtore commented May 23, 2022

@luca-della-vedova,
Below works on my machine:

ign gazebo -v3 "https://fuel.gazebosim.org/1.0/OpenRobotics/worlds/Edifice demo"

Only once I got error saing there is a assertion problem where the parameter of the argument of the loadDynamicImage function should be 6u. Other than that its runs.

actor.sdf doesnt work, but it is too complex to handle the whole world, so I started with a simpler version: MecanumLift.dae. I will work on that as next step.

mecanum lift.zip

Update: Looks like our skeleton loading code has problems. This will be a good oppurtunity to implement basic fallback system to default loaders as mentioned here.

Update (27/05/22):
Segmentation fault problem: RecursiveCreate function utilizes skeletons however there is an error in that logic. Furthermore, the RecursiveSkeletonCreate function has also problems. This means almost every part of the skeleton loading code has problems. I will solve this by copying the logic from collada loader and the assimp opengl tutorials.

Additional Problems: Removing all the skeleton loading code still does not gives a visually correct output. That means recursiveCreate function has also problems.

First I will fix the recursiveCreate function and then move on the Skeleton codes.

Update(28/05/22):
Unfortunetely, recursiveCreate function requires a recursiveSkeletonCreate function. In order to accomplish this I will first implement that by following here

@luca-della-vedova
Copy link
Member

@luca-della-vedova, Below works on my machine:

ign gazebo -v3 "https://fuel.gazebosim.org/1.0/OpenRobotics/worlds/Edifice demo"

Strange, but now I don't have a clean machine to test with binary 5.0.1 anymore! 😅

actor.sdf doesnt work, but it is too complex to handle the whole world, so I started with a simpler version: MecanumLift.dae. I will work on that as next step.

Yea it's a bit tricky because one actor with one animation works, seems to be an issue with either multiple animations or one of the ones I didn't manage to test yet.

mecanum lift.zip

Update: Looks like our skeleton loading code has problems. This will be a good oppurtunity to implement basic fallback system to default loaders as mentioned here.

Yea I saw the biggest issue right now is something about duplicated bone names, I'm sure it can be fixed at the AssimpLoader level.

Update from my side, I'm currently looking at PBR materials and embedded textures. PBR materials are reasonably simple to implement but a lot of interesting formats (i.e. fbx, glb) create a single binary blob with the mesh and all its related textures. gz-common does not currently support setting data structures (i.e. an array of bytes) as textures but only filenames so the change might be a bit more involved and span multiple packages.

@luca-della-vedova
Copy link
Member

luca-della-vedova commented May 31, 2022

Good news! There is pretty much feature parity in the demo worlds between the assimp loader and our custom STL / OBJ / DAE loaders! Edifice demo where every asset is loaded through assimp below:

image

@onurtore I can't see any issue with the mecanum lift, you can try the latest changes with assimp built from source and see how that goes?

The main issue right now is only in actor.sdf that might require some work here and there, for example it seems gz-sim expects the first animation to actually only be the skin and I don't think the loader is doing that right now.

I'll start looking at more niche features but this might soon be reaching a "good enough for basic operations" stage, as long as we are conservative with the custom loader behavior (for now there has been no issue with STL and OBJ but collada still has a few rough edges) and find a way to add the dependency for binary assimp.

Some example of more niche features that are still TODO:

  • Transparency - Done alpha transparency for GLTF, other formats will still need override from SDF
  • Finishing GLB PBR (It uses metallic roughness maps but they are unsupported by Ogre2 right now)
  • Embedded textures, as above, it's a large effort so might not do it for now
  • Full feature parity with ColladaLoader aka "why does actor.sdf crash"

@onurtore
Copy link
Contributor Author

onurtore commented Jun 1, 2022

Hi @luca-della-vedova,
Building [email protected] in Ubuntu 20.04 ends up with an error. Applying the author's fix does not resolve the issue.

In order to progress, I used the source installation of 5.1.1. I test some of the worlds, while MaceneumLift.dae works in empty world, it does not work when I try to add it to eddifice.

Meshes without normals still have problems, maybe this will help.

@luca-della-vedova
Copy link
Member

Hi @luca-della-vedova, Building [email protected] in Ubuntu 20.04 ends up with an error. Applying the author's fix does not resolve the issue.

For the error personally I just deleted the line with the issue and it seems to work

@onurtore
Copy link
Contributor Author

onurtore commented Jun 7, 2022

@luca-della-vedova thanks for the heads up.

Update:
I have been trying to load the actor.sdf into Gazebo for the past couple of days. The problem occurs during the trajectory interpolation of the actors in SceneManager::CreateActor function. Code snippet tries to access rootNode's frame count using rootNode->FrameCount(), however the RootNode is a nullptr and result with an seg fault. This happens because there is no animation by the name 'Scene'. I am trying to find a possible way to fix this problem.

Removing whole interpolation code loads the actor.sdf into Gazebo. Unfortunately, loaded meshes have weird arms and playing the animations ends up with a segmentation fault after a while. Fixing the weird arm problem looks easier and I am working on that.

weird1

weird2

@luca-della-vedova
Copy link
Member

Sounds good! One of the issues I can see from the screenshot is that the actors are also upside down, I guess both the upside down issue and the weird arms are issues with the node transforms and might have the same (or a similar) solution?

In the meanwhile I'm getting to work with embedded textures so there might be some pretty consistent changes in the branch to allow reading binary textures

@onurtore
Copy link
Contributor Author

onurtore commented Jun 10, 2022

Small update: Assimp can not parse all the inverse bind transform. This happens because skeleton creation uses nodes, but code sets the inverse bind transform using mesh' bones. If number of bones and the number of nodes are not 1-1 correspondence then this creates nodes without inverse transform. Assimp sometimes does not include bones to hierarchy and I am trying to fix that or find a way to remove nodes without bones or automatically find the transformation of nodes without bones.

@luca-della-vedova
Copy link
Member

Small update: Assimp can not parse all the inverse bind transform. This happens because skeleton creation uses nodes, but code sets the inverse bind transform using mesh' bones. If number of bones and the number of nodes are not 1-1 correspondence then this creates nodes without inverse transform. Assimp sometimes does not include bones to hierarchy and I am trying to fix that or find a way to remove nodes without bones or automatically find the transformation of nodes without bones.

Thanks for digging into this! I noticed that this was documented in assimp and a PR was merged literally yesterday to deal with the issue of nodes and bones not mapping in case a bone didn't have any vertex.
I wonder if this could be the issue we are facing? It seems to have only been implemented for FBX though so I'm not 100% sure how to test it, an option could be to export the animation into FBX, test it, build from source with that commit then test it again and see if it's fixed.
If that works the "right" path forward might be to look into how the PR fixes the issue for FBX and try to submit one for other Collada?

@onurtore
Copy link
Contributor Author

onurtore commented Jun 20, 2022

Hi @luca-della-vedova,
The problem we are facing is related to node-bone correspondence. I tried different approaches to eliminate this problem.

  1. Using Assimp's flags to preserve the bones without vertices.
    Unfortunately, flags related to this operations does not work.

  2. Using a generic skeleton refine function.
    I tried to solve the problem in the Gazebo's side, rather than Assimp side. For the past week I spent most of time to write a function that will take any Gazebo skeleton structure and Assimp scene and either add necessary transformations or remove bones without vertices. Unfortunately, after failed attemps I have realized that this is not a viable solution.

  3. Using new experimental bone structure to create Skeleton.
    I tried to use the new skeleton structure. When I convert our dae into fbx with blender, I couldn't access the new skeleton structure (Seg fault). I tried one of the meshes author pushed to repository for test. While I could access the node structure, I couldnt get the bone names (Seg fault.)

  4. Fixing Assimp's remove empty bones flag.
    This fixes the weird hand problem, however there are still some bones (for example 'scene' node) without inverse bindings, these nodes have identity transformations, and thats maybe the the reason why our models are reversed. Looks like colladaParser doesnt load them as bone and thats why these dont have inverse bindings. Maybe we can remove these nodes from our skeleton after recursive creation? This might also solve the FrameCount() segmentation fault.

We can move with option three or four depending on what type of solution we are looking for. I implemented the fourth in here, if you can test it it will be appreciated. I am planning to create a PR in assimp for this. (This branch might still create a seg fault, due to animations and below should be setted in Gazebo:

this->dataPtr->importer.SetPropertyBool(AI_CONFIG_IMPORT_REMOVE_EMPTY_BONES, false);

@luca-della-vedova
Copy link
Member

Thanks for the great work!

I agree solution 3 and 4 look like the best fits. I am quite neutral to which way to go for, I had a look at your branch and it seems n.4 is quite simple even at the assimp level however if it cannot fix the inverted skeleton it's maybe not super ideal.
I tried to test your branch and can't quite replicate the fix, basically I:

  • Changed my assimp from source to your branch
  • Added the importer setting you mentioned to the AssimpLoader constructor
  • Recompiled and ran ign gazebo actor.sdf

However I see the same segfaults, am I missing anything?

@onurtore
Copy link
Contributor Author

onurtore commented Jun 21, 2022

Hi @luca,
I believe interpolation is the reason of the segfault. I will work on that after fixing the upside down problem. Maybe you can test it after I fix that problem. If you wan to test the current version, you can comment out the code inside the if (animation && animation->InterpolateX()) in gz-sim->rendering->SceneManager.cc

@onurtore
Copy link
Contributor Author

Hi @luca-della-vedova,
I have 2 things to mention, first, I was doing some speed analysis on object files and created below graph. The column with Assimp Loader without flags does not utilize aiProcess_RemoveRedundantMaterials, aiProcess_SortByPType
aiProcess_PopulateArmatureData and significantly faster than other assimp loaders. I was wondering whether is it important for us to use these flags.

Figure_1

Second,
When I try to move the camera angle more than some degree with a world/mesh (edifice, campus.obj) loaded using assimp loader, mesh dissapears from screen. This does not happens with the original loader, did you realize such problem?

@azeey azeey linked a pull request Sep 28, 2022 that will close this issue
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants