fix: node appearance provider still issuing events after cad node dis…

…posal (#3626)
cognitedata · Aug 28, 2023 · ccaf55b · ccaf55b
1 parent f98c30b
commit ccaf55b
Show file tree

Hide file tree

Showing 6 changed files with 19 additions and 3 deletions.
diff --git a/viewer/packages/cad-geometry-loaders/src/CadModelUpdateHandler.ts b/viewer/packages/cad-geometry-loaders/src/CadModelUpdateHandler.ts
@@ -233,7 +233,7 @@ function createDetermineSectorsInput([settings, _, camera, clipping, models]: [
   ClippingInput,
   CadNode[]
 ]): DetermineSectorsPayload {
-  const prioritizedAreas = models.flatMap(model => model.prioritizedAreas);
+  const prioritizedAreas = models.filter(model => !model.isDisposed).flatMap(model => model.prioritizedAreas);
   return {
     ...camera,
     ...settings,

diff --git a/viewer/packages/cad-geometry-loaders/src/sector/SectorLoader.ts b/viewer/packages/cad-geometry-loaders/src/sector/SectorLoader.ts
@@ -62,7 +62,7 @@ export class SectorLoader {
     }
 
     const cadModels = input.models;
-    const visibleCadModels = cadModels.filter(x => x.visible);
+    const visibleCadModels = cadModels.filter(x => x.visible && !x.isDisposed);
 
     const sectorCullerInput: DetermineSectorsInput = {
       ...input,

diff --git a/viewer/packages/cad-model/src/wrappers/CadNode.ts b/viewer/packages/cad-model/src/wrappers/CadNode.ts
@@ -37,6 +37,8 @@ export class CadNode extends Object3D {
   private readonly _batchedGeometryMeshGroup: Group;
   private readonly _styledTreeIndexSets: StyledTreeIndexSets;
 
+  private _isDisposed: boolean = false;
+
   private _needsRedraw: boolean = false;
 
   public readonly treeIndexToSectorsMap = new TreeIndexToSectorsMap();
@@ -153,6 +155,10 @@ export class CadNode extends Object3D {
     return this._materialManager.getRenderMode();
   }
 
+  get isDisposed(): boolean {
+    return this._isDisposed;
+  }
+
   public loadSector(sector: WantedSector, abortSignal?: AbortSignal): Promise<ConsumedSector> {
     return this._sectorRepository.loadSector(sector, abortSignal);
   }
@@ -203,13 +209,15 @@ export class CadNode extends Object3D {
   }
 
   public dispose(): void {
+    this.nodeAppearanceProvider.dispose();
     this.nodeAppearanceProvider.off('changed', this._setModelRenderLayers);
     this._sectorRepository.clearCache();
     this._materialManager.removeModelMaterials(this._cadModelMetadata.modelIdentifier);
     this._geometryBatchingManager?.dispose();
     this._rootSector?.dereferenceAllNodes();
     this._rootSector?.clear();
     this.clear();
+    this._isDisposed = true;
 
     delete this._geometryBatchingManager;
     // @ts-ignore

diff --git a/viewer/packages/cad-styling/src/NodeAppearanceProvider.ts b/viewer/packages/cad-styling/src/NodeAppearanceProvider.ts
@@ -156,6 +156,13 @@ export class NodeAppearanceProvider {
     return this._styledCollections.some(x => x.nodeCollection.isLoading);
   }
 
+  dispose(): void {
+    this.scheduleNotifyChanged.cancel();
+    this._events.changed.unsubscribeAll();
+    this._events.loadingStateChanged.unsubscribeAll();
+    this._events.prioritizedAreasChanged.unsubscribeAll();
+  }
+
   private notifyChanged() {
     this._cachedPrioritizedAreas = undefined;
     this._events.changed.fire();

diff --git a/viewer/packages/cad-styling/visual-tests/Highlighted.VisualTest.ts b/viewer/packages/cad-styling/visual-tests/Highlighted.VisualTest.ts
@@ -22,7 +22,7 @@ export default class HighlightedVisualTest extends StreamingVisualTestFixture {
       .getModelNodeAppearanceProvider(modelIdentifier)
       .assignStyledNodeCollection(nodes, DefaultNodeAppearance.Highlighted);
 
-    // Styles are not applied immidiatly, so wait a little for styling to take effect
+    // Styles are not applied immediately, so wait a little for styling to take effect
     await new Promise(resolve => setTimeout(resolve, 100));
 
     model.geometryNode.position.set(25, 0, -15);

diff --git a/viewer/packages/rendering/src/CadMaterialManager.ts b/viewer/packages/rendering/src/CadMaterialManager.ts
@@ -267,6 +267,7 @@ export class CadMaterialManager {
     for (const [_, wrapper] of this.materialsMap) {
       wrapper.nodeAppearanceTextureBuilder.dispose();
       wrapper.nodeTransformTextureBuilder.dispose();
+      wrapper.nodeAppearanceProvider.dispose();
     }
   }