Skip to content

test: fix cross-platform failures in InputManager & TextUtils#3044

Draft
cptbtptpbcptdtptp wants to merge 3 commits into
galacean:dev/2.0from
cptbtptpbcptdtptp:fix/cross-platform-test-failures-3043
Draft

test: fix cross-platform failures in InputManager & TextUtils#3044
cptbtptpbcptdtptp wants to merge 3 commits into
galacean:dev/2.0from
cptbtptpbcptdtptp:fix/cross-platform-test-failures-3043

Conversation

@cptbtptpbcptdtptp

@cptbtptpbcptdtptp cptbtptpbcptdtptp commented Jun 18, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes the local (non-mac) unit-test failures reported in #3043. Both are test issues, not engine bugs — and both were hidden because the test job runs on macos-latest.

InputManager — blur and focus, change listener target

Root cause is cross-case state leakage, masked by a mac-only branch. The keyboard case dispatches a keydown KeyA with no following update, so the event stays queued and is consumed by a later case's update, marking KeyA as held down. On mac the meta-key cleanup in KeyboardManager happens to clear it; on other platforms it stays held, so the next keydown KeyA is filtered as a repeat and isKeyDown returns false.

Fix: reset the keyboard state at the end of the keyboard case so it doesn't leak.

TextUtils — measureTextWithWrap, measureTextWithoutWrap

lineHeight is derived from a per-pixel scan of glyphs rendered with the host Arial, which is a different font file and rasterizer per OS (27 on macOS, 26 on Windows).

Fix: bundle a small Roboto subset (Apache-2.0, ~22 KB, Latin only), load it via FontFace, point the renderers at it, and rebaseline the metrics. Advance-based metrics (width/lines/lineWidths) now come from the embedded file and are stable across platforms; CJK text still falls back to system fonts as before, so its full-width advances are unchanged.

Note: lineHeight is still a rasterized value. The fixed font removes the "different Arial per OS" variance, but rasterizer differences (DirectWrite vs CoreText vs FreeType) could in principle still shift it by ±1px on some platforms.

Verification

Reproduced the InputManager failures locally by forcing a non-mac platform (both target cases then fail exactly as in the issue), and confirmed both fixes make all 14 tests pass.

Closes #3043

Summary by CodeRabbit

  • Tests
    • Updated the text measurement test suite to use a deterministic Roboto subset font for more consistent rendering and updated related expectations.
    • Improved keyboard input testing by dispatching a browser blur event to reset keyboard state between assertions.

cptbtptpbcptdtptp and others added 2 commits June 18, 2026 16:01
The "keyboard" case dispatches a `keydown KeyA` without a following
`update`, so the event stays queued and is later consumed by another
case's `update`, marking KeyA as held down. On mac the meta-key cleanup
in KeyboardManager happens to clear it, but on other platforms it stays
held, so the next `keydown KeyA` is filtered as a repeat and isKeyDown
returns false. This made "blur and focus" and "change listener target"
fail locally on Windows while passing in CI (macOS).

Reset the keyboard state at the end of the case to keep it isolated.

Refs galacean#3043

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
TextUtils derives lineHeight from a per-pixel scan of rendered glyphs,
so relying on the host "Arial" (a different font file and rasterizer per
OS) produced platform-dependent values (lineHeight 27 on macOS vs 26 on
Windows). Bundle a small Roboto subset (Apache-2.0), load it via
FontFace in the test, point the renderers at it, and rebaseline the
metrics. CJK text still falls back to system fonts as before, so its
full-width advances stay stable.

Refs galacean#3043

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 18, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 1f23dd98-4b82-4c46-82f8-668595b0b2be

📥 Commits

Reviewing files that changed from the base of the PR and between 49c1544 and 782e860.

📒 Files selected for processing (1)
  • tests/src/core/2d/text/TextUtils.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • tests/src/core/2d/text/TextUtils.test.ts

Walkthrough

Two test-only fixes: TextUtils.test.ts replaces the host Arial font with a deterministically loaded Roboto subset font via FontFace/document.fonts, reassigns all TextRenderer instances to use it, and updates every metric assertion (lineHeight, width, height, lines, lineWidths) to match Roboto's values. InputManager.test.ts inserts a blur event dispatch before engine.update() to clear a KeyA keydown that would otherwise leak into a subsequent not-held assertion.

Changes

TextUtils: Roboto subset font and updated metric assertions

