InstancedMesh: Support copying/cloning

[aknoerig]

In order to copy/clone an InstancedMesh succesfully, its additional properties need to be copied, too.

(This commit is untested, but hopefully straightforward enough to not warrant me to setup a full dev/test environment?)

[WestLangley]

This commit is untested

So you filed a PR that you have not tested?

It is based on Mesh.copy() which does not even work correctly. #16224

[Mugen87]

I agree that the existing issues with copy() and clone() are a problem. For consistency reasons, I think it's still okay to add InstancedMesh.copy().

[WestLangley]

For consistency reasons, I think it's still okay to add InstancedMesh.copy()

The method does not work.

[aknoerig]

Well, I followed the current pattern and it's working for me.

[Mugen87]

The method does not work.

Do you mind elaborating in more detail what exactly does not work? 😇

[WestLangley]

For example, .copy() does not work.

var mesh1 = new THREE.InstancedMesh(
	new THREE.SphereBufferGeometry( 4, 32, 16 ),
	new THREE.MeshBasicMaterial(),
	1
);

mesh1.setMatrixAt( 0, new THREE.Matrix4() ); // identity
mesh1.position.set( - 5, 0, 0 );
scene.add( mesh1 );

var mesh2 = new THREE.InstancedMesh(
	new THREE.BoxBufferGeometry( 5, 5, 5 ),
	new THREE.MeshNormalMaterial(),
	1
);

mesh2.copy( mesh1 ); // both geometry and material differ from that of mesh1
mesh2.position.set( 5, 0, 0 );
scene.add( mesh2 );

mesh2 = mesh1.clone(); does not work either. In fact it is worse -- the instance matrix is zero.

[Mugen87]

Ah yes, now I remember. You're right, the problem was the fact that Mesh.copy() does not honor the material and geometry. The method only works in combination with Mesh.clone() which is obviously strange.

Argh, we should really fix this in Mesh.

[Mugen87]

@WestLangley Do you mind reopening #16224?

[WestLangley]

Well, I followed the current pattern and it's working for me.

@aknoerig Can you please provide the code pattern that works for you?

[aknoerig]

I was actually looking for a way to clone an InstancedMesh, with a copied geometry. What worked for me, doing it manually, was this:

let mesh = new THREE.InstancedMesh(
	new THREE.SphereBufferGeometry( 4, 32, 16 ),
	new THREE.MeshBasicMaterial(),
	1
)

let meshClone = mesh.clone()

// add missing properties for InstancedMesh
meshClone.instanceMatrix = mesh.instanceMatrix
meshClone.count = mesh.count

// selectively copy the geometry
meshClone.geometry = mesh.geometry.clone()

Since this worked well for me, I attempted to contribute back and this pull-request is my amateur attempt at that. The pull-request itself is untested, as it looked straightforward enough to me. I didn't realize that there was a philosophical discussion around cloning/copying. Sorry if my patch is actually making things worse or more confusing...!

The way I understood it, is that clone() is always a shallow clone, which makes sense to me. I'd then expect copy() to be a deep clone. Implementation-wise, I expected clone() to internally uses copy() non-recursively.

[WestLangley]

@aknoerig Thank you for your PR, anyway!

We do request that everyone test their PRs, however. :-)

[Mugen87]

Closing in favor of #19471.