From b8ecd529066ee6a94dfd1be1c2386b9a383d7285 Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Thu, 5 Nov 2026 12:49:49 -0500 Subject: [PATCH 1/2] fix(rendering): skip deferred renders after view disposal --- src/core/vtk/__tests__/useVtkView.spec.ts | 65 +++++++++++++++++++++++ src/core/vtk/useVtkView.ts | 2 + 2 files changed, 67 insertions(+) diff --git a/src/core/vtk/__tests__/useVtkView.spec.ts b/src/core/vtk/__tests__/useVtkView.spec.ts index b01a0fbee..ec9f265fa 100644 --- a/src/core/vtk/__tests__/useVtkView.spec.ts +++ b/src/core/vtk/__tests__/useVtkView.spec.ts @@ -4,6 +4,7 @@ import { mount } from '@vue/test-utils'; import vtkRenderWindow from '@kitware/vtk.js/Rendering/Core/RenderWindow'; import vtkRenderWindowInteractor from '@kitware/vtk.js/Rendering/Core/RenderWindowInteractor'; import vtkOpenGLRenderWindow from '@kitware/vtk.js/Rendering/OpenGL/RenderWindow'; +import vtkWidgetManager from '@kitware/vtk.js/Widgets/Core/WidgetManager'; import { VtkRenderWindowParentContext } from '@/src/components/vtk/context'; import { VtkRenderWindowParentApi } from '@/src/types/vtk-types'; import { useVtkView, useWebGLRenderWindow } from '@/src/core/vtk/useVtkView'; @@ -61,12 +62,76 @@ function mountViewUnder(parent: VtkRenderWindowParentApi) { describe('useVtkView teardown', () => { beforeEach(() => { vi.useFakeTimers(); + // newInstance freezes the API. Use the same implementation without freezing + // it so the tests can observe renders and inject a cleanup failure. + vi.spyOn(vtkRenderWindow, 'newInstance').mockImplementation(() => { + const api = {} as vtkRenderWindow; + vtkRenderWindow.extend(api, {}); + return api; + }); + vi.spyOn(vtkWidgetManager, 'newInstance').mockImplementation(() => { + const api = {} as vtkWidgetManager; + vtkWidgetManager.extend(api, {}); + return api; + }); }); afterEach(() => { + vi.restoreAllMocks(); vi.useRealTimers(); }); + it.each(['live', 'disposed', 'cleanup throws'] as const)( + 'only executes queued renders for a live view: %s', + (state) => { + const scope = effectScope(true); + const view = scope.run(() => useVtkView(ref(null)))!; + // Observe render dispatch without needing a WebGL context in the test DOM. + const render = vi + .spyOn(view.renderWindow, 'render') + .mockImplementation(() => {}); + const renderWidgets = vi + .spyOn(view.widgetManager, 'renderWidgets') + .mockImplementation(() => {}); + + try { + view.requestRender(); + expect(vi.getTimerCount()).toBe(1); + + if (state === 'cleanup throws') { + const error = new Error('selector cleanup failed'); + vi.spyOn(view.widgetManager, 'delete').mockImplementationOnce(() => { + throw error; + }); + expect(() => scope.stop()).toThrow(error); + expect(view.renderWindowView.isDeleted()).toBe(true); + // Cleanup stopped before reaching deferredRender.cancel(). + expect(vi.getTimerCount()).toBe(1); + } else if (state === 'disposed') { + scope.stop(); + expect(vi.getTimerCount()).toBe(0); + } + + vi.runAllTimers(); + + const expectedRenders = state === 'live' ? 1 : 0; + expect(renderWidgets).toHaveBeenCalledTimes(expectedRenders); + expect(render).toHaveBeenCalledTimes(expectedRenders); + } finally { + scope.stop(); + // A throwing scope cleanup does not resume on a second stop(). + [ + view.widgetManager, + view.renderer, + view.renderWindow, + view.interactor, + ].forEach((object) => { + if (!object.isDeleted()) object.delete(); + }); + } + } + ); + it('queues no render work once the view scope is disposed', () => { const container = document.createElement('div'); const scope = effectScope(true); diff --git a/src/core/vtk/useVtkView.ts b/src/core/vtk/useVtkView.ts index c7332e388..eaf853f3e 100644 --- a/src/core/vtk/useVtkView.ts +++ b/src/core/vtk/useVtkView.ts @@ -153,6 +153,8 @@ export function useVtkView( // render API const deferredRender = batchForNextTask(() => { + // Cleanup can throw before the queued render is cancelled. + if (disposed) return; // don't need to re-render during animation if (interactor.isAnimating()) return; widgetManager.renderWidgets(); From 22568e1e704441482924cd4185dea578b9efb06f Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Thu, 5 Nov 2026 14:41:48 -0500 Subject: [PATCH 2/2] test(rendering): observe deferred renders through public events --- src/core/vtk/__tests__/useVtkView.spec.ts | 50 ++++++++++------------- 1 file changed, 22 insertions(+), 28 deletions(-) diff --git a/src/core/vtk/__tests__/useVtkView.spec.ts b/src/core/vtk/__tests__/useVtkView.spec.ts index ec9f265fa..eaae758a7 100644 --- a/src/core/vtk/__tests__/useVtkView.spec.ts +++ b/src/core/vtk/__tests__/useVtkView.spec.ts @@ -4,7 +4,6 @@ import { mount } from '@vue/test-utils'; import vtkRenderWindow from '@kitware/vtk.js/Rendering/Core/RenderWindow'; import vtkRenderWindowInteractor from '@kitware/vtk.js/Rendering/Core/RenderWindowInteractor'; import vtkOpenGLRenderWindow from '@kitware/vtk.js/Rendering/OpenGL/RenderWindow'; -import vtkWidgetManager from '@kitware/vtk.js/Widgets/Core/WidgetManager'; import { VtkRenderWindowParentContext } from '@/src/components/vtk/context'; import { VtkRenderWindowParentApi } from '@/src/types/vtk-types'; import { useVtkView, useWebGLRenderWindow } from '@/src/core/vtk/useVtkView'; @@ -62,22 +61,9 @@ function mountViewUnder(parent: VtkRenderWindowParentApi) { describe('useVtkView teardown', () => { beforeEach(() => { vi.useFakeTimers(); - // newInstance freezes the API. Use the same implementation without freezing - // it so the tests can observe renders and inject a cleanup failure. - vi.spyOn(vtkRenderWindow, 'newInstance').mockImplementation(() => { - const api = {} as vtkRenderWindow; - vtkRenderWindow.extend(api, {}); - return api; - }); - vi.spyOn(vtkWidgetManager, 'newInstance').mockImplementation(() => { - const api = {} as vtkWidgetManager; - vtkWidgetManager.extend(api, {}); - return api; - }); }); afterEach(() => { - vi.restoreAllMocks(); vi.useRealTimers(); }); @@ -86,25 +72,33 @@ describe('useVtkView teardown', () => { (state) => { const scope = effectScope(true); const view = scope.run(() => useVtkView(ref(null)))!; - // Observe render dispatch without needing a WebGL context in the test DOM. - const render = vi - .spyOn(view.renderWindow, 'render') - .mockImplementation(() => {}); - const renderWidgets = vi - .spyOn(view.widgetManager, 'renderWidgets') - .mockImplementation(() => {}); + // With no container, the interactor emits render events without drawing. + const events: string[] = []; + const rendered = view.interactor.onRenderEvent(() => + events.push('render') + ); + const widgetsModified = view.widgetManager.onModified(() => + events.push('widgets') + ); try { view.requestRender(); expect(vi.getTimerCount()).toBe(1); if (state === 'cleanup throws') { - const error = new Error('selector cleanup failed'); - vi.spyOn(view.widgetManager, 'delete').mockImplementationOnce(() => { - throw error; + const error = new Error('view removal listener failed'); + // removeView emits Modified after the OpenGL view has been deleted, + // but before disposal reaches deferredRender.cancel(). + const failure = view.renderWindow.onModified(() => { + if (!view.renderWindow.hasView(view.renderWindowView)) throw error; }); - expect(() => scope.stop()).toThrow(error); + try { + expect(() => scope.stop()).toThrow(error); + } finally { + failure?.unsubscribe(); + } expect(view.renderWindowView.isDeleted()).toBe(true); + expect(view.renderWindow.hasView(view.renderWindowView)).toBe(false); // Cleanup stopped before reaching deferredRender.cancel(). expect(vi.getTimerCount()).toBe(1); } else if (state === 'disposed') { @@ -114,10 +108,10 @@ describe('useVtkView teardown', () => { vi.runAllTimers(); - const expectedRenders = state === 'live' ? 1 : 0; - expect(renderWidgets).toHaveBeenCalledTimes(expectedRenders); - expect(render).toHaveBeenCalledTimes(expectedRenders); + expect(events).toEqual(state === 'live' ? ['widgets', 'render'] : []); } finally { + rendered.unsubscribe(); + widgetsModified?.unsubscribe(); scope.stop(); // A throwing scope cleanup does not resume on a second stop(). [