From 66cc77ffc4bfdc11b8b99eadb9e7b3d5f9d15295 Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Fri, 28 Aug 2026 11:10:16 -0400 Subject: [PATCH] fix(Widgets): release owned picking resources on deletion vtkWidgetManager owns its per-view widgets and its hardware selector, and deletion left both alive. The selector holds a full-window framebuffer, color texture and depth renderbuffer per view, and a focused widget keeps an animation request on the interactor that only losing focus cancels. On delete, release focus, remove the view widgets, and delete the selector once any in-flight capture settles. vtkOpenGLHardwareSelector chains releaseGraphicsResources into delete so its framebuffer goes with it. setRenderer drops the captured buffers and the in-flight capture that belong to the previous wiring, and a capture that settles after the manager was deleted or re-targeted no longer publishes stale buffers. Selection handles a capture that produced nothing to select against. Tests cover teardown, renderer replacement and focused-widget deletion through the public API, counting live WebGL objects. --- .../Rendering/OpenGL/HardwareSelector/api.md | 5 + .../OpenGL/HardwareSelector/index.d.ts | 5 + .../OpenGL/HardwareSelector/index.js | 11 ++ Sources/Widgets/Core/WidgetManager/api.md | 4 + Sources/Widgets/Core/WidgetManager/index.js | 83 ++++++++++++-- .../WidgetManager/test/testWidgetManager.js | 106 +++++++++++++++++- 6 files changed, 205 insertions(+), 9 deletions(-) diff --git a/Sources/Rendering/OpenGL/HardwareSelector/api.md b/Sources/Rendering/OpenGL/HardwareSelector/api.md index 80562718176..dd73085fa00 100644 --- a/Sources/Rendering/OpenGL/HardwareSelector/api.md +++ b/Sources/Rendering/OpenGL/HardwareSelector/api.md @@ -25,3 +25,8 @@ if (hws.captureBuffers()) { hws.releasePixBuffers(); } ``` + +## releaseGraphicsResources() + +Releases the captured pixel buffers and the framebuffer resources owned by the +selector. This is called automatically when the selector is deleted. diff --git a/Sources/Rendering/OpenGL/HardwareSelector/index.d.ts b/Sources/Rendering/OpenGL/HardwareSelector/index.d.ts index ef7cabb94b0..8d64ece42e8 100644 --- a/Sources/Rendering/OpenGL/HardwareSelector/index.d.ts +++ b/Sources/Rendering/OpenGL/HardwareSelector/index.d.ts @@ -53,6 +53,11 @@ export interface vtkOpenGLHardwareSelector extends vtkHardwareSelector { */ releasePixBuffers(): void; + /** + * Releases the pixel buffers and GPU resources owned by this selector. + */ + releaseGraphicsResources(): void; + /** * Preps for picking the scene. * diff --git a/Sources/Rendering/OpenGL/HardwareSelector/index.js b/Sources/Rendering/OpenGL/HardwareSelector/index.js index 7dec9ef9598..a4f3cb93e29 100644 --- a/Sources/Rendering/OpenGL/HardwareSelector/index.js +++ b/Sources/Rendering/OpenGL/HardwareSelector/index.js @@ -347,6 +347,12 @@ function vtkOpenGLHardwareSelector(publicAPI, model) { model.zBuffer = null; }; + publicAPI.releaseGraphicsResources = () => { + publicAPI.releasePixBuffers(); + model.framebuffer?.delete(); + model.framebuffer = null; + }; + //---------------------------------------------------------------------------- publicAPI.beginSelection = () => { model._openGLRenderer = model._openGLRenderWindow.getViewNodeFor( @@ -667,6 +673,11 @@ function vtkOpenGLHardwareSelector(publicAPI, model) { model.propColorValue[2] = (Math.floor(val / 65536) % 256) / 255.0; }; + publicAPI.delete = macro.chain( + () => publicAPI.releaseGraphicsResources(), + publicAPI.delete + ); + // info has // valid // propId diff --git a/Sources/Widgets/Core/WidgetManager/api.md b/Sources/Widgets/Core/WidgetManager/api.md index 4d1a43fb757..2e0c1d8c21b 100644 --- a/Sources/Widgets/Core/WidgetManager/api.md +++ b/Sources/Widgets/Core/WidgetManager/api.md @@ -1,5 +1,9 @@ vtkWidgetManager manages view widgets for a given renderer. +Deleting a widget manager removes and deletes its per-view widgets and deletes +its hardware selector. If a selection capture is in progress, selector deletion +is deferred until that capture settles. + ## enablePicking() Enable widget picking in the renderer. diff --git a/Sources/Widgets/Core/WidgetManager/index.js b/Sources/Widgets/Core/WidgetManager/index.js index 772f5152bef..4248e4c2bad 100644 --- a/Sources/Widgets/Core/WidgetManager/index.js +++ b/Sources/Widgets/Core/WidgetManager/index.js @@ -58,6 +58,24 @@ function vtkWidgetManager(publicAPI, model) { model.classHierarchy.push('vtkWidgetManager'); const propsWeakMap = new WeakMap(); const subscriptions = []; + // not a model field: delete() wipes the model and this must survive it + let tearingDown = false; + + function deleteSelectorWhenReady(selector, captureInProgress) { + if (!selector) { + return; + } + const deleteSelector = () => { + if (!selector.isDeleted()) { + selector.delete(); + } + }; + if (captureInProgress) { + captureInProgress.then(deleteSelector, deleteSelector); + } else { + deleteSelector(); + } + } // -------------------------------------------------------------------------- // API internal @@ -129,8 +147,17 @@ function vtkWidgetManager(publicAPI, model) { async function updateSelection(callData, fromTouchEvent, callID) { const { position } = callData; + const selectedData = await publicAPI.getSelectedDataForXY( + position.x, + position.y + ); + + if (publicAPI.isDeleted()) { + return; + } + const { requestCount, selectedState, representation, widget } = - await publicAPI.getSelectedDataForXY(position.x, position.y); + selectedData; if (requestCount || callID !== model._currentUpdateSelectionCallID) { // requestCount > 0: Call activate only once @@ -229,14 +256,23 @@ function vtkWidgetManager(publicAPI, model) { renderPickingBuffer(); model._capturedBuffers = null; - model._captureInProgress = model._selector.getSourceDataAsync( + const captureInProgress = model._selector.getSourceDataAsync( model._renderer, x1, y1, x2, y2 ); - model._capturedBuffers = await model._captureInProgress; + model._captureInProgress = captureInProgress; + const capturedBuffers = await captureInProgress; + // deleted or re-targeted while awaiting: the buffers describe a stale scene + if ( + publicAPI.isDeleted() || + model._captureInProgress !== captureInProgress + ) { + return; + } + model._capturedBuffers = capturedBuffers; model._captureInProgress = null; model.previousSelectedData = null; renderFrontBuffer(); @@ -248,6 +284,11 @@ function vtkWidgetManager(publicAPI, model) { }; publicAPI.renderWidgets = () => { + // Losing focus re-enables picking, so a widget focused at deletion time + // would otherwise start a full window capture on the way out. + if (tearingDown) { + return; + } if (model.pickingEnabled && model.captureOn === CaptureOn.MOUSE_RELEASE) { const [w, h] = model._apiSpecificRenderWindow.getSize(); captureBuffers(0, 0, w, h); @@ -262,6 +303,9 @@ function vtkWidgetManager(publicAPI, model) { }; publicAPI.setRenderer = (renderer) => { + deleteSelectorWhenReady(model._selector, model._captureInProgress); + model._capturedBuffers = null; + model._captureInProgress = null; const renderingComponents = extractRenderingComponents(renderer); Object.assign(model, renderingComponents); macro.moveToProtected({}, model, Object.keys(renderingComponents)); @@ -355,8 +399,18 @@ function vtkWidgetManager(publicAPI, model) { }; function removeWidgetInternal(viewWidget) { - model._renderer.removeActor(viewWidget); - viewWidget.delete(); + if (model._renderer && !model._renderer.isDeleted()) { + model._renderer.removeActor(viewWidget); + } + if (!viewWidget.isDeleted()) { + viewWidget.delete(); + } + } + + function removeAllWidgetsInternal() { + model.widgets.forEach(removeWidgetInternal); + model.widgets = []; + model.widgetInFocus = null; } function onWidgetRemoved() { @@ -365,9 +419,7 @@ function vtkWidgetManager(publicAPI, model) { } publicAPI.removeWidgets = () => { - model.widgets.forEach(removeWidgetInternal); - model.widgets = []; - model.widgetInFocus = null; + removeAllWidgetsInternal(); onWidgetRemoved(); }; @@ -406,6 +458,12 @@ function vtkWidgetManager(publicAPI, model) { } } + // a capture can settle with nothing to select against: the manager was + // deleted or re-targeted while awaiting, or the selector had no view + if (publicAPI.isDeleted() || !model._capturedBuffers) { + return {}; + } + model.selections = model._capturedBuffers.generateSelection(x, y, x, y); } return publicAPI.getSelectedData(); @@ -467,9 +525,18 @@ function vtkWidgetManager(publicAPI, model) { const superDelete = publicAPI.delete; publicAPI.delete = () => { + if (publicAPI.isDeleted()) { + return; + } + tearingDown = true; while (subscriptions.length) { subscriptions.pop().unsubscribe(); } + // a focused widget holds an animation request on the interactor that only + // losing focus cancels + publicAPI.releaseFocus(); + removeAllWidgetsInternal(); + deleteSelectorWhenReady(model._selector, model._captureInProgress); superDelete(); }; } diff --git a/Sources/Widgets/Core/WidgetManager/test/testWidgetManager.js b/Sources/Widgets/Core/WidgetManager/test/testWidgetManager.js index c47ac6e5a7a..43cab9153c0 100644 --- a/Sources/Widgets/Core/WidgetManager/test/testWidgetManager.js +++ b/Sources/Widgets/Core/WidgetManager/test/testWidgetManager.js @@ -1,9 +1,12 @@ -import { it } from 'vitest'; +import { expect, it } from 'vitest'; import testUtils from 'vtk.js/Sources/Testing/testUtils'; +import { createTrackedRenderView } from 'vtk.js/Sources/Testing/renderTestUtils'; import 'vtk.js/Sources/Rendering/Misc/RenderingAPIs'; import vtkGenericRenderWindow from 'vtk.js/Sources/Rendering/Misc/GenericRenderWindow'; import vtkPolyLineWidget from 'vtk.js/Sources/Widgets/Widgets3D/PolyLineWidget'; +import vtkRenderer from 'vtk.js/Sources/Rendering/Core/Renderer'; import vtkWidgetManager from 'vtk.js/Sources/Widgets/Core/WidgetManager'; +import { CaptureOn } from 'vtk.js/Sources/Widgets/Core/WidgetManager/Constants'; import noScaleInPixelsWithPerspectiveBaseline from './testNoScaleInPixelsWithPerspectiveBaseline.png'; import noScaleInPixelsWithParallelBaseline from './testNoScaleInPixelsWithParallelBaseline.png'; @@ -151,3 +154,104 @@ it.skipIf(__VTK_TEST_NO_WEBGL__)('Test getPixelWorldHeightAtCoord', () => { .reduce((current, next) => current.then(next), Promise.resolve()) .finally(gc.releaseResources); }); + +it.skipIf(__VTK_TEST_NO_WEBGL__)( + 'cleans up an in-flight selection and its view widgets when deleted', + async () => { + const gc = testUtils.createGarbageCollector(); + const { tracker, renderer, renderWindow, emptySceneObjects } = + createTrackedRenderView(gc); + + const widgetManager = vtkWidgetManager.newInstance(); + widgetManager.setRenderer(renderer); + const widget = gc.registerResource(vtkPolyLineWidget.newInstance()); + const viewWidget = widgetManager.addWidget(widget); + renderWindow.render(); + + const selection = widgetManager.getSelectedDataForXY(0, 0); + expect(tracker.count()).toBeGreaterThan(emptySceneObjects); + + // application teardown order: manager first, with a selection in flight + widgetManager.delete(); + await expect(selection).resolves.toEqual({}); + + expect(renderer.getActors()).not.toContain(viewWidget); + expect(widget.getViewIds()).toEqual([]); + + // the view tree frees removed actors on the next render + renderWindow.render(); + expect(tracker.count()).toBe(emptySceneObjects); + gc.releaseResources(); + } +); + +it.skipIf(__VTK_TEST_NO_WEBGL__)( + 'releases the selector owned for a previous renderer', + async () => { + const gc = testUtils.createGarbageCollector(); + const { tracker, renderer, renderWindow, emptySceneObjects } = + createTrackedRenderView(gc); + + const widgetManager = vtkWidgetManager.newInstance(); + widgetManager.setRenderer(renderer); + await widgetManager.getSelectedDataForXY(0, 0); + const selectorObjects = tracker.count(); + expect(selectorObjects).toBeGreaterThan(emptySceneObjects); + + // same-renderer setRenderer is still a full re-wire and releases the + // selector built for the previous wiring + widgetManager.setRenderer(renderer); + expect(tracker.count()).toBe(emptySceneObjects); + expect(widgetManager.get('_camera')._camera).toBe( + renderer.getActiveCamera() + ); + + const otherRenderer = gc.registerResource(vtkRenderer.newInstance()); + renderWindow.addRenderer(otherRenderer); + // create the view node the selector will pick against + renderWindow.render(); + widgetManager.setRenderer(otherRenderer); + expect(tracker.count()).toBe(emptySceneObjects); + + await widgetManager.getSelectedDataForXY(0, 0); + expect(tracker.count()).toBeGreaterThan(emptySceneObjects); + widgetManager.delete(); + expect(tracker.count()).toBe(emptySceneObjects); + + renderWindow.removeRenderer(otherRenderer); + gc.releaseResources(); + } +); + +it.skipIf(__VTK_TEST_NO_WEBGL__)( + 'does not start a picking capture while deleting a focused widget', + async () => { + const gc = testUtils.createGarbageCollector(); + const { tracker, renderer, renderWindow, emptySceneObjects } = + createTrackedRenderView(gc); + + const widgetManager = vtkWidgetManager.newInstance({ + captureOn: CaptureOn.MOUSE_RELEASE, + }); + widgetManager.setRenderer(renderer); + const widget = gc.registerResource(vtkPolyLineWidget.newInstance()); + widgetManager.addWidget(widget); + renderWindow.render(); + + // settle the initial capture so nothing in flight defers selector deletion + await widgetManager.getSelectedDataForXY(0, 0); + await new Promise((resolve) => setTimeout(resolve, 0)); + + // deleting while focused would otherwise start a MOUSE_RELEASE capture + // and defer freeing the selector behind it + widgetManager.grabFocus(widget); + const beforeDelete = tracker.count(); + + widgetManager.delete(); + expect(tracker.count()).toBeLessThan(beforeDelete); + + renderWindow.render(); + expect(tracker.count()).toBe(emptySceneObjects); + gc.releaseResources(); + } +);