Skip to content

Canvas: text metrics, multi-GUI filters, drawImage(canvas), toDataURL - #1855

Open
bkaradzic-microsoft wants to merge 8 commits into
BabylonJS:masterfrom
bkaradzic-microsoft:pr/canvas-features
Open

Canvas: text metrics, multi-GUI filters, drawImage(canvas), toDataURL#1855
bkaradzic-microsoft wants to merge 8 commits into
BabylonJS:masterfrom
bkaradzic-microsoft:pr/canvas-features

Conversation

@bkaradzic-microsoft

@bkaradzic-microsoft bkaradzic-microsoft commented Aug 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Summary

Canvas2D feature work that does not depend on instance branding or JsRuntimeHost type tags.

Area Change
Text metrics Font sizes are em units; measureText uses advance + ink bounds and binds the same face as fillText
Multi-GUI filters nanovg_filterstack refcounts shared blur programs/uniforms (multiple ADTs)
drawImage(canvas) InstanceOf(Canvas) then unwrap; CPU pixel mirror via bound context
toDataURL PNG only; bimg_encode + base64

Not in this PR

Unforgeable Path2D/Gradient/Image type checks → #1844 (held for type tags / optional). Accidental wrong-type args that are not instanceof Canvas still take the existing Image path.

Test plan

  • Unit: drawImage(canvas) + toDataURL → data:image/png;base64,...
  • CI green

Copilot AI lite review requested due to automatic review settings August 27, 2026 23:19

This comment was marked as outdated.

Sign in to view

@bkaradzic-microsoft bkaradzic-microsoft mentioned this pull request Aug 27, 2026
Draft
@bkaradzic-microsoft
bkaradzic-microsoft requested a balanced review from Copilot August 28, 2026 00:19
Copilot AI reviewed Aug 28, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 9 out of 10 changed files in this pull request and generated 4 comments.

Comment thread Polyfills/Canvas/Source/Canvas.cpp Outdated
Comment thread Polyfills/Canvas/Source/Context.cpp Outdated
Comment thread Polyfills/Canvas/Source/nanovg/fontstash.h
Comment thread Polyfills/Canvas/Source/Canvas.cpp Outdated
@bkaradzic-microsoft
bkaradzic-microsoft requested review from CedricGuillemet and bghgary and a balanced review from Copilot August 28, 2026 03:01
Copilot AI reviewed Aug 28, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 16 out of 18 changed files in this pull request and generated 4 comments.

Comment thread Polyfills/Canvas/Source/Context.cpp
Comment thread Polyfills/Canvas/Source/nanovg/fontstash.h
Comment thread Apps/UnitTests/JavaScript/src/tests.javaScript.all.ts
Comment thread Polyfills/Canvas/Source/MeasureText.cpp Outdated
@bkaradzic-microsoft

bkaradzic-microsoft commented Sep 1, 2026

Copy link
Copy Markdown
Member Author

