fix(OpenGL): release poly data mapper buffers when the view node is deleted - #3618
Open
PaulHax wants to merge 1 commit into
Open
fix(OpenGL): release poly data mapper buffers when the view node is deleted#3618PaulHax wants to merge 1 commit into
PaulHax wants to merge 1 commit into
Conversation
…eleted vtkOpenGLPolyDataMapper, vtkOpenGLPolyDataMapper2D and vtkOpenGLGlyph3DMapper had no way to free the VBOs, VAOs and color texture they own on the GL context. Deleting a view node, which is what happens when an actor is removed or a render window is closed, left them allocated. Fifty addActor/removeActor cycles orphan 150 WebGL objects on a live context, and getGraphicsMemoryInfo does not account for them. The leak is not recoverable for a render window that shares a context with a parent: releaseGraphicsResources is proxied to the root render window, so calling it from a child frees resources its siblings still draw with. Add releaseGraphicsResources to those mappers and chain it into delete, so a view frees its own buffers and nothing else. The buffers are rebuilt on the next render. The release logic is shared between the two poly data mappers, which own the same buffer-holding model shape. The tests drive the public API and count live WebGL objects by wrapping the create/delete pairs on the rendering context prototypes, added to testUtils as trackWebGLObjects alongside createRenderContainer. The tracked-view setup and the same-image-after-release assertion they share live in renderTestUtils.
PaulHax
marked this pull request as ready for review
August 28, 2026 21:31
sankhesh
reviewed
Aug 29, 2026
| renderWindow.render(); | ||
| expect(tracker.count()).toBe(emptySceneObjects); | ||
|
|
||
| gc.releaseResources(); |
Collaborator
There was a problem hiding this comment.
Since this is inside an async call, this may never get called if the test fails early leaking resources. Recommend enclosing inside an afterEach to ensure to ensure cleanup.
| expect(tracker.count()).toBeLessThan(bothViewsObjects); | ||
| expect(await survivingAfter).toBe(await survivingBefore); | ||
|
|
||
| gc.releaseResources(); |
| expect(tracker.count()).toBeGreaterThan(emptySceneObjects); | ||
| expect(await afterRelease).toBe(await beforeRelease); | ||
|
|
||
| gc.releaseResources(); |
| expect(tracker.count()).toBe(bothViewsObjects); | ||
|
|
||
| rootRenderWindow.removeRenderWindow(closing.childRenderWindow); | ||
| closing.childRenderWindow.delete(); |
Collaborator
There was a problem hiding this comment.
Wouldn't this lead to double deletion if the garbage collector already tracks resources?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
vtkOpenGLPolyDataMapper,vtkOpenGLPolyDataMapper2DandvtkOpenGLGlyph3DMapperhave no way to free the VBOs, VAOs and color texture they own on the GL context. Deleting a view node, which happens when an actor is removed or a render window closes, leaves them allocated.#2887 gave the image and volume mappers
unregisterGraphicsResourceUserchained intodelete(), so their shared textures are freed when a view closes. The poly data mappers never got the per-view equivalent. Applications cannot recover the leak themselves: when views share one WebGL context, a child view'sreleaseGraphicsResources()is forwarded to the root view and releases the resources of every view on the context, so an application closing one view would break the views that stay open.Found while tightening per-view GPU teardown in Kitware/VolView#932, which documents this as a known gap that needs a vtk.js-side fix: its layout swaps add and remove actors on views sharing one WebGL context, so the orphaned buffers accumulate across the session.
Results
Before: fifty
addActor/removeActorcycles orphan 150 live WebGL objects on a context that stays alive, andgetGraphicsMemoryInfo()reports none of it (it only walks the shared-resource map). After: deleting the view node returns the live-object count to the empty-scene baseline. Separately, releasing a live mapper's resources and rendering again produces an identical image.Changes
vtkOpenGLPolyDataMapper,vtkOpenGLPolyDataMapper2DandvtkOpenGLGlyph3DMappergain a publicreleaseGraphicsResources(), chained intodelete(), so a view node frees its own buffers and nothing else. The release logic is shared between the two poly data mappers, which own the same buffer-holding model shape.Testing/testUtils.jsgainstrackWebGLObjects(counts live WebGL objects by wrapping create/delete pairs on the rendering context prototypes) andcreateRenderContainer; the tracked-view setup and the same-image-after-release assertion live in a newTesting/renderTestUtils.PR and Code Checklist
npm run reformatto have correctly formatted code