perf(pipeline): share internal RT across cameras via per-frame pool lease

[cptbtptpbcptdtptp]

Summary

Issue context: galacean/migration-agent#304

Two concrete problems on multi-camera scenes (e.g. Camera3D + CameraUI both rendering to screen):

1. Multi-camera scratch-RT over-allocation — BasicRenderPipeline._internalColorTarget was a per-pipeline long-lived member, so each camera allocated its own full-canvas internal RT. On a 1078×2249 canvas with MSAA 4x that's 2 × 74 MB = 148 MB just for scratch buffers.
2. Resize memory never reclaimed — on canvas resize the old-size scratch RTs are returned to RenderTargetPool's free list, but since the canvas never goes back to the old size those entries never match an allocation again and linger forever. Memory climbs on every resize and never comes back down.

What changed

BasicRenderPipeline — the internal color target and copy-background texture are now leased from RenderTargetPool per frame (pool.allocateRenderTarget / allocateTexture) and returned to the pool at the end of render(), after _drawRenderPass() has fully consumed them. Cameras with matching shape share one underlying RT via the pool's existing match key (width, height, colorFormat, depthFormat, mipmap, isSRGBColorSpace, antiAliasing); mismatched HDR/MSAA/format still get their own entries. Net occupancy for the multi-camera case drops to 1 × full-canvas RT — fixes problem 1.

Release happens on the happy path (sequential, after the pass that produces the RT finishes). It is intentionally not wrapped in try/finally: the render pipeline trusts its passes not to throw (WebGL errors surface via getError, not exceptions), and if a pass ever does throw that is a bug to surface and fix, not to paper over — a defensive finally would also have been asymmetric (it only covered the internal RT, while SAO / PostProcess / DepthOnly RTs in the same draw span would leak just the same).

Engine — subscribes to canvas._sizeUpdateFlagManager and calls RenderTargetPool.gc() on canvas resize, so the now-stale full-canvas entries are destroyed at the moment of resize instead of lingering in the free list. This is what fixes problem 2; the listener is removed on engine destroy.

That's the whole fix. The pool keeps its existing reclamation contract (free list flushed by gc(), driven by canvas resize, ResourceManager.gc(), and engine destroy) — the same contract PostProcess / DepthOnlyPass / SAO have always relied on. No per-frame aging, no frame-stamp bookkeeping.

Trade-offs

• Debug attribution gets weaker: _internalColorTarget no longer stably belongs to one camera; pool ownership is the right level of abstraction now.
• Pool match scan runs per camera per frame instead of per pipeline once. It's an O(n) scan over a tiny free list — measured negligible.
• onCanvasResize flushes the whole free list, not just the old canvas size, so a few unrelated fixed-size entries (e.g. shadow/SAO) may be rebuilt the next frame. Canvas resize is low-frequency, so this is an acceptable trade for keeping the logic trivial.

Follow-up

Extending the same per-frame cross-camera lease to other pass RTs (FinalPass sRGB, bloom mip chain, SAO) is tracked in #3056. DepthOnly is intentionally excluded there (its depth texture is published to camera.shaderData and outlives render()).

Test plan

New tests/src/core/RenderPipeline/RenderTargetPool.test.ts (browser mode, all passing):

• Matching reuse: free → alloc same shape returns the same instance; mismatched shape allocates fresh
• Multi-camera frame-internal reuse (the headline scenario)
• Texture free-list reuse / mismatch
• gc() destroys all free-list entries

Build + full suite on CI.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

Walkthrough

Engine integrates RenderTargetPool ticking and canvas-resize eviction. RenderTargetPool gains frame-age and size-based eviction with frame-tracking. BasicRenderPipeline returns per-pass leased targets to the pool. Tests cover reuse, age-based eviction, size-based eviction, and gc cleanup.

Changes

RenderTargetPool frame-age eviction and resource lifecycle

