refactor(rhi-webgl): rework canvas resolution API, follow display size by default#3037
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:
Walkthrough
ChangesOffscreenCanvas detection and auto-resize
Sequence Diagram(s)sequenceDiagram
participant Caller
participant WebGLEngine
participant ResizeObserver
participant WebCanvas
Caller->>WebGLEngine: enableAutoResize(pixelRatio?)
WebGLEngine->>ResizeObserver: disconnect() (prior observer, if any)
WebGLEngine->>ResizeObserver: new ResizeObserver(callback)
WebGLEngine->>ResizeObserver: observe(canvas element)
WebGLEngine->>WebGLEngine: on(Shutdown, _cleanupResizeObserver)
Note over ResizeObserver,WebCanvas: canvas client dimensions change
ResizeObserver->>WebCanvas: resizeByClientSize(pixelRatio)
Caller->>WebGLEngine: disableAutoResize()
WebGLEngine->>WebGLEngine: off(Shutdown, _cleanupResizeObserver)
WebGLEngine->>ResizeObserver: disconnect()
Note over WebGLEngine: engine.destroy() triggers Shutdown
WebGLEngine->>WebGLEngine: _cleanupResizeObserver()
WebGLEngine->>ResizeObserver: disconnect()
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 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 |
![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/rhi-webgl/WebGLEngine.test.ts`:
- Around line 140-145: The spy on the _cleanupResizeObserver method is created
after enableAutoResize() has already registered the Shutdown listener, meaning
the listener holds a reference to the original function rather than the spied
version. Move the vi.spyOn call for cleanupResizeObserverSpy to before the
enableAutoResize() call to ensure the registered Shutdown listener captures the
spied function reference, allowing the assertion on
cleanupResizeObserverSpy.toHaveBeenCalledTimes(1) to work correctly.
🪄 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: ca7819a5-d724-46f3-ad7d-879ae0800382
📒 Files selected for processing (3)
packages/core/src/input/InputManager.tspackages/rhi-webgl/src/WebGLEngine.tstests/src/rhi-webgl/WebGLEngine.test.ts
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指针问题我考虑到了，但不知为何代码经过修改后我的本地测试可以跑通。 
我会再去处理这个问题，原本以为已经修复了。 |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
这里有一个时序问题需要提一下，根据 W3C 规范，浏览器每帧的渲染管线顺序是： 在发生 resize 那帧，引擎绘制 rAF 时使用的还是旧的画布尺寸，随后 ResizeObserver 回调修改画布的尺寸会清空 canvas buffer ，表现就是这一帧会白，但 window 监听 resize 也不可以直接用，因为 window.resize 仅监听浏览器窗口大小变化，所以其他引擎要么完全手动触发，要么轮询检测，避免本帧 resize 与渲染不同步。 |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## dev/2.0 #3037 +/- ##
===========================================
+ Coverage 79.52% 79.81% +0.28%
===========================================
Files 904 904
Lines 101168 101285 +117
Branches 11377 11405 +28
===========================================
+ Hits 80456 80838 +382
+ Misses 20528 20265 -263
+ Partials 184 182 -2
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:
|
luo2430
changed the title
|
代码处理完毕，vitest问题我会去开一个issue，有部分解决方案和提议，但需要验证。另外，isOffscreenCanvas中的 |
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 这里有好康的 |
这个是当前 PR 在真实 resize 时的表现 期望应该是这样的（时序导致） |
|
@cptbtptpbcptdtptp 这个示例能分享一下吗？我用于测试。 |
examples/src/device-restore.ts 这个，设置 enableAutoResize 就可以。 |
github-actions
Bot
added
the
documentation
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.
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.
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.
本轮改动总结（
|
GuoLei1990
left a comment
GuoLei1990
left a comment
There was a problem hiding this comment.
复审 @ e25286149fa5（HEAD 在我上一条 review 提交瞬间又线性推进 1 个 commit：e2528614 fix: feature-detect device-pixel-content-box before observing to avoid throwing on Safari——这个 commit 正是修复我历轮 flag 的 [P0]〔删掉的 dpcb try/catch → 改用 feature-detection 门控，比 try/catch 更干净〕，P0 闭环。但我上一轮同时 flag 的 [P1]〔rename 漏改测试文件〕仍未修：WebGLEngine.test.ts:186/233 仍引用旧名 _setSize/_setSizeByClientSizeFallback，2 个单测确定性抛 TypeError，codecov〔vitest〕job 仍 FAILURE。gate 保持，--request-changes 重锚到 e25286149fa5。）
时序校准（我上一条 CHANGES_REQUESTED 落到了这个我尚未审的 commit 上）
我上一条 review 审的是 b128f70c〔rename commit，flag P1 测试漏改 + 重申 P0〕。提交 review 的瞬间 HEAD 已推进到 e25286149fa5——compare b128f70c...e2528614 = ahead_by: 1：
e25286149fa5 fix: feature-detect device-pixel-content-box before observing to avoid throwing on Safari〔仅WebCanvas.ts，+15/-3〕
GitHub 把我的 CHANGES_REQUESTED 记到了 e2528614——但我尚未审它。本轮从 b128f70c 审这一 delta〔且因它触碰我 flag 的 P0 边界，我逐链 trace + 拉 CI annotation〕。上一条已 minimize。
我上轮 [P0] 直接闭环——feature-detection 门控替代 try/catch，更优
e2528614 逐字核对，正是修复我历轮反复 flag 的 P0〔WebCanvas.ts:45 删掉 observe({box:"device-pixel-content-box"}) 外 try/catch，不支持 dpcb 的浏览器上 WebIDL enum 抛 TypeError → create() 崩溃〕：
+ private static _devicePixelContentBoxSupported?: boolean;
+ private static _supportsDevicePixelContentBox(): boolean {
+ // Passing { box: "device-pixel-content-box" } to observe() throws on browsers whose
+ // ResizeObserverBoxOptions enum lacks that value (Safari, Firefox < 93), so callers must gate on this
+ return (this._devicePixelContentBoxSupported ??=
+ typeof ResizeObserverEntry !== "undefined" && "devicePixelContentBoxSize" in ResizeObserverEntry.prototype);
+ }
...
- // Browsers without device-pixel-content-box ignore the box option and fall back to content-box, ...
- this._resizeObserver.observe(this._webCanvas, { box: "device-pixel-content-box" });
+ // Observe exact device pixels where supported, otherwise the content-box and size from the client size
+ if (WebCanvas._supportsDevicePixelContentBox()) {
+ this._resizeObserver.observe(this._webCanvas, { box: "device-pixel-content-box" });
+ } else {
+ this._resizeObserver.observe(this._webCanvas);
+ }逐点核对，正确且优于我建议的 try/catch：
- feature-detection 是标准 dpcb 检测惯例：
"devicePixelContentBoxSize" in ResizeObserverEntry.prototype——支持device-pixel-content-boxbox 值的浏览器与在 entry 上暴露devicePixelContentBoxSize是同批次 ship 的〔同一规范增量〕，用后者探测前者是社区公认写法。typeof ResizeObserverEntry !== "undefined"独立守卫引用〔虽已在ResizeObserver存在分支内，仍单独 guard，稳健〕。??=记忆化避免每次setAutoResolution重复探测。 - 不再抛异常：门控命中 →
observe({box:"dpcb"})；不支持 →observe(this._webCanvas)〔纯 content-box〕。根本消除了 TypeError 抛出路径，比我建议的 try/catch 更干净〔无异常成本、无 catch〕。pump 的 device-pixel 分支读devicePixelContentBoxSize?.[0]→undefined→client-size在纯 content-box 下正确降级。 - 注释改如实：新注释「throws on browsers whose ResizeObserverBoxOptions enum lacks that value (Safari, Firefox < 93)」——与 WebIDL 规范一致，替换了旧的 aspirational lie「browsers ignore the box option」。
- build×3 + e2e 4/4 全绿坐实。
[P0] 闭环。 我此前坚持的诉求〔别让不支持 dpcb 的浏览器崩溃 + 注释改如实〕两条都满足，且落地方式更优。
我上轮 [P1] 未闭环——本 commit 只碰 WebCanvas.ts，测试文件旧名仍在
[P1]（重申，e2528614 未触及）tests/src/rhi-webgl/WebGLEngine.test.ts:186,233 — rename 漏改测试文件，2 个单测确定性抛 TypeError，codecov〔vitest〕job FAILURE
b128f70c rename _setSize/_setSizeByClientSizeFallback→_setResolution* 时漏改测试文件，e2528614 只修 P0〔仅 WebCanvas.ts〕，未碰测试。逐字核对 tip e25286149fa5 的 WebGLEngine.test.ts〔fetch 独立复核〕：
// :181 注释仍写 _setSize
// :186 const originalSetSize = (webCanvas as any)._setSize.bind(webCanvas); ← _setSize 已 undefined
// :233 (webCanvas as any)._setSizeByClientSizeFallback(1); ← 已 undefinedCI annotation 逐字坐实〔e25286149fa5 的 codecov check-run id 86780018739，仍 FAILURE〕：
TypeError: Cannot read properties of undefined (reading 'bind')
❯ src/rhi-webgl/WebGLEngine.test.ts:186:56
TypeError: webCanvas._setSizeByClientSizeFallback is not a function
❯ src/rhi-webgl/WebGLEngine.test.ts:233:23
:186— 测试「disables itself on a layout feedback loop」〔我77f5571b轮 approve 的测试 ②〕：_setSize已改名_setResolution→undefined.bind()→ TypeError。:233— 测试「client-size fallback skips a canvas with no layout size」〔测试 ④〕：_setSizeByClientSizeFallback已改名_setResolutionByClientSizeFallback→ is not a function。
回归网确认：b128f70c 的父 commit 8383c3b4 codecov = SUCCESS——回归唯一由 rename 引入。codecov job 跑 vitest，本体 FAILURE + annotation 含 TypeError = 真单测失败，非「覆盖率比值 artifact」〔后者仅 codecov/patch〕。e2e 全绿因这两个是白盒 vitest 单测、只在 codecov job 跑。
修复：测试文件三处旧名同步改名〔:186/:187 _setSize→_setResolution、:233 _setSizeByClientSizeFallback→_setResolutionByClientSizeFallback、:181 注释同步〕：
const originalSetResolution = (webCanvas as any)._setResolution.bind(webCanvas);
(webCanvas as any)._setResolution = (w: number, h: number) => { ... };
...
(webCanvas as any)._setResolutionByClientSizeFallback(1);改完 codecov〔vitest〕转绿即闭环。审查动作：rename PR 每次必须全仓 grep 旧符号名〔含 tests/〕——production 改名后编译器抓不到测试里的 as any cast 引用，运行时才崩。这是本 PR 第二次同型 miss〔前一次 7dc5112〕。
已闭环清单（历史全部项，逐条对 tip e25286149fa5 核对，未回退）
- [P0 我历轮] 删掉 dpcb observe 外 try/catch → 不支持 dpcb 的浏览器崩溃 →
e2528614改用_supportsDevicePixelContentBox()feature-detection 门控 + 注释改如实。本轮闭环，且落地优于 try/catch。 - [P0 我更早轮]
_pumpPendingResize漏改名 →28f639171修复，_pumpPendingResolution在，未回退。闭环。 - [P0 更早轮] TS4113 跨包 build 破坏 → build×3 SUCCESS。闭环。
- [P1 by cptbtptpbcptdtptp] 小程序 ResizeObserver ReferenceError →
typeof ResizeObserver守卫在。闭环。 - [P1 by cptbtptpbcptdtptp] 无 CSS 反馈环 → 检测 + 测试 ②〔但被 rename 打穿=本轮 P1〕。
- [P1 by cptbtptpbcptdtptp] content-box 接不住 DPR-only →
device-pixel-content-box门控在〔现由 feature-detection 保护，不再崩〕。闭环。 - [P2 by cptbtptpbcptdtptp]
setResolution不整数化 /@internald.ts 泄漏 →Math.round//** @internal */在。闭环。 - [API]
scaledeprecation →85ee362f删除。闭环。 - [P3 我更早轮] 反馈环 + RO 降级无 CI 测试 → 测试 ②③ 补上——但被 rename 漏改把 ②④ 打穿=本轮 P1。
- [naming]
_setSize*→_setResolution*production →b128f70c落地正确〔_onResolutionChangedoverride 保留正确、build×3/e2e 坐实〕，唯测试文件漏改=本轮 P1。
CI 说明
tip e25286149fa5：build×3 SUCCESS、e2e 1,2,3,4/4 全 SUCCESS、lint SUCCESS。codecov〔vitest〕= FAILURE〔annotation 含两处 TypeError，见 P1，非 codecov/patch 比值 artifact〕。
结论
request-changes：HEAD 在我上条 review 提交瞬间线性推进到 e25286149fa5〔fix: feature-detect device-pixel-content-box〕。这个 commit 正确闭环了我历轮反复 flag 的 [P0]——用 _supportsDevicePixelContentBox()〔"devicePixelContentBoxSize" in ResizeObserverEntry.prototype，标准 dpcb 探测惯例，??= 记忆化〕门控 observe 分支，不支持 dpcb 的浏览器走纯 content-box observe(webCanvas)，根本消除 TypeError 抛出路径，比我建议的 try/catch 更干净；注释也改成如实描述「throws on browsers whose enum lacks that value」，替换了 aspirational lie。P0 闭环，落地优于 try/catch。但我上一轮同时 flag 的 [P1] 仍未修：b128f70c rename _setSize*→_setResolution* 时漏改测试文件 WebGLEngine.test.ts，e2528614 只碰 WebCanvas.ts 未碰测试 → :186 _setSize.bind → undefined.bind → TypeError、:233 _setSizeByClientSizeFallback → is not a function，2 个单测〔恰是我 77f5571b 轮 approve 的测试 ②④〕确定性崩，codecov〔vitest〕job 仍 FAILURE〔annotation 逐字坐实，父 commit 8383c3b4 SUCCESS = 回归唯一由 rename 引入〕。修复=测试文件三处旧名同步改名〔:186/:187/:233 + :181 注释〕，改完 codecov 转绿。因当前 tip 单测确定性红、不可合并，P1 未闭环，提交 REQUEST_CHANGES 保持门控。8383c3b4〔P0 仍开版本〕和 e25286149fa5〔HEAD 竞态、P0 已在该 commit 修好但我尚未审〕的两条 review 均已 minimize，本轮重锚到 e25286149fa5。
3 participants
概述
一次 canvas 分辨率 API 的易用性增强：把 auto-resize 能力归位到
WebCanvas，让渲染分辨率默认自动跟随显示尺寸——大多数应用create({ canvas })之后不用写任何 resize 代码。主要改动
1. 归属：auto-resize 全部归
WebCanvas，pump 下沉到基类observer 观察 canvas、算 canvas 尺寸、改 canvas 分辨率——概念上 100% 属于 canvas。原本放 engine 是因为白屏修复需要帧时序（实现依赖绑架了归属）。现在：
WebCanvasEngine.update()（this._canvas._pumpPendingResize()），_pumpPendingResize是基类Canvas的默认 no-op、WebCanvasoverride——WebGLEngine不再需要 overrideupdate()2. 默认自动跟随显示尺寸（零配置）
ResizeObserver监听 canvas 元素本身 → 接住窗口、CSS、flex/容器、侧边栏/面板等所有来源的尺寸变化（window.resize只能接住窗口变化）3. 两个分辨率入口
scale的 base 是物理分辨率：1= 满物理分辨率（原生清晰），0.7= 降 30% 提性能，2= 超采样。device pixel ratio 自动吸收，用户无需理解 DPRcreate({ canvas })后调setResolution(w, h)（退出自动，无闪烁）4.
width/height改只读分辨率是原子的整体：单独设
width不动height会产生畸变中间态 + 两次 buffer 重建，且裸写canvas.width会让引擎缓存尺寸与真实 buffer 分叉。改只读后，所有分辨率变更走setResolution一个原子入口。width/height是"画布分辨率"的分量。5. 分数 DPR 精度：
devicePixelContentBoxSizeclientWidth * dpr在分数 DPR（Windows 1.25/1.5）下有 ≤1px 舍入误差。改用ResizeObserverentry 的devicePixelContentBoxSize（精确物理像素）消除它，Safari 等不支持的浏览器自动降级回clientWidth * dpr（特性检测，无人退化）。four 大 web3D 引擎（three.js/Babylon/Pixi/PlayCanvas）均未采用此特性。6. 删
resizeByClientSize被默认自动 +
setAutoResolution覆盖。Breaking Changes
major 版本级：
canvas.width/canvas.height改为只读（用setResolution设分辨率）resizeByClientSizecreate后调setResolution）设计参照
create({...})配置模式对齐 three.js / Babylon（web 引擎标准）scale（物理分辨率之上的性能倍数）对齐 UE ScreenPercentagesetResolution命名对齐 Unity / UE（SetResolution/SetScreenResolution）验证
devicePixelContentBoxSize精确路径 + 首帧时序 jsdom 不忠实复现，靠规范 + 代码审查（建议真机验一次）供讨论。 @luo2430 有不同意见或更好想法请直接回复。