instanceColor attribute is not defined when InstancedMesh is populated asynchronously

Just as the title says. It all boils down to this line https://github.com/mrdoob/three.js/blob/dev/src/renderers/webgl/WebGLPrograms.js#L171. Setting any color for any instance just after the instanced mesh is created, solves the issue. But this is not obvious and looks like it's not documented.

To Reproduce

1. Prepare a scene with InstancedMesh
2. Render it
3. Populate the instanced mesh asynchronously

Live example

jsfiddle

Expected behavior

When setColorAt is called, the shader should be recompiled to include the instanceColor. Or there should be a valid and documented way to make sure that this attribute is always defined.

Platform

• Device: Desktop
• OS: Windows
• Browser: Chrome
• Three.js version: r128

Just after I filed the issue, I realized I just need to guard the shader with #undef's. Although it would be more convenient to have some explicit way to configure this and avoid relying on pre-processor so heavily.

[mrdoob]

Hmm, this definitely shouldn't crash...

[Mugen87]

@mrdoob It does not crash when a built-in material is used. https://jsfiddle.net/we76xsvr/1/

The OP also missed to set material.needsUpdate = true;. InstancedMesh is created without instanceColor. When added, the shader program needs a recompile. If the renderer should hide this, it would be necessary to check each frame if colors are present or not. For performance reasons, I do not vote to do this since the presented use case seems rare to me.

The OP also missed to set material.needsUpdate = true;

Oh, nice! This is definitely better than random setColorAt during init. Thanks!

it would be necessary to check each frame if colors are present

Not necessarily, since setColorAt can both initialize attribute buffer and mark material for update. But this looks a bit dirty. A little note in the docs on InstancedMesh would have solved the issue for me.

[Mugen87]

Not necessarily, since setColorAt can both initialize attribute buffer and mark material for update.

If @mrdoob is fine with this approach, I guess it's worth a try.

When colors are once set via setColorAt(), it seems unlikely that colors are set to null at a later point. So it's not necessary to handle this particular use case.

Otherwise we can start with a note in the documentation and change the code if more users bump into this issue.

[mrdoob]

Hmm, I wonder... what would happen in this case?

const material = new THREE.MeshBasicMaterial();

const mesh1 = new THREE.InstancedMesh( geometry, material, 10 );
scene.add( mesh1 );

const mesh2 = new THREE.InstancedMesh( geometry, material 10 );
scene.add( mesh2 );

And asynchronously we did:

mesh2.setColorAt();
material.needsUpdate = true;

Because material is shared between mesh1 and mesh2 then it's possible that WebGLRenderer will handle mesh1 first and set material.needsUpdate to false and by the time it handles mesh2 (which now has instanced colors) it won't know that it needs to update because material.needsUpdate was set to false...

[Mugen87]

This behavior is indeed confusing.

I guess we can handle this use case without relying on Material.needsUpdate by directly checking for instanceColor in the renderer.

[mrdoob]

Which we do already... 🤔

three.js/src/renderers/webgl/WebGLPrograms.js

Line 171 in 8c21891

instancingColor: object.isInstancedMesh === true && object.instanceColor !== null,

[Mugen87]

This code block is only executed when a program change is detected. A possible place for an enhancement would be:

three.js/src/renderers/WebGLRenderer.js

Lines 1482 to 1533 in 35eaf19

	if ( material.version === materialProperties.__version ) {
	if ( materialProperties.needsLights && ( materialProperties.lightsStateVersion !== lights.state.version ) ) {
	needsProgramChange = true;
	} else if ( materialProperties.outputEncoding !== encoding ) {
	needsProgramChange = true;
	} else if ( object.isInstancedMesh && materialProperties.instancing === false ) {
	needsProgramChange = true;
	} else if ( ! object.isInstancedMesh && materialProperties.instancing === true ) {
	needsProgramChange = true;
	} else if ( object.isSkinnedMesh && materialProperties.skinning === false ) {
	needsProgramChange = true;
	} else if ( ! object.isSkinnedMesh && materialProperties.skinning === true ) {
	needsProgramChange = true;
	} else if ( materialProperties.envMap !== envMap ) {
	needsProgramChange = true;
	} else if ( material.fog && materialProperties.fog !== fog ) {
	needsProgramChange = true;
	} else if ( materialProperties.numClippingPlanes !== undefined &&
	( materialProperties.numClippingPlanes !== clipping.numPlanes ||
	materialProperties.numIntersection !== clipping.numIntersection ) ) {
	needsProgramChange = true;
	} else if ( materialProperties.vertexAlphas !== vertexAlphas ) {
	needsProgramChange = true;
	}
	} else {
	needsProgramChange = true;
	materialProperties.__version = material.version;
	}