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

TinyGltfImporter: Import Weights and JointIds mesh attributes #84

Closed
wants to merge 2 commits into from

Conversation

Squareys
Copy link
Contributor

@Squareys Squareys commented May 8, 2020

Hi @mosra !

Building on top of mosra/magnum#441, here's the import of the new mesh attributes in TinyGltfImporter.

Best,
Jonathan

TODO

  • Docs: Advertise support
  • Tests
  • What are "skins"?
  • Is this sufficient to import skinned mesh animations?

@Squareys
Copy link
Contributor Author

Squareys commented May 8, 2020

From gitter to archive here:

It appears that in addition to the Skin Data, we will need to import some form of "Skin" or "Skeleton", which requires extension of the Trade:: Apis.
I.e. this could be done with SkeletonData / SkinData or a Trade::SkinnedMeshObjectData
There are inverseBindMatrices that are stored per Bone in there (Some Matrix accessor in gltf), a reference to the root node and the nodes that are used as bones/joints
Since ObjectType::Light will redirect to LightData, I assume we'd have ObjectType::SkinnedMesh and redirect to skin, mesh and possibly nodes of the skeleton

@@ -1171,6 +1171,41 @@ Containers::Optional<MeshData> TinyGltfImporter::doMesh(const UnsignedInt id, Un
return Containers::NullOpt;
}

/* Joint attribute ends with _0, _1 ... */
} else if(Utility::String::beginsWith(attribute.first, "JOINTS")) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO for testing: the code relies on the attributes being sorted (so JOINTS_0 before JOINTS_1) and without gaps. I think sorting is ensured implicitly due to how JSON dict data is stored, but the gaps need to be checked.

Comment on lines +1183 to +1184
if(!(accessor.componentType == TINYGLTF_COMPONENT_TYPE_UNSIGNED_BYTE) &&
!(accessor.componentType == TINYGLTF_COMPONENT_TYPE_UNSIGNED_SHORT)) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if(!(accessor.componentType == TINYGLTF_COMPONENT_TYPE_UNSIGNED_BYTE) &&
!(accessor.componentType == TINYGLTF_COMPONENT_TYPE_UNSIGNED_SHORT)) {
if(!(accessor.componentType == TINYGLTF_COMPONENT_TYPE_UNSIGNED_BYTE && !accessor.normalized) &&
!(accessor.componentType == TINYGLTF_COMPONENT_TYPE_UNSIGNED_SHORT && !accessor.normalized)) {

IDs can't be normalized. Also needs a test ;)

Signed-off-by: Squareys <[email protected]>
@mosra mosra added this to the 2023.0a milestone Feb 18, 2023
@mosra mosra added the scrapped label Feb 18, 2023
@mosra
Copy link
Owner

mosra commented Feb 18, 2023

Huh, what's this, completely forgot this was even attempted in the old plugin 😆 Two complete plugin reimplementations later, this is done in GltfImporter in cf9c300.

@mosra mosra closed this Feb 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

2 participants