Skip to content

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

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

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

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

总结

两处均为测试修复,方向正确。

  • InputManager:根因定位准确。keyboard 用例里 keydown KeyA 之后没有 update,事件滞留在 _nativeEvents 队列,被后续用例的 update 消费;Mac 上 KeyboardManager._update 的 meta 键清理分支恰好把它清掉,非 Mac 平台不走该分支,于是 KeyA 残留在 _curHeldDownKeyToIndexMap,下一次 keydown KeyA 被当作重复过滤掉 → 失败。修复用 blur(清空所有按键 map 和 _nativeEvents)+ update(排空队列),对照 KeyboardManager.ts 核实无误,是最小且对根因的修复,不是掩盖 flakiness。第 198 行 isKeyHeldDown(KeyA) === false 的断言在 update 之前读 map,语义不受影响。
  • TextUtils:内置一个真实的 Roboto-Regular 子集(name 表核实为 v2.137、Apache-2.0、~22KB),通过 FontFace/document.fonts 加载。这让 advance 类指标(width/lines/lineWidths)真正跨平台确定——字体字节在所有平台一致,measureText 的步进不依赖光栅器。回归测试反转可破(字体换回 Arial 则 macOS 给 27、值停在 28 即失败;保留字体把值改回 27 则 Roboto 给 28 即失败),约束有效。

无阻塞问题,方向上严格优于现状。

问题

[P2] lineHeight/height 的光栅器依赖仍未根除TextUtils.test.ts,所有 lineHeight/height 断言,如第 179/277/502/537 行)

换固定字体只消除了"各 OS 的 Arial 是不同字体文件"这一项方差,但没有消除该断言失败的真正根因:lineHeight = fontSizeInfo.size + lineSpacing,而 size = ascent + descent 来自 TextUtils._measureFontOrChar 里的 getImageData 逐像素 alpha 扫描(TextUtils.ts:397/425-428)。这个扫描依赖光栅器(CoreText vs DirectWrite vs FreeType),即便 Roboto 字形轮廓完全一致,顶/底边缘像素仍可能差 ±1px。新基线 28 是 CI(macos-latest)/CoreText 下的值;提 #3043 的 Windows 同学很可能仍拿到 27 而继续红——而且本 PR 只在非 Mac 平台验证了 InputManager,TextUtils 没有在 Windows 上验证过。

建议:对光栅化的纵向指标用容差断言,例如 expect(result.lineHeight).to.be.closeTo(28, 1)heightlineHeight * 行数,同理),同时把 advance 类的 width/lineWidths/lines 保持精确相等。这样既守住回归意义,又容忍作者自己已承认可能存在的 ±1px。

[P3] 内置字体缺少就地的来源/许可说明Roboto-Regular-subset.ttf

子集的 name 表只保留了简短版权串("Copyright 2011 Google Inc."),许可条目(Apache-2.0)在子集化时似乎被丢掉了(二进制里 strings 不到 "Apache"/"License")。PR 描述写明了 Apache-2.0 是对的,但仓库里落了个无就地溯源的二进制 blob。建议在测试里加一行注释或在文件旁放一行 NOTICE 记录"Roboto-Regular v2.137, Apache-2.0, subset for tests",便于追溯许可来源。

[P3] 字体可用性可更稳TextUtils.test.ts:35-36

await testFont.load() + document.fonts.add(testFont) 之后字体已加载、canvas 一般能拿到;如再 await document.fonts.ready,可在任何测量前彻底消除字体就绪竞态。可选,非必须。

简化建议

无。InputManager 的修复是最小化的局部清理(用 afterEach 统一 blur/update 更系统,但对单个泄漏用例属过度设计);TextUtils 的 new URL(..., import.meta.url) + FontFace 加载是 Vite/browser-mode 的惯用写法,没有引入多余复杂度。

@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端依旧相同的报错,测试不通过

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