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

GLTFExporter: Add MAT3 accessor support. #25426

Merged
merged 1 commit into from
Feb 6, 2023

Conversation

paugit
Copy link
Contributor

@paugit paugit commented Feb 3, 2023

GLTFExporter generates invalid GLTFs if the source scene contains glTF models with MAT3 accessor type.

These type of accessors are generated by Houdini for example. GLTFImporter does know this type but the GLTFExporter does not and it will omit the accessor type from the generated glTF. The generated file will not open in any known 3d software like Blender, Houdini and so on.

This small PR will fix the issue.

@donmccurdy
Copy link
Collaborator

These type of accessors are generated by Houdini for example...

Would you be able to share an example? I'm not sure I've ever seen MAT3 accessors in glTF, are they used for vertex attributes, animation samplers, or something else?

@paugit
Copy link
Contributor Author

paugit commented Feb 4, 2023

Can't give the actual gltf that made me notice the issue but here is a part of the original that will be invalid after export.

{ "bufferView": 8, "componentType": 5126, "count": 6043, "type": "MAT3", "min": [ 1, 0, 0, 0, 1, 0, 0, 0, 1 ], "max": [ 1, 0, 0, 0, 1, 0, 0, 0, 1 ], "byteOffset": 0 },

I can ask if our 3d artist knows what type of data causes it. Our model is quite complex skinned mesh with blend shapes but with no animation. I can also ask if he could replicate the MAT3 accessor with some smaller model that I can give to you.

@paugit
Copy link
Contributor Author

paugit commented Feb 4, 2023

If you check glTF specs (https://registry.khronos.org/glTF/specs/2.0/glTF-2.0.html#_accessor_type) you can see that it should be supported. You are also missing MAT2 but for that you would need to do a bit more magic because it has the same item count as VEC4. Luckily we don't need MAT2 support so I would be really happy if we could get just this oneliner in fast :)

Copy link
Collaborator

@donmccurdy donmccurdy left a comment

Choose a reason for hiding this comment

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

I'll assume it's a MAT3 vertex attribute, and not an animation channel or something else accessors could be used for... I've never seen these used in any glTF file, and in general it's hard to maintain or test code paths that we don't know how to reach, so any other information would be really helpful in ensuring this doesn't break in the future.

But it's a simple enough change for sure, and so no objection to the PR here!

@mrdoob mrdoob added this to the r150 milestone Feb 6, 2023
@mrdoob mrdoob merged commit 9e9466d into mrdoob:dev Feb 6, 2023
@paugit
Copy link
Contributor Author

paugit commented Feb 8, 2023

Here is a simple test asset exported from Houdini that has MAT3. There is a way to remove it though but by default it is exported. Sorry for the big comment, I guess there is no other way to attach the file here permanently.

mat3_test.gltf.zip

@donmccurdy
Copy link
Collaborator

Thanks @paugit! Seems to be the _transform custom vertex atribute.

Screenshot 2023-02-08 at 9 27 49 AM

@Mugen87 Mugen87 changed the title Fix GLTFExporter MAT3 accessor support GLTFExporter: Fix MAT3 accessor support. Feb 19, 2023
@Mugen87 Mugen87 changed the title GLTFExporter: Fix MAT3 accessor support. GLTFExporter: Add MAT3 accessor support. Feb 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants