refactor: render pass options initialization and execute logic

[wumaolinmaoan]

Summary by CodeRabbit

Release Notes

• New Features

  • Explicit render pass naming for improved debugging and identification

• Refactor

  • Streamlined render pass system architecture and simplified configuration workflows
  • Enhanced mesh management through explicit addition methods
  • Removed legacy delegation callbacks for a cleaner rendering pipeline
  • Simplified rendering lifecycle and state handling

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

Major refactoring of the RenderPass system: simplified public API surface by removing legacy constructs (RenderPassAttachmentOptions, RenderPassDelegate), introduced default values for priority/name/meshes/meshOrder, replaced "destroyed" with "disposed" flag, moved mesh configuration from constructor to runtime addMesh() method, and removed delegate-based lifecycle hooks from renderer.

Changes

Cohort / File(s)	Summary
RenderPass Core Architecture packages/effects-core/src/render/render-pass.ts	Consolidated public API by removing RenderPassAttachmentOptions, RenderPassDelegate, and SemanticMap types; introduced default initializations for priority (0), name (auto-generated), meshes (empty array), and meshOrder (ascending); replaced destroyed flag with disposed; simplified execute/frameCleanup to no-ops; removed custom viewport logic from getViewport; updated constructor signature to remove name, clearAction, semantics, priority, meshOrder, meshes, and delegate parameters.
DrawObjectPass Implementation packages/effects-core/src/render/draw-object-pass.ts	Set priority to RenderPassPriorityNormal and name to 'DrawObjectPass' in constructor; added execute override that performs color/depth/stencil clearing via TextureLoadAction.clear, renders meshes, then clears storeAction; added dispose override that detaches resize listener and calls super.dispose; enhanced onResize to resize framebuffer with current dimensions.
Render Frame Simplification packages/effects-core/src/render/render-frame.ts	Removed imports of OrderType and RenderPassPriorityNormal; removed name, priority, meshOrder, and clearAction parameters from DrawObjectPass and post-process pass constructors (BloomThresholdPass, HQGaussianDownSamplePass, HQGaussianUpSamplePass); eliminated drawObjectPassClearAction object configuration.
Post-Process Pass Naming packages/effects-core/src/render/post-process-pass.ts	Added this.name assignments in constructors: BloomThresholdPass ('BloomThresholdPass'), HQGaussianDownSamplePass ('GaussianDownPass' + type + level), HQGaussianUpSamplePass ('GaussianUpPass' + level), ToneMappingPass ('ToneMappingPass').
Renderer Delegate Removal packages/effects-webgl/src/gl-renderer.ts	Removed delegate-based lifecycle hooks: eliminated delegate.willBeginRenderPass and delegate.didEndRenderPass calls around pass execution; removed delegate.willRenderMesh and delegate.didRenderMesh calls during mesh rendering; replaced delegated mesh rendering with direct mesh.render(this) calls.
Specialized Gizmo Passes plugin-packages/editor-gizmo/src/gizmo-component.ts	Replaced inline RenderPass instantiations with three new specialized subclasses: DrawGizmoBehindPass (priority: Postprocess + Postprocess, clears depth), DrawGizmoFrontPass (priority: Prepare + 2), DrawGizmoEditorPass (priority: Postprocess + 2); each implements custom execute method.
Outline Pass Constructor web-packages/imgui-demo/src/ge.ts	Created OutlinePass constructor that centralizes priority (RenderPassPriorityPostprocess) and name ('OutlinePass') configuration; removed explicit priority/name/meshOrder parameters from call sites.
Disposal Configuration plugin-packages/model/src/utility/plugin-helper.ts	Removed semantics field from RenderPassDestroyOptions passed to WebGLHelper.deleteRenderPass.
Test Updates: Mesh Configuration web-packages/test/unit/src/effects-webgl/gl-dispose.spec.ts, gl-geometry.spec.ts, gl-render-pass.spec.ts	Replaced constructor-time mesh registration (via meshes array in options) with runtime addMesh() calls; made meshOrder publicly assignable post-construction instead of constructor parameter.
Test Updates: Expectations web-packages/test/unit/src/effects-webgl/gl-render-frame.spec.ts	Changed GL_STENCIL_CLEAR_VALUE assertion from 0xff to 0.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant RenderPass
    participant DrawObjectPass
    participant Renderer
    participant Framebuffer

    Note over Client,Framebuffer: New: Constructor-based defaults with specialized subclasses
    Client->>DrawObjectPass: new DrawObjectPass(renderer, options)
    DrawObjectPass->>DrawObjectPass: this.priority = RenderPassPriorityNormal
    DrawObjectPass->>DrawObjectPass: this.name = 'DrawObjectPass'
    Client->>DrawObjectPass: addMesh(mesh)
    DrawObjectPass->>DrawObjectPass: meshes.push(mesh)
    Client->>Renderer: executeRenderPass(drawObjectPass)
    Renderer->>DrawObjectPass: execute(renderer)
    DrawObjectPass->>Framebuffer: clear(color, depth, stencil via TextureLoadAction)
    DrawObjectPass->>Renderer: renderMeshes (direct, no delegate)
    DrawObjectPass->>Framebuffer: storeAction.clear()
    Renderer->>DrawObjectPass: dispose()
    DrawObjectPass->>Renderer: engine.off('resize', onResize)
    DrawObjectPass->>DrawObjectPass: this.disposed = true

    Note over Client,Framebuffer: Old: Constructor-based configuration with delegates
    Note over Client,Framebuffer: (Before: name/priority/meshes passed in constructor, delegate lifecycle hooks invoked)

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

