[WIP] Add position/quaternion/scale cache into Object3D

[takahirox]

This PR adds position/quaternion/scale cache into Object3D. This is one of the ideas, discussed in #14138, to skip unnecessary matrix update in .updateMatrixWorld().

This PR is still WIP because it could have performance penalty in some cases. But you can see how small the change is from this PR.

[takahirox]

As I posted #14138 (comment) I'd like to suggest we go this cache approach so far, and we think of the other options if we realize performance gain isn't good enough or performance penalty for applications including a lot of dynamic objects isn't small.

So I removed "[WIP]" from the title of this PR.

[arpu]

works for me, but cannot say what the performance win is

[WestLangley]

@Mugen87 Can you please explain what your approval of this PR means?

Do you approve the implementation -- meaning you tested it and you believe it works correctly?

Do you approve the performance difference in memory and speed? Wasn't performance worse in some cases?

[WestLangley]

This looks like a failure point with this PR...

var a = new THREE.Object3D();
var b = new THREE.Object3D();
var v = new THREE.Vector3();

a.position.set( 1, 2, 3 );
a.updateMatrix();
v.setFromMatrixPosition( a.matrix ); // 1, 2, 3

b.copy( a );
b.position.set( 0, 0, 0 );
b.updateMatrix();
v.setFromMatrixPosition( b.matrix ); // 1, 2, 3 -- oops

[takahirox]

Good catch. To pass your test case, I needed to update ._position/quaternion/scaleCache in .copy().

But came up with a more serious problem. Users can directly touch object.position/quaternion/scale/matrix and make the same problem. For example, based on your test case

var a = new THREE.Object3D();
var b = new THREE.Object3D();
var v = new THREE.Vector3();

a.position.set( 1, 2, 3 );
a.updateMatrix();
v.setFromMatrixPosition( a.matrix ); // 1, 2, 3

b.matrix.copy( a.matrix );
b.position.set( 0, 0, 0 );
b.updateMatrix();
v.setFromMatrixPosition( b.matrix ); // 1, 2, 3 -- oops

Thinking if there's any solutions to this issue...

[Mugen87]

Do you approve the performance difference in memory and speed? Wasn't performance worse in some cases?

@takahirox already made statistics about this approach in #14138. TBH, I trust his data.

Besides, caching values like this is a typical approach in engine design. I was just not aware about your mentioned side effect in three.js.

[Mugen87]

@takahirox Have you tried to init the values in the cache differently? For example with Infinity or NaN:

this._positionCache = new Vector3( Infinity, Infinity, Infinity );

Notice that e.g. Box3 is also initialized with Infinity values.

[takahirox]

@Mugen87 I don't think it solves the problem because this problem can happen any time, not only when ._*Cache has initial values.

More simply

b.updateMatrix();  // local matrix is updated from the .position/quaternion/scale and
                   // they are copied to ._*Cache 
b.matrix.copy( a.matrix ); // manually update local matrix
b.updateMatrix();  // local matrix should be calculated from the
                   //  .position/quaternion/scale but the matrix remains the same
                   //  because .position/quaternion/scale has the same values as
                   //  ._*Cache values and matrix.compose() inside is skipped

[Mugen87]

It seems it's necessary to detect changes to matrix in some way. This makes the whole approach more complicated though since you need some sort of dirty flag or proxy mechanism 😞

If the matrix property would not be direct accessible but via an interface, things would be easier, too. But this is a design limitation of the library and can't be changed.

[takahirox]

Added "[WIP]" to the title again so far.

[takahirox]

Realized that the mechanism we needed on the current API is the one detecting if updated local matrix hasn't changed or if .position/quaternion/scale are synched with local matrix rather than the one detecting if .position/quaternion/scale haven't changed.

So, some ideas I quickly came up with without Object.defineProperties or Proxy are

1. Adding local matrix cache.

	updateMatrix: function () {

		if ( ! this._positionCache.equals( this.position ) ||
			! this._quaternionCache.equals( this.quaternion ) ||
			! this._scaleCache.equals( this.scale ) ||
			! this._matrixCache.equals( this.matrix ) ) {

			this.matrix.compose( this.position, this.quaternion, this.scale );

			this.matrixWorldNeedsUpdate = true;

			this._positionCache.copy( this.position );
			this._quaternionCache.copy( this.quaternion );
			this._scaleCache.copy( this.scale );
			this._matrixCache.copy( this.matrix );

		}

	},

