KHR_materials_variants got lost during clone of mesh

DavidG · January 24, 2023, 7:44am

Hello,

i think i found a bug in the KHR_materials_variants class. We want to make use of the clone method of meshes to increase performance and memory usage. We are also making use of KHR_materials_variants. It seems that KHR_materials_variants are not correctly copied during the clone process.

I created a playground to prove this Bug report KHR_materials_variants missed during clone of mesh | Babylon.js Playground (babylonjs.com)

The clone is missing the variants. I also had a look at the KHR_materials_variants class.
For me it seems like it is at least planned that KHR_materials_variants are aswell copied.

github.com

BabylonJS/Babylon.js/blob/c43ee29e015358fbeb7f032ca4491baa39e6e209/packages/dev/loaders/src/glTF/2.0/Extensions/KHR_materials_variants.ts#L224


      
          for (let mappingVariantIndex = 0; mappingVariantIndex < mapping.variants.length; ++mappingVariantIndex) {

              const variantIndex = mapping.variants[mappingVariantIndex];

              const variant = ArrayItem.Get(`/extensions/${NAME}/variants/${variantIndex}`, this._variants, variantIndex);

              extensionMetadata.variants[variant.name] = extensionMetadata.variants[variant.name] || [];

              extensionMetadata.variants[variant.name].push({

                  mesh: babylonMesh,

                  material: babylonMaterial,

              });

          

              // Replace the target when original mesh is cloned

              babylonMesh.onClonedObservable.add((newOne: Node) => {

                  const newMesh = newOne as Mesh;

                  let metadata: Nullable<IExtensionMetadata> = null;

                  let newRoot: Nullable<Node> = newMesh;

          

                  // Find root to get medata

                  do {

                      newRoot = newRoot!.parent;

                      if (!newRoot) {

                          return;

                      }

As far as I understand the code in line

github.com

BabylonJS/Babylon.js/blob/c43ee29e015358fbeb7f032ca4491baa39e6e209/packages/dev/loaders/src/glTF/2.0/Extensions/KHR_materials_variants.ts#L239


      
          // Find root to get medata

          do {

              newRoot = newRoot!.parent;

              if (!newRoot) {

                  return;

              }

              metadata = KHR_materials_variants._GetExtensionMetadata(newRoot as Mesh);

          } while (metadata === null);

          

          // Need to clone the metadata on the root (first time only)

          if (root && metadata === KHR_materials_variants._GetExtensionMetadata(root)) {

              // Copy main metadata

              newRoot._internalMetadata = {};

              for (const key in root._internalMetadata) {

                  newRoot._internalMetadata[key] = root._internalMetadata[key];

              }

          

              // Copy the gltf metadata

              newRoot._internalMetadata.gltf = [];

              for (const key in root._internalMetadata.gltf) {

                  newRoot._internalMetadata.gltf[key] = root._internalMetadata.gltf[key];

the if block with metadata === KHR_materials_variants._GetExtensionMetadata(root) will never be true, because the new mesh does not got the new metadata at this point. From my point of view it should be metadata !== KHR_materials_variants._GetExtensionMetadata(root).

Or am I doing something wrong with the clone call?

srzerbetto · January 24, 2023, 1:08pm

Hello @DavidG , how are you doing?

Thanks for finding that bug! I believe you are right on your assumptions.

I will just cc @sebavan so we can take a look, just to be 100% sure.

DavidG · January 25, 2023, 7:39am

Hey @srzerbetto thanks for your fast reply. Im doing fine, hope you too.
Also debugged the code yesterday a little bit. Seems to me that also the “Find root to get metadata” part is actually not searching the metadata on the existing mesh but the cloned one. From my understanding it can never find the metadata there.

sebavan · January 28, 2023, 12:07am

Will be fixed by Fix GLTF Variants Clone by sebavan · Pull Request #13481 · BabylonJS/Babylon.js · GitHub

DavidG · January 31, 2023, 7:43am

Thank you for this really fast response and fix, awesome!

Topic		Replies	Views
Possible bug with cloning CustomMaterial Bugs	3	279	April 20, 2020
Randomly Generated Meshes Questions	8	998	May 2, 2020
Highlight or changing material in instance meshes Questions	10	1216	May 19, 2022
Sandbox material binding problem Bugs material	11	383	October 29, 2020
Custom material vs clones Questions	4	249	September 30, 2020

KHR_materials_variants got lost during clone of mesh

Related Topics