Skip to content

Commit 720eef7

Browse files
committed
fix(model-viewer): defer bounding box recalculations to render loop for extra models
1 parent d6d96c8 commit 720eef7

4 files changed

Lines changed: 54 additions & 4 deletions

File tree

packages/model-viewer/src/features/controls.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,7 @@ const maxCameraOrbitIntrinsics = (element: ModelViewerElementBase) => {
193193
};
194194

195195
export const cameraTargetIntrinsics = (element: ModelViewerElementBase) => {
196+
element[$scene].updateBoundingBoxAndShadowIfDirty();
196197
const center = element[$scene].boundingBox.getCenter(new Vector3());
197198

198199
return {

packages/model-viewer/src/features/loading.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,10 +252,12 @@ export const LoadingMixin = <T extends Constructor<ModelViewerElementBase>>(
252252
* turntable rotation.
253253
*/
254254
getDimensions(): Vector3D {
255+
this[$scene].updateBoundingBoxAndShadowIfDirty();
255256
return toVector3D(this[$scene].size);
256257
}
257258

258259
getBoundingBoxCenter(): Vector3D {
260+
this[$scene].updateBoundingBoxAndShadowIfDirty();
259261
return toVector3D(this[$scene].boundingBox.getCenter(new Vector3()));
260262
}
261263

packages/model-viewer/src/test/features/extra-model-spec.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,36 @@ suite('ExtraModel', () => {
7575

7676
expect(scene._models[1].position.x).to.equal(5);
7777
});
78+
79+
test('does not calculate bounding box synchronously when offset changes', async () => {
80+
element.loading = 'eager';
81+
element.src = CUBE_GLB_PATH;
82+
83+
const extra = document.createElement('extra-model');
84+
extra.setAttribute('src', CUBE_GLB_PATH);
85+
extra.setAttribute('offset', '1 0 0');
86+
element.appendChild(extra);
87+
88+
await waitForEvent(element, 'load');
89+
const scene = (element as any)[$scene];
90+
91+
// Ensure bounds are clean initially
92+
scene.updateBoundingBoxAndShadowIfDirty();
93+
const oldMaxX = scene._boundingBox.max.x;
94+
95+
// Change offset
96+
extra.setAttribute('offset', '10 0 0');
97+
await timePasses(); // allow MutationObserver to trigger updateModelTransforms
98+
99+
// The bounds should be dirty, but the actual boundingBox value hasn't mathematically updated yet!
100+
expect(scene.boundsAndShadowDirty).to.be.true;
101+
expect(scene._boundingBox.max.x).to.equal(oldMaxX);
102+
103+
// However, reading via getDimensions flushes it
104+
element.getDimensions();
105+
expect(scene.boundsAndShadowDirty).to.be.false;
106+
expect(scene._boundingBox.max.x).to.be.greaterThan(oldMaxX);
107+
});
78108
});
79109

80110
suite('hotspot attachment', () => {

packages/model-viewer/src/three-components/ModelScene.ts

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,7 @@ export class ModelScene extends Scene {
117117

118118
private _currentGLTFs: ModelViewerGLTFInstance[] = [];
119119
private _models: Object3D[] = [];
120+
private boundsAndShadowDirty = false;
120121
private mixers: AnimationMixer[] = [];
121122
private mixerPausedStates: boolean[] = [];
122123
private cancelPendingSourceChange: (() => void)|null = null;
@@ -320,9 +321,8 @@ export class ModelScene extends Scene {
320321
this.setGroundedSkybox();
321322
}
322323

323-
updateModelTransforms(index: number, offset?: string|null, orientation?: string|null, scale?: string|null) {
324+
updateModelTransforms(index: number, offset?: string|null, _orientation?: string|null, scale?: string|null) {
324325
const model = this._models[index];
325-
console.log(`[ModelScene] updateModelTransforms index: ${index} modelFound: ${!!model} offset: ${offset} orientation: ${orientation}`);
326326
if (!model) return;
327327

328328
if (offset) {
@@ -342,11 +342,27 @@ export class ModelScene extends Scene {
342342
}
343343

344344
model.updateMatrixWorld(true);
345-
this.updateBoundingBox();
346-
this.updateShadow();
345+
// Defer bounding box and shadow recalculations.
346+
// If developers animate `<extra-model>` offset or scale properties via requestAnimationFrame,
347+
// recalculating bounding boxes synchronously every single frame here blocks the main thread and tanks frame rates.
348+
// Instead, we mark the bounds as dirty and wait for the render loop or a public dimensions getter to flush the changes.
349+
this.boundsAndShadowDirty = true;
347350
this.queueRender();
348351
}
349352

353+
/**
354+
* Evaluates bounding box recalculations asynchronously.
355+
* Flushed right before a frame is rendered or when dimension properties are formally queried
356+
* to ensure that high-frequency layout changes don't stall execution natively.
357+
*/
358+
updateBoundingBoxAndShadowIfDirty() {
359+
if (this.boundsAndShadowDirty) {
360+
this.boundsAndShadowDirty = false;
361+
this.updateBoundingBox();
362+
this.updateShadow();
363+
}
364+
}
365+
350366
reset() {
351367
this.url = null;
352368
this.renderCount = 0;
@@ -1241,6 +1257,7 @@ export class ModelScene extends Scene {
12411257
}
12421258

12431259
renderShadow(renderer: WebGLRenderer) {
1260+
this.updateBoundingBoxAndShadowIfDirty();
12441261
const shadow = this.shadow;
12451262
if (shadow != null && shadow.needsUpdate == true) {
12461263
shadow.render(renderer, this);

0 commit comments

Comments
 (0)