1759686 - Enable Canvas2d text methods to work without a presShell or presContext

Status: RESOLVED FIXED

(Core :: Layout: Text and Fonts, enhancement)

Description

[Jonathan Kew [:jfkthame]]

The font/text code in CanvasRenderingContext2d (SetFontInternal, DrawOrMeasureText) assumes availability of a presShell and presContext; for offscreen-canvas/worker-thread use, we need to allow it to work independently of these.

Comment 1

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1759686 - patch 1 - Remove unused mPresContext member from CanvasRenderingContext2D.

Nothing uses this member, so remove it to make it clear we don't need to provide it
when setting up a CanvasRenderingContext2D.

Comment 2

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1759686 - patch 2 - Remove redundant null-checks in CanvasRenderingContext2D, as GetPresShell() will check these members.

We don't need to null-check the mCanvasElement and mDocShell members in these methods,
and bail out if neither is provided, because in that case the following GetPresShell() call
will return null and we'll bail out there anyhow.

Removing these extra checks will simplify the changes involved in enabling the canvas code
to be used in a Worker context, where it is not connected to a document.

Depends on D144184

Comment 3

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1759686 - patch 3 - Ensure prefs that may be accessed by canvas2d text code are atomic.

These need to be atomic so that they can be accessed by canvas code when running on a worker thread
without calling main-thread-only Preferences APIs.

Depends on D144185

Comment 4

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1759686 - patch 4 - Extend ServoCSSParser::ParseFontShorthandForMatching to (optionally) return font size in addition to family/style.

When running in a Worker, the canvas2d SetFontInternal method can't call GetFontStyleForServo
because it doesn't have a canvasElement or presShell to pass. But we can use the method
ServoCSSParser::ParseFontShorthandForMatching to parse the canvas 'font' property, if we
extend it so as to also return the size from the font shorthand.

Depends on D144186

Comment 5

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1759686 - patch 5 - Adapt canvas2d text methods to allow use from worker threads without a Document or PresShell. r=aosmond

This enables an offscreen-canvas Worker to do text rendering, but only with system-installed fonts
because we don't yet have a FontFaceSet available in the Worker. (That's bug 1758946.)

Depends on D144187

Comment 6

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1759686 - Extend part 4 to resolve keyword sizes correctly, and reuse existing code to turning lengths into absolute pixels. r=jfkthame

I think it doesn't make sense to support em/percentages without
supporting smaller/bigger/other font-relative things.

I have many questions about how other units like viewport-units etc are
supposed to behave. Blink seems to resolve viewport units to zero:

https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/css/resolver/font_style_resolver.cc;l=15-27;drc=5e62802ab420dc1743a1824ec4b537a730b2b24b

Which might be reasonable, but I think keeping a simple model consistent
with what other code like DOMMatrix does seems better for now.

Comment 7

[Jonathan Kew [:jfkthame]]

Queued the preliminary patches for landing, to avoid bit-rot and get them out of my local tree. Marking this as leave-open, pending review/landing of part 5.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d411980f1328
patch 1 - Remove unused mPresContext member from CanvasRenderingContext2D. r=lsalzman
https://hg.mozilla.org/integration/autoland/rev/bdc36ea77f24
patch 2 - Remove redundant null-checks in CanvasRenderingContext2D, as GetPresShell() will check these members. r=lsalzman
https://hg.mozilla.org/integration/autoland/rev/cde3c45e16a5
patch 3 - Ensure prefs that may be accessed by canvas2d text code are atomic. r=lsalzman
https://hg.mozilla.org/integration/autoland/rev/8a2c9882e141
patch 4 - Extend ServoCSSParser::ParseFontShorthandForMatching to (optionally) return font size in addition to family/style. r=emilio
https://hg.mozilla.org/integration/autoland/rev/0b6f31f3ce0b
Extend part 4 to resolve keyword sizes correctly, and reuse existing code to turning lengths into absolute pixels. r=jfkthame

Comment 9

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/d411980f1328
https://hg.mozilla.org/mozilla-central/rev/bdc36ea77f24
https://hg.mozilla.org/mozilla-central/rev/cde3c45e16a5
https://hg.mozilla.org/mozilla-central/rev/8a2c9882e141
https://hg.mozilla.org/mozilla-central/rev/0b6f31f3ce0b

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c07302e77455
patch 5 - Adapt canvas2d text methods to allow use from worker threads without a Document or PresShell. r=aosmond

Comment 11

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/c07302e77455