Skip to content

Commit fc142e7

Browse files
authored
Create abstraction layer over the inspector gizmo layer (#17177)
This change fixes some issues with the inspector due to its `gizmoManager` being treated as an `any` type. ## Issue Description To see the issue this is attempting to address, go to [https://sandbox.babylonjs.com/](https://sandbox.babylonjs.com/?asset=https://playground.babylonjs.com/scenes/skull.babylon) and load a model, then open the inspector and click this button: <img width="536" height="220" alt="image" src="https://github.com/user-attachments/assets/638ce37e-f310-40f8-a424-e557f35ab062" /> A message will appear on screen: `Script error. Check the developer console.` (nothing appears in console). The actual issue stems from this error: ```log sceneTreeItemComponent.tsx:185 Uncaught TypeError: Cannot read properties of null (reading 'gizmoManager') // or sceneTreeItemComponent.tsx:187 Uncaught TypeError: Cannot set properties of null (setting 'coordinatesMode') ``` Which leads to here: https://github.com/BabylonJS/Babylon.js/blob/197d8f6728cc57e46df8e63eaab776a023e5a3ff/packages/dev/inspector/src/components/sceneExplorer/entities/sceneTreeItemComponent.tsx#L185-L187 The issue being that this assumes that this value has been initialized, which in this case it has not been. ## Change Description This change makes all access to the Inspector's `gizmoManager` use one of two helper functions: one for getting and optionally creating, and one for disposing. This abstracts the type-unsafety of the `any` types to one file, rather than spreading them across ~10 different locations. This code should not make any changes to when the inspector creates the `gizmoManager`, as the code path to create it should only be activated in the place where it was originally created.
1 parent 1c96319 commit fc142e7

File tree

5 files changed

+56
-37
lines changed

5 files changed

+56
-37
lines changed

packages/dev/inspector/src/components/sceneExplorer/entities/cameraTreeItemComponent.tsx

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { TreeItemLabelComponent } from "../treeItemLabelComponent";
1010
import { ExtensionsComponent } from "../extensionsComponent";
1111
import * as React from "react";
1212
import type { GlobalState } from "../../globalState";
13+
import { GetInspectorGizmoManager } from "../../../inspectorGizmoManager";
1314

1415
interface ICameraTreeItemComponentProps {
1516
camera: Camera;
@@ -73,9 +74,7 @@ export class CameraTreeItemComponent extends React.Component<ICameraTreeItemComp
7374
toggleGizmo(): void {
7475
const camera = this.props.camera;
7576
if (camera.reservedDataStore && camera.reservedDataStore.cameraGizmo) {
76-
if (camera.getScene().reservedDataStore && camera.getScene().reservedDataStore.gizmoManager) {
77-
camera.getScene().reservedDataStore.gizmoManager.attachToMesh(null);
78-
}
77+
GetInspectorGizmoManager(camera.getScene(), false)?.attachToMesh(null);
7978
this.props.globalState.enableCameraGizmo(camera, false);
8079
this.setState({ isGizmoEnabled: false });
8180
} else {

packages/dev/inspector/src/components/sceneExplorer/entities/lightTreeItemComponent.tsx

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { TreeItemLabelComponent } from "../treeItemLabelComponent";
99
import { ExtensionsComponent } from "../extensionsComponent";
1010
import * as React from "react";
1111
import type { GlobalState } from "../../globalState";
12+
import { GetInspectorGizmoManager } from "../../../inspectorGizmoManager";
1213

1314
interface ILightTreeItemComponentProps {
1415
light: Light;
@@ -39,9 +40,7 @@ export class LightTreeItemComponent extends React.Component<ILightTreeItemCompon
3940
toggleGizmo(): void {
4041
const light = this.props.light;
4142
if (light.reservedDataStore && light.reservedDataStore.lightGizmo) {
42-
if (light.getScene().reservedDataStore && light.getScene().reservedDataStore.gizmoManager) {
43-
light.getScene().reservedDataStore.gizmoManager.attachToMesh(null);
44-
}
43+
GetInspectorGizmoManager(light.getScene(), false)?.attachToMesh(null);
4544
this.props.globalState.enableLightGizmo(light, false);
4645
this.setState({ isGizmoEnabled: false });
4746
} else {

packages/dev/inspector/src/components/sceneExplorer/entities/sceneTreeItemComponent.tsx

Lines changed: 14 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ import type { Observer, Observable } from "core/Misc/observable";
33
import type { PointerInfo } from "core/Events/pointerEvents";
44
import { PointerEventTypes } from "core/Events/pointerEvents";
55
import type { IExplorerExtensibilityGroup } from "core/Debug/debugLayer";
6-
import { GizmoManager } from "core/Gizmos/gizmoManager";
76
import type { Scene } from "core/scene";
87

98
import { FontAwesomeIcon } from "@fortawesome/react-fontawesome";
@@ -22,8 +21,8 @@ import { GizmoCoordinatesMode } from "core/Gizmos/gizmo";
2221
import type { Bone } from "core/Bones/bone";
2322

2423
import { setDebugNode } from "../treeNodeDebugger";
25-
import { FrameGraphUtils } from "core/FrameGraph/frameGraphUtils";
2624
import type { DragStartEndEvent } from "core/Behaviors/Meshes/pointerDragEvents";
25+
import { DisposeInspectorGizmoManager, GetInspectorGizmoManager } from "../../../inspectorGizmoManager";
2726

2827
interface ISceneTreeItemComponentProps {
2928
scene: Scene;
@@ -53,8 +52,8 @@ export class SceneTreeItemComponent extends React.Component<
5352

5453
const scene = this.props.scene;
5554
let gizmoMode = 0;
56-
if (scene.reservedDataStore && scene.reservedDataStore.gizmoManager) {
57-
const manager: GizmoManager = scene.reservedDataStore.gizmoManager;
55+
const manager = GetInspectorGizmoManager(scene, false);
56+
if (manager) {
5857
if (manager.positionGizmoEnabled) {
5958
gizmoMode = 1;
6059
} else if (manager.rotationGizmoEnabled) {
@@ -85,9 +84,8 @@ export class SceneTreeItemComponent extends React.Component<
8584
}
8685

8786
updateGizmoAutoPicking(isInPickingMode: boolean) {
88-
const scene = this.props.scene;
89-
if (scene.reservedDataStore && scene.reservedDataStore.gizmoManager) {
90-
const manager: GizmoManager = scene.reservedDataStore.gizmoManager;
87+
const manager = GetInspectorGizmoManager(this.props.scene, false);
88+
if (manager) {
9189
manager.enableAutoPicking = isInPickingMode;
9290
}
9391
}
@@ -100,9 +98,8 @@ export class SceneTreeItemComponent extends React.Component<
10098
const scene = this.props.scene;
10199
this._onSelectionChangeObserver = this.props.onSelectionChangedObservable.add((entity) => {
102100
this._selectedEntity = entity;
103-
if (entity && scene.reservedDataStore && scene.reservedDataStore.gizmoManager) {
104-
const manager: GizmoManager = scene.reservedDataStore.gizmoManager;
105-
101+
const manager = GetInspectorGizmoManager(scene, false);
102+
if (entity && manager) {
106103
const className = entity.getClassName();
107104

108105
if (className === "TransformNode" || className.indexOf("Mesh") !== -1) {
@@ -181,10 +178,11 @@ export class SceneTreeItemComponent extends React.Component<
181178
}
182179

183180
onCoordinatesMode() {
184-
const scene = this.props.scene;
185-
const manager: GizmoManager = scene.reservedDataStore.gizmoManager;
181+
const manager = GetInspectorGizmoManager(this.props.scene, false);
186182
// flip coordinate system
187-
manager.coordinatesMode = this.state.isInWorldCoodinatesMode ? GizmoCoordinatesMode.Local : GizmoCoordinatesMode.World;
183+
if (manager) {
184+
manager.coordinatesMode = this.state.isInWorldCoodinatesMode ? GizmoCoordinatesMode.Local : GizmoCoordinatesMode.World;
185+
}
188186
this.setState({ isInWorldCoodinatesMode: !this.state.isInWorldCoodinatesMode });
189187
}
190188
onPickingMode() {
@@ -266,27 +264,17 @@ export class SceneTreeItemComponent extends React.Component<
266264
setGizmoMode(mode: number) {
267265
const scene = this.props.scene;
268266

269-
if (!scene.reservedDataStore) {
270-
scene.reservedDataStore = {};
271-
}
272-
273267
if (this._gizmoLayerOnPointerObserver) {
274268
scene.onPointerObservable.remove(this._gizmoLayerOnPointerObserver);
275269
this._gizmoLayerOnPointerObserver = null;
276270
}
277271

278-
if (!scene.reservedDataStore.gizmoManager) {
279-
const layer1 = scene.frameGraph ? FrameGraphUtils.CreateUtilityLayerRenderer(scene.frameGraph) : new UtilityLayerRenderer(scene);
280-
const layer2 = scene.frameGraph ? FrameGraphUtils.CreateUtilityLayerRenderer(scene.frameGraph) : new UtilityLayerRenderer(scene);
281-
282-
scene.reservedDataStore.gizmoManager = new GizmoManager(scene, undefined, layer1, layer2);
283-
}
272+
const manager = GetInspectorGizmoManager(scene, true);
284273

285274
if (this.props.gizmoCamera) {
286-
scene.reservedDataStore.gizmoManager.utilityLayer.setRenderCamera(this.props.gizmoCamera);
275+
manager.utilityLayer.setRenderCamera(this.props.gizmoCamera);
287276
}
288277

289-
const manager: GizmoManager = scene.reservedDataStore.gizmoManager;
290278
// Allow picking of light gizmo when a gizmo mode is selected
291279
this._gizmoLayerOnPointerObserver = UtilityLayerRenderer.DefaultUtilityLayer.utilityLayerScene.onPointerObservable.add((pointerInfo) => {
292280
if (pointerInfo.type == PointerEventTypes.POINTERDOWN) {
@@ -312,8 +300,7 @@ export class SceneTreeItemComponent extends React.Component<
312300

313301
if (this.state.gizmoMode === mode) {
314302
mode = 0;
315-
manager.dispose();
316-
scene.reservedDataStore.gizmoManager = null;
303+
DisposeInspectorGizmoManager(scene);
317304
} else {
318305
switch (mode) {
319306
case 1:

packages/dev/inspector/src/inspector.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import type { IPopupComponentProps } from "./components/popupComponent";
1919
import { PopupComponent } from "./components/popupComponent";
2020
import { CopyStyles } from "shared-ui-components/styleHelper";
2121
import { CreatePopup } from "shared-ui-components/popupHelper";
22+
import { DisposeInspectorGizmoManager } from "./inspectorGizmoManager";
2223

2324
interface IInternalInspectorOptions extends IInspectorOptions {
2425
popup: boolean;
@@ -538,10 +539,7 @@ export class Inspector {
538539
this._GlobalState.enableCameraGizmo(g.camera, false);
539540
}
540541
}
541-
if (this._Scene && this._Scene.reservedDataStore && this._Scene.reservedDataStore.gizmoManager) {
542-
this._Scene.reservedDataStore.gizmoManager.dispose();
543-
this._Scene.reservedDataStore.gizmoManager = null;
544-
}
542+
DisposeInspectorGizmoManager(this._Scene);
545543

546544
if (this._NewCanvasContainer) {
547545
this._DestroyCanvasContainer();
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
import type { Scene } from "core/scene";
2+
import type { Nullable } from "core/types";
3+
import { UtilityLayerRenderer } from "core/Rendering/utilityLayerRenderer";
4+
import { GizmoManager } from "core/Gizmos/gizmoManager";
5+
import { FrameGraphUtils } from "core/FrameGraph/frameGraphUtils";
6+
7+
function CreateInspectorGizmoManager(scene: Scene): GizmoManager {
8+
const layer1 = scene.frameGraph ? FrameGraphUtils.CreateUtilityLayerRenderer(scene.frameGraph) : new UtilityLayerRenderer(scene);
9+
const layer2 = scene.frameGraph ? FrameGraphUtils.CreateUtilityLayerRenderer(scene.frameGraph) : new UtilityLayerRenderer(scene);
10+
const gizmoManager = new GizmoManager(scene, undefined, layer1, layer2);
11+
scene.reservedDataStore ??= {};
12+
scene.reservedDataStore.gizmoManager = gizmoManager;
13+
return gizmoManager;
14+
}
15+
16+
export function GetInspectorGizmoManager(scene: Nullable<Scene> | undefined, create: true): GizmoManager;
17+
export function GetInspectorGizmoManager(scene: Nullable<Scene> | undefined, create: false): Nullable<GizmoManager>;
18+
export function GetInspectorGizmoManager(scene: Nullable<Scene> | undefined, create: boolean): Nullable<GizmoManager> {
19+
let gizmoManager = scene && scene.reservedDataStore && scene.reservedDataStore.gizmoManager ? (scene.reservedDataStore.gizmoManager as GizmoManager) : null;
20+
if (!gizmoManager && create) {
21+
if (!scene) {
22+
throw new Error("Invalid scene provided to GetInspectorGizmoManager");
23+
}
24+
gizmoManager = CreateInspectorGizmoManager(scene);
25+
}
26+
return gizmoManager;
27+
}
28+
29+
export function DisposeInspectorGizmoManager(scene: Nullable<Scene> | undefined): void {
30+
const gizmoManager = GetInspectorGizmoManager(scene, false);
31+
if (!gizmoManager) {
32+
return;
33+
}
34+
gizmoManager.dispose();
35+
scene!.reservedDataStore.gizmoManager = null;
36+
}

0 commit comments

Comments
 (0)