• Particular areas requiring extra attention:
  • render-pass.ts: Extensive refactoring of constructor, lifecycle methods, and public API surface; verify all initialization defaults and disposed flag logic are correct
  • draw-object-pass.ts: New execute and dispose overrides; verify TextureLoadAction.clear behavior and framebuffer resizing logic
  • gl-renderer.ts: Removal of delegate hooks; ensure all render pass execution paths are unaffected and meshes still render correctly
  • Test updates across multiple files: Verify new addMesh() API works correctly across different test scenarios and that mesh ordering still functions as expected
  • Interaction between render-frame.ts and new defaults: Ensure that removing explicit configuration doesn't cause unexpected behavior in post-processing passes

Possibly related PRs

• refactor: clean up render pass code #1288: Modifies and simplifies the render-pass subsystem including RenderPass defaults, DrawObjectPass behavior, and framebuffer/viewport handling with parallel architectural changes.
• refactor: renderer draw geometry matrix setting #1119: Alters how uniform semantics are handled and updates renderer.drawGeometry signature across the codebase alongside render pass changes.
• Merge origin/feat/2.0 into origin/main #574: Refactors RenderPass/render-frame APIs and introduces default values for priority/name, aligning with this PR's consolidation of render pass initialization.

Suggested reviewers

• yiiqii
• liuxi150
• Sruimeng

Poem

🐰 Hoppy refactoring day!
Render passes cleaner now, defaults in place,
No more delegates cluttering the space,
AddMesh at runtime, disposed with care,
Architecture leaner—let's render with flair! ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main refactoring changes: simplifying render pass initialization by moving configuration logic from constructor options into dedicated pass subclasses and the base RenderPass class itself.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch refactor/render-pass-options

---

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cd79cd2 and 1ab2598.

📒 Files selected for processing (12)

• packages/effects-core/src/render/draw-object-pass.ts (1 hunks)
• packages/effects-core/src/render/post-process-pass.ts (4 hunks)
• packages/effects-core/src/render/render-frame.ts (1 hunks)
• packages/effects-core/src/render/render-pass.ts (8 hunks)
• packages/effects-webgl/src/gl-renderer.ts (1 hunks)
• plugin-packages/editor-gizmo/src/gizmo-component.ts (5 hunks)
• plugin-packages/model/src/utility/plugin-helper.ts (0 hunks)
• web-packages/imgui-demo/src/ge.ts (2 hunks)
• web-packages/test/unit/src/effects-webgl/gl-dispose.spec.ts (17 hunks)
• web-packages/test/unit/src/effects-webgl/gl-geometry.spec.ts (2 hunks)
• web-packages/test/unit/src/effects-webgl/gl-render-frame.spec.ts (1 hunks)
• web-packages/test/unit/src/effects-webgl/gl-render-pass.spec.ts (6 hunks)

💤 Files with no reviewable changes (1)

• plugin-packages/model/src/utility/plugin-helper.ts

🧰 Additional context used

🧠 Learnings (2)

📚 Learning: 2024-10-18T09:15:08.038Z

Learnt from: wumaolinmaoan
Repo: galacean/effects-runtime PR: 691
File: packages/effects-core/src/plugins/timeline/playables/color-property-mixer-playable.ts:39-42
Timestamp: 2024-10-18T09:15:08.038Z
Learning: In `packages/effects-core/src/plugins/timeline/playables/color-property-mixer-playable.ts`, when accumulating color components, avoid using `clone()` methods because it can cause additional garbage collection overhead.

Applied to files:

