-
-
Notifications
You must be signed in to change notification settings - Fork 35.5k
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
Animation Tracks: Convert to es6 classes #20013
Conversation
return new QuaternionLinearInterpolant( this.times, this.values, this.getValueSize(), result ); | ||
|
||
}, | ||
|
||
InterpolantFactoryMethodSmooth: undefined // not yet implemented |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I noticed this pattern where prototype methods are undefined to drive fallback logic in base KeyframeTrack
. It feels kind of wrong to mutate the KeyframeTrack interface, although I guess if these can be considered internal it doesn't matter. No need to change now, just pointing it out as another candidate for some cleanup down the road.
Object.assign( KeyframeTrack.prototype, { | ||
|
||
TimeBufferType: Float32Array, | ||
|
||
ValueBufferType: Float32Array, | ||
|
||
DefaultInterpolation: InterpolateLinear, | ||
|
||
} ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I debated whether or not to preserve these prototype properties. They should reduce memory footprint with a large number of animation tracks, though I don't know if that matters in real world usage. They seem better suited as static props than instance props? Anyways, let me know what direction is good and I can tweak this PR as needed.
Honestly, all of the inheritance doesn't seem that useful. In the long run maybe the track system could just be refactored with a more data-driven approach.
Thanks! |
Btw, seems like |
It seems this PR has broken https://raw.githack.com/mrdoob/three.js/dev/editor/index.html The following runtime error occurs:
At this place: three.js/src/animation/KeyframeTrack.js Line 32 in 4d90e23
|
Fiddle demonstrating the bug: https://jsfiddle.net/th1m48a9/ @mrdoob Since the release is tomorrow, this is in some sense a blocker. If not fixed in time, it's better to revert this PR. |
Supports #19986