Skip to content

Commit

Permalink
Merge pull request #14385 from donmccurdy/animation-optimize-optout-v3
Browse files Browse the repository at this point in the history
Animation: Do not validate/optimize by default.
  • Loading branch information
mrdoob committed Jul 31, 2018
2 parents 0b340e7 + 63e3a85 commit 251ebc8
Show file tree
Hide file tree
Showing 7 changed files with 63 additions and 26 deletions.
11 changes: 8 additions & 3 deletions docs/api/animation/AnimationClip.html
Expand Up @@ -66,23 +66,28 @@ <h3>[property:String uuid]</h3>
<h2>Methods</h2>


<h3>[method:AnimationClip optimize]()</h3>
<h3>[method:this optimize]()</h3>
<p>
Optimizes each track by removing equivalent sequential keys (which are common in morph target
sequences).
</p>

<h3>[method:null resetDuration]()</h3>
<h3>[method:this resetDuration]()</h3>
<p>
Sets the [page:.duration duration] of the clip to the duration of its longest
[page:KeyframeTrack].
</p>

<h3>[method:AnimationClip trim]()</h3>
<h3>[method:this trim]()</h3>
<p>
Trims all tracks to the clip's duration.
</p>

<h3>[method:Boolean validate]()</h3>
<p>
Performs minimal validation on each track in the clip. Returns true if all tracks are valid.
</p>


<h2>Static Methods</h2>