2. Check if .position/quaternion/scale is synched by using matrix.decompose

	updateMatrix: function () {

		this.matrix.decompose( tmpPosition, tmpQuaternion, tmpScale );

		if ( ! this.position.equals( tmpPosition ) ||
			! this.quaternion.equals( tmpQuaternion ) ||
			! this.scale.equals( tmpScale ) {

			this.matrix.compose( this.position, this.quaternion, this.scale );

			this.matrixWorldNeedsUpdate = true;

		}

	},

3. Check if local matrix values are updated

	updateMatrix: function () {

		tmpMatrix.compose( this.position, this.quaternion, this.scale );

		if ( ! this.matrix.equals( tmpMatrix ) ) {

			this.matrix.copy( tmpMatrix );
			this.matrixWorldNeedsUpdate = true;

		}

	},

But I don't think they are performant. Probably there is no simple solution on our APIs...?

[Mugen87]

I think this is one reason why certain engines don't grant direct access to matrices like the world matrix. Or they make these properties read only. This makes it easier to control how and when to execute an update.

Instead you use methods like .getWorldMatrix() (from Babylon.js) or a read-only property like localToWorldMatrix (from Unity) to retrieve it. AFAIK, Unity does not even provide access to the local matrix. Users have to compose it themselves if they need it via Matrix4x4.TRS. I think this also true for Babylon.js.

We might need an API change to handle caching properly.

[takahirox]

Yeah I think so too, we need to change API if we'd like to introduce cache. But I can't imagine how much this change requires to edit core code, example code, and user code...

[takahirox]

If we don't change the API for flexibility, maybe this type of optimization should be done in user side? For example users can define their updateMatrixWorld() if they can confirm they don't touch local matrix.

function updateMatrix( object ) {

	if ( object.userData.cache === undefined ) {

		object.userData.cache = {
			position: new THREE.Vector3( NaN, NaN, NaN ),
			quaternion: new THREE.Quaternion( NaN, NaN, NaN, NaN ),
			scale: new THREE.Vector3( NaN, NaN, NaN )
		};

	}

	if ( ! object.userData.cache.position.equals( object.cache ) ||
		! object.userData.cache.quaternion.equals( object.quaternion ) ||
		! object.userData.cache.scale.equals( object.scale ) ) {

		object.matrix.compose( object.position, object.quaternion, object.scale );

		object.matrixWorldNeedsUpdate = true;

		object.userData.cache.position.copy( object.position );
		object.userData.cache.quaternion.copy( object.quaternion );
		object.userData.cache.scale.copy( object.scale );

	}

}

function updateMatrixWorld( object, force ) {

	if ( object.matrixAutoUpdate ) updateMatrix( object );

	if ( object.matrixWorldNeedsUpdate || force ) {

		if ( object.parent === null ) {

			object.matrixWorld.copy( object.matrix );

		} else {

			object.matrixWorld.multiplyMatrices( object.parent.matrixWorld, object.matrix );

		}

		object.matrixWorldNeedsUpdate = false;

		force = true;

	}

	// update children

	var children = object.children;

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

		updateMatrixWorld( children[ i ], force );

	}

}

var scene = new THREE.Scene();
scene.autoUpdate = false;
scene.onBeforeRender = function () {

	updateMatrixWorld( scene );

};

[Mugen87]

This of course an option 👍

But in general I have to admit that a caching approach is not suitable to the flexibel API or three.js. It's just not consistent. With a caching system, you would have to declare Object3D.matrix as read-only or remove it completely from the public API (that's what I would prefer). You can then also remove Object3D.matrixAutoUpdate. It think this is actually a cleaner design since position, rotation and scale should be the only interface for changing/accessing the local transformation of an object. But as you said, changes like this are just not doable without breaking code.

[Mugen87]

Reminds me at introducing Object3D.forward (#14703). Would be super useful but unfortunately a breaking change.

[takahirox]

@Mugen87 Could you remove approval from this PR for clear state?

WIP

[Mugen87]

Done^^

[arpu]

@takahirox how can i use this #15706 (comment) , with aframe on user side?

[gkjohnson]

To handle the case the @WestLangley brought up would it make sense to add a force parameter to the updateMatrix function that defaults to true so it's backwards compatible?

If force is true then it skips the cache check and always updates the matrix. My general expectation is that if matrixAutoUpdate is set to false then the user expects to handle the matrix manipulation themselves.

If it's important to handle the case where matrixAutoUpdate changes from false to true then maybe when that flag changes we can recalculate the matrix to make sure it's synchronized.

[gkjohnson]

@takahirox Also this idea of drop-in replacements for the Object3D class is really interesting and something I've been considering for my own projects. It might complicate things but being able to specify a different type of Object3D base class depending on how you are planning to use the library that could enable different optimizations could be pretty nice.

[speigg]

@gkjohnson A default force=true parameter on updateMatrix() sounds like a great backwards-compatible solution.

[Mugen87]

see #14138 (comment)