CI was red across the board after merging master (#1857): Context.cpp still called the old 14-arg encoder->blit. Updated CaptureRGBA to the TextureRegion pair API (same pattern as NativeEngine/FrameBuffer).

Pushed in the latest commit.

bkaradzic-microsoft and others added 8 commits September 2, 2026 07:39
@bkaradzic-microsoft
Canvas: text metrics, multi-GUI filters, drawImage(canvas), toDataURL
e3f0bbf 
Independent of instance-branding (BabylonJS#1844 / type tags):

- fontstash: CSS/canvas font-size as em units; MeasureText advance + ink bounds
- MeasureText binds the same face FillText uses
- nanovg_filterstack: refcount shared blur programs (multi-ADT lifetime)
- drawImage(canvas) via Canvas InstanceOf + CPU pixel mirror (no NativeInstanceRegistry)
- toDataURL("image/png") via bimg_encode; link bimg_encode

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 60c2ec68-6de1-445d-9fc9-b699db737eae
@bkaradzic-microsoft
Canvas: fix GCC filterstack nested Uniforms default-init
9f6d585 
GCC rejects default member initializers on a nested type used as an
inline static member of the enclosing class. Match master: plain
UniformHandle fields + value-init the static Uniforms instance.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 60c2ec68-6de1-445d-9fc9-b699db737eae
@bkaradzic-microsoft
Canvas: GPU framebuffer readback for toDataURL and drawImage(canvas)
ef7bebf 
toDataURL and drawImage(canvas) previously read only the CPU pixel mirror
(populated by putImageData/drawImage of bitmaps), so NanoVG draws such as
fillRect/text/paths were missing from the result.

CaptureRGBA now flushes pending NanoVG commands, blits the canvas RT to a
READ_BACK texture, and blocks on DeviceContext::ReadTextureAsync. Mid-frame
flushes complete readTexture requests so the wait works under an active
FrameCompletionScope (RAF / unit-test open frame) without deadlocking.

Also:
- ForceMidFrameFlush API; process readTexture queue in PerformMidFrameViewFlush
- Bilateral Canvas/Context detach on destroy (JS cycle finalizer order)
- nvgCreateImageFromHandle helper for external bgfx textures (NODELETE)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 60c2ec68-6de1-445d-9fc9-b699db737eae
@bkaradzic-microsoft
Canvas: regenerate native-canvas.png golden for em-scale text fix
4dc8ed8 
The fontstash em-square scale change makes canvas2d text match CSS px sizing
(glyphs were previously ~0.86x too small for Droid Sans). Native Canvas
playground (#TKVFSA#8) draws large stroked/filled text over shapes, so the
old golden (captured at the undersized scale) fails pixel compare at ~4.1%.

Same golden update as shotgun 520be83; CI rendered result is within 0.125%
of this reference (limit 2.5%).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 60c2ec68-6de1-445d-9fc9-b699db737eae
@bkaradzic-microsoft
Canvas: address Copilot review on readback alpha and text metrics
ee9c4ab 
- CaptureRGBA: unpremultiply GPU readback so toDataURL/getImageData/drawImage
  expose straight alpha (NanoVG RT is ONE/INV_SRC_ALPHA premultiplied).
- MeasureText: use nvgTextBoundsInk so height/actualBoundingBox* are glyph ink
  extents; nvgTextBounds still uses the line box for layout callers.
- FreeType fontstash path: FT_Set_Pixel_Sizes from em `size` directly so raster
  matches em-scaled advances (stb path already did).
- Unit test: semi-transparent fill + getImageData channel asserts on
  drawImage(canvas) destination.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 60c2ec68-6de1-445d-9fc9-b699db737eae
@bkaradzic-microsoft
Canvas: update CaptureRGBA blit to TextureRegion API after bgfx bump.
4dd1ce5 
Master BabylonJS#1857 changed encoder.blit to TextureRegion pairs; Context.cpp still
used the old 14-arg form and broke CI after merging master.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d47cbab2-d751-4cf9-984f-4412dd9ec601
@bkaradzic-microsoft
Guard NanoVG bgfx destroys with isValid on context dispose.
40e5d75 
UBSan on MacOS_Sanitizers aborted during Canvas Context GC because
nvgRenderDelete used `if (gl->prog.idx)` — idx 0 is valid and
BGFX_INVALID_HANDLE is 65535, so never-created programs still called
destroyProgram and indexed ProgramRef[65535].

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d47cbab2-d751-4cf9-984f-4412dd9ec601
@bkaradzic-microsoft
Initialize NanoVG bgfx handles to BGFX_INVALID_HANDLE after zero-init.
a41eec2 
memSet left prog.idx at 0, which bgfx::isValid treats as a real handle. Dispose
then called destroyProgram(0) and asserted on Win32. Pair that with isValid for
lazy program create on first flush (replacing the old !prog.idx gate).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d47cbab2-d751-4cf9-984f-4412dd9ec601
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort, defaults to Balanced
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@CedricGuillemet CedricGuillemet Awaiting requested review from CedricGuillemet
@bghgary bghgary Awaiting requested review from bghgary

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@bkaradzic-microsoft