Skip to content

[manual merge] glTF improvements - #16086

Merged
appgurueu merged 2 commits into
luanti-org:masterfrom
appgurueu:fix/gltf-ignore-skinned-transform
Apr 26, 2025
Merged

[manual merge] glTF improvements#16086
appgurueu merged 2 commits into
luanti-org:masterfrom
appgurueu:fix/gltf-ignore-skinned-transform

Conversation

@appgurueu

@appgurueu appgurueu commented Apr 25, 2025

Copy link
Copy Markdown
Contributor

First commit is the trivial bugfix.
Second commit is for testing only, revert before merge: The model used for testing is NC-licensed.
I've also committed the JSON so you can conveniently view this using the glTF validator extension in VS Code.

How to test

Check out this PR, spawn eddie (https://sketchfab.com/3d-models/ai2u-catgirl-eddie-official-game-character-6f4d2278b2ba4c7f86e3a0c5a0491334), confirm that it looks correct:

Screenshot from 2025-04-25 14-44-39

it will not look correct without this PR.

Confirm that other glTF test models still work as expected.

c.f. https://registry.khronos.org/glTF/specs/2.0/glTF-2.0.html#joint-hierarchy:

Only the joint transforms are applied to the skinned mesh; the transform of the skinned mesh node MUST be ignored.

note that Eddie gets a warning in the glTF validator for this due to having non-identity transformed skinned nodes.

@appgurueu appgurueu added Trivial The change is a trivial bug fix, documentation or maintenance change, as per the Git Guidelines Bugfix 🐛 PRs that fix a bug @ Client rendering labels Apr 25, 2025
@appgurueu appgurueu changed the title [manual merge] Fix glTF reader not ignoring parent transforms for skinned meshes [manual merge] glTF improvements Apr 25, 2025
@appgurueu

Copy link
Copy Markdown
Contributor Author

I added a third commit. You can test it by observing that the frog still works.

@appgurueu
appgurueu force-pushed the fix/gltf-ignore-skinned-transform branch from a3276ac to 706dc96 Compare April 25, 2025 21:37

@sfan5 sfan5 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🐸

@sfan5 sfan5 added One approval ✅ ◻️ and removed Trivial The change is a trivial bug fix, documentation or maintenance change, as per the Git Guidelines labels Apr 26, 2025
@appgurueu
appgurueu force-pushed the fix/gltf-ignore-skinned-transform branch from 706dc96 to bd38ce2 Compare April 26, 2025 14:03
@appgurueu
appgurueu merged commit 01e4395 into luanti-org:master Apr 26, 2025
@appgurueu
appgurueu deleted the fix/gltf-ignore-skinned-transform branch April 26, 2025 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants