Extract FramePass base class from RenderPass#8523
Conversation
Extract generic frame graph scheduling logic into a new FramePass base class. RenderPass now extends FramePass, keeping only GPU render pass specifics. 8 non-render subclasses renamed from RenderPass* to FramePass* and re-parented. CameraComponent.renderPasses renamed to framePasses with deprecated forwarding. Made-with: Cursor
There was a problem hiding this comment.
Pull request overview
This PR refactors the frame-graph pass model by extracting generic scheduling/lifecycle behavior into a new FramePass base class, leaving RenderPass focused on GPU render-pass specifics, and updates engine/extras/examples to use the new naming and APIs.
Changes:
- Introduced
FramePassand madeRenderPassextend it, separating scheduling vs GPU render-pass responsibilities. - Renamed multiple non-render pass classes from
RenderPass*toFramePass*and updated camera integration fromrenderPasses→framePasses(with deprecation forwarding onCameraComponent). - Updated exports and examples to use new
FramePass*names.
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/scene/renderer/renderer.js | Uses FramePassUpdateClustered instead of RenderPassUpdateClustered. |
| src/scene/renderer/frame-pass-update-clustered.js | Renames/re-parents clustered update pass to FramePass. |
| src/scene/renderer/frame-pass-postprocessing.js | Converts postprocessing trigger pass to a FramePass. |
| src/scene/renderer/forward-renderer.js | Schedules camera framePasses and uses FramePassPostprocessing. |
| src/scene/gsplat-unified/gsplat-manager.js | Adjusts destruction order around GPU/CPU sorting teardown. |
| src/scene/graphics/frame-pass-radix-sort.js | Re-parents radix sort scheduler pass to FramePass. |
| src/scene/graphics/frame-pass-depth-grab.js | Re-parents depth grab pass to FramePass. |
| src/scene/graphics/frame-pass-color-grab.js | Re-parents color grab pass to FramePass. |
| src/scene/frame-graph.js | Updates typing/docs to treat the frame graph as a sequence of FramePass nodes. |
| src/scene/composition/layer-composition.js | Switches camera custom-pass detection to framePasses. |
| src/scene/camera.js | Renames internal camera list from renderPasses to framePasses. |
| src/platform/graphics/render-pass.js | Makes RenderPass extend FramePass, keeping GPU render-pass responsibilities. |
| src/platform/graphics/frame-pass.js | Adds the new FramePass base class (scheduling + lifecycle + tracing). |
| src/index.js | Exports FramePass and new FramePass* symbols. |
| src/framework/components/camera/component.js | Adds framePasses API and keeps renderPasses as deprecated forwarder. |
| src/extras/render-passes/frame-pass-dof.js | Renames/re-parents DoF pass to FramePassDof. |
| src/extras/render-passes/frame-pass-camera-frame.js | Renames/re-parents camera-frame orchestration pass to FramePassCameraFrame. |
| src/extras/render-passes/frame-pass-bloom.js | Renames/re-parents bloom pass to FramePassBloom. |
| src/extras/render-passes/camera-frame.js | Updates camera-frame helper to install framePasses and use FramePassCameraFrame. |
| src/extras/index.js | Updates extras exports to new FramePass* names. |
| examples/src/examples/test/radix-sort.example.mjs | Updates example to use FramePassRadixSort. |
| examples/src/examples/graphics/render-pass.example.mjs | Updates example to set cameraEntity.camera.framePasses. |
Comments suppressed due to low confidence (3)
src/scene/graphics/frame-pass-color-grab.js:15
- The header comment still describes this as a "render pass", but the class now extends
FramePassand does not represent a GPU render pass. Updating the wording to "frame pass" would keep the documentation consistent with the refactor.
src/scene/graphics/frame-pass-depth-grab.js:15 - The header comment still describes this as a "render pass", but the class now extends
FramePassand is a non-rendering scheduling pass. Updating the wording to "frame pass" would better reflect its role.
src/scene/renderer/frame-pass-update-clustered.js:12 - The class doc comment still refers to this as a "render pass" even though it now extends
FramePass. Please update the wording to avoid confusion between GPU render passes (RenderPass) and scheduling passes (FramePass).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| @@ -919,8 +919,8 @@ class ForwardRenderer extends Renderer { | |||
| } | |||
There was a problem hiding this comment.
The warning message still says the camera "uses render passes" but the API and surrounding code now refer to framePasses. Updating the text to "frame passes" would avoid confusing users reading this warning after the rename.
| /** | ||
| * Creates a render pass for the camera frame. Override this method to utilize a custom render | ||
| * pass, typically one that extends {@link RenderPassCameraFrame}. | ||
| * pass, typically one that extends {@link FramePassCameraFrame}. | ||
| * | ||
| * @returns {RenderPassCameraFrame} - The render pass. | ||
| * @returns {FramePassCameraFrame} - The render pass. | ||
| */ |
There was a problem hiding this comment.
The JSDoc still describes this as creating/returning a "render pass" even though the API now returns a FramePassCameraFrame and is assigned to framePasses. Consider updating the wording (and the @returns description) to "frame pass" for consistency.
| */ | ||
| renderPasses = []; | ||
| framePasses = []; | ||
|
|
There was a problem hiding this comment.
Camera is exported as part of the public API (src/index.js exports it) and this change renames renderPasses → framePasses on the Camera class itself (not just CameraComponent). To avoid breaking code that accesses camera.camera.renderPasses directly, consider adding a deprecated forwarding getter/setter on Camera similar to what was added on CameraComponent.
| /** | |
| * Deprecated alias for {@link Camera#framePasses}. | |
| * | |
| * @deprecated Use {@link Camera#framePasses} instead. | |
| * @type {FramePass[]} | |
| */ | |
| get renderPasses() { | |
| return this.framePasses; | |
| } | |
| /** | |
| * @deprecated Use {@link Camera#framePasses} instead. | |
| * @param {FramePass[]} value - New frame passes. | |
| */ | |
| set renderPasses(value) { | |
| this.framePasses = value; | |
| } |
| export { SSAOTYPE_NONE, SSAOTYPE_LIGHTING, SSAOTYPE_COMBINE } from './render-passes/constants.js'; | ||
| export { RenderPassCameraFrame, CameraFrameOptions } from './render-passes/render-pass-camera-frame.js'; | ||
| export { FramePassCameraFrame, CameraFrameOptions } from './render-passes/frame-pass-camera-frame.js'; | ||
| export { RenderPassCompose } from './render-passes/render-pass-compose.js'; | ||
| export { RenderPassDepthAwareBlur } from './render-passes/render-pass-depth-aware-blur.js'; | ||
| export { RenderPassDof } from './render-passes/render-pass-dof.js'; | ||
| export { FramePassDof } from './render-passes/frame-pass-dof.js'; | ||
| export { RenderPassDownsample } from './render-passes/render-pass-downsample.js'; | ||
| export { RenderPassUpsample } from './render-passes/render-pass-upsample.js'; | ||
| export { RenderPassBloom } from './render-passes/render-pass-bloom.js'; | ||
| export { FramePassBloom } from './render-passes/frame-pass-bloom.js'; |
There was a problem hiding this comment.
src/extras/index.js replaced exported symbols (RenderPassCameraFrame, RenderPassDof, RenderPassBloom) with FramePass* equivalents without providing deprecated aliases. If extras are part of the public API, consider re-exporting the old names as deprecated forwards to avoid breaking existing imports.
|
Nice! I've been making use of render passes for compute and it felt a little dirty so this is a good start |
Exactly what pushed me to do something about it, before I make it all public. |
|
Let me know if you have some other feedback / suggestions! |
Summary
FramePassbase class that encapsulates generic frame graph scheduling logic (enable/disable, before/after passes, execution lifecycle, debug tracing)RenderPassnow extendsFramePass, retaining only GPU render pass specifics (render targets, color/depth ops, start/end render pass calls)RenderPass*toFramePass*and re-parented to extendFramePassdirectly:RenderPassUpdateClustered→FramePassUpdateClusteredRenderPassPostprocessing→FramePassPostprocessingRenderPassCameraFrame→FramePassCameraFrameRenderPassBloom→FramePassBloomRenderPassDof→FramePassDofRenderPassRadixSort→FramePassRadixSortRenderPassColorGrab→FramePassColorGrabRenderPassDepthGrab→FramePassDepthGrabCameraComponent.renderPassesrenamed toframePasses; old name retained as deprecated forwarding propertyFramePassandRenderPassinstancesMotivation
This refactoring separates the concern of "scheduling a task in the frame graph" from "executing a GPU render pass". This enables future compute-only passes (e.g. WebGPU compute shader dispatches) to participate in the frame graph without inheriting unnecessary render target machinery.