• packages/effects-core/src/render/draw-object-pass.ts
• packages/effects-webgl/src/gl-renderer.ts
• packages/effects-core/src/render/render-frame.ts
• web-packages/imgui-demo/src/ge.ts

📚 Learning: 2025-08-05T07:50:26.317Z

Learnt from: ChengYi996
Repo: galacean/effects-runtime PR: 1118
File: packages/effects-core/src/components/ffd-component.ts:135-219
Timestamp: 2025-08-05T07:50:26.317Z
Learning: In packages/effects-core/src/components/ffd-component.ts, the mesh subdivision logic (lines 147-219) within the collectSpriteComponents method is temporary debug code that will be properly implemented later, as confirmed by ChengYi996.

Applied to files:

• packages/effects-webgl/src/gl-renderer.ts
• plugin-packages/editor-gizmo/src/gizmo-component.ts

🧬 Code graph analysis (3)

web-packages/test/unit/src/effects-webgl/gl-render-frame.spec.ts (1)

packages/effects-webgl/src/gl-renderer.ts (1)

• gl (36-38)

packages/effects-core/src/render/draw-object-pass.ts (2)

packages/effects-core/src/render/render-pass.ts (3)

• RenderPass (229-532)
• RenderPassOptions (219-222)
• RenderPassPriorityNormal (16-16)

packages/effects-core/src/render/renderer.ts (1)

• Renderer (12-121)

plugin-packages/editor-gizmo/src/gizmo-component.ts (1)

packages/effects-core/src/render/render-pass.ts (4)

• RenderPass (229-532)
• RenderPassOptions (219-222)
• RenderPassPriorityPostprocess (17-17)
• RenderPassPriorityPrepare (15-15)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Analyze (javascript-typescript)

🔇 Additional comments (23)

packages/effects-webgl/src/gl-renderer.ts (1)

111-113: Clean iteration style.

Using for...of is more idiomatic and readable than indexed iteration when the index isn't needed.

packages/effects-core/src/render/draw-object-pass.ts (2)

10-12: LGTM!

Setting priority and name defaults in the constructor provides clear, consistent initialization for DrawObjectPass instances.

---

17-25: Verify the intention of clear(this.storeAction) after rendering.

The execute method clears at the start (correct), renders meshes, then calls renderer.clear(this.storeAction). The storeAction is initialized with TextureStoreAction.store values by default (from the base RenderPass constructor), not TextureLoadAction.clear.

Since Renderer.clear() checks for TextureLoadAction.clear before performing any clearing, this second clear call will likely be a no-op. If this is intentional (perhaps for future use or as a placeholder), consider adding a comment. If a post-render clear is needed, a separate clear action should be defined.

web-packages/test/unit/src/effects-webgl/gl-render-frame.spec.ts (1)

74-74: Verify alignment with new DrawObjectPass clear behavior.

The expected value changed from 0xff to 0. This appears to reflect that DrawObjectPass.execute() now clears with default stencil value (0) rather than using the clearStencil: 0xff specified in the RenderFrame's clearAction.

Confirm this is the intended behavior—the RenderFrame's clearAction still specifies clearStencil: 0xff on line 58, but that value is no longer being used during the render pass execution.

web-packages/test/unit/src/effects-webgl/gl-geometry.spec.ts (2)

541-544: LGTM!

Test correctly updated to use the new addMesh() API instead of passing meshes in constructor options.

---

563-567: LGTM!

Consistent with the updated RenderPass API pattern.

web-packages/imgui-demo/src/ge.ts (2)

1-2: LGTM!

Imports correctly updated to include RenderPassOptions type and RenderPassPriorityPostprocess constant.

---

106-110: LGTM!

Constructor follows the established pattern from this PR—setting priority and name internally rather than requiring them in the options object. This is consistent with DrawObjectPass and other pass implementations in this refactor.

web-packages/test/unit/src/effects-webgl/gl-dispose.spec.ts (1)

69-72: LGTM! API usage updated correctly.

The test now correctly uses the new API pattern where RenderPass is instantiated without meshes and they are added post-construction via addMesh().

plugin-packages/editor-gizmo/src/gizmo-component.ts (3)

1-1: LGTM! Required import for new pass classes.

The RenderPassOptions type is correctly imported to support the new pass class constructors.

---

884-906: LGTM! Well-structured pass classes.

Both DrawGizmoFrontPass and DrawGizmoEditorPass are correctly implemented with appropriate priorities (2 and 3002 respectively) and execute logic.

---

871-882: The priority calculation is intentional, but the expression could be clearer.

