From cae8e14780a0bfc57f4aebb6be6c164a9c77ac7e Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Fri, 28 Aug 2026 11:10:09 -0400 Subject: [PATCH] fix(OpenGL/Framebuffer): release owned resources on delete populateFramebuffer creates a color texture and a depth renderbuffer that nothing ever freed: releaseGraphicsResources deleted the framebuffer but not the attachments, and delete did not call it. Track ownership of what populateFramebuffer creates and free it together with the framebuffer when the framebuffer is released or deleted. Caller-supplied attachments stay borrowed. When the render window is already deleted but the shared GL context is still alive, the owned texture is freed directly on the context. No caller in the tree deletes a framebuffer yet, so the delete chain has no consumer until vtkOpenGLHardwareSelector gains one. The attachment release does take effect immediately on the paths that already run: setOpenGLRenderWindow releases when the render window changes, which the volume mapper, hardware selector and the four render passes reach on every render, and create releases before it rebuilds. Nulling glFramebuffer on release also repairs the getSize and getGLFramebuffer resize checks in OpenGL/VolumeMapper, ForwardPass, Convolution2DPass and RadialDistortionPass, which previously reused a deleted framebuffer object. removeColorBuffer assigned the result of splice back to colorBuffers, which keeps the removed entry and drops every later one. Clear the slot in place instead, so indices stay aligned with GL color attachment points, and let bind skip the holes. create re-binds the new framebuffer when the one it released was bound, since deleting a bound framebuffer resets the GL binding to the default one, and repoints a saved binding that named the released framebuffer. Tests cover attachment ownership, repopulation, binding restoration and delete through the public API, counting live WebGL objects. --- Sources/Rendering/OpenGL/Framebuffer/index.js | 79 ++++++++++- .../test/testReleaseGraphicsResources.js | 133 ++++++++++++++++++ 2 files changed, 206 insertions(+), 6 deletions(-) create mode 100644 Sources/Rendering/OpenGL/Framebuffer/test/testReleaseGraphicsResources.js diff --git a/Sources/Rendering/OpenGL/Framebuffer/index.js b/Sources/Rendering/OpenGL/Framebuffer/index.js index 799889ebab8..1f732fe70a1 100644 --- a/Sources/Rendering/OpenGL/Framebuffer/index.js +++ b/Sources/Rendering/OpenGL/Framebuffer/index.js @@ -10,6 +10,31 @@ function vtkFramebuffer(publicAPI, model) { // Set our className model.classHierarchy.push('vtkFramebuffer'); + function releaseOwnedColorBuffer() { + if (model.ownedColorBuffer) { + if (model._openGLRenderWindow && !model._openGLRenderWindow.isDeleted()) { + model.ownedColorBuffer.releaseGraphicsResources( + model._openGLRenderWindow + ); + } else { + // a shared GL context can outlive the render window + model.context?.deleteTexture(model.ownedColorBuffer.getHandle()); + } + model.ownedColorBuffer.delete(); + model.ownedColorBuffer = null; + } + } + + // Releases owned attachments; borrowed ones and the framebuffer stay alive. + function releaseAttachments() { + releaseOwnedColorBuffer(); + if (model.depthTexture) { + model.context?.deleteRenderbuffer(model.depthTexture); + model.depthTexture = null; + } + model.colorBuffers = []; + } + publicAPI.getBothMode = () => model.context.FRAMEBUFFER; // publicAPI.getDrawMode = () => model.context.DRAW_FRAMEBUFFER; // publicAPI.getReadMode = () => model.context.READ_FRAMEBUFFER; @@ -73,9 +98,8 @@ function vtkFramebuffer(publicAPI, model) { mode = model.context.FRAMEBUFFER; } model.context.bindFramebuffer(mode, model.glFramebuffer); - for (let i = 0; i < model.colorBuffers.length; i++) { - model.colorBuffers[i].bind(); - } + // removed attachments leave empty slots; indices match attachment points + model.colorBuffers.forEach((buffer) => buffer?.bind()); model._openGLRenderWindow.setActiveFramebuffer(publicAPI); }; @@ -87,9 +111,23 @@ function vtkFramebuffer(publicAPI, model) { return; } - model.glFramebuffer = model.context.createFramebuffer(); + const gl = model.context; + const replaced = model.glFramebuffer; + const wasBound = + replaced && gl.getParameter(gl.FRAMEBUFFER_BINDING) === replaced; + const wasSaved = replaced && model.previousDrawBinding === replaced; + publicAPI.releaseGraphicsResources(); + model.glFramebuffer = gl.createFramebuffer(); model.glFramebuffer.width = width; model.glFramebuffer.height = height; + if (wasSaved) { + // the saved binding named the framebuffer this call just deleted + model.previousDrawBinding = model.glFramebuffer; + } + if (wasBound) { + // deleting a bound framebuffer resets the GL binding to default + publicAPI.bind(); + } }; publicAPI.setColorBuffer = (texture, attachment = 0) => { @@ -102,6 +140,13 @@ function vtkFramebuffer(publicAPI, model) { return; } + if ( + model.colorBuffers[attachment] === model.ownedColorBuffer && + texture !== model.ownedColorBuffer + ) { + releaseOwnedColorBuffer(); + } + let glAttachment = gl.COLOR_ATTACHMENT0; if (attachment > 0) { glAttachment += attachment; @@ -126,6 +171,10 @@ function vtkFramebuffer(publicAPI, model) { return; } + if (model.colorBuffers[attachment] === model.ownedColorBuffer) { + releaseOwnedColorBuffer(); + } + let glAttachment = gl.COLOR_ATTACHMENT0; if (attachment > 0) { glAttachment += attachment; @@ -139,7 +188,14 @@ function vtkFramebuffer(publicAPI, model) { 0 ); - model.colorBuffers = model.colorBuffers.splice(attachment, 1); + // clear without shifting: indices map to GL attachment points + model.colorBuffers[attachment] = null; + while ( + model.colorBuffers.length && + model.colorBuffers[model.colorBuffers.length - 1] == null + ) { + model.colorBuffers.pop(); + } }; publicAPI.setDepthBuffer = (texture) => { @@ -193,8 +249,10 @@ function vtkFramebuffer(publicAPI, model) { }; publicAPI.releaseGraphicsResources = () => { + releaseAttachments(); if (model.glFramebuffer) { - model.context.deleteFramebuffer(model.glFramebuffer); + model.context?.deleteFramebuffer(model.glFramebuffer); + model.glFramebuffer = null; } }; @@ -214,6 +272,8 @@ function vtkFramebuffer(publicAPI, model) { publicAPI.bind(); const gl = model.context; + releaseAttachments(); + const texture = vtkOpenGLTexture.newInstance(); texture.setOpenGLRenderWindow(model._openGLRenderWindow); texture.setMinificationFilter(Filter.LINEAR); @@ -225,6 +285,7 @@ function vtkFramebuffer(publicAPI, model) { dataType: VtkDataTypes.UNSIGNED_CHAR, data: null, }); + model.ownedColorBuffer = texture; publicAPI.setColorBuffer(texture); // use a renderbuffer for depth; no consumer samples this @@ -245,6 +306,11 @@ function vtkFramebuffer(publicAPI, model) { ); }; + publicAPI.delete = macro.chain( + publicAPI.releaseGraphicsResources, + publicAPI.delete + ); + // For backwards compatibility. Use getColorBuffers()[0] going forward. publicAPI.getColorTexture = () => model.colorBuffers[0]; } @@ -256,6 +322,7 @@ const DEFAULT_VALUES = { // _openGLRenderWindow: null, glFramebuffer: null, colorBuffers: null, + ownedColorBuffer: null, depthTexture: null, previousDrawBinding: 0, previousReadBinding: 0, diff --git a/Sources/Rendering/OpenGL/Framebuffer/test/testReleaseGraphicsResources.js b/Sources/Rendering/OpenGL/Framebuffer/test/testReleaseGraphicsResources.js new file mode 100644 index 00000000000..908c2f7aebb --- /dev/null +++ b/Sources/Rendering/OpenGL/Framebuffer/test/testReleaseGraphicsResources.js @@ -0,0 +1,133 @@ +import { expect, it } from 'vitest'; +import testUtils from 'vtk.js/Sources/Testing/testUtils'; +import { createTrackedRenderView } from 'vtk.js/Sources/Testing/renderTestUtils'; + +import { VtkDataTypes } from 'vtk.js/Sources/Common/Core/DataArray/Constants'; +import vtkOpenGLFramebuffer from 'vtk.js/Sources/Rendering/OpenGL/Framebuffer'; +import vtkOpenGLTexture from 'vtk.js/Sources/Rendering/OpenGL/Texture'; + +function createTexture(renderWindow) { + const texture = vtkOpenGLTexture.newInstance(); + texture.setOpenGLRenderWindow(renderWindow); + texture.create2DFromRaw({ + width: 32, + height: 32, + numComps: 4, + dataType: VtkDataTypes.UNSIGNED_CHAR, + data: null, + }); + return texture; +} + +function releaseTexture(texture, renderWindow) { + texture.releaseGraphicsResources(renderWindow); + texture.delete(); +} + +it.skipIf(__VTK_TEST_NO_WEBGL__)( + 'releases owned resources without releasing borrowed attachments', + () => { + const gc = testUtils.createGarbageCollector(); + const { tracker, view, emptySceneObjects } = createTrackedRenderView(gc); + + const borrowedTexture = createTexture(view); + const borrowedTextureObjects = tracker.count(); + expect(borrowedTextureObjects).toBeGreaterThan(emptySceneObjects); + + const framebuffer = vtkOpenGLFramebuffer.newInstance(); + framebuffer.setOpenGLRenderWindow(view); + framebuffer.saveCurrentBindingsAndBuffers(); + framebuffer.create(32, 32); + framebuffer.populateFramebuffer(); + const populatedObjects = tracker.count(); + expect(populatedObjects).toBeGreaterThan(borrowedTextureObjects); + + // replacing the owned attachment frees it; the replacement stays borrowed + framebuffer.setColorBuffer(borrowedTexture); + expect(tracker.count()).toBe(populatedObjects - 1); + expect(view.getContext().isTexture(borrowedTexture.getHandle())).toBe(true); + + // rebuilding replaces the owned objects instead of accumulating them + framebuffer.create(64, 64); + framebuffer.populateFramebuffer(); + expect(tracker.count()).toBe(populatedObjects); + + const beforeRemove = tracker.count(); + framebuffer.removeColorBuffer(); + expect(tracker.count()).toBe(beforeRemove - 1); + expect(() => framebuffer.bind()).not.toThrow(); + framebuffer.restorePreviousBindingsAndBuffers(); + + framebuffer.delete(); + expect(tracker.count()).toBe(borrowedTextureObjects); + + releaseTexture(borrowedTexture, view); + expect(tracker.count()).toBe(emptySceneObjects); + gc.releaseResources(); + } +); + +// The surface LIC passes attach at sparse locations, remove attachments while +// others stay, and recreate the framebuffer while it is bound. +it.skipIf(__VTK_TEST_NO_WEBGL__)( + 'keeps attachment slots aligned and stays bound across create', + () => { + const gc = testUtils.createGarbageCollector(); + const { view } = createTrackedRenderView(gc); + const gl = view.getContext(); + + const texture0 = createTexture(view); + const texture2 = createTexture(view); + const framebuffer = vtkOpenGLFramebuffer.newInstance(); + framebuffer.setOpenGLRenderWindow(view); + framebuffer.saveCurrentBindingsAndBuffers(); + framebuffer.create(32, 32); + framebuffer.bind(); + framebuffer.setColorBuffer(texture0, 0); + framebuffer.setColorBuffer(texture2, 2); + + // Removing one attachment must not shift the others off their slots. + framebuffer.removeColorBuffer(0); + expect(framebuffer.getColorBuffers()[2]).toBe(texture2); + expect(() => framebuffer.bind()).not.toThrow(); + + // recreating while bound must keep the caller on the new framebuffer + framebuffer.create(32, 32); + expect(gl.getParameter(gl.FRAMEBUFFER_BINDING)).toBe( + framebuffer.getGLFramebuffer() + ); + + framebuffer.restorePreviousBindingsAndBuffers(); + framebuffer.delete(); + releaseTexture(texture0, view); + releaseTexture(texture2, view); + gc.releaseResources(); + } +); + +it.skipIf(__VTK_TEST_NO_WEBGL__)( + 'restores a saved binding that create() replaced', + () => { + const gc = testUtils.createGarbageCollector(); + const { renderWindow, view } = createTrackedRenderView(gc); + renderWindow.render(); + const gl = view.getContext(); + + const framebuffer = vtkOpenGLFramebuffer.newInstance(); + framebuffer.setOpenGLRenderWindow(view); + framebuffer.create(32, 32); + framebuffer.bind(); + + // the saved binding is this framebuffer, which the next create() deletes + framebuffer.saveCurrentBindingsAndBuffers(); + framebuffer.create(64, 64); + framebuffer.restorePreviousBindingsAndBuffers(); + + expect(gl.getError()).toBe(gl.NO_ERROR); + expect(gl.getParameter(gl.FRAMEBUFFER_BINDING)).toBe( + framebuffer.getGLFramebuffer() + ); + + framebuffer.delete(); + } +);