fix(OpenGL/Framebuffer): release owned resources on delete

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?.isDeleted()) {
+        model.ownedColorBuffer.releaseGraphicsResources(
+          model._openGLRenderWindow
+        );
+      } else if (model.ownedColorBuffer.getHandle()) {
+        // 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;
@@ -74,7 +99,8 @@ function vtkFramebuffer(publicAPI, model) {
     }
     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[i]?.bind();
     }
     model._openGLRenderWindow.setActiveFramebuffer(publicAPI);
   };
@@ -87,9 +113,18 @@ function vtkFramebuffer(publicAPI, model) {
       return;
     }
 
-    model.glFramebuffer = model.context.createFramebuffer();
+    const gl = model.context;
+    const wasBound =
+      model.glFramebuffer &&
+      gl.getParameter(gl.FRAMEBUFFER_BINDING) === model.glFramebuffer;
+    publicAPI.releaseGraphicsResources();
+    model.glFramebuffer = gl.createFramebuffer();
     model.glFramebuffer.width = width;
     model.glFramebuffer.height = height;
+    if (wasBound) {
+      // deleting a bound framebuffer resets the GL binding to default
+      publicAPI.bind();
+    }
   };
 
   publicAPI.setColorBuffer = (texture, attachment = 0) => {
@@ -102,6 +137,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 +168,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 +185,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 +246,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 +269,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 +282,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 +303,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 +319,7 @@ const DEFAULT_VALUES = {
   // _openGLRenderWindow: null,
   glFramebuffer: null,
   colorBuffers: null,
+  ownedColorBuffer: null,
   depthTexture: null,
   previousDrawBinding: 0,
   previousReadBinding: 0,

Sources/Rendering/OpenGL/Framebuffer/test/testReleaseGraphicsResources.js

@@ -0,0 +1,106 @@
+import { expect, it } from 'vitest';
+import testUtils from 'vtk.js/Sources/Testing/testUtils';
+import renderTestUtils 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 } =
+      renderTestUtils.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()).toBeLessThan(populatedObjects);
+
+    // 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 } = renderTestUtils.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();
+  }
+);