Layer / File(s) Summary
Roboto font loading and TextRenderer wiring
tests/src/core/2d/text/TextUtils.test.ts
beforeAll loads a Roboto subset FontFace and registers it with document.fonts; all TextRenderer instances switch from "Arial" to Font.createFromOS(engine, testFontName).
measureTextWithWrap assertion updates
tests/src/core/2d/text/TextUtils.test.ts
All expected lineHeight, height, width, and lines values in measureTextWithWrap truncate and overflow mode cases are updated to Roboto metrics (lineHeight 28, revised heights and line content).
measureTextWithoutWrap assertion updates
tests/src/core/2d/text/TextUtils.test.ts
All expected lineHeight, width, lineWidths, and lines values in measureTextWithoutWrap truncate and overflow mode cases (including empty/undefined text) are updated to Roboto metrics.

InputManager: keyboard state leak fix

Layer / File(s) Summary
blur dispatch to reset keyboard state
tests/src/core/input/InputManager.test.ts
A blur event is dispatched after queuing KeyA keydown and before engine.update(), preventing the pending keydown from persisting into the subsequent not-held assertion.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A rabbit swapped fonts — Arial, goodbye!
Now Roboto metrics reach toward the sky.
Line heights of 28, widths all aligned,
No leaky key presses left behind.
Tests run deterministic, clean, and bright —
The warren of assertions feels just right! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely identifies the main fixes: cross-platform test failures in InputManager and TextUtils, matching the changeset's scope.
Linked Issues check ✅ Passed Changes fully address both requirements from issue #3043: InputManager keyboard state leakage is fixed by dispatching a blur event, and TextUtils metrics are rebaselined using a deterministic Roboto font.
Out of Scope Changes check ✅ Passed All changes are scoped to fixing the reported cross-platform test failures; no unrelated modifications to production code or unrelated test areas are present.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

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

@codecov

codecov Bot commented Jun 18, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.28%. Comparing base (5153244) to head (782e860).
⚠️ Report is 10 commits behind head on dev/2.0.

Additional details and impacted files
@@             Coverage Diff             @@
##           dev/2.0    #3044      +/-   ##
===========================================
- Coverage    79.28%   79.28%   -0.01%     
===========================================
  Files          919      919              
  Lines       102452   102452              
  Branches     11354    11353       -1     
===========================================
- Hits         81230    81225       -5     
- Misses       21034    21039       +5     
  Partials       188      188              
Flag Coverage Δ
unittests 79.28% <ø> (-0.01%) ⬇️

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

This comment was marked as outdated.

@luo2430

luo2430 commented Jun 18, 2026

Copy link
Copy Markdown

我错了,似乎与字体文件差异无关,是跨平台的渲染差异。@GuoLei1990expect(result.lineHeight).to.be.closeTo(28, 1) 方案应该是最优解了。输入问题确认已解决。

 FAIL   @galacean/engine-tests (chromium)  src/core/2d/text/TextUtils.test.ts > TextUtils > measureTextWithWrap
AssertionError: expected [ 'T', 'h', 'e ', 'w', 'e', …(11) ] to deeply equal [ 'T', 'h', 'e ', 'w', 'e', …(11) ]

Failure screenshot:
  - tests/src/core/2d/text/__screenshots__/TextUtils.test.ts/TextUtils-measureTextWithWrap-1.png

- Expected
+ Received

@@ -11,8 +11,8 @@
    "gr",
    "e",
    "at",
    "to",
    "d",
