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

AssimpImporter: Support multiple meshes per node #68

Closed
wants to merge 1 commit into from

Conversation

ikalevatykh
Copy link
Contributor

If a node has several meshes, creates one Object3D per mesh.

object3DCount() query returns a number of all nodes extended with a number of extra nodes for each additional mesh.
object3DForName() points to the first object containing the first mesh,
object3DName() returns the same name for all objects in the given sequence.
doObject3D() for multi-meshes returns node for first mesh as usual but also returns indices of extra meshes nodes as children.

@mosra mosra added this to TODO in Asset management via automation Sep 24, 2019
@mosra
Copy link
Owner

mosra commented Sep 24, 2019

Wow, you were fast :)

So, if I see correctly, this is the same as I outlined in #66 (comment) (and as documented in TinyGltfImporter), right? If so, can you:

  • add a similar "Mesh import" section to AssimpImporter.h like it was in TinyGltfImporter, so people know the semantics
  • any chance you could add a test case for this? In an Ideal World™, the same glTF file as is used to test this exact thing in TinyGltfImporter could be used here (and the same test code as well, so basically copy the test part of da58734 over), but realistically you might hit a bunch of crash bugs with Assimp :/ Then the second easiest would be converting that glTF file to Collada somehow and then letting Assimp load that.

@mosra mosra moved this from TODO to In progress in Asset management Oct 8, 2019
@mosra
Copy link
Owner

mosra commented Oct 15, 2019

@ikalevatykh any chance you could have a look at the tests & docs? It would be great to have this in for the release, but I probably won't be able to find time to add those myself.

Thank you!

@ikalevatykh
Copy link
Contributor Author

@mosra I had to switch to another task but I will do it Thursday

@mosra
Copy link
Owner

mosra commented Oct 15, 2019

You're great, thanks!

@mosra
Copy link
Owner

mosra commented Mar 22, 2020

Merged a variant of this in #81. Thanks for providing the initial version! 👍

@mosra mosra closed this Mar 22, 2020
Asset management automation moved this from In progress to Done Mar 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants