fix(Widgets): release owned picking resources on deletion

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.

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.
    *

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

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.

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();
   };
 }

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 renderTestUtils 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 } =
+      renderTestUtils.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 } =
+      renderTestUtils.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 } =
+      renderTestUtils.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();
+  }
+);