Use assimp for loading meshes #393

luca-della-vedova · 2022-07-13T04:06:26Z

🎉 New feature

Closes #374

Summary

This PR introduces using the assimp library for parsing models and, with it, support for GLB, GLTF and FBX formats.
For now it overrides all the custom loaders, there is no known regression with obj and stl, while there are a few regression with collada files.
Most of the regressions have been handled upstream at the assimp library level, however the fixes didn't make it to the released Debian package so they might not reflect unless assimp is built from source or a new release is triggered upstream.
A regression in actor.sdf that segfaults is known and currently under investigation.
Imo a nice stretch goal would be to deprecate as many loaders as possible and only maintain one but this can be a scary change so the alternative could be to just add yet another loader for "all other formats" and keep using the internal ones for obj, stl and dae. However, obj and stl seem to be simple and reliable enough that I believe they could be removed.

Depends on a gz-cmake PR and release to add support for finding assimp in gz_find_package for CI to be green.

Test it

All the unit tests from STL, Collada and OBJ loaders have been ported to the assimp loader and updated in cases where the behavior changed.
New unit tests for the new formats have been merged in #385 by @onurtore.
You will need a few glb assets to test this PR, I provided a few in the matching gz-rendering PR.

Checklist

Signed all commits for DCO
Added tests
Added example and/or tutorial
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.

Signed-off-by: Luca Della Vedova <[email protected]>

Signed-off by: Onur Berk Tore [email protected] Co-authored-by: Luca Della Vedova <[email protected]>

Signed-off-by: Luca Della Vedova <[email protected]>

…p_sandbox Signed-off-by: Luca Della Vedova <[email protected]>

Signed-off-by: Luca Della Vedova <[email protected]>

* Fix: Works even the root node doesnt have child * Assimp also generates normals * Revert "Fix: Works even the root node doesnt have child" This reverts commit 7687252. Signed-off by: Onur Berk Tore, [email protected] Signed-off by: Onur Berk Tore, [email protected] Co-authored-by: Luca Della Vedova <[email protected]>

Signed-off-by: Luca Della Vedova <[email protected]>

onurtore

Tested locally, looks good to me

onurtore · 2022-08-04T07:18:05Z

graphics/src/MeshManager.cc

+  this->dataPtr->fileExtensions.insert("gltf");
+  this->dataPtr->fileExtensions.insert("glb");
+  this->dataPtr->fileExtensions.insert("fbx");
+


Hi @luca-della-vedova,
Should we remove this newline?

onurtore · 2022-08-07T13:23:25Z

Hi @luca-della-vedova, @ahcorde,

While I trying to fix seg fault of actor.sdf, I have realized there is at least two reason for the seg fault, one is solved in this PR, second is bvh animation in actor.sdf. Before bvh animation are added to skeleton in here we are checking whether loaded bvh skeleton count are same with the skeleton loaded through mesh manager. If we use scene node in mesh manager, then those numbers are not same (The code is here). Actually, even if we use assimp for loading bvh file, which I tried, then the rootNode will not be scene, bvh loader in assimp do not utilize scene node. (I also utilized a check for any occurence of error during bvhloading in here)

So there are two things we can do, if this PR, and this PR are merged, then seg fault problem will dissappear. Otherwise, we have to modify the code during Bvhloading in SkeletonAnimation. I dont think that will be hard, and even we might deprecate the bvhloader and use assimp there as well.

The hotfix looks little bit hackish so I am not sure it is the right direction, so I am waiting your views on the topic to move forward.

onurtore · 2022-08-09T16:48:25Z

graphics/src/AssimpLoader.cc

