Render pass teardown - #3622
Open
PaulHax wants to merge 5 commits into
Open
Conversation
Render passes are not view nodes and are not registered graphics resources, so none of the teardown walkers reached them: deleting a render window left every framebuffer its passes owned on the GPU. Nothing reclaims those later either, since deleteGLContext only decrements a counter and the context outlives the render window. The WebGPU render window already frees its passes this way. vtkRenderPass gains a releaseGraphicsResources that reaches the passes it delegates to, and each OpenGL pass owning GPU objects chains its own onto it. vtkOpenGLRenderWindow calls the chain from delete and from releaseGraphicsResources, through a module local function because the public method is proxied to the root window, whose passes a child window must not free. The delegate step is what makes a post processing pass releasable: it is the pass installed on the render window, and the forward pass doing the actual rendering is reachable only through it. Without it a released view kept the forward pass and its translucent pass bound to a shader cache that had just been cleared, and drew a different image on the next render. vtkOpenGLOrderIndependentTranslucentPass had such a method already, unreachable from outside the class since it was added, and it threw when called: the shader cache owns the programs it hands out, so there is no releaseGraphicsResources to call on one, and nulling tris left the next traverse with no helper to build a vertex buffer from. Drop the shader reference instead and keep tris, which the trailing modified() refills. Dropping the reference is also what lets the copy vertex array be rebuilt, since it is only ever constructed alongside the shader. Both walk sites skip deleted passes. An application that tears down a pass it owns before the view would otherwise leave the walk reading delegates off a wiped model, and that TypeError propagated out of the render window delete, so the GL context and the container were never released. Tests count live WebGL objects across a view teardown for the translucent, convolution and radial distortion passes, check that a released view draws the same image and reads depth again on the next render, and check that deleting a pass before the view still tears the view down.
The delete chain unsubscribed from the animation rate listener and unregistered the shared scalar and transfer function textures, but left the objects the mapper allocates for itself: the framebuffer it renders through while interacting, its attachments, the jitter texture and the vertex buffers behind tris and copyVAO. Removing a volume from a scene or tearing down a view leaked all of them. Add releaseGraphicsResources and chain it into delete, so the objects go when the view node does, whether removeUnusedNodes prunes it or the view tree is torn down. The copy shader comes from the shader cache, which owns it, so only the reference is dropped. One test drives an interaction so the mapper allocates its framebuffer, then checks the live WebGL object count returns to baseline once the volume leaves the renderer. A second releases the mapper and renders again. It asserts the volume still draws rather than comparing images, because the ray cast jitter texture is rebuilt from fresh noise and the pixels do not repeat.
setRenderPasses swapped the pass list without freeing what the outgoing passes owned. A window that had already rendered stranded its default forward pass, and with it the translucent framebuffer and the three attachments that pass allocates, for the life of the context. The setter now releases the passes absent from the incoming list, so a pass reinstalled at another index keeps what it has. Membership is compared at the top level only: a pass that moves into or out of a delegate chain is released and rebuilds on the next traverse.
A child render window never gets a context of its own, so the delete path skipped its render pass release and stranded everything the passes had allocated in the root's context: with one translucent actor, the order independent translucent pass left its framebuffer, three textures, a vertex array and a vertex buffer on the GPU for the life of the context. The public releaseGraphicsResources is proxied to the root window, so no other path reaches a child's passes either. A child frees GPU objects through methods proxied to the root window, which makes the release order-sensitive: it can only run while the root is alive. The gate now also admits a child whose root is not deleted. Deleting the root first stays safe, since the view node tree deletes children before the root model is wiped, so a child's release still reaches the live root context.
…ion change A new kernel dimension recompiles the convolution shader, and the block that rebuilds it assigned a fresh vtkVertexArrayObject over the existing one. The GL vertex array behind the replaced object was never deleted, so every change stranded one for the life of the context. Tell the existing vertex array its program changed instead. That deletes the GL object it holds and clears the program handle it remembers, which is what the addAttributeArray calls below need: they reject a program that does not match the one the vertex array was built against. The test renders through the pass, changes the kernel dimension, and checks the live WebGL object count is unchanged. Programs are not counted, so the extra object the old code left behind is the stranded vertex array.
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
1 participant
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
Render passes are not view nodes and are not registered graphics resources, so none of the teardown walkers reached them. Deleting a
vtkOpenGLRenderWindowleft every framebuffer its passes owned on the GPU, and nothing reclaimed them later:deleteGLContextonly decrements a counter and the context outlives the render window. The same passes were stranded whensetRenderPassesswapped them out, and a child render window, which has no context of its own, skipped the release entirely.vtkOpenGLVolumeMapperhad the same gap for the objects it allocates for itself, andvtkConvolution2DPassstranded a vertex array on every kernel dimension change.With one translucent actor, the order independent translucent pass alone leaves a framebuffer, three textures, a vertex array and a vertex buffer on the GPU for the life of the context, per deleted view. The WebGPU render window already frees its passes on delete.
Stacked on #3620
Results
Deleting a render window, replacing its render passes, deleting a child render window, or deleting the root before its children returns the live WebGL object count to baseline. A view that releases its graphics resources draws the same image and reads depth again on the next render. Removing a volume from a scene frees the mapper's interaction framebuffer, jitter texture and vertex buffers. Changing a convolution kernel dimension no longer leaks a vertex array.
Changes
vtkRenderPassgainsreleaseGraphicsResources(viewNode), which reaches the passes it delegates to.vtkForwardPass,vtkOpenGLOrderIndependentTranslucentPass,vtkConvolution2DPassandvtkRadialDistortionPasschain their own release onto it. The translucent pass had such a method already, unreachable and throwing when called; it now drops its shader reference instead of releasing a program the shader cache owns, and keeps its helper so the next traverse can rebuild.vtkOpenGLRenderWindowreleases its render passes ondelete()and fromreleaseGraphicsResources(), through a module local walk because the public method is proxied to the root window. The context-wide release now walks child render windows too. A child releases while its root is still alive, which the view node tree guarantees when the root is deleted first.vtkOpenGLRenderWindow.setRenderPasses()releases the passes absent from the incoming list. A pass kept at another index keeps what it has. Membership is compared at the top level only.vtkOpenGLVolumeMappergainsreleaseGraphicsResources(), chained intodelete().vtkConvolution2DPassresets its vertex array viashaderProgramChanged()on a kernel dimension change instead of replacing it.PR and Code Checklist
npm run reformatto have correctly formatted codeTesting
New tests count live WebGL objects by wrapping the create/delete pairs on the rendering context prototypes, across view teardown for the translucent, convolution and radial distortion passes, across
setRenderPasses, across child window deletion in both orders, and across a volume leaving the scene.Image tests check that a released view draws the same image and reads depth again. One test deletes a pass before the view and checks the view still tears down.