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

KHR animation pointer #12767

Merged
merged 16 commits into from
Aug 2, 2022
Merged

Conversation

pandaGaume
Copy link
Contributor

@pandaGaume pandaGaume commented Jul 19, 2022

This is the first version of the KHR_animation_pointer for the GLTF loader.
The basic idea behind the code is to turn every kind of animation into a pointer, in order to keep only one code path for all. This has been tested with success with all the samples provided as part of the Khronos proposal.
Even if functional, the code is still not very clean and many ehancement must be done in order to reach a certain level of quality. However, calendar issue drive us to put this onto PR rapidely.
AnimateAllTheThings.zip
This can be tested with the file above, which encompass all the cases supported by the KHR_animation_pointer.
The main changes are located into 3 files.
GLTFLoader has been simplified by moving all the animation related code to the KHR_animation_pointer.ts where the extension lies. KHR_animation_pointer.map.ts contains the pointer tree mapping and miscellaneous function to serve the pointer parsing and logic.
All the other change are to support indirection between GLTF entities and Babylon ones. This is subject to discussion, because not very "clean".
May close #12544

@pandaGaume pandaGaume marked this pull request as draft July 19, 2022 20:54
@pandaGaume pandaGaume requested a review from bghgary July 19, 2022 20:59
@azure-pipelines
Copy link

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

@azure-pipelines
Copy link

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

@azure-pipelines
Copy link

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

@azure-pipelines
Copy link

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

@azure-pipelines
Copy link

@azure-pipelines
Copy link

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

@azure-pipelines
Copy link

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

@azure-pipelines
Copy link

@azure-pipelines
Copy link

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

@azure-pipelines
Copy link

@bghgary
Copy link
Contributor

bghgary commented Jul 21, 2022

One last comment to resolve, then it looks good to me. We should also add tests and add this extension to the inspector. I can take care of this later.

@azure-pipelines
Copy link

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

@azure-pipelines
Copy link

@bghgary bghgary marked this pull request as ready for review August 2, 2022 16:07
@bghgary bghgary merged commit 013d803 into BabylonJS:master Aug 2, 2022
RaananW pushed a commit that referenced this pull request Dec 9, 2022
Former-commit-id: 21b271c7ffb657d84726bca28e1bad194145da26
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.

Implement KHR_animation_pointer
2 participants