Layer / File(s)	Summary
RenderTargetPool frame-age tracking and eviction methods packages/core/src/RenderPipeline/RenderTargetPool.ts	Introduces maxFreeAgeFrames field and parallel frame-tracking arrays for free pooled entries. Swap-pop helpers update allocation paths to keep tracking aligned. tick(currentFrame) destroys entries exceeding age threshold. evictBySize(width, height) destroys entries matching given dimensions. Updated gc() clears frame arrays. Centralized render-target destruction with double-destroy identity checks.
BasicRenderPipeline per-pass resource cleanup packages/core/src/RenderPipeline/BasicRenderPipeline.ts	Removes conditional free in render(). _drawRenderPass now explicitly returns leased _internalColorTarget and _copyBackgroundTexture to engine._renderTargetPool after rendering output work and nulls fields to prevent frame-to-frame leaks.
Engine canvas resize tracking and pool lifecycle packages/core/src/Engine.ts	Engine tracks last canvas width/height and registers a resize listener that calls evictBySize(oldW, oldH) when dimensions change. Engine update loop calls pool.tick(frameCount) each frame. Resize listener is removed during engine destruction.
RenderTargetPool eviction test coverage tests/src/core/RenderPipeline/RenderTargetPool.test.ts	Validates allocation/reuse of freed render targets at matching dimensions, allocation of fresh targets at differing dimensions, frame-age eviction within and beyond maxFreeAgeFrames threshold, size-based eviction via evictBySize(), and full cleanup via gc(). Mirrors behaviors for standalone textures.

Sequence Diagram(s)

sequenceDiagram
  participant Engine
  participant RenderTargetPool
  participant BasicRenderPipeline
  participant Canvas

  BasicRenderPipeline->>RenderTargetPool: allocateRenderTarget(width,height)
  alt free match
    RenderTargetPool->>RenderTargetPool: _removeFreeRenderTargetAt(i)
    RenderTargetPool-->>BasicRenderPipeline: reuse RT
  else no match
    RenderTargetPool->>RenderTargetPool: create RT and textures
    RenderTargetPool-->>BasicRenderPipeline: new RT
  end

  BasicRenderPipeline->>RenderTargetPool: freeRenderTarget(rt) (end-of-pass)
  RenderTargetPool->>RenderTargetPool: record freed frame (engine.time.frameCount)

  Engine->>RenderTargetPool: tick(currentFrame) (each update)
  alt aged beyond maxFreeAgeFrames
    RenderTargetPool->>RenderTargetPool: destroy aged free entries
  end

  Canvas->>Engine: size change event
  Engine->>RenderTargetPool: evictBySize(oldWidth,oldHeight)
  RenderTargetPool->>RenderTargetPool: destroy matching-size free entries

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 I hop through pools by frame-count rule,
I free old textures before they get cool,
When canvas grows, I toss the past,
Per-pass I return what never should last,
Tests cheer as I tidy the pool.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The PR title 'perf(pipeline): share internal RT across cameras via per-frame pool lease' directly and specifically describes the main change: enabling multiple cameras to share internal render targets via a per-frame pooling mechanism.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch claude/rt-pool-multicamera-reuse

