Animations: storing key frames in typed arrays #13434
Conversation
|
Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s). |
myfreeer
force-pushed
the
compact-animation
branch
from
0102e63
to
ae3e9eb
Compare
|
Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s). |
myfreeer
force-pushed
the
compact-animation
branch
from
ae3e9eb
to
23b0ec6
Compare
|
Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s). |
|
Snapshot stored with reference name: Test environment: To test a playground add it to the URL, for example: https://babylonsnapshots.z22.web.core.windows.net/refs/pull/13434/merge/index.html#WGZLGJ#4600 Links to test babylon tools with this snapshot: https://playground.babylonjs.com/?snapshot=refs/pull/13434/merge To test the snapshot in the playground with a playground ID add it after the snapshot query string: https://playground.babylonjs.com/?snapshot=refs/pull/13434/merge#BCU1XR#0 |
|
Visualization tests for webgl1 have failed. If some tests failed because the snapshots do not match, the report can be found at If tests were successful afterwards, this report might not be available anymore. |
|
Visualization tests for webgl2 have failed. If some tests failed because the snapshots do not match, the report can be found at If tests were successful afterwards, this report might not be available anymore. |
myfreeer
force-pushed
the
compact-animation
branch
from
23b0ec6
to
b3886b6
Compare
|
Visualization tests for webgl1 have failed. If some tests failed because the snapshots do not match, the report can be found at If tests were successful afterwards, this report might not be available anymore. |
|
Visualization tests for webgl2 have failed. If some tests failed because the snapshots do not match, the report can be found at If tests were successful afterwards, this report might not be available anymore. |
myfreeer
force-pushed
the
compact-animation
branch
from
b3886b6
to
b08c8f2
Compare
|
It's quite a mammoth PR! I'm not really proficient with the animation system of Babylon.js, so I'm not sure I can help a lot on the structural/architectural review side... However, I can always review the code itself! Also, I think our animation system will possibly(/probably?) undergo some changes in the next weeks or months, so I wonder if it's the right time for such a big update to the system... cc @bghgary, @sebavan, @deltakosh, @RaananW |
|
Wow! From quickly browsing the changes, it seems like some changes were made to central processes like the gltf loading process. are there any breaking changes here? |
|
Let me convert as a draft during the discussions to prevent any accidental merges |
myfreeer
force-pushed
the
compact-animation
branch
from
b08c8f2
to
05ef3c0
Compare
In gltf loader, a flag |
myfreeer
force-pushed
the
compact-animation
branch
2 times, most recently
from
f23ac1e
to
53de25e
Compare
|
Visualization tests for webgl2 have failed. If some tests failed because the snapshots do not match, the report can be found at If tests were successful afterwards, this report might not be available anymore. |
Keyframes in Babylon.js are stored with array of objects. Some animations exported with millions of key frames, with the object mode, it would take more memory than stored with typed arrays, like Float32Array. Without the object per frame, it would be much harder for features like per-frame interpolation and tangent to be implemented, so this could be an optional feature for those suffering too many keyframes without these advanced features. For AnimationCurveEditor it is hard to fully adapt it to CompactAnimation, maybe the fast way it to convert CompactAnimation to Animation, and replace all possible usage to converted animation. Not supported yet: * CUBICSPLINE animation with tangent * per-frame interpolation or tangent * serializing GLTF animation with quantization and 4-bytes alignment Forum link: https://forum.babylonjs.com/t/animations-storing-key-frames-in-typed-arrays/36566
myfreeer
force-pushed
the
compact-animation
branch
from
53de25e
to
0360be8
Compare
|
Long overdue update. @myfreeer you can not imagine how many discussions you helped us having in the core team regarding this topic :-) and this is for the best !!! Basically, we would like to use a system similar to what you are proposing here for our overall animation system :-) Unfortunately, we are too close to our 6.0 release to integrate those changes now and we would like them to be fully back compatible and working with all our feature set. What I propose is to close this PR and let @Popov72 integrate your changes as part of a broader revamp starting as soon as 6.0 will be released (in a couple months). You can track the issue here: #13534 Big thanks for the great contribution and sorry again we can not integrate immediately, hope it does not disrupt your plans too much. |
Keyframes in Babylon.js are stored with array of objects. Some animations exported with millions of key frames, with the object mode, it would take more memory than stored with typed arrays, like Float32Array.
Without the object per frame, it would be much harder for features like per-frame interpolation and tangent to be implemented, so this could be an optional feature for those suffering too many keyframes without these advanced features.
For AnimationCurveEditor it is hard to fully adapt it to CompactAnimation, maybe the fast way it to convert CompactAnimation to Animation, and replace all possible usage to converted animation.
Not supported yet:
Forum link:
https://forum.babylonjs.com/t/animations-storing-key-frames-in-typed-arrays/36566