Fix variants with multiple internal materials (#4882)

* refactor to handle multiple internal materials for each gltf material

* simplify

Author: elalish

packages/model-viewer/src/features/scene-graph/material.ts

@@ -40,7 +40,6 @@ export const $gltfIndex = Symbol('gltfIndex');
 export const $setActive = Symbol('setActive');
 export const $variantIndices = Symbol('variantIndices');
 const $isActive = Symbol('isActive');
-export const $variantSet = Symbol('variantSet');
 const $modelVariants = Symbol('modelVariants');
 const $name = Symbol('name');
 const $pbrTextures = Symbol('pbrTextures');
@@ -56,7 +55,7 @@ export class Material extends ThreeDOMElement implements MaterialInterface {
   private[$lazyLoadGLTFInfo]?: LazyLoader;
   private[$gltfIndex]: number;
   private[$isActive]: boolean;
-  private[$variantSet] = new Set<number>();
+  public[$variantIndices] = new Set<number>();
   private[$name]?: string;
   readonly[$modelVariants]: Map<string, VariantData>;
   private[$pbrTextures] = new Map<TextureUsage, TextureInfo>();
@@ -147,12 +146,9 @@ export class Material extends ThreeDOMElement implements MaterialInterface {
     createTextureInfo(TextureUsage.Anisotropy);
   }
 
-  async[$getLoadedMaterial](): Promise<MeshPhysicalMaterial> {
+  async[$getLoadedMaterial](): Promise<MeshPhysicalMaterial|null> {
     if (this[$lazyLoadGLTFInfo] != null) {
-      const {set, material} = await this[$lazyLoadGLTFInfo]!.doLazyLoad();
-
-      // Fills in the missing data.
-      this[$correlatedObjects] = set as Set<MeshPhysicalMaterial>;
+      const material = await this[$lazyLoadGLTFInfo]!.doLazyLoad();
 
       this[$initialize]();
       // Releases lazy load info.
@@ -161,7 +157,7 @@ export class Material extends ThreeDOMElement implements MaterialInterface {
       this.ensureLoaded = async () => {};
       return material as MeshPhysicalMaterial;
     }
-    return this[$correlatedObjects]!.values().next().value;
+    return null;
   }
 
   private colorFromRgb(rgb: RGB|string): Color {
@@ -240,13 +236,9 @@ export class Material extends ThreeDOMElement implements MaterialInterface {
     return this[$gltfIndex];
   }
 
-  [$variantIndices]() {
-    return this[$variantSet];
-  }
-
   hasVariant(name: string): boolean {
     const variantData = this[$modelVariants].get(name);
-    return variantData != null && this[$variantSet].has(variantData.index);
+    return variantData != null && this[$variantIndices].has(variantData.index);
   }
 
   setEmissiveFactor(rgb: RGB|string) {

packages/model-viewer/src/features/scene-graph/model.ts

@@ -15,11 +15,11 @@
 
 import {Intersection, Material as ThreeMaterial, Mesh, MeshPhysicalMaterial, Object3D} from 'three';
 
-import {CorrelatedSceneGraph, GLTFElementToThreeObjectMap, ThreeObjectSet} from '../../three-components/gltf-instance/correlated-scene-graph.js';
+import {CorrelatedSceneGraph, GLTFElementToThreeObjectMap} from '../../three-components/gltf-instance/correlated-scene-graph.js';
 import {GLTF, GLTFElement} from '../../three-components/gltf-instance/gltf-2.0.js';
 
 import {Model as ModelInterface} from './api.js';
-import {$setActive, $variantSet, Material} from './material.js';
+import {$setActive, $variantIndices, Material} from './material.js';
 import {Node, PrimitiveNode} from './nodes/primitive-node.js';
 import {$correlatedObjects} from './three-dom-element.js';
 
@@ -46,12 +46,10 @@ export class LazyLoader {
   gltf: GLTF;
   gltfElementMap: GLTFElementToThreeObjectMap;
   mapKey: GLTFElement;
-  doLazyLoad: () => Promise<{set: ThreeObjectSet, material: ThreeMaterial}>;
+  doLazyLoad: () => Promise<ThreeMaterial>;
   constructor(
       gltf: GLTF, gltfElementMap: GLTFElementToThreeObjectMap,
-      mapKey: GLTFElement,
-      doLazyLoad:
-          () => Promise<{set: ThreeObjectSet, material: ThreeMaterial}>) {
+      mapKey: GLTFElement, doLazyLoad: () => Promise<ThreeMaterial>) {
     this.gltf = gltf;
     this.gltfElementMap = gltfElementMap;
     this.mapKey = mapKey;
@@ -88,29 +86,28 @@ export class Model implements ModelInterface {
 
     for (const [i, material] of gltf.materials!.entries()) {
       const correlatedMaterial =
-          gltfElementMap.get(material) as Set<MeshPhysicalMaterial>;
+          gltfElementMap.get(material) as Set<MeshPhysicalMaterial>| null;
 
       if (correlatedMaterial != null) {
         this[$materials].push(new Material(
-            onUpdate, i, true, this[$variantData], correlatedMaterial, material.name));
+            onUpdate,
+            i,
+            true,
+            this[$variantData],
+            correlatedMaterial,
+            material.name));
       } else {
         const elementArray = gltf['materials'] || [];
         const gltfMaterialDef = elementArray[i];
 
-        // Loads the three.js material.
-        const capturedMatIndex = i;
+        const threeMaterialSet = new Set<MeshPhysicalMaterial>();
+        gltfElementMap.set(gltfMaterialDef, threeMaterialSet);
         const materialLoadCallback = async () => {
-          const threeMaterial =
-              await threeGLTF.parser.getDependency(
-                  'material', capturedMatIndex) as MeshPhysicalMaterial;
-
-          // Adds correlation, maps the variant gltf-def to the
-          // three material set containing the variant material.
-          const threeMaterialSet = new Set<MeshPhysicalMaterial>();
-          gltfElementMap.set(gltfMaterialDef, threeMaterialSet);
+          const threeMaterial = await threeGLTF.parser.getDependency(
+                                    'material', i) as MeshPhysicalMaterial;
           threeMaterialSet.add(threeMaterial);
 
-          return {set: threeMaterialSet, material: threeMaterial};
+          return threeMaterial;
         };
 
         // Configures the material for lazy loading.
@@ -119,7 +116,7 @@ export class Model implements ModelInterface {
             i,
             false,
             this[$variantData],
-            correlatedMaterial,
+            threeMaterialSet,
             material.name,
             new LazyLoader(
                 gltf, gltfElementMap, gltfMaterialDef, materialLoadCallback)));
@@ -387,7 +384,7 @@ export class Model implements ModelInterface {
 
     for (const material of this.materials) {
       if (material.hasVariant(variantName)) {
-        material[$variantSet].delete(variant.index);
+        material[$variantIndices].delete(variant.index);
       }
     }
 

packages/model-viewer/src/features/scene-graph/nodes/primitive-node.ts

@@ -117,7 +117,7 @@ export class PrimitiveNode extends Node {
           const {name} = variantNames[variant];
           this.variantToMaterialMap.set(variant, mvMaterial);
           // Provides variant info for material self lookup.
-          mvMaterial[$variantIndices]().add(variant);
+          mvMaterial[$variantIndices].add(variant);
           // Updates the models variant data.
           if (!modelVariants.has(name)) {
             modelVariants.set(name, {name, index: variant} as VariantData);
@@ -128,18 +128,20 @@ export class PrimitiveNode extends Node {
   }
 
   async setActiveMaterial(material: number): Promise<ThreeMaterial|null> {
-    const mvMaterial = this.materials.get(material);
-    if (mvMaterial != null && material !== this.activeMaterialIdx) {
-      this.mesh.material = await mvMaterial[$getLoadedMaterial]();
-      const {normalScale} = this.mesh.material as MeshPhysicalMaterial;
-      // TODO: remove this hack in favor of properly storing the different
-      // three.js materials that are potentially created for different meshes
-      // that share a glTF material.
-      if (normalScale != null &&
-          (normalScale.y * normalScale.x < 0) !=
-              (this.mesh.geometry.attributes.tangent == null)) {
-        this.parser.assignFinalMaterial(this.mesh);
+    const mvMaterial = this.materials.get(material)!;
+    if (material !== this.activeMaterialIdx) {
+      const backingMaterials =
+          mvMaterial[$correlatedObjects] as Set<MeshPhysicalMaterial>;
+
+      const baseMaterial = await mvMaterial[$getLoadedMaterial]();
+      if (baseMaterial != null) {
+        this.mesh.material = baseMaterial;
+      } else {
+        this.mesh.material = backingMaterials.values().next().value;
       }
+
+      this.parser.assignFinalMaterial(this.mesh);
+      backingMaterials.add(this.mesh.material as MeshPhysicalMaterial);
       this.activeMaterialIdx = material;
     }
     return this.mesh.material as ThreeMaterial;
@@ -210,7 +212,7 @@ export class PrimitiveNode extends Node {
     const variantIndex = modelVariantData.index;
 
     // Updates materials mapped to the variant.
-    materialVariant[$variantIndices]().add(variantIndex);
+    materialVariant[$variantIndices].add(variantIndex);
 
     // Updates internal mappings.
     this.variantToMaterialMap.set(variantIndex, materialVariant);
@@ -236,7 +238,7 @@ export class PrimitiveNode extends Node {
   private updateVariantUserData(
       variantIndex: number, materialVariant: Material) {
     // Adds variants name to material variants set.
-    materialVariant[$variantIndices]().add(variantIndex);
+    materialVariant[$variantIndices].add(variantIndex);
 
     this.mesh.userData.variantData = this.modelVariants;
     // Updates import data (see VariantMaterialLoaderPlugin.ts).

packages/model-viewer/src/features/scene-graph/three-dom-element.ts

@@ -29,10 +29,9 @@ type CorrelatedObjects = Set<Object3D>|Set<Material>|Set<Texture>;
 export class ThreeDOMElement {
   readonly[$onUpdate]: () => void;
   // The Three.js scene graph construct for this element.
-  [$correlatedObjects]: CorrelatedObjects|null;
+  [$correlatedObjects]: CorrelatedObjects;
 
-  constructor(
-      onUpdate: () => void, correlatedObjects: CorrelatedObjects|null = null) {
+  constructor(onUpdate: () => void, correlatedObjects: CorrelatedObjects) {
     this[$onUpdate] = onUpdate;
     this[$correlatedObjects] = correlatedObjects;
   }

packages/model-viewer/src/test/features/scene-graph-spec.ts

@@ -147,6 +147,29 @@ suite('SceneGraph', () => {
         expect(gltfRoot.children[1].userData.variantMaterials.size).to.be.eq(3);
       });
 
+      test('allows the scene graph to be manipulated', async () => {
+        element.variantName = 'Yellow Red';
+        await waitForEvent(element, 'variant-applied');
+
+        const material =
+            (element[$scene].model!.children[1] as Mesh).material as
+            MeshStandardMaterial;
+
+        const mat = element.model!.getMaterialByName('red')!;
+
+        expect(mat.isActive).to.be.true;
+
+        mat.pbrMetallicRoughness.setBaseColorFactor([0.5, 0.5, 0.5, 1]);
+
+        const color = mat.pbrMetallicRoughness.baseColorFactor;
+
+        expect(color).to.be.eql([0.5, 0.5, 0.5, 1]);
+
+        console.log(material.name, ': actual material ', material.uuid);
+
+        expect(material.color).to.include({r: 0.5, g: 0.5, b: 0.5});
+      });
+
       test(
           `Setting variantName to null results in primitive
            reverting to default/initial material`,

packages/model-viewer/src/test/features/scene-graph/model-spec.ts

@@ -83,12 +83,12 @@ suite('scene-graph/model', () => {
       test('Switch variant and lazy load', async () => {
         const threeGLTF = await loadThreeGLTF(CUBES_GLTF_PATH);
         const model = new Model(CorrelatedSceneGraph.from(threeGLTF));
-        expect(model[$materials][2][$correlatedObjects]).to.be.null;
+        expect(model[$materials][2][$correlatedObjects]).to.be.empty;
         expect(model[$materials][2][$lazyLoadGLTFInfo]).to.be.ok;
 
         await model[$switchVariant]('Yellow Red');
 
-        expect(model[$materials][2][$correlatedObjects]).to.not.be.null;
+        expect(model[$materials][2][$correlatedObjects]).to.not.be.empty;
         expect(model[$materials][2][$lazyLoadGLTFInfo]).to.not.be.ok;
       });