-   "ay",
-   ".",
+   "a",
+   "y.",
  ]

 ❯ src/core/2d/text/TextUtils.test.ts:190:36
    188|     expect(result.width).to.be.equal(24);
    189|     expect(result.height).to.be.equal(100);
    190|     expect(result.lines).to.be.deep.equal([
       |                                    ^
    191|       "T",
    192|       "h",

Address review feedback on galacean#3044:
- lineHeight/height derive from a per-pixel glyph scan and stay rasterizer
  dependent (CoreText vs DirectWrite) even with a fixed font, so assert
  them with a +/-1px tolerance (closeTo) while keeping advance-based
  width/lines/lineWidths exact. This preserves the regression guard while
  letting non-mac platforms pass too.
- Record the bundled font source/license inline (Roboto-Regular v2.137,
  Apache-2.0).
- Await document.fonts.ready to remove a font-readiness race.

Refs galacean#3043

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@luo2430

luo2430 commented Jun 20, 2026

Copy link
Copy Markdown

windows端依旧相同的报错,测试不通过

@GuoLei1990 GuoLei1990 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

总结

782e860bc3 把所有 lineHeight/height 断言改成 closeTo(28, 1) / closeTo(H, tol),采纳了上轮 P2;同时上轮两条 P3(字体来源注释、await document.fonts.ready)也都落地了。InputManager 修复未变,reviewer 已确认输入问题解决。

但 luo2430 在 Windows 上仍然失败——而且失败点已经从 lineHeight 移到了 lines["d","ay","."] 期望 vs ["d","a","y."] 实际)。这条不是上轮 lineHeight 那条的换皮,是一个我上轮明确判错的新问题:我当时断言 width/lines/lineWidths 这类 advance 指标"跨平台确定",Windows 的结果证伪了它。下面是根因。

问题

[P1] lines 的换行切分点仍依赖光栅器,固定字体没能消除它TextUtils.test.ts,所有 wrap 的 lines/lineWidths/width 断言,如第 192-208 行)

我上轮的判断错了。换行切分不是纯 advance 决定的。逐链 trace:

  • 行/词的宽度累加用的是 charInfo.xAdvanceTextUtils.ts:194/200/223/228),xAdvance = Math.round(actualWidth)TextUtils.ts:450)——这一项是字体文件确定的,跨平台稳定,我上轮没说错。
  • 是否断词/断行的阈值判断用的是 charInfo.wTextUtils.ts:184lineWidth + w > rendererWidthTextUtils.ts:205wordWidth + charInfo.w > rendererWidth)。而 w = Math.round(Math.max(actualBoundingBoxRight + |actualBoundingBoxLeft|, actualWidth))TextUtils.ts:371-374)——actualBoundingBox*字形墨水盒,恰恰是 DirectWrite vs CoreText vs FreeType 会差的量。

所以即便 Roboto 轮廓字节一致,墨水盒右沿经 Math.round 后在 Windows 上抖 ±1px,就会让 wordWidth + wrendererWidth=24 边界附近提前/延后跨线,断词点平移一个字符 → "today." 切成 ["d","a","y."] 而非 ["d","ay","."]。本 PR 把 lines 重新基线成了 macOS-CoreText 的切分(diff 里 "a","y.""ay","."" "" "lineWidths [63,111][54,105] 都是 mac 值),Windows 自然继续红。

这跟之前的 lineHeight同一个根(墨水盒逐像素扫描的光栅器依赖),只是这次显形在切分而非行高。所以 closeTo 容差治不了它——lines 是离散的 deep-equal,没有"容差"一说。

注意 w 的墨水盒口径不能在源码侧改掉TextUtils.ts:369-370 注释 + 引入它的 #2247(commit 192f5a874 "Fix measure text width error")明确说明,纯 advance 会少量导致渲染裁切,max(boundingBox, advance) 才贴近 native 渲染宽度。这是有意为之的渲染正确性取舍,不是 bug。所以修复落点只能在测试侧——不要去动 _measureFontOrCharw 公式。

建议(测试侧,二选一):

  • 首选:对 wrap 结果只断"重组等于原文 + 段数/宽度在容差内",不断逐段切分。即 expect(result.lines.join("")).to.equal(原文去换行) + expect(result.lineHeight).to.be.closeTo(28,1),放弃 deep.equal(["d","ay","."]) 这种锁死切分点的断言。切分点本就坐在 ±1px 抖动的边界上,断它等于把光栅器锁进测试。
  • 次选:把 rendererWidth 调离临界值(如把 width 从 0.24 调成能让每个词整体落入或整体溢出、不卡在单字符边界的值),让切分点远离 ±1px 翻转区。代价是要重新设计每个 case 的期望,且无法保证所有 case 都能躲开边界。

无论哪种,都要在 reviewer 的 Windows 机器上实跑确认,而不是只在 mac CI 验证——这条 PR 至今 lines 从未在 Windows 上验过。

简化建议

measureTextWithoutWraplineWidths(如第 421 行 [516]、439 行 [421])走的是 xAdvance 累加,理论上是 advance 确定的,可以保留精确相等;真正不稳的是 wrap 路径里坐在 rendererWidth 边界上lines 切分。如果走"首选"方案,可以只对 wrap 的 lines 放宽,no-wrap 的 lineWidths/width 维持精确——这样既守住 advance 类回归,又只对真正光栅器耦合的切分让步。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants