Remove force flag set from Object3D.updateMatrixWorld(), which makes Object3D.matrixWorldAutoUpdate useless

[DolphinIQ]

Remove force flag set from Object3D.updateMatrixWorld(), which makes Object3D.matrixWorldAutoUpdate useless

Motivation

I am setting object.matrixWorldAutoUpdate = false; in my app and updating the world matrices manually. However due to the issue, Object3D.matrixWorldAutoUpdate simply doesn't work.

Description

At the moment Object3D.matrixWorldAutoUpdate flag doesn't do anything. Its true by default (which is why it hasnt been caught yet I assume), but if you set it to false, the object will still have its world matrix recalculated every frame. Why? Because of force = true inside Object3D.updateMatrixWorld().

The Problem

So what happens?

1. Every frame WebGLRenderer.render() calls if ( scene.matrixWorldAutoUpdate === true ) scene.updateMatrixWorld();
2. Then scene.updateMatrixWorld() calls if ( this.matrixAutoUpdate ) this.updateMatrix();
3. scene.updateMatrix() composes the local matrix and sets this.matrixWorldNeedsUpdate = true;
4. Back inside scene.updateMatrixWorld the condition is called: if ( this.matrixWorldNeedsUpdate || force ) {, which will run because of point 3.
5. World matrix is calculated and most importantly for this PR: force = true; is called (for what reason, I do not know)
6. Next scene updates its children. How does it determine whether they should be updated? The check is performed:
   if ( child.matrixWorldAutoUpdate === true || force === true ) { - HERE WE SEE child.matrixWorldAutoUpdate DOES NOT MATTER! because force is true.
7. Scene calls child.updateMatrixWorld( force ); updating the entire scene graph, now with force flag set to true SO FROM NOW ON IT WILL ALWAYS BE TRUE FOR EVERY OBJECT IN THE SCENE GRAPH! And since it will always be passed down as true, the check from point 6. if ( child.matrixWorldAutoUpdate === true || force === true ) { will always run, making child.matrixWorldAutoUpdate not matter at all!

Here is a rundown in the Object3D matrix updates code, with steps above pointed out in the comments:

    updateMatrix() {
        this.matrix.compose( this.position, this.quaternion, this.scale );
        this.matrixWorldNeedsUpdate = true; // <-- 3rd point
    }

    updateMatrixWorld( force ) {
                // on 1st run "this" is Scene and "force" is false

        if ( this.matrixAutoUpdate ) this.updateMatrix(); // <-- 2nd point - sets 
        // `this.matrixWorldNeedsUpdate = true` which makes the next condition always run

        if ( this.matrixWorldNeedsUpdate || force ) { // <-- 4th point - `matrixWorldNeedsUpdate` is set
        // in `updateMatrix()` above, so condition always happens

            if ( this.parent === null ) {

                this.matrixWorld.copy( this.matrix );

            } else {

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

            }

            this.matrixWorldNeedsUpdate = false;

            force = true; // <-- 5th point - "force" is now true and will ALWAYS 
            // be true throughout this `updateMatrixWorld` traversal

        }

        // update children

        const children = this.children;

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

            const child = children[ i ];

            if ( child.matrixWorldAutoUpdate === true || force === true ) { // <-- 6th point - 
            // `child.matrixWorldAutoUpdate` now doesnt matter because `force` is true

                child.updateMatrixWorld( force ); // <-- 7th point - and now `force` will ALWAYS be true 
                // throughout this entire scene graph `updateMatrixWorld` traversal

            }

        }

    }

We see that in the current configuration as long as the root Scene object updates its local matrix, ALL of its descendants will have their world matrices updated, whether they like it or not (regardless of child.matrixWorldAutoUpdate. Here is JSfiddle proof: https://jsfiddle.net/fpb09am8/
Its very easy to test. Just do:

// Setup
const parent = new THREE.Mesh( geometry, material );

const child= mesh.clone();
child.matrixWorldAutoUpdate = false;
child.position.x = 2;

scene.add( parent);
parent.add( child );

// Render loop
parent.rotation.y = timeElapsed / 1000;

Here we see that even though child doesnt want to have its world matrix updated automatically (child.matrixWorldAutoUpdate = false) it still happens because the scene will force it on all its descendants as shown in the 7 points above.

Solution

Remove force = true; from inside Object3D.updateMatrixWorld. It is setting itself to true by itself, making auto update checks ineffective. It is confusing if you set force to false (or leave undefined) because it will turn to true in just 1 generation by itself anyway. It is not clear at all that its basically always true.
In my opinion force should be an external flag set by the users if they want it.

Important tests note!

Running tests with force = true; removed, causes one failed test:

not ok 568 Core > Object3D > updateMatrixWorld
  ---
  message: "Updating parent world matrix has effect to children world matrices even if children local matrices aren't changed"
  severity: failed
  actual: "[\n  1,\n  0,\n  0,\n  0,\n  0,\n  1,\n  0,\n  0,\n  0,\n  0,\n  1,\n  0,\n  4,\n  5,\n  6,\n  1\n]"
  expected: "[\n  1,\n  0,\n  0,\n  0,\n  0,\n  1,\n  0,\n  0,\n  0,\n  0,\n  1,\n  0,\n  1,\n  2,\n  3,\n  1\n]"
  stack: "    at Object.<anonymous> (file:///C:/DDDDDDDD/0_Coding_0/Web%20JS%20Projects/THREE.js/PRs/three.js/test/unit/src/core/Object3D.tests.js:1010:11)"

The test in question:

// -- Propagation to children world matrices test

child.position.set( 0, 0, 0 );
parent.position.set( 1, 2, 3 );
child.matrixWorldAutoUpdate = true;
parent.matrixAutoUpdate = true;
parent.updateMatrixWorld();

assert.deepEqual( child.matrixWorld.elements, [
    1, 0, 0, 0,
    0, 1, 0, 0,
    0, 0, 1, 0,
    1, 2, 3, 1
], 'Updating parent world matrix has effect to children world matrices even if children local matrices aren\'t changed' );

This is caused because in previous tests (which are all inline so they risk mixing flags) there is child.matrixAutoUpdate = false;. This affects the failed test, by causing the child to not run child.updateMatrix() and thus not set child.matrixWorldNeedsUpdate = true; (point 3.). And now since the force flag just doesnt forcefully update everything by itself regardless of any user concerns, the if ( this.matrixWorldNeedsUpdate || force ) { check (point 4.) will fail and not update the child's world matrix.

This test only worked previously, because force was broken and is now fixed. I fixed the test by adding child.matrixAutoUpdate = true; which is the default anyway.
Now we could argue that the Object3D.matrixWorldNeedsUpdate flag shouldn't rely solely on Object3D.updateMatrix() and I kinda agree, but it is outside the scope of this PR. It works this way right now and I kept it working this way.

[github-actions]

📦 Bundle size

Full ESM build, minified and gzipped.

Filesize dev	Filesize PR	Diff
668.4 kB (165.9 kB)	668.4 kB (165.9 kB)	-5 B

🌳 Bundle size after tree-shaking

Minimal build including a renderer, camera, empty scene, and dependencies.

Filesize dev	Filesize PR	Diff
449.6 kB (108.9 kB)	449.6 kB (108.9 kB)	-5 B

[Mugen87]

Sorry but this change is incorrect. It breaks a series of examples.

I suggest you make sure all examples work as before reopening the PR.

[makc]

lol @Mugen87 did you check?

List of failed screenshots: webgl_camera

basically you set matrixAutoUpdate to false, and helper's matrix itself points to the camera.matrix
since updateMatrix is never called, it now thinks world matrix is fine

this is how it should work, really. because what you have now in that example/helper is h4x

@DolphinIQ could have probably fixed it easily by setting world matrix update flag to true in helper's update()

[Mugen87]

@makc webgl_camera is not the only example that fails.