Skip to content

Commit

Permalink
fix: Disposed geometryGroup softlocks sector load (#3946)
Browse files Browse the repository at this point in the history
* fix: Disposed geometryGroup softlocks sector load

* Check dispose before dereference

* Update lockfile

* Refactor
  • Loading branch information
Strepto authored May 8, 2024
1 parent 6723b94 commit cb258f3
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 22 deletions.
35 changes: 18 additions & 17 deletions viewer/packages/cad-parsers/package.json
Original file line number Diff line number Diff line change
@@ -1,17 +1,18 @@
{
"name": "@reveal/cad-parsers",
"private": true,
"main": "index.ts",
"scripts": {
"start": "yarn ws:start",
"test": "yarn ws:test --config ./../../jest.config.js",
"lint": "yarn ws:lint"
},
"dependencies": {
"@reveal/data-providers": "workspace:*",
"@reveal/metrics": "workspace:*",
"@reveal/model-base": "workspace:*",
"@reveal/sector-parser": "workspace:*",
"@reveal/utilities": "workspace:*"
}
}
{
"name": "@reveal/cad-parsers",
"private": true,
"main": "index.ts",
"scripts": {
"start": "yarn ws:start",
"test": "yarn ws:test --config ./../../jest.config.js",
"lint": "yarn ws:lint"
},
"dependencies": {
"@reveal/data-providers": "workspace:*",
"@reveal/logger": "workspace:*",
"@reveal/metrics": "workspace:*",
"@reveal/model-base": "workspace:*",
"@reveal/sector-parser": "workspace:*",
"@reveal/utilities": "workspace:*"
}
}
21 changes: 16 additions & 5 deletions viewer/packages/cad-parsers/src/sector/SectorNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import * as THREE from 'three';
import { AutoDisposeGroup } from '@reveal/utilities';
import { LevelOfDetail } from '../cad/LevelOfDetail';
import { Log } from '@reveal/logger';

export class SectorNode extends THREE.Group {
public readonly sectorPath: string;
Expand Down Expand Up @@ -37,12 +38,18 @@ export class SectorNode extends THREE.Group {
return this._updatedTimestamp;
}

updateGeometry(geomtryGroup: AutoDisposeGroup | undefined, levelOfDetail: LevelOfDetail): void {
updateGeometry(geometryGroup: AutoDisposeGroup | undefined, levelOfDetail: LevelOfDetail): void {
this.resetGeometry();
this._group = geomtryGroup;
if (this._group !== undefined) {
this._group.reference();

if (geometryGroup) {
if (geometryGroup.isDisposed()) {
Log.warn('Tried to add an already disposed geometry group to sector:', this.sectorId);
} else {
geometryGroup.reference();
}
}

this._group = geometryGroup;
this._lod = levelOfDetail;
this._updatedTimestamp = Date.now();
this.visible = this._lod !== LevelOfDetail.Discarded;
Expand All @@ -57,7 +64,11 @@ export class SectorNode extends THREE.Group {

resetGeometry(): void {
if (this._group !== undefined) {
this._group.dereference();
if (!this._group.isDisposed()) {
this._group.dereference();
} else {
Log.warn('Tried to dereference an already disposed geometryGroup in sector:', this.sectorId);
}
this.remove(this._group);
}

Expand Down
4 changes: 4 additions & 0 deletions viewer/packages/utilities/src/three/AutoDisposeGroup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ export class AutoDisposeGroup extends THREE.Group {
private _isDisposed = false;
private _referenceCount = 0;

isDisposed(): boolean {
return this._isDisposed;
}

reference(): void {
this.ensureNotDisposed();
this._referenceCount++;
Expand Down
1 change: 1 addition & 0 deletions viewer/yarn.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit cb258f3

Please sign in to comment.