Skip to content

Commit

Permalink
fix: contexts not being cleaned up properly (#4499)
Browse files Browse the repository at this point in the history
* fix: context leak

* chore: remove debug setup

* chore: bump reveal to 4.12.2

* fix: additional wrong cleanup in ComboControls
  • Loading branch information
christjt authored May 15, 2024
1 parent b58df94 commit ee5abf5
Show file tree
Hide file tree
Showing 8 changed files with 24 additions and 8 deletions.
2 changes: 1 addition & 1 deletion viewer/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@cognite/reveal",
"version": "4.14.1",
"version": "4.14.2",
"description": "WebGL based 3D viewer for CAD and point clouds processed in Cognite Data Fusion.",
"homepage": "https://github.com/cognitedata/reveal/tree/master/viewer",
"repository": {
Expand Down
3 changes: 1 addition & 2 deletions viewer/packages/api/src/api-helpers/Image360ApiHelper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -482,8 +482,7 @@ export class Image360ApiHelper {
public dispose(): void {
this._onBeforeSceneRenderedEvent.unsubscribe(this._eventHandlers.updateHoverStateOnRender);
this._domElement.removeEventListener('mousemove', this._eventHandlers.setHoverIconEventHandler);
this._domElement.addEventListener('pointerup', this._eventHandlers.enter360Image);
this._domElement.addEventListener('keydown', this._eventHandlers.exit360ImageOnEscapeKey);
this._domElement.removeEventListener('keydown', this._eventHandlers.exit360ImageOnEscapeKey);

if (this._stationaryCameraManager && this._cachedCameraManager) {
if (this._activeCameraManager.innerCameraManager === this._stationaryCameraManager) {
Expand Down
2 changes: 2 additions & 0 deletions viewer/packages/api/src/public/RevealManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,8 @@ export class RevealManager {
this._pointCloudManager.dispose();
this._pipelineExecutor.dispose();
this._renderPipeline.dispose();
this._resizeHandler.dispose();
this._updateSubject.unsubscribe();
this._subscriptions.unsubscribe();
this._isDisposed = true;

Expand Down
6 changes: 4 additions & 2 deletions viewer/packages/api/src/public/migration/Cognite3DViewer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -298,15 +298,17 @@ export class Cognite3DViewer {
(useFlexibleCameraManager
? new FlexibleCameraManager(
this._domElement,
this.modelIntersectionCallback.bind(this),
(offsetX: number, offsetY: number, pickBoundingBox: boolean) =>
this.modelIntersectionCallback(offsetX, offsetY, pickBoundingBox),
undefined,
this._sceneHandler.scene,
options.hasEventListeners
)
: new DefaultCameraManager(
this._domElement,
this._mouseHandler,
this.modelIntersectionCallback.bind(this),
(offsetX: number, offsetY: number, pickBoundingBox: boolean) =>
this.modelIntersectionCallback(offsetX, offsetY, pickBoundingBox),
undefined
));

Expand Down
2 changes: 1 addition & 1 deletion viewer/packages/camera-manager/src/ComboControls.ts
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,7 @@ export class ComboControls extends EventDispatcher<ComboControlsEventType> {
private addEventListeners() {
this._keyboard.addEventListeners(this._domElement);
this._domElement.addEventListener('pointerdown', this.onPointerDown);
this._domElement.addEventListener('wheel', event => this.onMouseWheel(event));
this._domElement.addEventListener('wheel', this.onMouseWheel);
this._domElement.addEventListener('contextmenu', this.onContextMenu);

// canvas has focus by default, but it's possible to set tabindex on it,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@ export class FlexibleCameraManager extends PointerEvents implements IFlexibleCam
public dispose(): void {
this._isDisposed = true;
this.removeEventListeners();
this._markers?.dispose();
}

//================================================
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { FlexibleControlsType } from './FlexibleControlsType';

export class FlexibleCameraMarkers {
private readonly _scene: Scene;
private _targetMarker: Object3D | undefined;
private _targetMarker: Sprite | undefined;

//================================================
// CONSTRUCTOR
Expand Down Expand Up @@ -39,6 +39,16 @@ export class FlexibleCameraMarkers {
}
}

public dispose(): void {
if (this._targetMarker) {
this._scene.remove(this._targetMarker);
this._targetMarker.material.map?.dispose();
this._targetMarker.material.dispose();
this._targetMarker.geometry.dispose();
this._targetMarker = undefined;
}
}

private isVisible(manager: FlexibleCameraManager): boolean {
if (!manager.options.showTarget) {
return false;
Expand Down
4 changes: 3 additions & 1 deletion viewer/packages/rendering/src/ResizeHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export class ResizeHandler {
private readonly _onCameraChangeCallback: () => void;
private readonly _onCameraStopCallback: () => void;

private readonly _resizeObserver: ResizeObserver | undefined;
private _resizeObserver: ResizeObserver | undefined;

private _shouldResize: boolean = false;

Expand Down Expand Up @@ -141,7 +141,9 @@ export class ResizeHandler {
}

dispose(): void {
this._resizeObserver?.unobserve(this._renderer.domElement.parentElement!);
this._resizeObserver?.disconnect();
this._resizeObserver = undefined;
}
}

Expand Down

0 comments on commit ee5abf5

Please sign in to comment.