Browse Source

Merge pull request #14385 from donmccurdy/animation-optimize-optout-v3

Animation: Do not validate/optimize by default.
Mr.doob 7 years ago
parent
commit
251ebc873f

+ 8 - 3
docs/api/animation/AnimationClip.html

@@ -66,23 +66,28 @@
 		<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>
 

+ 3 - 4
docs/api/animation/AnimationMixer.html

@@ -55,10 +55,9 @@
 			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>

+ 11 - 9
docs/api/animation/KeyframeTrack.html

@@ -202,13 +202,12 @@
 			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 />
 
@@ -217,26 +216,29 @@
 			[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>

+ 16 - 2
src/animation/AnimationClip.js

@@ -31,8 +31,6 @@ function AnimationClip( name, duration, tracks ) {
 
 	}
 
-	this.optimize();
-
 }
 
 function getTrackTypeForValueTypeName( typeName ) {
@@ -407,6 +405,8 @@ Object.assign( AnimationClip.prototype, {
 
 		this.duration = duration;
 
+		return this;
+
 	},
 
 	trim: function () {
@@ -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 ++ ) {

+ 3 - 4
src/animation/KeyframeTrack.js

@@ -30,9 +30,6 @@ function KeyframeTrack( name, times, values, interpolation ) {
 
 	this.setInterpolation( interpolation || this.DefaultInterpolation );
 
-	this.validate();
-	this.optimize();
-
 }
 
 // Static methods
@@ -157,12 +154,14 @@ Object.assign( KeyframeTrack.prototype, {
 			}
 
 			console.warn( 'THREE.KeyframeTrack:', message );
-			return;
+			return this;
 
 		}
 
 		this.createInterpolant = factoryMethod;
 
+		return this;
+
 	},
 
 	getInterpolation: function () {

+ 6 - 0
test/unit/src/animation/AnimationClip.tests.js

@@ -72,6 +72,12 @@ export default QUnit.module( 'Animation', () => {
 
 		} );
 
+		QUnit.todo( "validate", ( assert ) => {
+
+			assert.ok( false, "everything's gonna be alright" );
+
+		} );
+
 	} );
 
 } );

+ 16 - 4
test/unit/src/animation/KeyframeTrack.tests.js

@@ -4,6 +4,7 @@
 /* global QUnit */
 
 import { KeyframeTrack } from '../../../../src/animation/KeyframeTrack';
+import { NumberKeyframeTrack } from '../../../../src/animation/tracks/NumberKeyframeTrack';
 
 export default QUnit.module( 'Animation', () => {
 
@@ -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 ] );
 
 		} );