---

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.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/src/core/RenderPipeline/RenderTargetPool.test.ts`:
- Around line 36-38: Add an afterAll teardown to mirror the beforeAll that
created the real WebGLEngine: locate the beforeAll that calls WebGLEngine.create
and the shared engine variable, and add an afterAll which properly tears down
the engine instance (call the engine's destruction method—e.g., engine.destroy()
or engine.dispose()—await it if it returns a promise) to avoid leaking WebGL
resources between tests.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 933780af-0b74-470e-a933-e8af3273060b

📥 Commits

Reviewing files that changed from the base of the PR and between 956b5c2 and a8c141d.

📒 Files selected for processing (5)

• packages/core/src/Engine.ts
• packages/core/src/RenderPipeline/BasicRenderPipeline.ts
• packages/core/src/RenderPipeline/RenderTargetPool.ts
• packages/core/src/texture/RenderTarget.ts
• tests/src/core/RenderPipeline/RenderTargetPool.test.ts

[coderabbitai]

🧹 Nitpick comments (1)

tests/src/core/RenderPipeline/RenderTargetPool.test.ts (1)

79-140: ⚡ Quick win

Add texture-path eviction/reuse coverage alongside RT coverage.

The suite validates RT paths well, but the new _freeTextureFrames + texture eviction codepaths (freeTexture, tick, evictBySize, gc) are currently untested. A couple of focused texture cases would catch frame-array drift/regressions quickly.

Suggested test additions

+  describe("texture free-list eviction", () => {
+    it("reuses a freed texture within maxFreeAgeFrames and evicts after threshold", () => {
+      pool.maxFreeAgeFrames = 2;
+      const t1 = pool.allocateTexture(
+        128, 128, TextureFormat.R8G8B8A8, false, false, TextureWrapMode.Clamp, TextureFilterMode.Bilinear
+      );
+      pool.freeTexture(t1);
+      const base = engine.time.frameCount;
+      pool.tick(base + 1);
+      const t2 = pool.allocateTexture(
+        128, 128, TextureFormat.R8G8B8A8, false, false, TextureWrapMode.Clamp, TextureFilterMode.Bilinear
+      );
+      expect(t2).to.equal(t1);
+      pool.freeTexture(t2);
+      pool.tick(base + 10);
+      expect(t1.destroyed).to.equal(true);
+    });
+
+    it("evictBySize removes only matching free textures", () => {
+      const a = pool.allocateTexture(
+        800, 600, TextureFormat.R8G8B8A8, false, false, TextureWrapMode.Clamp, TextureFilterMode.Bilinear
+      );
+      const b = pool.allocateTexture(
+        1024, 768, TextureFormat.R8G8B8A8, false, false, TextureWrapMode.Clamp, TextureFilterMode.Bilinear
+      );
+      pool.freeTexture(a);
+      pool.freeTexture(b);
+      pool.evictBySize(800, 600);
+      expect(a.destroyed).to.equal(true);
+      expect(b.destroyed).to.equal(false);
+    });
+  });

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/src/core/RenderPipeline/RenderTargetPool.test.ts` around lines 79 -
140, Add tests mirroring the RenderTargetPool RT cases but exercising the
texture paths: allocate textures (use the texture allocator helper, e.g.,
allocTexture or alloc(pool, w,h, /*type*/ 'texture') if present), call
pool.freeTexture(...) and assert reuse within pool.maxFreeAgeFrames by using
pool.tick(frame), assert destruction after aging by checking texture.destroyed
and that re-allocation returns a new object, test pool.evictBySize(width,height)
only destroys matching free textures (leave others intact and reusable), and
verify pool.gc() destroys all entries; reference _freeTextureFrames,
freeTexture, tick, evictBySize, and gc to locate code paths.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@tests/src/core/RenderPipeline/RenderTargetPool.test.ts`:
- Around line 79-140: Add tests mirroring the RenderTargetPool RT cases but
exercising the texture paths: allocate textures (use the texture allocator
helper, e.g., allocTexture or alloc(pool, w,h, /*type*/ 'texture') if present),
call pool.freeTexture(...) and assert reuse within pool.maxFreeAgeFrames by
using pool.tick(frame), assert destruction after aging by checking
texture.destroyed and that re-allocation returns a new object, test
pool.evictBySize(width,height) only destroys matching free textures (leave
others intact and reusable), and verify pool.gc() destroys all entries;
reference _freeTextureFrames, freeTexture, tick, evictBySize, and gc to locate
code paths.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 56134fba-2b79-4034-8ece-d511b57dc429

📥 Commits

Reviewing files that changed from the base of the PR and between 60e4b03 and 3a67d1a.

📒 Files selected for processing (2)

• packages/core/src/RenderPipeline/RenderTargetPool.ts
• tests/src/core/RenderPipeline/RenderTargetPool.test.ts

[codecov]

Codecov Report

❌ Patch coverage is 75.00000% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.36%. Comparing base (956b5c2) to head (007c3a2).
⚠️ Report is 20 commits behind head on dev/2.0.

Files with missing lines	Patch %	Lines
...ges/core/src/RenderPipeline/BasicRenderPipeline.ts	71.42%	4 Missing ⚠️
e2e/config.ts	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           dev/2.0    #3015      +/-   ##
===========================================
+ Coverage    79.23%   79.36%   +0.13%     
===========================================
  Files          902      903       +1     
  Lines        99853   100628     +775     
  Branches     10298    11256     +958     
===========================================
+ Hits         79117    79866     +749     
- Misses       20563    20578      +15     
- Partials       173      184      +11     

Flag	Coverage Δ
unittests	79.36% <75.00%> (+0.13%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[GuoLei1990]

总结

本轮自上次 review（1be6eed72）以来新增一个 commit 007c3a240（git merge-base --is-ancestor 1be6eed72 007c3a240 = YES，真增量非回退），整 PR 现已 MERGED 到 dev/2.0。新 commit 只动一行：

• 007c3a240（test(e2e): relax CharacterSpacing diff threshold for font raster drift）— 把 e2e/config.ts 里 text-character-spacing 的 diffPercentage 从 0.0 调到 0.0129，专门吸收我连续多轮记录为环境 flake 的 CoreText 亚像素光栅漂移。threshold 保持 0.0 不变——即每像素零色差容忍不松，只放宽「允许多少比例的像素整体不同」。

这条修法是对的，且经 CI 实证：本轮 e2e (22.x, 4/4) 由连续多轮三 retry 全红翻成首跑绿（绿 run 日志 ✅ [Text_text-character-spacing] Test passed (958ms total)，不再 retry）。符合我自己的 e2e 阈值铁律——阈值由 CI 实测验证而非本地拍板。threshold: 0.0 保留意味着真正的 glyph 形变回归仍会远远突破 0.0129% 被抓到，回归网没破。

问题

无阻塞性问题（无 P0/P1/P2）。下面一条 P3 不阻塞，仅作风险记录。

P3

• e2e/config.ts:504 — diffPercentage: 0.0129 相对实测漂移 0.0128125% 的 buffer 极薄（≈1.007×），未来 runner 镜像升级若把漂移再抬高一丝即会复红

  我的 e2e 阈值经验通常建议 CI 实测值的 2-3× buffer，本处只取了贴着实测值上方的最小 round number。客观说这不是缺陷、不必改：① 该漂移源是 CoreText 字体光栅的 glyph-edge 亚像素抗锯齿，有界于边缘像素数、非无界 GPU jitter，且 0.0128125% 跨多轮逐字稳定（同值复现），说明是 per-runner-image 确定性偏移而非噪声；② 薄 buffer 在这里反而是特性不是 bug——配合保留 threshold: 0.0，把回归网收得越紧越能抓真 glyph 回归，宽 diffPercentage 才会掩盖真问题。仅记录：若日后 macOS runner 镜像再升级把漂移推到 0.013%+，此 case 会以 1.007× 的薄边复红，届时按同法读 CI 实测值再抬一档即可。这是「下次镜像漂移时的 reminder」，非本轮待办。

关于历史 P1（throw-path RT 泄漏）— 已闭环，不再提

我前轮（1be6eed72）设的 P1：8bcdee627 删掉 1dac9863f 的 throw-safety try/finally，重新引入 isGCIgnored 内部 RT 在 throw 路径上的进程寿命级泄漏。我当时给的二选一是「保留 finally 或 若坚持删除则必须记录撤销依据」。

PR 合入时走的是第二条：PR 描述正文已完整记录删除依据——「intentionally not wrapped in try/finally」段逐条列出三点理由（WebGL 错误经 getError 而非异常浮现 / pass 真抛是该暴露的 bug 不该兜底 / 一个只覆盖 internal RT 的 finally 对同 draw span 的 SAO/PostProcess/DepthOnly RT 不对称）。这正是我提供的「有据可查地撤销」路径。泄漏的 trade-off 现在是被有意识、有记录地接受，而非悄悄回退。按 cr 流程「作者已合理记录依据即视为关闭」，这条 P1 闭环，不再以任何形式重提。

CI 状态

• e2e (22.x, 4/4) 本轮转绿 — 007c3a240 的阈值放宽生效，CharacterSpacing 首跑通过。多轮挂着的 e2e 红本轮闭环。
• codecov/patch 红 = 既有阈值漂移噪声：codecov/project 绿。同 [project_codecov_patch_artifact] 判例，非覆盖回退、非本 commit 引入。

build / lint / e2e 1-3/4 / codecov(project) 全绿。

自检

• 当前 tip 007c3a240 经 gh headRefOid + git fetch pull/3015/head 双确认；git merge-base --is-ancestor 1be6eed72 007c3a240 = YES，真增量。PR state=MERGED，本 review 为合入后收尾记录（已无门控意义），前两条针对旧 commit 的 review 已 minimize 为 OUTDATED。
• 新 commit 逐行核对：e2e/config.ts:504 diffPercentage 0.0→0.0129；消费语义经 e2e/utils/screenshot.ts:74（result.diffPercentage <= diffPercentage 时强制 match=true）+ threshold 经 odiff antialiasing:true 亲自读源确认；绿 run 日志（job 83835638612）确认 CharacterSpacing pass。Git History Lens：该 case 的 threshold:0.0/diffPercentage:0.0 是早期 PR 引入的 aspirational bit-exact 基线，被新 macOS runner 镜像打破——放宽是对 runner 漂移的正确响应，非还原任何 fix:，无矛盾。
• 历史已闭环项（memory cap 拆分/双计数、>→>=、texture 测试、evictBySize 子视口洞、_lastCanvasWidth 初始化、PipelineUtils 死路径、@internal 导出、afterAll teardown、self-assign 守卫删除、tick 删除、PR 描述过期、stranded-shape 措辞、descriptor key/生命周期/有界增长/listener 生命周期、样式 inline、hoist no-op、throw-path finally 泄漏 P1 经 PR 描述记录依据闭环）一律不重提。

LGTM。本轮唯一改动是一行 CI-validated 的 e2e 字体光栅漂移阈值放宽，多轮 e2e 红本轮转绿闭环；历史 P1 经 PR 描述记录撤销依据闭环。PR 已合入。