WebGL renderer crashes when Mesh object uses an array of Node Materials

[njarraud]

Description

If an array of node materials e.g. MeshPhysicalNodeMaterial is used as the material for a Mesh, The Threejs WebGL renderer will exit with the following error -

Uncaught TypeError: Cannot set properties of undefined (setting 'value')
    at getProgram (three.module.js:29540:38)
    at setProgram (three.module.js:29740:15)
    at WebGLRenderer.renderBufferDirect (three.module.js:28759:20)
    at renderObject (three.module.js:29451:11)
    at renderObjects (three.module.js:29420:6)
    at renderScene (three.module.js:29289:36)
    at WebGLRenderer.render (three.module.js:29107:5)
    at animate (?editor_console=true:196:14)

It works as intended with other regular materials e.g. MeshPhongMaterial and used to work with old versions of the node material system (years ago).

Reproduction steps

1. Add a mesh to a scene with an array of node materials
2. Render

Code

const geometry = new THREE.SphereGeometry( 5, 12, 8 ).toNonIndexed();
geometry.addGroup(0, 252, 0)
geometry.addGroup(252, 252, 1)

const nodeMaterial1 = new MeshPhysicalNodeMaterial({color:new THREE.Color('red')});
const nodeMaterial2 = new MeshPhysicalNodeMaterial({color:new THREE.Color('yellow')});
const nodeMaterial = [nodeMaterial1, nodeMaterial2]

mesh = new THREE.Mesh( geometry, nodeMaterial );

Live example

Comment/Uncomment the mesh lines at the end of the script to toggle between regular and node materials.
(MeshPhongMaterial is uncommented by default)

• Live example

Screenshots

No response

Version

156

Device

No response

Browser

No response

OS

No response

[mrdoob]

Hmm, I'm still not sure the the addGroup() and array materials API...

const geometry = new THREE.SphereGeometry( 5, 12, 8 ).toNonIndexed();
geometry.addGroup(   0, 252, 0 );
geometry.addGroup( 252, 252, 1 );
const mesh = new THREE.Mesh( geometry, [ material1, material2 ] );

Maybe for the new renderer we can move to something like this?

const geometry = new THREE.SphereGeometry( 5, 12, 8 ).toNonIndexed();
const geometry1 = new THREE.GeometryView( geometry,   0, 252 );
const geometry2 = new THREE.GeometryView( geometry, 255, 252 ); 
const mesh1 = new THREE.Mesh( geometry1, material1 );
const mesh2 = new THREE.Mesh( geometry2, material2 );

/cc @sunag @Mugen87

[Mugen87]

The implementation in the renderers will be more clear if a material property of 3D objects does not hold an array anymore. Ideally we have a 1:1 geometry-material relationship.

Something like GeometryView sounds good to me. It reminds me to the JavaScript DataView class which has a similar purpose. We could actually derive GeometryView from BufferGeometry and create an ordinary geometry at construction time. In this way, the renderer can treat it like a normal geometry.

[njarraud]

The current system is pretty handy for me to combine multiple elements (geometries) together from different sources and use the one Mesh object in the scene. It avoids a more complicated logic to keep track of everything including picking, etc.

If this functionality is on its last leg, I'd better rethink my pipeline 🙂

[sunag]

The implementation of both in WebGPURenderer doesn't seem so different in terms of complexity and performance, in both cases the RenderObject would only have one .geometry and one .material a now a new RenderObject.drawRange property.

An array of materials seems to save on the number of tests in the Frustum.intersectsObject() and is a little more similar to the artistic workflow e.g ( https://youtu.be/ms89wP8m4ZU?t=33, https://youtu.be/qYwOHoS0x8g?t=69 ).

Edit: We will have some properties like mesh.matrixWorld shared too.

[gkjohnson]

Hmm, I'm still not sure the the addGroup() and array materials API...
...
Maybe for the new renderer we can move to something like this?

I'm strongly against this kind of API change - there are good reasons to bookkeep the geometry group / submesh information along with the geometry. Geometry attributes are designed and laid out with this kind of multi material rendering in mind and need to be accounted for when performing geometry operations. Merge vertices, for example, needs to modify the group layout so materials continue to map correctly - and packages like three-mesh-bvh and three-bvh-csg rearrange attribute data and need to do so with these render groups considered.

Not to mention that supporting multi-material meshes is basically a standard across rendering engines. Multi-materials are supported in Babylon, Unity, Unreal Engine, Godot, and others.

This kind of change will only shift complexity to end users and make for significantly more confusing and cumbersome APIs in other dependent packages.

[LeviPesin]

@sunag This should've been fixed now, so we can close this issue?