Add engine.renderingInfo for runtime memory tracking#2910
Open
GuoLei1990 wants to merge 13 commits intogalacean:dev/2.0from
Open
Add engine.renderingInfo for runtime memory tracking#2910GuoLei1990 wants to merge 13 commits intogalacean:dev/2.0from
GuoLei1990 wants to merge 13 commits intogalacean:dev/2.0from
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:
WalkthroughRendering memory accounting added: new RenderingStatistics tracks texture and buffer memory; Engine exposes it and resets on device loss. Textures, buffers, render targets update statistics on create, rebuild, and destroy. Tests validate memory accounting and device-loss/restored behavior. Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant Engine
participant Texture as Texture/System
participant Buffer as Buffer/System
participant RenderTarget as RT/System
rect rgba(100,150,240,0.5)
App->>Engine: create Texture2D(width,height,format...)
Engine->>Texture: construct Texture2D
Texture->>Engine: compute _gpuMemorySize via TextureUtils
Texture->>Engine: Engine.renderingStatistics._textureMemory += size
end
rect rgba(120,200,140,0.5)
App->>Engine: create Buffer(data)
Engine->>Buffer: construct Buffer
Buffer->>Engine: Engine.renderingStatistics._bufferMemory += byteLength
end
rect rgba(240,180,90,0.5)
App->>Engine: create RenderTarget(...)
Engine->>RenderTarget: construct RT & platform target
RenderTarget->>Engine: compute renderbuffer size
RenderTarget->>Engine: Engine.renderingStatistics._textureMemory += rbSize
end
rect rgba(200,100,180,0.5)
App->>Engine: deviceLost()
Engine->>Engine: Engine.renderingStatistics._reset()
App->>Engine: deviceRestored()
Engine->>Texture: rebuild textures -> re-increment _textureMemory
Engine->>Buffer: rebuild buffers -> re-increment _bufferMemory
Engine->>RenderTarget: rebuild RT -> re-increment rb memory
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
packages/core/src/asset/RenderingInfo.ts (1)
6-8: Encapsulate writable counters to preserve the readonly API contract.At Line 6 and Line 8,
_textureMemory/_bufferMemoryare publicly writable on an exported class. External code can mutate them directly, which underminestextureMemory,bufferMemory, andtotalMemoryas authoritative stats.♻️ Proposed refactor
export class RenderingInfo { /** `@internal` */ - _textureMemory: number = 0; + private _textureMemory: number = 0; /** `@internal` */ - _bufferMemory: number = 0; + private _bufferMemory: number = 0; + + /** `@internal` */ + _addTextureMemory(delta: number): void { + this._textureMemory = Math.max(0, this._textureMemory + delta); + } + + /** `@internal` */ + _addBufferMemory(delta: number): void { + this._bufferMemory = Math.max(0, this._bufferMemory + delta); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/asset/RenderingInfo.ts` around lines 6 - 8, Make _textureMemory and _bufferMemory non-public to preserve the readonly API: change the fields on class RenderingInfo to private (e.g., private _textureMemory: number = 0; private _bufferMemory: number = 0;) and replace external direct mutations with internal mutator methods (e.g., add private or `@internal` methods like addTextureMemory(amount: number) / addBufferMemory(amount: number) or setTextureMemory(value: number) / setBufferMemory(value: number)) so external code can only read via the existing textureMemory, bufferMemory and totalMemory getters; update any call sites to use the new internal mutators rather than writing to the underscored fields.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/src/texture/RenderTarget.ts`:
- Line 232: When destroying a RenderTarget, avoid unguarded subtraction of GPU
memory counters: in the RenderTarget method that currently does
"this._engine._renderingInfo._textureMemory -= this._renderbufferGpuMemorySize",
first verify the counter and size are valid (e.g. ensure
this._renderbufferGpuMemorySize > 0 and this._engine._renderingInfo and
this._engine._renderingInfo._textureMemory are defined and >=
this._renderbufferGpuMemorySize) before subtracting; after a successful subtract
set this._renderbufferGpuMemorySize = 0 so it won't be double-subtracted, and
skip the subtraction entirely if the checks fail (covers device/context-loss
reset windows).
- Line 196: The depth renderbuffer memory calculation in RenderTarget (variable
renderbufferMemory using TextureUtils.getMipLevelByteCount with depth, width,
height) is not accounting for multisampling; multiply the computed depth
mip-level byte count by the antiAliasing sample count (the same antiAliasing
value used for color renderbuffers) when antiAliasing > 1 so the depth memory
matches multisample allocations (consistent with GLRenderTarget.ts behavior
where depth uses renderbufferStorageMultisample).
In `@packages/core/src/texture/Texture.ts`:
- Line 241: The destruction path in Texture.ts always subtracts
this._gpuMemorySize from this._engine._renderingInfo._textureMemory which can go
negative if Engine._onDeviceLost() reset counters to zero; modify the destroy
logic in the Texture class (where this._engine._renderingInfo._textureMemory is
decremented) to clamp or guard the subtraction (e.g., only subtract up to the
current _textureMemory or set the result with Math.max(0, current -
this._gpuMemorySize)) so totals never become negative and undercount after
device loss/restoration.
In `@packages/core/src/texture/TextureUtils.ts`:
- Around line 153-154: In TextureUtils where TextureFormat.Depth32Stencil8 is
handled (the switch/case returning a byte count), fix the per-texel size from 5
to 8 bytes: replace the return expression currently using width * height * 5
with width * height * 8 so DEPTH32F_STENCIL8 reflects 64-bit (8-byte) allocation
per texel; keep the same function/switch structure and update only the numeric
multiplier for TextureFormat.Depth32Stencil8.
---
Nitpick comments:
In `@packages/core/src/asset/RenderingInfo.ts`:
- Around line 6-8: Make _textureMemory and _bufferMemory non-public to preserve
the readonly API: change the fields on class RenderingInfo to private (e.g.,
private _textureMemory: number = 0; private _bufferMemory: number = 0;) and
replace external direct mutations with internal mutator methods (e.g., add
private or `@internal` methods like addTextureMemory(amount: number) /
addBufferMemory(amount: number) or setTextureMemory(value: number) /
setBufferMemory(value: number)) so external code can only read via the existing
textureMemory, bufferMemory and totalMemory getters; update any call sites to
use the new internal mutators rather than writing to the underscored fields.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
packages/core/src/Engine.tspackages/core/src/asset/RenderingInfo.tspackages/core/src/graphic/Buffer.tspackages/core/src/index.tspackages/core/src/texture/RenderTarget.tspackages/core/src/texture/Texture.tspackages/core/src/texture/Texture2D.tspackages/core/src/texture/Texture2DArray.tspackages/core/src/texture/TextureCube.tspackages/core/src/texture/TextureUtils.ts
Comment on lines
+153
to
+154
| case TextureFormat.Depth32Stencil8: | ||
| return width * height * 5; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n packages/core/src/texture/TextureUtils.ts | head -170 | tail -30Repository: galacean/engine
Length of output: 1261
🌐 Web query:
WebGL DEPTH32F_STENCIL8 texture format memory allocation bytes
💡 Result:
In WebGL 2, DEPTH32F_STENCIL8 corresponds to a 64-bit depth-stencil texel: 32-bit float depth + 8-bit stencil + 24 unused bits (padding/alignment), i.e. 8 bytes per texel. [1], [2]
So a single-level (no mipmaps) 2D texture allocation is roughly:
- bytes ≈ width × height × 8
Actual GPU memory use can be higher due to implementation details (tiling/compression, row/page alignment, mipmaps, multisampling, etc.), but 8 B/texel is the correct nominal size for this format. [1], [2]
Sources:
[1] StackOverflow answer quoting GL_ARB_depth_buffer_float (“64-bit pixels … 32-bit float depth, 8 stencil, 24 unused”) (stackoverflow.com)
[2] Khronos Vulkan VkFormat entry for VK_FORMAT_D32_SFLOAT_S8_UINT (“optionally 24 bits unused”) (registry.khronos.org)
Citations:
- 1: https://stackoverflow.com/questions/22398575/depth-stencil-framebuffer-concerns?utm_source=openai
- 2: https://registry.khronos.org/vulkan/specs/latest/man/html/VkFormat.html?utm_source=openai
Fix Depth32Stencil8 memory allocation calculation to 8 bytes.
The WebGL 2 DEPTH32F_STENCIL8 format allocates 64 bits (8 bytes) per texel: 32-bit float depth + 8-bit stencil + 24 bits padding for alignment. The current calculation of 5 bytes under-reports actual GPU memory usage.
Current code
case TextureFormat.Depth32Stencil8:
return width * height * 5;
Change 5 to 8.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/texture/TextureUtils.ts` around lines 153 - 154, In
TextureUtils where TextureFormat.Depth32Stencil8 is handled (the switch/case
returning a byte count), fix the per-texel size from 5 to 8 bytes: replace the
return expression currently using width * height * 5 with width * height * 8 so
DEPTH32F_STENCIL8 reflects 64-bit (8-byte) allocation per texel; keep the same
function/switch structure and update only the numeric multiplier for
TextureFormat.Depth32Stencil8.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev/2.0 #2910 +/- ##
===========================================
- Coverage 78.36% 78.35% -0.01%
===========================================
Files 880 882 +2
Lines 96111 96513 +402
Branches 9602 9670 +68
===========================================
+ Hits 75315 75624 +309
- Misses 20634 20726 +92
- Partials 162 163 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
packages/core/src/texture/RenderTarget.ts (2)
196-196:⚠️ Potential issue | 🟠 MajorAccount depth renderbuffer memory with MSAA sample count.
Line 196 adds only single-sample depth bytes; for MSAA targets this under-reports memory.
🐛 Proposed fix
- renderbufferMemory += TextureUtils.getMipLevelByteCount(<TextureFormat>depth, width, height); + const depthBytes = TextureUtils.getMipLevelByteCount(<TextureFormat>depth, width, height); + renderbufferMemory += antiAliasing > 1 ? depthBytes * antiAliasing : depthBytes;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/texture/RenderTarget.ts` at line 196, The depth renderbuffer memory calculation in RenderTarget adds only a single-sample byte count (renderbufferMemory += TextureUtils.getMipLevelByteCount(depth, width, height)); update it to multiply the returned byte count by the MSAA sample count so MSAA targets are accounted for (e.g., renderbufferMemory += TextureUtils.getMipLevelByteCount(depth, width, height) * Math.max(1, this.samples || sampleCount)); reference the RenderTarget class, the renderbufferMemory variable and TextureUtils.getMipLevelByteCount when making the change and ensure you use the correct property name for the sample count used by this render target.
232-232:⚠️ Potential issue | 🟠 MajorAvoid unguarded subtraction after context-loss reset.
Line 232 can drive
textureMemorynegative when a render target is destroyed after counters were reset on device loss.🐛 Proposed fix
protected override _onDestroy(): void { super._onDestroy(); - this._engine._renderingStatistics._textureMemory -= this._renderbufferGpuMemorySize; + const stats = this._engine._renderingStatistics; + stats._textureMemory = Math.max(0, stats._textureMemory - this._renderbufferGpuMemorySize); + this._renderbufferGpuMemorySize = 0; this._platformRenderTarget.destroy();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/texture/RenderTarget.ts` at line 232, When subtracting the renderbuffer GPU memory from the engine counters in RenderTarget (the line using this._engine._renderingStatistics._textureMemory and this._renderbufferGpuMemorySize), guard against context-loss resets so the counter cannot go negative: either check that _renderbufferGpuMemorySize > 0 and that _textureMemory >= _renderbufferGpuMemorySize before subtracting, or clamp the result with a max(0, current - size) assignment; apply this change where the render target is destroyed/released so repeated/device-loss resets cannot drive the counter below zero.
🧹 Nitpick comments (1)
tests/src/core/RenderingStatistics.test.ts (1)
100-127: Add regression tests for MSAA depth and destroy-during-loss paths.Current coverage misses two important cases: (1) depth renderbuffer bytes with
antiAliasing > 1, and (2) destroying resources while context is lost (counter underflow guard).✅ Suggested test additions
+ it("RenderTarget depth renderbuffer scales with antiAliasing", () => { + const colorTexture = new Texture2D(engine, 64, 64, TextureFormat.R8G8B8A8, false, false); + const before = engine.renderingStatistics.textureMemory; + const rt = new RenderTarget(engine, 64, 64, colorTexture, TextureFormat.Depth24Stencil8, 4); + const after = engine.renderingStatistics.textureMemory; + // 64*64*4(depth) * 4 samples + expect(after - before).to.equal(64 * 64 * 4 * 4); + rt.destroy(); + colorTexture.destroy(); + }); + + it("destroy during device loss does not make counters negative", async () => { + const buffer = new Buffer(engine, BufferBindFlag.VertexBuffer, 256, BufferUsage.Static); + // `@ts-ignore` + engine._onDeviceLost(); + buffer.destroy(); + expect(engine.renderingStatistics.bufferMemory).to.equal(0); + });Also applies to: 129-157
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/src/core/RenderingStatistics.test.ts` around lines 100 - 127, Add two tests: (1) a MSAA depth test that creates a Texture2D and a RenderTarget with antiAliasing > 1 (use RenderTarget(..., antiAliasing: 4) or set renderTarget.antiAliasing = 4) and assert engine.renderingStatistics.textureMemory increases by width * height * 4 * samples (use the same width/height as the test); (2) a destroy-during-loss test that simulates a lost context (set engine.isContextLost / the engine context-lost flag) before calling RenderTarget.destroy and verify engine.renderingStatistics.textureMemory does not underflow (i.e., equals the value before creation/destruction). Reference the RenderTarget constructor, renderTarget.antiAliasing, RenderTarget.destroy, Texture2D, TextureFormat and engine.renderingStatistics.textureMemory when adding these assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/src/graphic/Buffer.ts`:
- Line 251: When destroying a Buffer, the line subtracting memory
(this._engine._renderingStatistics._bufferMemory -= this._byteLength) can drive
the counter negative during device-loss resets; update the Buffer.destroy logic
to guard the subtraction by either checking current value and only subtracting
Math.min(current, this._byteLength) or performing the subtraction then clamping
this._engine._renderingStatistics._bufferMemory = Math.max(0, ...). Refer to the
symbols this._engine._renderingStatistics._bufferMemory and this._byteLength in
Buffer.destroy to add the guard so stats never go below zero.
---
Duplicate comments:
In `@packages/core/src/texture/RenderTarget.ts`:
- Line 196: The depth renderbuffer memory calculation in RenderTarget adds only
a single-sample byte count (renderbufferMemory +=
TextureUtils.getMipLevelByteCount(depth, width, height)); update it to multiply
the returned byte count by the MSAA sample count so MSAA targets are accounted
for (e.g., renderbufferMemory += TextureUtils.getMipLevelByteCount(depth, width,
height) * Math.max(1, this.samples || sampleCount)); reference the RenderTarget
class, the renderbufferMemory variable and TextureUtils.getMipLevelByteCount
when making the change and ensure you use the correct property name for the
sample count used by this render target.
- Line 232: When subtracting the renderbuffer GPU memory from the engine
counters in RenderTarget (the line using
this._engine._renderingStatistics._textureMemory and
this._renderbufferGpuMemorySize), guard against context-loss resets so the
counter cannot go negative: either check that _renderbufferGpuMemorySize > 0 and
that _textureMemory >= _renderbufferGpuMemorySize before subtracting, or clamp
the result with a max(0, current - size) assignment; apply this change where the
render target is destroyed/released so repeated/device-loss resets cannot drive
the counter below zero.
---
Nitpick comments:
In `@tests/src/core/RenderingStatistics.test.ts`:
- Around line 100-127: Add two tests: (1) a MSAA depth test that creates a
Texture2D and a RenderTarget with antiAliasing > 1 (use RenderTarget(...,
antiAliasing: 4) or set renderTarget.antiAliasing = 4) and assert
engine.renderingStatistics.textureMemory increases by width * height * 4 *
samples (use the same width/height as the test); (2) a destroy-during-loss test
that simulates a lost context (set engine.isContextLost / the engine
context-lost flag) before calling RenderTarget.destroy and verify
engine.renderingStatistics.textureMemory does not underflow (i.e., equals the
value before creation/destruction). Reference the RenderTarget constructor,
renderTarget.antiAliasing, RenderTarget.destroy, Texture2D, TextureFormat and
engine.renderingStatistics.textureMemory when adding these assertions.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
packages/core/src/Engine.tspackages/core/src/asset/RenderingStatistics.tspackages/core/src/graphic/Buffer.tspackages/core/src/index.tspackages/core/src/texture/RenderTarget.tspackages/core/src/texture/Texture.tspackages/core/src/texture/Texture2D.tspackages/core/src/texture/Texture2DArray.tspackages/core/src/texture/TextureCube.tstests/src/core/RenderingStatistics.test.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/core/src/texture/TextureCube.ts
- packages/core/src/texture/Texture2DArray.ts
- packages/core/src/texture/Texture2D.ts
- packages/core/src/texture/Texture.ts
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/core/src/texture/TextureUtils.ts (1)
155-156:⚠️ Potential issue | 🟡 MinorFix
Depth32Stencil8memory calculation to 8 bytes.The WebGL 2
DEPTH32F_STENCIL8format allocates 64 bits (8 bytes) per texel: 32-bit float depth + 8-bit stencil + 24 bits padding. The current value of 5 under-reports actual GPU memory.🐛 Proposed fix
case TextureFormat.Depth32Stencil8: - return width * height * 5; + return width * height * 8;,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/texture/TextureUtils.ts` around lines 155 - 156, The memory calculation for TextureFormat.Depth32Stencil8 is incorrect (returns width * height * 5); update the case handling for TextureFormat.Depth32Stencil8 in the switch inside TextureUtils (the branch that currently returns width * height * 5) to return width * height * 8 to reflect the 64-bit (8 bytes) per-texel allocation for DEPTH32F_STENCIL8.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/core/src/texture/TextureUtils.ts`:
- Around line 155-156: The memory calculation for TextureFormat.Depth32Stencil8
is incorrect (returns width * height * 5); update the case handling for
TextureFormat.Depth32Stencil8 in the switch inside TextureUtils (the branch that
currently returns width * height * 5) to return width * height * 8 to reflect
the 64-bit (8 bytes) per-texel allocation for DEPTH32F_STENCIL8.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/core/src/graphic/Buffer.tspackages/core/src/texture/RenderTarget.tspackages/core/src/texture/Texture.tspackages/core/src/texture/TextureUtils.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/core/src/texture/Texture.ts
- packages/core/src/graphic/Buffer.ts
… in ResourceManager
…n RenderingStatistics tests
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.
Summary
RenderingInfoclass exposed viaengine.renderingInfowith three readonly properties:textureMemory,bufferMemory, andtotalMemory(bytes)TextureUtils.getTextureByteCount()andgetMipLevelByteCount()supporting all 30+ TextureFormat values including compressed formats (BC, ETC, PVRTC, ASTC)Motivation
Web engines have no access to native GPU profiling tools. Unlike desktop engines where external profilers (RenderDoc, Xcode GPU Debugger) can inspect VRAM usage, WebGL provides zero visibility into how much memory resources consume. This makes several real-world problems hard to solve:
totalMemorybefore and after operations makes leaks immediately visible.totalMemoryagainst pre-loss values confirms whether all resources were successfully rebuilt.API
Test plan
Summary by CodeRabbit
New Features
Bug Fixes
Tests