+    // Now add the bones to the skeleton
+    if (assimpMesh->HasBones() && _scene->HasAnimations())
+    {
+      // TODO(luca) merging skeletons here


Hi @luca-della-vedova,
Can you eloborate on this TODO little bit more, I want to implement it, but I did not quite understand it.

onurtore · 2022-08-09T18:09:57Z

graphics/src/AssimpLoader.cc

+      // TODO(luca) merging skeletons here
+      auto skeleton = _mesh->MeshSkeleton();
+      // TODO(luca) Append to existing skeleton if multiple submeshes?
+      skeleton->SetNumVertAttached(subMesh.VertexCount());


Hi @luca-della-vedova,
Is this TODO necessary ? By the way, resizing this vector in each iteration looks like heavy operation, but I don't think we have better alternative.

onurtore · 2022-08-11T12:20:02Z

graphics/src/AssimpLoader.cc

+      math::Vector3d texcoords;
+      texcoords.X(_assimpMesh->mTextureCoords[uvIdx][vertexIdx].x);
+      texcoords.Y(_assimpMesh->mTextureCoords[uvIdx][vertexIdx].y);
+      // TODO(luca) why do we need 1.0 - Y?


Hi @luca-della-vedova,
Looks like aiProcess_FlipUVs flag solves this issue, should we use that?

onurtore · 2022-08-22T09:19:15Z

graphics/src/AssimpLoader.cc

+    auto [texName, texData] = this->LoadTexture(_scene,
+        texturePath, this->GenerateTextureName(_scene, assimpMat, "Diffuse"));
+    if (texData != nullptr)
+      mat->SetTextureImage(texName, texData);


Hi @luca-della-vedova,
Do you know how we can combine multiple diffuse in here? I want to implement it but my knowledge is weak. I know we can access them using a while loop but dont know how to merge them.

luca-della-vedova and others added 30 commits May 13, 2022 12:20

First draft of assimp loader, can load obj and dae meshes

33c8d2f

Signed-off-by: Luca Della Vedova <[email protected]>

WIP correct material assignments

bce766d

Signed-off-by: Luca Della Vedova <[email protected]>

Refactor mesh creation into separate function

3b08c5b

Signed-off-by: Luca Della Vedova <[email protected]>

WIP adding actor support

0ed8f69

Signed-off-by: Luca Della Vedova <[email protected]>

[FEATURE] Drag and drop support for gltf and fbx (#354)

531b2a8

WIP actors with single skeletons working

8f63b21

Signed-off-by: Luca Della Vedova <[email protected]>

Migrate to assimp 5.2.0

db823ca

Signed-off-by: Luca Della Vedova <[email protected]>

Fix: Works even the root node doesnt have child (#358)

1795314

Signed-off by: Onur Berk Tore [email protected] Co-authored-by: Luca Della Vedova <[email protected]>

Only load bones if model has animations

ce4cfc8

Naming style to follow Gazebo convention

47e42a3

Signed-off-by: Luca Della Vedova <[email protected]>

Merge branch 'main' of github.com:gazebosim/gz-common into luca/assim…

2a94bfc

…p_sandbox Signed-off-by: Luca Della Vedova <[email protected]>

Fix after namespace migration

c106035

Signed-off-by: Luca Della Vedova <[email protected]>

Parse alpha from texture from GLTF materials

d5f9935

Signed-off-by: Luca Della Vedova <[email protected]>

Minor cleanups, parse metalness and roughness

1f28b89

Signed-off-by: Luca Della Vedova <[email protected]>

WIP create API for having in memory textures

1c9771d

Signed-off-by: Luca Della Vedova <[email protected]>

Diffuse textures working

fd3a0f5

Signed-off-by: Luca Della Vedova <[email protected]>

Add API to load compressed images and read RGBA data

08f1102

Signed-off-by: Luca Della Vedova <[email protected]>

Add test

661823c

Signed-off-by: Luca Della Vedova <[email protected]>

Deallocate memory in tests

b5ab18f

Signed-off-by: Luca Della Vedova <[email protected]>

Merge branch 'feature/compressed_images' into luca/inmemory_textures

97b459e

Migrate to image API, minor cleanups

d08554f

Signed-off-by: Luca Della Vedova <[email protected]>

Merge branch 'luca/inmemory_textures' into luca/assimp_sandbox

304cd93

Add PBR materials to inmemory textures

d6c59b1

Signed-off-by: Luca Della Vedova <[email protected]>

Merge branch 'luca/inmemory_textures' into luca/assimp_sandbox

6879e1c

Use correct mesh index to load bones

435fe13

Signed-off-by: Luca Della Vedova <[email protected]>

Add metallic roughness map support

91e32ea

Signed-off-by: Luca Della Vedova <[email protected]>

Merge branch 'main' into luca/assimp_sandbox

d09f892

Signed-off-by: Luca Della Vedova <[email protected]>

Add API to return image data as vector of bytes

ee7fb08

Signed-off-by: Luca Della Vedova <[email protected]>

Update migration guide

7ee1c45

Signed-off-by: Luca Della Vedova <[email protected]>

chapulina deleted the luca/assimp_sandbox branch July 30, 2022 02:01

chapulina mentioned this pull request Jul 30, 2022

Depend on assimp gazebo-release/gz-common5-release#20

Merged

luca-della-vedova mentioned this pull request Aug 1, 2022

Add a test for loading gltf with external textures #409

Merged

9 tasks

This was referenced Aug 2, 2022

[ign to gz] Deduplicate QML files gazebosim/gz-gui#449

Merged

Introduces GZ_MESH_FORCE_ASSIMP flag #403

Merged

onurtore reviewed Aug 4, 2022

View reviewed changes

This was referenced Aug 4, 2022

Implement XDisplacement function for checking x displacement for all nodes #411

Merged

Default name assignment to animations without names #413

Merged

Utilizes return value of AddBvhAnimation func gazebosim/gz-sim#1632

Closed

onurtore reviewed Aug 9, 2022

View reviewed changes

onurtore reviewed Aug 11, 2022

View reviewed changes

onurtore reviewed Aug 22, 2022

View reviewed changes

onurtore mentioned this pull request Aug 22, 2022

Adds dummy animation for the root scene node #429

Closed

8 tasks

This was linked to issues Sep 28, 2022

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

Open

[Meta-ticket][GSoC22] Improving Gazebo’s Mesh Format Support #374

Open

onurtore mentioned this pull request Nov 10, 2022

Collada and Assimp test for default animation names #476

Merged

8 tasks

azeey mentioned this pull request Jun 4, 2024

Support loading meshes in glTF format #173

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use assimp for loading meshes #393

Use assimp for loading meshes #393

luca-della-vedova commented Jul 13, 2022 •

edited

Loading

onurtore left a comment

onurtore Aug 4, 2022

onurtore commented Aug 7, 2022 •

edited

Loading

onurtore Aug 9, 2022

onurtore Aug 9, 2022 •

edited

Loading

onurtore Aug 11, 2022 •

edited

Loading

onurtore Aug 22, 2022 •

edited

Loading

Use assimp for loading meshes #393

Use assimp for loading meshes #393

Conversation

luca-della-vedova commented Jul 13, 2022 • edited Loading

🎉 New feature

Summary

Test it

Checklist

onurtore left a comment

Choose a reason for hiding this comment

onurtore Aug 4, 2022

Choose a reason for hiding this comment

onurtore commented Aug 7, 2022 • edited Loading

onurtore Aug 9, 2022

Choose a reason for hiding this comment

onurtore Aug 9, 2022 • edited Loading

Choose a reason for hiding this comment

onurtore Aug 11, 2022 • edited Loading

Choose a reason for hiding this comment

onurtore Aug 22, 2022 • edited Loading

Choose a reason for hiding this comment

luca-della-vedova commented Jul 13, 2022 •

edited

Loading

onurtore commented Aug 7, 2022 •

edited

Loading

onurtore Aug 9, 2022 •

edited

Loading

onurtore Aug 11, 2022 •

edited

Loading

onurtore Aug 22, 2022 •

edited

Loading