Material Index not matched

[dongho-shin]

Describe the bug

this model has all the same Materials (8 mesh but share 1 material, share 1 material(same uuid) is a key point of this bug)

1. PathTrace Rendering first after import this model
2. change only material of mesh

3. PathTrace render again

Model

FYI) zip from mac

Blender_Smeg (2).glb.zip

Platform:

• Device: Desktop
• OS: MacOS
• Browser: Chrome
• Three.js version: r162
• Library version: 0.0.23

This bug causes here

three-gpu-pathtracer/src/core/utils/GeometryPreparationUtils.js

Line 46 in d5b1f14

const materialIndex = allMaterials.indexOf( mat );

sorry for ugly notes

indexOf, if all meshes share 1 materials(in this notes a) geometry.materialIndex attributes are all 0

and then material of mesh has changed(a -> b)

StaticGeometryGenerator, bvhChanged => no changes(because in this scenario no geometry changes) so materialIndex not updated, geometry reuse materialIndex 0 so nothing has been updated

so my sugestion is
before

		const mat = Array.isArray( materials ) ? materials[ group.materialIndex ] : materials;
		const materialIndex = allMaterials.indexOf( mat );

after

	//	const mat = Array.isArray( materials ) ? materials[ group.materialIndex ] : materials; // not used
		const materialIndex = group.materialIndex;

always welcome any ideas & questions

[gkjohnson]

Thanks for reporting and digging in as always!

I suspect this is because the geometry has not changed, causing the static geometry generator to return "NO_CHANGE" meaning the material index buffer attribute is not changed - see here.

I think the easy fix would be the following:

// change this
if ( result.changeType !== NO_CHANGE ) {

	updateMaterialIndexAttribute( geometry, materials, materials );

}

// to this
updateMaterialIndexAttribute( geometry, materials, materials );

So the material index is always updated. Would you be able to test that?

A more optimal change would be to check if any of the materials have changed before regenerating. But that's a bit more complicated.

[dongho-shin]

first try that you suggest me, it fails so I tried workaround weird way(use group.materialIndex)
I found I miss this part

three-gpu-pathtracer/src/core/WebGLPathTracer.js

Lines 353 to 365 in d5b1f14

	if ( bvhChanged ) {
	material.bvh.updateFrom( bvh );
	material.attributesArray.updateFrom(
	geometry.attributes.normal,
	geometry.attributes.tangent,
	geometry.attributes.uv,
	geometry.attributes.color,
	);
	material.materialIndexAttribute.updateFrom( geometry.attributes.materialIndex );
	}

your suggestion + change of code below fix this issue

//previous
 if ( bvhChanged ) { 
  
 	material.bvh.updateFrom( bvh ); 
 	material.attributesArray.updateFrom( 
 		geometry.attributes.normal, 
 		geometry.attributes.tangent, 
 		geometry.attributes.uv, 
 		geometry.attributes.color, 
 	); 
  
 	material.materialIndexAttribute.updateFrom( geometry.attributes.materialIndex ); 
  
 } 
 //after
  if ( bvhChanged ) { 
  
 	material.bvh.updateFrom( bvh ); 
 	material.attributesArray.updateFrom( 
 		geometry.attributes.normal, 
 		geometry.attributes.tangent, 
 		geometry.attributes.uv, 
 		geometry.attributes.color, 
 	); 
    
 }
 //No matter of bvhChanged
 material.materialIndexAttribute.updateFrom( geometry.attributes.materialIndex );  

if you agree this fixed i'll open pr, always thanks for feedback @gkjohnson

[gkjohnson]

Good catch - a PR would be great.

It could be improved further if the material list information were saved as a hash to check if the list of materials had actually been updated or not - but I'l leave it up to you if you want to look into that.

[dongho-shin]

I'll open a PR this week fixing this issue in this way. if hash improvement of material is not urgent I'll look into it

[gkjohnson]

Fixed in #664