Skip to content

Conversation

@Fyrestar
Copy link

To create a new clip from a existing from-to frame with optional new name. Some tools only export animations as a single clip.

To create a new clip from a existing from-to frame with optional new name.
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.

Related: #13430

I'm OK with merging either of the PRs; mine would need a rebase anyway. Do you have an example model with multiple subclips, to test that this works as expected and gives smoothly-looping clips? There are some edge cases to be aware of — like:

-------------clip1-------------↓-------------clip2-------------↓-------------clip3-------------|
track1 ----|----|----|----|----|----|----|----|----|----|----|----|----|----|----|----|----|---|
track2----|------------------------------------------------------------------------------------|
track3----|----|----|----|----|-------------------------------------|----|----|----|----|----|-|

... where track2 and track3 actually do have values in clip2, but no keyframes. For a correctly authored animation that wouldn't be a problem, but automatic optimizations at export or import can mess things up...


var track = this.tracks[i];

var stride = track.values.length / track.times.length;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would use track.getValueSize() here.


tracks.push(
new track.constructor( track.name, times, values, track.createInterpolant )
);
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's possible that some of the tracks won't have any keyframes in the sliced range, so it might be best to keep empty tracks out of the new clip.


}

duration = Math.max(times[ to ], duration);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Minor: Duration is computed automatically, so it might be simpler just to pass it undefined in the constructor.


slice: function(from, to, name) {

var frames = to - from, duration = 0;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Code style: We use hard tabs, not soft tabs.


},

slice: function(from, to, name) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Code style: Extra spaces are necessary after opening bracket and before closing bracket slice: function( from, to, name ) {

https://github.com/mrdoob/three.js/wiki/Mr.doob's-Code-Style%E2%84%A2#blocks


for ( var i = 0, l = this.tracks.length; i < l; i ++ ) {

var track = this.tracks[i];
Copy link
Collaborator

Choose a reason for hiding this comment

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

Code style: Extra spaces are necessary after opening bracket and before closing bracket.

https://github.com/mrdoob/three.js/wiki/Mr.doob's-Code-Style%E2%84%A2#arrays

@Fyrestar
Copy link
Author

Fyrestar commented Jan 21, 2019

When i export animations with optimizations it will basically remove the start/end keyframe of the desired sequence of a track when smoothing it out in some cases. I'll change it to compute the values at these times,

@donmccurdy donmccurdy mentioned this pull request Jan 30, 2019
3 tasks
@Mugen87
Copy link
Collaborator

Mugen87 commented Sep 21, 2019

Thanks for the work here but I think it's better to focus on #13430 (which is already tested and approved).

@Mugen87 Mugen87 closed this Sep 21, 2019
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.

4 participants