The priority of 6000 (RenderPassPriorityPostprocess + RenderPassPriorityPostprocess) is intentionally set to render behind gizmos after all other rendering passes, including post-processing effects (which use priority 5000) and editor gizmos (which use priority 3002). The "behind-gizmo" naming confirms this is by design.

However, the expression RenderPassPriorityPostprocess + RenderPassPriorityPostprocess is confusing. Compared to sibling passes like DrawGizmoEditorPass (which uses RenderPassPriorityPostprocess + 2), this double-addition lacks clarity. Consider using a literal constant value or adding a comment explaining the render order intent.

web-packages/test/unit/src/effects-webgl/gl-render-pass.spec.ts (2)

107-109: LGTM! Correct API usage.

Meshes are now added dynamically via addMesh() in a loop, which properly tests the new API.

---

388-392: LGTM! Correct testing of meshOrder property.

The test correctly sets meshOrder as a property before adding meshes, validating that the order is applied when meshes are added.

packages/effects-core/src/render/post-process-pass.ts (1)

56-56: LGTM! Explicit naming improves debugging.

Adding explicit name assignments to post-processing passes improves identification and debugging. The names are descriptive and follow a consistent pattern.

Also applies to: 146-146, 231-231, 317-317

packages/effects-core/src/render/render-frame.ts (2)

244-247: LGTM! Simplified pass construction.

The DrawObjectPass construction now only passes necessary attachment options, with other configuration handled internally by the class. This aligns with the API simplification goals.

---

14-14: The review comment contains an inaccuracy regarding RenderPassAttachmentStorageType.

The verification shows:

• OrderType and RenderPassPriorityNormal were never imported in render-frame.ts and are correctly absent from the file.
• RenderPassAttachmentStorageType is still present on line 20 (import { RenderPassAttachmentStorageType, RenderTargetHandle } from './render-pass';) and is actively used on line 241 (storageType: RenderPassAttachmentStorageType.depth_stencil_opaque).

The import was not removed and cannot be removed without breaking the code.

Likely an incorrect or invalid review comment.

packages/effects-core/src/render/render-pass.ts (6)

219-222: LGTM! Simplified options interface.

The RenderPassOptions interface is now focused on attachment configuration only, with other settings managed through properties or subclass constructors. This significantly simplifies the API.

---

264-264: LGTM! Better semantic naming.

Renaming destroyed to disposed is more semantically accurate and aligns with the Disposable interface convention.

Also applies to: 289-289, 491-491, 514-514

---

272-286: LGTM! Constructor greatly simplified.

The constructor now focuses only on essential attachment configuration, with other properties managed through defaults, property assignment, or subclass implementations. This is a significant and positive API simplification.

---

337-346: LGTM! Appropriate template method pattern.

Making execute and frameCleanup no-ops in the base class is correct. Subclasses override these methods to implement their specific rendering behavior.

---

397-406: LGTM! Simplified viewport logic.

The viewport is now determined solely by the framebuffer or renderer dimensions, removing custom viewport handling. This simplifies the logic and makes behavior more predictable.

---

246-246: Based on my verification, I now have a complete picture. Let me generate the rewritten review comment:

---

Remove readonly modifiers from meshes and meshOrder properties or add setter methods if these properties should remain mutable.

The properties meshes (line 246) and meshOrder (line 250) are declared as readonly, but the test file ./web-packages/test/unit/src/effects-webgl/gl-render-pass.spec.ts directly assigns values to meshOrder (e.g., rp1.meshOrder = OrderType.descending). The test bypasses TypeScript checking with // @ts-nocheck at the top of the file, which masks this conflict.

If meshOrder is intended to be immutable after construction, the test should not modify it directly. If it should be settable, the readonly modifier should be removed from both properties, or alternatively, public setter methods should be provided to control how these properties can be modified.

Tip

📝 Customizable high-level summaries are now available in beta!

You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.

• Provide your own instructions using the high_level_summary_instructions setting.
• Format the summary however you like (bullet lists, tables, multi-section layouts, contributor stats, etc.).
• Use high_level_summary_in_walkthrough to move the summary from the description to the walkthrough section.

Example instruction:

"Divide the high-level summary into five sections:

1. 📝 Description — Summarize the main change in 50–60 words, explaining what was done.
2. 📓 References — List relevant issues, discussions, documentation, or related PRs.
3. 📦 Dependencies & Requirements — Mention any new/updated dependencies, environment variable changes, or configuration updates.
4. 📊 Contributor Summary — Include a Markdown table showing contributions:
   | Contributor | Lines Added | Lines Removed | Files Changed |
5. ✔️ Additional Notes — Add any extra reviewer context.
   Keep each section concise (under 200 words) and use bullet or numbered lists for clarity."

Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}