Skip to content

Add DOM-free font offset fallback#18463

Open
matthargett wants to merge 3 commits into
BabylonJS:masterfrom
rebeckerspecialties:codex/native-font-offset
Open

Add DOM-free font offset fallback#18463
matthargett wants to merge 3 commits into
BabylonJS:masterfrom
rebeckerspecialties:codex/native-font-offset

Conversation

@matthargett
Copy link
Copy Markdown
Contributor

What

  • Falls back from DOM layout font metrics to canvas text metrics when DOM layout is unavailable or returns zero-sized bounds.
  • Uses a CSS pixel-size estimate as the final fallback.
  • Adds focused unit coverage for the zero-layout path.

Why

Native runtimes need GUI resize-to-fit code to work without carrying validation-only font metric shims.

Validation

  • npx vitest run --project=unit packages/dev/core/test/unit/Engines/engine.common.test.ts

Make GetFontOffset robust when DOM layout is unavailable or reports zero metrics by falling back to canvas text metrics, then to a CSS font-size estimate.

This lets native runtimes use GUI resizeToFit without carrying validation-only font metric shims.
Copilot AI review requested due to automatic review settings May 17, 2026 02:38
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR improves GetFontOffset so it can produce usable font metrics in non-DOM environments by falling back from DOM layout measurement to canvas text metrics, with a final CSS pixel-size estimate fallback, and adds a unit test covering the “DOM layout returns zero bounds” path.

Changes:

  • Refactors GetFontOffset into DOM-based, canvas-based, and estimated fallback strategies.
  • Adds a CSS px font-size parser to support the final fallback when no measurement APIs are available.
  • Adds a focused jsdom unit test that forces the zero-layout DOM case and validates the canvas-metrics fallback behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
packages/dev/core/src/Engines/engine.common.ts Implements multi-stage DOM/canvas/estimated fallbacks for font metric extraction.
packages/dev/core/test/unit/Engines/engine.common.test.ts Adds unit coverage for the “DOM returns zero bounds, use canvas metrics” fallback.

Comment on lines +213 to +214
const ascent = Number(metrics.fontBoundingBoxAscent ?? metrics.actualBoundingBoxAscent);
const descent = Number(metrics.fontBoundingBoxDescent ?? metrics.actualBoundingBoxDescent);
Comment thread packages/dev/core/src/Engines/engine.common.ts Outdated
@RaananW
Copy link
Copy Markdown
Member

RaananW commented May 17, 2026

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@bjsplat
Copy link
Copy Markdown
Collaborator

bjsplat commented May 17, 2026

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

@bjsplat
Copy link
Copy Markdown
Collaborator

bjsplat commented May 17, 2026

Snapshot stored with reference name:
refs/pull/18463/merge

Test environment:
https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/refs/pull/18463/merge/index.html

To test a playground add it to the URL, for example:

https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/refs/pull/18463/merge/index.html#WGZLGJ#4600

Links to test your changes to core in the published versions of the Babylon tools (does not contain changes you made to the tools themselves):

https://playground.babylonjs.com/?snapshot=refs/pull/18463/merge
https://sandbox.babylonjs.com/?snapshot=refs/pull/18463/merge
https://gui.babylonjs.com/?snapshot=refs/pull/18463/merge
https://nme.babylonjs.com/?snapshot=refs/pull/18463/merge

To test the snapshot in the playground with a playground ID add it after the snapshot query string:

https://playground.babylonjs.com/?snapshot=refs/pull/18463/merge#BCU1XR#0

If you made changes to the sandbox or playground in this PR, additional comments will be generated soon containing links to the dev versions of those tools.

@bjsplat
Copy link
Copy Markdown
Collaborator

bjsplat commented May 17, 2026

@bjsplat
Copy link
Copy Markdown
Collaborator

bjsplat commented May 17, 2026

🟢 Memory Leak Test Results

13 passed, 0 leaked out of 13 scenarios

🟢 All memory leak tests passed — no leaks detected.

Passed Scenarios (13)
Scenario Package
Core Feature Stack @babylonjs/core
Core Rendering Materials Shadows Stack @babylonjs/core
Core Textures Render Targets PostProcess Stack @babylonjs/core
GUI Fullscreen UI Controls @babylonjs/gui
GUI Mesh ADT Controls @babylonjs/gui
Loaders Boombox Import @babylonjs/loaders
Loaders OBJ Direct Load @babylonjs/loaders
Loaders STL Direct Load @babylonjs/loaders
Materials Library Stack @babylonjs/materials
Serializers glTF Export @babylonjs/serializers
Serializers GLB Export @babylonjs/serializers
PostProcesses Digital Rain Stack @babylonjs/post-processes
Procedural Textures Stack @babylonjs/procedural-textures

@bjsplat
Copy link
Copy Markdown
Collaborator

bjsplat commented May 17, 2026

@bjsplat
Copy link
Copy Markdown
Collaborator

bjsplat commented May 17, 2026

@bjsplat
Copy link
Copy Markdown
Collaborator