Expand Down
7 changes: 3 additions & 4 deletions docs/api/animation/AnimationMixer.html
Expand Up @@ -55,10 +55,9 @@ <h3>[method:AnimationAction clipAction]([param:AnimationClip clip], [param:Objec
from the mixer's default root. The first parameter can be either an [page:AnimationClip] object
or the name of an AnimationClip.<br /><br />

If an action fitting these parameters doesn't yet exist, it will be created by this method.<br /><br />

Note: Calling this method several times with the same parameters returns always the same clip
instance.
If an action fitting the clip and root parameters doesn't yet exist, it will be created by
this method. Calling this method several times with the same clip and root parameters always
returns the same clip instance.
</p>

<h3>[method:AnimationAction existingAction]([param:AnimationClip clip], [param:Object3D optionalRoot])</h3>
Expand Down
20 changes: 11 additions & 9 deletions docs/api/animation/KeyframeTrack.html
Expand Up @@ -202,13 +202,12 @@ <h3>[method:null InterpolantFactoryMethodSmooth]( [link:https://developer.mozill
created automatically.
</p>

<h3>[method:null optimize]()</h3>
<h3>[method:this optimize]()</h3>
<p>
Removes equivalent sequential keys, which are common in morph target sequences. Called
automatically by the constructor.
Removes equivalent sequential keys, which are common in morph target sequences.
</p>

<h3>[method:null scale]()</h3>
<h3>[method:this scale]()</h3>
<p>
Scales all keyframe times by a factor.<br /><br />

Expand All @@ -217,26 +216,29 @@ <h3>[method:null scale]()</h3>
[page:AnimationClip.CreateFromMorphTargetSequence animationClip.CreateFromMorphTargetSequence]).
</p>

<h3>[method:null setInterpolation]( [param:Constant interpolationType] )</h3>
<h3>[method:this setInterpolation]( [param:Constant interpolationType] )</h3>
<p>
Sets the interpolation type. See [page:Animation Animation Constants] for choices.
</p>

<h3>[method:null shift]( [param:Number timeOffsetInSeconds] )</h3>
<h3>[method:this shift]( [param:Number timeOffsetInSeconds] )</h3>
<p>
Moves all keyframes either forward or backward in time.
</p>


<h3>[method:null trim]( [param:Number startTimeInSeconds], [param:Number endTimeInSeconds] )</h3>
<h3>[method:this trim]( [param:Number startTimeInSeconds], [param:Number endTimeInSeconds] )</h3>
<p>
Removes keyframes before *startTime* and after *endTime*,
without changing any values within the range [*startTime*, *endTime*].
</p>

<h3>[method:null validate]()</h3>
<h3>[method:Boolean validate]()</h3>
<p>
Performs minimal validation on the tracks. Returns true if valid.
</p>

<p>
Performs minimal validation on the tracks. Called automatically by the constructor.<br /><br />
This method logs errors to the console, if a track is empty, if the [page:.valueSize value size] is not valid, if an item
in the [page:.times times] or [page:.values values] array is not a valid number or if the items in the *times* array are out of order.
</p>
Expand Down
18 changes: 16 additions & 2 deletions src/animation/AnimationClip.js
Expand Up @@ -31,8 +31,6 @@ function AnimationClip( name, duration, tracks ) {

}

this.optimize();

}

function getTrackTypeForValueTypeName( typeName ) {
Expand Down Expand Up @@ -407,6 +405,8 @@ Object.assign( AnimationClip.prototype, {

this.duration = duration;

return this;

},

trim: function () {
Expand All @@ -421,6 +421,20 @@ Object.assign( AnimationClip.prototype, {

},

validate: function () {

var valid = true;

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

valid = valid && this.tracks[ i ].validate();

}

return valid;

},

optimize: function () {

for ( var i = 0; i < this.tracks.length; i ++ ) {
Expand Down
7 changes: 3 additions & 4 deletions src/animation/KeyframeTrack.js
Expand Up @@ -30,9 +30,6 @@ function KeyframeTrack( name, times, values, interpolation ) {

this.setInterpolation( interpolation || this.DefaultInterpolation );

this.validate();
this.optimize();

}

// Static methods
Expand Down Expand Up @@ -157,12 +154,14 @@ Object.assign( KeyframeTrack.prototype, {
}

console.warn( 'THREE.KeyframeTrack:', message );
return;
return this;

}

this.createInterpolant = factoryMethod;

return this;

},

getInterpolation: function () {
Expand Down
6 changes: 6 additions & 0 deletions test/unit/src/animation/AnimationClip.tests.js
Expand Up @@ -72,6 +72,12 @@ export default QUnit.module( 'Animation', () => {

} );

QUnit.todo( "validate", ( assert ) => {

assert.ok( false, "everything's gonna be alright" );

} );

} );

} );
20 changes: 16 additions & 4 deletions test/unit/src/animation/KeyframeTrack.tests.js
Expand Up @@ -4,6 +4,7 @@
/* global QUnit */

import { KeyframeTrack } from '../../../../src/animation/KeyframeTrack';
import { NumberKeyframeTrack } from '../../../../src/animation/tracks/NumberKeyframeTrack';

export default QUnit.module( 'Animation', () => {

Expand Down Expand Up @@ -96,15 +97,26 @@ export default QUnit.module( 'Animation', () => {

} );

QUnit.todo( "validate", ( assert ) => {
QUnit.test( 'validate', ( assert ) => {

assert.ok( false, "everything's gonna be alright" );
var validTrack = new NumberKeyframeTrack( '.material.opacity', [ 0, 1 ], [ 0, 0.5 ] );
var invalidTrack = new NumberKeyframeTrack( '.material.opacity', [ 0, 1 ], [ 0, NaN ] );

assert.ok( validTrack.validate() );
assert.notOk( invalidTrack.validate() );

} );

QUnit.todo( "optimize", ( assert ) => {
QUnit.test( 'optimize', ( assert ) => {

assert.ok( false, "everything's gonna be alright" );
var track = new NumberKeyframeTrack( '.material.opacity', [ 0, 1, 2, 3, 4 ], [ 0, 0, 0, 0, 1 ] );

assert.equal( track.values.length, 5 );

track.optimize();

assert.smartEqual( Array.from( track.times ), [ 0, 3, 4 ] );
assert.smartEqual( Array.from( track.values ), [ 0, 0, 1 ] );

} );

Expand Down

0 comments on commit 251ebc8

Please sign in to comment.