Merge pull request #20759 from takahirox/FixAttach

Object3D: Fix world matrix update bug in .attach()

src/core/Object3D.js

@@ -409,10 +409,10 @@ Object3D.prototype = Object.assign( Object.create( EventDispatcher.prototype ),
 
 		object.applyMatrix4( _m1 );
 
-		object.updateWorldMatrix( false, false );
-
 		this.add( object );
 
+		object.updateWorldMatrix( false, true );
+
 		return this;
 
 	},

test/unit/src/core/Object3D.tests.js

@@ -13,6 +13,22 @@ import {
 	eps
 } from '../math/Constants.tests';
 
+const matrixEquals4 = ( a, b ) => {
+
+	for ( let i = 0; i < 16; i ++ ) {
+
+		if ( Math.abs( a.elements[ i ] - b.elements[ i ] ) >= eps ) {
+
+			return false;
+
+		}
+
+	}
+
+	return true;
+
+};
+
 export default QUnit.module( 'Core', () => {
 
 	QUnit.module( 'Object3D', () => {
@@ -426,6 +442,63 @@ export default QUnit.module( 'Core', () => {
 
 		} );
 
+		QUnit.test( "attach", ( assert ) => {
+
+			const object = new Object3D();
+			const oldParent = new Object3D();
+			const newParent = new Object3D();
+			const expectedMatrixWorld = new Matrix4();
+
+			// Attach to a parent
+
+			object.position.set( 1, 2, 3 );
+			object.rotation.set( Math.PI / 2, Math.PI / 3, Math.PI / 4 );
+			object.scale.set( 2, 3, 4 );
+			newParent.position.set( 4, 5, 6 );
+			newParent.rotation.set( Math.PI / 5, Math.PI / 6, Math.PI / 7 );
+			newParent.scale.set( 5, 5, 5 );
+
+			object.updateMatrixWorld();
+			newParent.updateMatrixWorld();
+			expectedMatrixWorld.copy( object.matrixWorld );
+
+			newParent.attach( object );
+
+			assert.ok( object.parent && object.parent == newParent &&
+				oldParent.children.indexOf( object ) === - 1,
+				"object is a child of a new parent" );
+
+			assert.ok( matrixEquals4( expectedMatrixWorld, object.matrixWorld ), "object's world matrix is maintained" );
+
+			// Attach to a new parent from an old parent
+
+			object.position.set( 1, 2, 3 );
+			object.rotation.set( Math.PI / 2, Math.PI / 3, Math.PI / 4 );
+			object.scale.set( 2, 3, 4 );
+			oldParent.position.set( 4, 5, 6 );
+			oldParent.rotation.set( Math.PI / 5, Math.PI / 6, Math.PI / 7 );
+			oldParent.scale.set( 5, 5, 5 );
+			newParent.position.set( 7, 8, 9 );
+			newParent.rotation.set( Math.PI / 8, Math.PI / 9, Math.PI / 10 );
+			newParent.scale.set( 6, 6, 6 );
+
+			oldParent.add( object );
+			oldParent.updateMatrixWorld();
+			newParent.updateMatrixWorld();
+			expectedMatrixWorld.copy( object.matrixWorld );
+
+			newParent.attach( object );
+
+			assert.ok( object.parent && object.parent == newParent &&
+				newParent.children.indexOf( object ) !== - 1 &&
+				oldParent.children.indexOf( object ) === - 1,
+				"object is no longer a child of an old parent and is a child of a new parent now" );
+
+			assert.ok( matrixEquals4( expectedMatrixWorld, object.matrixWorld ),
+				"object's world matrix is maintained even it had a parent" );
+
+		} );
+
 		QUnit.test( "getObjectById/getObjectByName/getObjectByProperty", ( assert ) => {
 
 			var parent = new Object3D();