bjsplat commented May 17, 2026

⚡ Performance Test Results

🟢 All performance tests passed — no regressions detected.

Co-authored-by: Copilot Autofix powered by AI <[email protected]>
@RaananW
Copy link
Copy Markdown
Member

RaananW commented May 18, 2026

Moving all active PRs to draft for the next 24 to 48 hours.
This is due to the tree-shaking PR #18441 being merged later today.

@RaananW RaananW marked this pull request as draft May 18, 2026 08:39
@RaananW RaananW marked this pull request as ready for review May 18, 2026 16:34
@RaananW
Copy link
Copy Markdown
Member

RaananW commented May 18, 2026

Please make sure to merge from master

@bjsplat
Copy link
Copy Markdown
Collaborator

bjsplat commented May 18, 2026

Reviewer - this PR has made changes to one or more package.json files.

@bjsplat
Copy link
Copy Markdown
Collaborator

bjsplat commented May 18, 2026

Reviewer - this PR has made changes to the build configuration file.

This build will release a new package on npm

If that was unintentional please make sure to revert those changes or close this PR.

@bjsplat
Copy link
Copy Markdown
Collaborator

bjsplat commented May 18, 2026

@bjsplat
Copy link
Copy Markdown
Collaborator

bjsplat commented May 18, 2026

🟢 Memory Leak Test Results

13 passed, 0 leaked out of 13 scenarios

🟢 All memory leak tests passed — no leaks detected.

Passed Scenarios (13)
Scenario Package
Core Feature Stack @babylonjs/core
Core Rendering Materials Shadows Stack @babylonjs/core
Core Textures Render Targets PostProcess Stack @babylonjs/core
GUI Fullscreen UI Controls @babylonjs/gui
GUI Mesh ADT Controls @babylonjs/gui
Loaders Boombox Import @babylonjs/loaders
Loaders OBJ Direct Load @babylonjs/loaders
Loaders STL Direct Load @babylonjs/loaders
Materials Library Stack @babylonjs/materials
Serializers glTF Export @babylonjs/serializers
Serializers GLB Export @babylonjs/serializers
PostProcesses Digital Rain Stack @babylonjs/post-processes
Procedural Textures Stack @babylonjs/procedural-textures

@bjsplat
Copy link
Copy Markdown
Collaborator

bjsplat commented May 18, 2026

@bjsplat
Copy link
Copy Markdown
Collaborator

bjsplat commented May 18, 2026

⚡ Performance Test Results

🟢 All performance tests passed — no regressions detected.

@bjsplat
Copy link
Copy Markdown
Collaborator

bjsplat commented May 18, 2026

@matthargett
Copy link
Copy Markdown
Contributor Author

Merged latest master (9.9.1) into the branch.

The earlier package/build-configuration bot comments are stale after the rebase/merge: the current PR diff is only packages/dev/core/src/Engines/engine.common.ts plus packages/dev/core/test/unit/Engines/engine.common.test.ts.

Local validation with the reconciled 18460 + 18463 patches:

  • Focused unit tests passed, including engine.common.
  • nx build babylonjs and nx build babylonjs-gui passed at 9.9.1.
  • BabylonNative NativeWebGPU focused screenshots passed for the related GUI3D/font-offset coverage.

Fresh PR checks currently show GitGuardian passing, with Azure/Mergify entries neutral/skipping rather than failing.

@Popov72
Copy link
Copy Markdown
Contributor

Popov72 commented May 22, 2026

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@bjsplat
Copy link
Copy Markdown
Collaborator

bjsplat commented May 22, 2026

@bjsplat
Copy link
Copy Markdown
Collaborator

bjsplat commented May 22, 2026

🟢 Memory Leak Test Results

13 passed, 0 leaked out of 13 scenarios

🟢 All memory leak tests passed — no leaks detected.

Passed Scenarios (13)
Scenario Package
Core Feature Stack @babylonjs/core
Core Rendering Materials Shadows Stack @babylonjs/core
Core Textures Render Targets PostProcess Stack @babylonjs/core
GUI Fullscreen UI Controls @babylonjs/gui
GUI Mesh ADT Controls @babylonjs/gui
Loaders Boombox Import @babylonjs/loaders
Loaders OBJ Direct Load @babylonjs/loaders
Loaders STL Direct Load @babylonjs/loaders
Materials Library Stack @babylonjs/materials
Serializers glTF Export @babylonjs/serializers
Serializers GLB Export @babylonjs/serializers
PostProcesses Digital Rain Stack @babylonjs/post-processes
Procedural Textures Stack @babylonjs/procedural-textures

@bjsplat
Copy link
Copy Markdown
Collaborator

bjsplat commented May 22, 2026

@bjsplat
Copy link
Copy Markdown
Collaborator

bjsplat commented May 22, 2026

@RaananW
Copy link
Copy Markdown
Member

RaananW commented May 22, 2026

one last thing - the copilot comment seems logical to me, does it make sense? feel free to resolve or commit these changes, and I will merge it right after

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.

5 participants