InstancedMesh: duplicated call to refreshBoundingInfo in clone()

kzhsw · August 7, 2025, 1:08am

In InstancedMesh.clone(), it calls refreshBoundingInfo after the cloned instance constructed.

packages/dev/core/src/Meshes/instancedMesh.ts

0a28aed8d


      
                  "behaviors",

                  "worldMatrixFromCache",

                  "hasThinInstances",

                  "hasBoundingInfo",

                  "geometry",

              ],

              []

          );

          

          // Bounding info

          this.refreshBoundingInfo();

          

          // Parent

          if (newParent) {

              result.parent = newParent;

          }

          

          if (!doNotCloneChildren) {

              // Children

              for (let index = 0; index < this.getScene().meshes.length; index++) {

                  const mesh = this.getScene().meshes[index];

But createInstance , which internally called Mesh._instancedMeshFactory, which called the InstancedMesh constructor, where refreshBoundingInfo is called when needed.

github.com/BabylonJS/Babylon.js

packages/dev/core/src/Meshes/instancedMesh.ts

0a28aed8d


      
          import { DeepCopier } from "../Misc/deepCopier";

          import { TransformNode } from "./transformNode";

          import type { Light } from "../Lights/light";

          import { VertexBuffer } from "../Buffers/buffer";

          import { Tools } from "../Misc/tools";

          import type { ThinEngine } from "../Engines/thinEngine";

          import { RegisterClass } from "../Misc/typeStore";

          import type { Geometry } from "./geometry";

          

          Mesh._instancedMeshFactory = (name: string, mesh: Mesh): InstancedMesh => {

              const instance = new InstancedMesh(name, mesh);

          

              if (mesh.instancedBuffers) {

                  instance.instancedBuffers = {};

          

                  for (const key in mesh.instancedBuffers) {

                      instance.instancedBuffers[key] = mesh.instancedBuffers[key];

                  }

              }

          

              return instance;

Since refreshBoundingInfo is a heavy option that iterates every vertex from source mesh, this duplicated call should be avoided.

github.com/BabylonJS/Babylon.js

packages/dev/core/src/Meshes/instancedMesh.ts

0a28aed8d


      
              if (typeof applySkeletonOrOptions === "object") {

                  options = applySkeletonOrOptions;

              } else {

                  options = {

                      applySkeleton: applySkeletonOrOptions,

                      applyMorph: applyMorph,

                  };

              }

          

              const bias = this._sourceMesh.geometry ? this._sourceMesh.geometry.boundingBias : null;

              this._refreshBoundingInfo(this._sourceMesh._getData(options, null, VertexBuffer.PositionKind), bias);

              return this;

          }

          

          /** @internal */

          public override _preActivate(): InstancedMesh {

              if (this._currentLOD) {

                  this._currentLOD._preActivate();

              }

              return this;

          }

Also link the previous thread about this here:

A quick blame shows that this call origins 11 years ago where the InstancedMesh was created.

roland · August 7, 2025, 5:45am

Great catch and nice analysis! Would you be up for creating a PR? That would be awesome.

kzhsw · August 7, 2025, 6:13am

I would propose a simple removal of that call, I’ll submit a PR if that’s ok, or we can discuss further here.

roland · August 7, 2025, 7:24am

Let’s dig deeper:

You propose to remove to call this.refreshBoundingInfo() at line 582 if I am right.

I’m not sure whether it’s not a safety call to prepare the BB before cloning the mesh/meshes. The vertex data might have been altered without calling this function on the source mesh. I’m also not sure whether removing it could affect the cloning process (with incorrect BB). @kzhsw did you do some tests already?

On the other hand clone was always considered as a CPU heavy operation, so the programmer should be aware of this.

Maybe you could add a parameter to the clone funtion called doNotRefreshBoundingInfo: boolean to give the programmer better control of the clone function.

EDIT: maybe this could be a better option so we don’t end up with too many parameters and we don’t break backwards compatibility:

public override clone(name: string, newParent: Nullable = null, doNotCloneChildrenOrOptions?: boolean | { doNotCloneChildren?: boolean, doNotRefreshBoundingInfo?: boolean }, newSourceMesh?: Mesh): InstancedMesh

@Deltakosh @sebavan Any thoughts?

Deltakosh · August 7, 2025, 3:04pm

I would agree with @roland here

kzhsw · August 8, 2025, 12:59am

So do you know the original reason to do a full rebuild of bounding info from vertex here?

Deltakosh · August 8, 2025, 2:50pm

let me check deeply

Deltakosh · August 8, 2025, 2:56pm

After further checking, I think we are fine to go

I’ll let all the PR tests run to be sure though:

Remove unnecessary BBox computation by deltakosh · Pull Request #16996 · BabylonJS/Babylon.js

Topic		Replies	Views
InstancedMesh: why fully refreshBoundingInfo even without bones Questions performance , instances	5	90	May 21, 2025
refreshBoundingInfo called every time an InstancedMesh is created, even if not needed Questions	2	231	February 16, 2024
Duplicating meshes without binding their geometry Questions	7	1143	August 27, 2021
Shared skeleton between GLB import instances incorrectly culls instances Bugs	9	713	August 8, 2020
InstancedBuffers not working with cloned mesh Questions	2	339	October 11, 2020

InstancedMesh: duplicated call to refreshBoundingInfo in clone()

Related topics