perf(pipeline): share internal RT across cameras via per-frame pool lease#3015
Conversation
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughEngine 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. ChangesRenderTargetPool frame-age eviction and resource lifecycle
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
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
…ease Each camera held its own `_internalColorTarget` for the lifetime of its `BasicRenderPipeline`, so a scene with N on-screen cameras pinned N full-canvas RTs. On a 1078x2249 canvas with MSAA 4x that is 2 * 74 MB = 148 MB just for the scratch buffers (see investigation in galacean/migration-agent#304). Convert `_internalColorTarget` and `_copyBackgroundTexture` to per-frame leases: `BasicRenderPipeline._drawRenderPass` returns both to `RenderTargetPool` at end of every frame, so the next camera in the frame finds a matching free entry and reuses the same underlying RT. Cameras with mismatched format / MSAA / depth still get their own entries -- the pool's existing match key handles that. `RenderTargetPool` gains three bounded-growth strategies so the free list cannot leak across canvas resizes or shape churn: * `tick(currentFrame)` -- destroys entries idle longer than `maxFreeAgeFrames` (default 60). Engine calls this once per `update()`. * `evictBySize(width, height)` -- destroys entries matching the given dimensions. Engine subscribes to canvas size changes and evicts at the previous canvas size, so old full-canvas RTs do not linger. * `maxFreeBytes` -- when a `free*` push would exceed the cap, the oldest entries (by `lastUsedFrame`) are destroyed until the total fits. Scoped to free-list contents only (not total GPU memory), so the cap is device-independent. `RenderTarget._memorySize` becomes `@internal` so the pool can compute per-entry byte size without re-deriving from format/aa. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
cptbtptpbcptdtptp
force-pushed
the
claude/rt-pool-multicamera-reuse
branch
from
ce8814d to
1f3f2e1
Compare
github-actions
Bot
added
the
documentation
Three review fixes on top of the previous commit: 1. `maxFreeBytes` now applies to the combined free-list total (RT + Texture) instead of each list independently. Previously, with the default 64 MB cap, the pool could actually hold up to 128 MB (64 MB RT + 64 MB Texture) — inconsistent with what `freeListByteSize` reports. The unified `_enforceMemoryCap` picks the older entry across both pools by `lastUsedFrame` and evicts until the combined sum is at or below the cap. 2. `_computeRtBytes` now documents the contract it depends on: that `RenderTarget._memorySize` covers only the RT's own renderbuffers (MSAA + depth RBO) and excludes the attached `colorTexture` / `depthTexture`, whose bytes live on `Texture._memorySize`. So the sum does not double-count. 3. `RenderTargetPool` is no longer re-exported from `RenderPipeline/index.ts` — it stays `@internal`. The test imports it via a relative source path instead, keeping the public surface unchanged. Added a 12th unit test verifying the unified cap actually bounds the combined total. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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
📒 Files selected for processing (5)
packages/core/src/Engine.tspackages/core/src/RenderPipeline/BasicRenderPipeline.tspackages/core/src/RenderPipeline/RenderTargetPool.tspackages/core/src/texture/RenderTarget.tstests/src/core/RenderPipeline/RenderTargetPool.test.ts
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
CR follow-ups: * `tick()` now calls `_enforceMemoryCap()` at the end, so a mid-run reduction of `maxFreeBytes` takes effect within one frame instead of waiting for the next `free*` call. Cost is one extra scan per frame over an already-tiny free list. * Test file adds `afterAll(() => engine.destroy())` to release the WebGL context between test files. * New test locks in the tick-re-enforces-cap behavior. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…nough The byte cap was defending against pathological churn within the age window — a scenario covered in practice by canvas-resize eviction (shape coupled to canvas) and frame-age (steady state). With the default 64 MB cap, a single full-canvas MSAA 4x RT (~86 MB on a 1078x2249 RGBA8+D24S8 canvas) was larger than the cap. Every free immediately destroyed the just-pushed RT, defeating the multi-camera sharing this PR exists to enable. The abstraction was unfortunately calibrated against a fictional worst case; the realistic worst cases are already bounded. Dropping it removes a tunable that's hard to set well (device-dependent, no single number works) and a sizable chunk of byte-tracking machinery (`_freeRenderTargetBytes`, `_freeRenderTargetByteTotal`, the combined-pool LRU in `_enforceMemoryCap`, `_computeRtBytes`, `_findOldestIndex`, `freeListByteSize`). `RenderTarget._memorySize` reverts to `private` — pool no longer reads it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/src/core/RenderPipeline/RenderTargetPool.test.ts (1)
79-140: ⚡ Quick winAdd 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
📒 Files selected for processing (2)
packages/core/src/RenderPipeline/RenderTargetPool.tstests/src/core/RenderPipeline/RenderTargetPool.test.ts
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Address CR (P3, raised by GuoLei1990 + CodeRabbit): * `tick()` boundary is now `>= maxFreeAgeFrames` so an entry idle for exactly `maxFreeAgeFrames` frames is destroyed, matching the field name (was `>`, which kept it one extra frame). * Add texture free-list tests: reuse-then-age-evict, evictBySize selectivity, and gc; gc test now also covers a pooled texture. Adds an explicit boundary test locking the inclusive age semantics. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
…decov The codecov job builds packages and runs the suite against the built bundles. The test imported `WebGLEngine` from `@galacean/engine-rhi-webgl` while importing core symbols from `@galacean/engine-core`; against the built output these resolve to two separate copies of core, so the `WebGLEngine` (extending one `Engine`) crashed during construction reading a class that lived in the other copy — failing only this file while 108 others passed. Local `pnpm test` masked it by resolving every package to source via the `debug` mainField (single module graph). Import `WebGLEngine` from the `@galacean/engine` umbrella like the other engine tests. `RenderTargetPool` (still `@internal`, not barrel-exported) is now referenced via a type-only import plus its runtime constructor from `engine._renderTargetPool`, avoiding a value import of the source file that would reintroduce the dual-core split. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
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
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
With per-frame leasing, `_internalColorTarget` / `_copyBackgroundTexture` are always null when `render()` runs, so the `recreateRenderTargetIfNeeded` match-or-realloc path was dead for this caller and falsely implied cross-frame reuse. Shape matching now happens inside the pool (free at end of frame, match on next allocate), so call `pool.allocateRenderTarget` / `pool.allocateTexture` directly. Behavior-identical; no cross-frame reuse path remained to preserve. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Drop the `index !== last` guard (it only avoided a harmless self-assign) and the local aliases, leaving the plain swap-with-last form. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
…size-keyed evict `evictBySize` only matched entries whose dimensions equalled the previous canvas size, so it missed canvas-derived-but-scaled entries (sub-viewport cameras, down/upsampled targets) — those lingered until frame-age. A canvas resize invalidates every canvas-coupled size at once, and the pool can't tell canvas-coupled from fixed-size entries, so just flush the whole free list via `gc()` (active leases are untouched; next frame reallocates). This matches Godot's reconfigure-on-resize and is simpler: drops `evictBySize` and the `_lastCanvasWidth/_lastCanvasHeight` tracking. Canvas setters only dispatch on real size changes, so no guard is needed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
cptbtptpbcptdtptp
force-pushed
the
claude/rt-pool-multicamera-reuse
branch
from
4308347 to
59d45b5
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
…throw can't strand it The per-frame internal color target / copy-background texture were freed only at the tail of `_drawRenderPass()`. A throw in the SAO pass or anywhere in the draw span skipped the release: next frame's `render()` overwrote the still non-null field with a fresh lease, orphaning the previous RT — it is `isGCIgnored`, so `ResourceManager.gc()` never reclaims it — for the engine's lifetime. Wrap the pass body in `try/finally` and release the leases there, so they are always returned to the pool and the fields always nulled, even on throw. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Internal-RT pooling (multi-camera sharing) plus canvas-resize gc() already cover the two real issues this PR targets: resize memory not being reclaimed, and multi-camera RT over-allocation. The per-frame tick()/maxFreeAgeFrames frame-age eviction only handled shape churn that bypasses canvas resize (e.g. per-frame viewport resizing) -- an unobserved scenario, and inconsistent with the pool's gc-based reclamation already relied on by PostProcess/DepthOnly/SAO. RenderTargetPool returns to baseline (no frame-stamp parallel arrays); Engine no longer ticks the pool each frame. onCanvasResize -> gc() stays. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
GuoLei1990
added
bug
GuoLei1990
left a comment
GuoLei1990
left a comment
There was a problem hiding this comment.
总结
本轮自上次 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，真增量。PRstate=MERGED，本 review 为合入后收尾记录（已无门控意义），前两条针对旧 commit 的 review 已 minimize 为 OUTDATED。 - 新 commit 逐行核对：
e2e/config.ts:504diffPercentage 0.0→0.0129；消费语义经e2e/utils/screenshot.ts:74（result.diffPercentage <= diffPercentage时强制match=true）+threshold经 odiffantialiasing: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 已合入。
2 participants
Summary
Issue context: galacean/migration-agent#304
Two concrete problems on multi-camera scenes (e.g. Camera3D + CameraUI both rendering to screen):
BasicRenderPipeline._internalColorTargetwas 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.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 fromRenderTargetPoolper frame (pool.allocateRenderTarget/allocateTexture) and returned to the pool at the end ofrender(), 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 viagetError, not exceptions), and if a pass ever does throw that is a bug to surface and fix, not to paper over — a defensivefinallywould 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 tocanvas._sizeUpdateFlagManagerand callsRenderTargetPool.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
_internalColorTargetno longer stably belongs to one camera; pool ownership is the right level of abstraction now.onCanvasResizeflushes 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.shaderDataand outlivesrender()).Test plan
New
tests/src/core/RenderPipeline/RenderTargetPool.test.ts(browser mode, all passing):gc()destroys all free-list entriesBuild + full suite on CI.