diff --git a/README.Rmd b/README.Rmd index 923d244..71379f5 100644 --- a/README.Rmd +++ b/README.Rmd @@ -31,3 +31,8 @@ devtools::install_github("bbuchsbaum/neurosurf") See examples of use of `neurosurf` in the [vignettes](https://bbuchsbaum.github.io/neurosurf/articles/index.html). + +## Cleaning Up + +When removing a surface from a viewer, call the `dispose()` method on the +`NeuroSurface` object to free associated WebGL resources and event listeners. diff --git a/README.md b/README.md index ca4fad4..b30adb1 100644 --- a/README.md +++ b/README.md @@ -23,3 +23,8 @@ See examples of use of `neurosurf` in the [vignettes](https://bbuchsbaum.github.io/neurosurf/articles/index.html). + +## Cleaning Up + +When removing a surface from a viewer, call the `dispose()` method on the +`NeuroSurface` object to free associated WebGL resources and event listeners. diff --git a/inst/htmlwidgets/lib/neurosurface/neurosurface.js b/inst/htmlwidgets/lib/neurosurface/neurosurface.js index a4bb71c..ade8c0a 100644 --- a/inst/htmlwidgets/lib/neurosurface/neurosurface.js +++ b/inst/htmlwidgets/lib/neurosurface/neurosurface.js @@ -1,3 +1,19 @@ + + dispose() { + if (this.mesh) { + this.mesh.geometry.dispose(); + this.mesh.material.dispose(); + this.mesh = null; + } + } + dispose() { + if (this.rangeListener) this.rangeListener(); + if (this.thresholdListener) this.thresholdListener(); + if (this.alphaListener) this.alphaListener(); + super.dispose(); + } + dispose() { + this.mouse = new Vector2(); this.intersectionPoint = new Vector3(); @@ -39,4 +55,5 @@ this.paneContainer = null; } } + } diff --git a/inst/htmlwidgets/neurosurface/src/ColorMap.js b/inst/htmlwidgets/neurosurface/src/ColorMap.js index 7472323..c3d37de 100644 --- a/inst/htmlwidgets/neurosurface/src/ColorMap.js +++ b/inst/htmlwidgets/neurosurface/src/ColorMap.js @@ -1,5 +1,6 @@ import colormap from 'colormap'; import { EventEmitter } from './EventEmitter.js'; +import { debugLog } from './debug.js'; class ColorMap extends EventEmitter { constructor(colors, options = {}) { @@ -18,7 +19,7 @@ class ColorMap extends EventEmitter { setRange(range) { if (Array.isArray(range) && range.length === 2 && range.every(v => typeof v === 'number')) { this.range = range; - console.log('ColorMap: Emitting rangeChanged event', this.range); + debugLog('ColorMap: Emitting rangeChanged event', this.range); this.emit('rangeChanged', this.range); } else { this.range = [0, 1]; @@ -28,7 +29,7 @@ class ColorMap extends EventEmitter { setThreshold(threshold) { if (Array.isArray(threshold) && threshold.length === 2 && threshold.every(v => typeof v === 'number')) { this.threshold = threshold; - console.log('ColorMap: Emitting thresholdChanged event', this.threshold); + debugLog('ColorMap: Emitting thresholdChanged event', this.threshold); this.emit('thresholdChanged', this.threshold); } else { this.threshold = [0, 0]; diff --git a/inst/htmlwidgets/neurosurface/src/EventEmitter.js b/inst/htmlwidgets/neurosurface/src/EventEmitter.js index 61d1c05..5f67439 100644 --- a/inst/htmlwidgets/neurosurface/src/EventEmitter.js +++ b/inst/htmlwidgets/neurosurface/src/EventEmitter.js @@ -1,9 +1,13 @@ export class EventEmitter { constructor() { - this._events = {}; + // Use an object without a prototype to avoid prototype pollution + this._events = Object.create(null); } on(event, listener) { + if (typeof listener !== 'function') { + throw new TypeError('listener must be a function'); + } if (!this._events[event]) { this._events[event] = []; } @@ -11,9 +15,21 @@ export class EventEmitter { return () => this.removeListener(event, listener); } + once(event, listener) { + if (typeof listener !== 'function') { + throw new TypeError('listener must be a function'); + } + const wrapped = (...args) => { + this.removeListener(event, wrapped); + listener(...args); + }; + return this.on(event, wrapped); + } + emit(event, ...args) { if (this._events[event]) { - this._events[event].forEach((listener) => listener(...args)); + // Copy listeners to avoid issues if the array is modified during emit + [...this._events[event]].forEach((listener) => listener(...args)); } } @@ -22,6 +38,17 @@ export class EventEmitter { this._events[event] = this._events[event].filter( (listener) => listener !== listenerToRemove ); + if (this._events[event].length === 0) { + delete this._events[event]; + } + } + } + + removeAllListeners(event) { + if (event) { + delete this._events[event]; + } else { + this._events = Object.create(null); } } -} \ No newline at end of file +} diff --git a/inst/htmlwidgets/neurosurface/src/NeuroSurfaceViewer.js b/inst/htmlwidgets/neurosurface/src/NeuroSurfaceViewer.js index 8215e61..4544574 100644 --- a/inst/htmlwidgets/neurosurface/src/NeuroSurfaceViewer.js +++ b/inst/htmlwidgets/neurosurface/src/NeuroSurfaceViewer.js @@ -3,6 +3,7 @@ import { TrackballControls } from 'three/examples/jsm/controls/TrackballControls import { ColorMappedNeuroSurface, VertexColoredNeuroSurface } from './classes.js'; import { Pane } from 'tweakpane'; import * as EssentialsPlugin from '@tweakpane/plugin-essentials'; +import { debugLog } from './debug.js'; export class NeuroSurfaceViewer { constructor(container, width, height, config = {}, viewpoint = 'lateral') { @@ -265,9 +266,9 @@ export class NeuroSurfaceViewer { umat = new THREE.Matrix4().identity(); } - console.log('Setting viewpoint to:', viewpoint); - console.log('Current viewpoint state:', this.viewpointState); - console.log("umat", umat); + debugLog('Setting viewpoint to:', viewpoint); + debugLog('Current viewpoint state:', this.viewpointState); + debugLog('umat', umat); // Calculate the bounding box of all surfaces const box = new THREE.Box3(); @@ -334,7 +335,7 @@ export class NeuroSurfaceViewer { } updateIntensityRange() { - console.log('NeuroSurfaceViewer: Updating intensity range', [this.intensityRange.range.min, this.intensityRange.range.max]); + debugLog('NeuroSurfaceViewer: Updating intensity range', [this.intensityRange.range.min, this.intensityRange.range.max]); this.surfaces.forEach(surface => { if (surface instanceof ColorMappedNeuroSurface) { surface.colorMap.setRange([this.intensityRange.range.min, this.intensityRange.range.max]); @@ -344,7 +345,7 @@ export class NeuroSurfaceViewer { } updateThresholdRange() { - console.log('NeuroSurfaceViewer: Updating threshold range', [this.thresholdRange.range.min, this.thresholdRange.range.max]); + debugLog('NeuroSurfaceViewer: Updating threshold range', [this.thresholdRange.range.min, this.thresholdRange.range.max]); this.surfaces.forEach(surface => { if (surface instanceof ColorMappedNeuroSurface) { surface.colorMap.setThreshold([this.thresholdRange.range.min, this.thresholdRange.range.max]); @@ -361,7 +362,7 @@ export class NeuroSurfaceViewer { } addSurface(surface, id) { - console.log('Adding surface:', surface, 'with id:', id); + debugLog('Adding surface:', surface, 'with id:', id); this.surfaces.set(id, surface); if (!surface.mesh) { console.warn('Surface mesh not created. Creating now.'); @@ -370,7 +371,7 @@ export class NeuroSurfaceViewer { this.scene.add(surface.mesh); if (surface instanceof ColorMappedNeuroSurface) { - console.log('Updating data range for ColorMappedNeuroSurface'); + debugLog('Updating data range for ColorMappedNeuroSurface'); this.updateDataRange(surface.data); this.updateRangeControls(); } @@ -418,6 +419,9 @@ export class NeuroSurfaceViewer { removeSurface(id) { const surface = this.surfaces.get(id); if (surface) { + if (typeof surface.dispose === 'function') { + surface.dispose(); + } this.scene.remove(surface.mesh); this.surfaces.delete(id); } @@ -481,7 +485,7 @@ export class NeuroSurfaceViewer { this.updateRangeControls(); surface.updateColors(); this.render(); - console.log(`Updated data for surface with id: ${id}`); + debugLog(`Updated data for surface with id: ${id}`); } else { console.warn(`No ColorMappedNeuroSurface found with id: ${id}`); } @@ -492,7 +496,7 @@ export class NeuroSurfaceViewer { if (surface && surface instanceof VertexColoredNeuroSurface) { surface.setColors(colors); this.render(); - console.log(`Updated colors for surface with id: ${id}`); + debugLog(`Updated colors for surface with id: ${id}`); } else { console.warn(`No VertexColoredNeuroSurface found with id: ${id}`); } diff --git a/inst/htmlwidgets/neurosurface/src/classes.js b/inst/htmlwidgets/neurosurface/src/classes.js index b4df5d6..2c5d381 100644 --- a/inst/htmlwidgets/neurosurface/src/classes.js +++ b/inst/htmlwidgets/neurosurface/src/classes.js @@ -1,5 +1,6 @@ import * as THREE from 'three'; import ColorMap from './ColorMap.js'; +import { debugLog } from './debug.js'; export class SurfaceGeometry { constructor(vertices, faces, hemi) { @@ -8,10 +9,10 @@ export class SurfaceGeometry { this.hemi = hemi; this.mesh = null; - console.log("SurfaceGeometry constructor called"); - console.log("Vertices:", this.vertices.length); - console.log("Faces:", this.faces.length); - console.log("Hemi:", this.hemi); + debugLog('SurfaceGeometry constructor called'); + debugLog('Vertices:', this.vertices.length); + debugLog('Faces:', this.faces.length); + debugLog('Hemi:', this.hemi); this.createMesh(); } @@ -28,8 +29,8 @@ export class SurfaceGeometry { }); this.mesh = new THREE.Mesh(geometry, material); - console.log("SurfaceGeometry construction complete"); - console.log("Mesh:", this.mesh); + debugLog('SurfaceGeometry construction complete'); + debugLog('Mesh:', this.mesh); } } @@ -153,6 +154,18 @@ export class NeuroSurface { this.mesh.material.transparent = this.colorMap.hasAlpha; this.mesh.material.needsUpdate = true; } + + /** + * Dispose of the mesh resources associated with this surface. + * Geometry and material are released and the mesh reference cleared. + */ + dispose() { + if (this.mesh) { + this.mesh.geometry.dispose(); + this.mesh.material.dispose(); + this.mesh = null; + } + } } export class ColorMappedNeuroSurface extends NeuroSurface { @@ -195,17 +208,17 @@ export class ColorMappedNeuroSurface extends NeuroSurface { // Set up new listeners this.rangeListener = this.colorMap.on('rangeChanged', (range) => { - console.log('ColorMappedNeuroSurface: Received rangeChanged event', range); + debugLog('ColorMappedNeuroSurface: Received rangeChanged event', range); this.irange = range; this.updateColors(); }); this.thresholdListener = this.colorMap.on('thresholdChanged', (threshold) => { - console.log('ColorMappedNeuroSurface: Received thresholdChanged event', threshold); + debugLog('ColorMappedNeuroSurface: Received thresholdChanged event', threshold); this.threshold = threshold; this.updateColors(); }); this.alphaListener = this.colorMap.on('alphaChanged', (alpha) => { - console.log('ColorMappedNeuroSurface: Received alphaChanged event', alpha); + debugLog('ColorMappedNeuroSurface: Received alphaChanged event', alpha); this.config.alpha = alpha; this.updateColors(); }); @@ -239,11 +252,11 @@ export class ColorMappedNeuroSurface extends NeuroSurface { } updateColors() { - console.log('Updating colors. Mesh:', !!this.mesh, 'ColorMap:', !!this.colorMap); + debugLog('Updating colors. Mesh:', !!this.mesh, 'ColorMap:', !!this.colorMap); if (!this.mesh || !this.colorMap) { console.warn('Mesh or ColorMap not initialized in updateColors'); - console.log('Mesh:', this.mesh); - console.log('ColorMap:', this.colorMap); + debugLog('Mesh:', this.mesh); + debugLog('ColorMap:', this.colorMap); return; } @@ -251,10 +264,10 @@ export class ColorMappedNeuroSurface extends NeuroSurface { const componentsPerColor = 4; // Always use RGBA const colors = new Float32Array(vertexCount * componentsPerColor); - console.log("threshold", this.threshold); - console.log("irange", this.irange); - console.log("alpha", this.config.alpha); - console.log("data", this.data); + debugLog('threshold', this.threshold); + debugLog('irange', this.irange); + debugLog('alpha', this.config.alpha); + debugLog('data', this.data); const baseSurfaceColor = new THREE.Color(this.config.color); @@ -322,6 +335,16 @@ export class ColorMappedNeuroSurface extends NeuroSurface { this.data = newData; this.updateColors(); } + + /** + * Remove listeners and dispose of resources. + */ + dispose() { + if (this.rangeListener) this.rangeListener(); + if (this.thresholdListener) this.thresholdListener(); + if (this.alphaListener) this.alphaListener(); + super.dispose(); + } } export class VertexColoredNeuroSurface extends NeuroSurface { @@ -364,6 +387,13 @@ export class VertexColoredNeuroSurface extends NeuroSurface { this.updateColors(); return mesh; } + + /** + * Dispose of this surface's resources. + */ + dispose() { + super.dispose(); + } } diff --git a/inst/htmlwidgets/neurosurface/src/debug.js b/inst/htmlwidgets/neurosurface/src/debug.js new file mode 100644 index 0000000..a5b1b52 --- /dev/null +++ b/inst/htmlwidgets/neurosurface/src/debug.js @@ -0,0 +1,9 @@ +export let DEBUG = false; +export function setDebug(value) { + DEBUG = value; +} +export function debugLog(...args) { + if (DEBUG) { + console.log(...args); + } +} diff --git a/inst/htmlwidgets/neurosurface/src/index.js b/inst/htmlwidgets/neurosurface/src/index.js index 25d7fda..d504198 100644 --- a/inst/htmlwidgets/neurosurface/src/index.js +++ b/inst/htmlwidgets/neurosurface/src/index.js @@ -1,6 +1,7 @@ import * as THREE from 'three'; import { NeuroSurfaceViewer } from './NeuroSurfaceViewer'; import { SurfaceGeometry, NeuroSurface, ColorMappedNeuroSurface, VertexColoredNeuroSurface } from './classes'; +import { debugLog, setDebug } from './debug.js'; // Export the classes so they're available to the widget export { @@ -9,7 +10,9 @@ export { NeuroSurface, ColorMappedNeuroSurface, VertexColoredNeuroSurface, - THREE + THREE, + debugLog, + setDebug }; // Optionally, you can also attach these to the global window object @@ -21,8 +24,9 @@ if (typeof window !== 'undefined') { NeuroSurface, ColorMappedNeuroSurface, VertexColoredNeuroSurface, - THREE + THREE, + debugLog, + setDebug }; } - -console.log('Neurosurface module initialized'); +debugLog('Neurosurface module initialized'); diff --git a/inst/htmlwidgets/surfwidget.js b/inst/htmlwidgets/surfwidget.js index 7c52539..355b6e8 100644 --- a/inst/htmlwidgets/surfwidget.js +++ b/inst/htmlwidgets/surfwidget.js @@ -12,7 +12,7 @@ HTMLWidgets.widget({ renderValue: function(x) { if (!viewer) { - console.log("Creating NeuroSurfaceViewer"); + neurosurface.debugLog('Creating NeuroSurfaceViewer'); viewer = new neurosurface.NeuroSurfaceViewer(el, width, height, { ...x.config, cmap: x.cmap, @@ -26,13 +26,13 @@ HTMLWidgets.widget({ } try { - console.log("Creating SurfaceGeometry"); + neurosurface.debugLog('Creating SurfaceGeometry'); var geometry = new neurosurface.SurfaceGeometry(x.vertices, x.faces, x.hemi); - console.log("SurfaceGeometry created:", geometry); + neurosurface.debugLog('SurfaceGeometry created:', geometry); var surface; if (x.cmap) { - console.log("Creating ColorMappedNeuroSurface"); + neurosurface.debugLog('Creating ColorMappedNeuroSurface'); surface = new neurosurface.ColorMappedNeuroSurface( geometry, x.indices, @@ -41,7 +41,7 @@ HTMLWidgets.widget({ { irange: x.irange, thresh: x.thresh, alpha: x.alpha, ...x.config } ); } else if (x.vertexColors) { - console.log("Creating VertexColoredNeuroSurface"); + neurosurface.debugLog('Creating VertexColoredNeuroSurface'); surface = new neurosurface.VertexColoredNeuroSurface( geometry, x.indices, @@ -52,8 +52,8 @@ HTMLWidgets.widget({ throw new Error("Neither color map nor vertex colors provided"); } - console.log("Surface created:", surface); - console.log("Adding surface to viewer"); + neurosurface.debugLog('Surface created:', surface); + neurosurface.debugLog('Adding surface to viewer'); viewer.addSurface(surface, surfaceId); viewer.animate();