Closed Bug 1919906 Opened 1 year ago Closed 1 year ago

Text rendering looks off on maps.apple.com on Windows

Tracking

()

Status:

RESOLVED FIXED

Milestone:

133 Branch

Tracking Flags:

Tracking

Status

firefox-esr115

---

wontfix

firefox-esr128

---

wontfix

firefox130

---

wontfix

firefox131

---

wontfix

firefox132

---

wontfix

firefox133

---

wontfix

firefox134

---

wontfix

firefox135

---

fixed

People

(Reporter: bgrins, Assigned: jfkthame)

References

(Blocks 1 open bug, Regression)

Details

(Keywords: regression, webcompat:platform-bug, webcompat:site-report)

Attachments

(5 files)

firefox.jpeg 1 year ago Brian Grinstead [:bgrins] 4.73 MB, image/jpeg		Details
edge.jpeg 1 year ago Brian Grinstead [:bgrins] 2.72 MB, image/jpeg		Details
comparison of HTML (top) vs Canvas2d (below) rendering 1 year ago Jonathan Kew [:jfkthame] 6.22 KB, image/png		Details
Bug 1919906 - Ensure fonts used in canvas2d do not apply forced-GDI-classic mode during layout/measurement. r=jrmuizel,lsalzman 1 year ago Jonathan Kew [:jfkthame] 48 bytes, text/x-phabricator-request		Details \| Review
Bug 1919906 - Use HTML text measurement in browser_less_common_selection_manipulations.js, as canvas text metrics may not precisely match HTML metrics. r=mak 1 year ago Jonathan Kew [:jfkthame] 48 bytes, text/x-phabricator-request		Details \| Review

Brian Grinstead [:bgrins]

Reporter

Description

•

1 year ago

Attached image firefox.jpeg — Details

STR:

Load https://beta.maps.apple.com/?address=600+Montgomery+St%2C+San+Francisco%2C+CA+94111%2C+United+States&auid=13461352009204515340&ll=37.795167%2C-122.4027996&lsp=9902&q=Transamerica+Pyramid on Firefox in Windows
Look at the "Transamerica" text

Brian Grinstead [:bgrins]

Reporter

Comment 1

•

1 year ago

Attached image edge.jpeg — Details

Same text on Edge

Lee Salzman [:lsalzman]

Comment 2

•

1 year ago

Jonathan, any ideas what's wrong with the glyph layout here? Apple seems interested in fixing this.

Flags: needinfo?(jfkthame)

Jonathan Kew [:jfkthame]

Assignee

Comment 3

•

1 year ago

I can't seem to inspect that text to see how it's being styled, but my guess is this might be a bad interaction between letter-spacing, scaling, and pixel-snapping of glyph positions.

Flags: needinfo?(jfkthame)

Alexandre Poirot [:ochameau]

Comment 4

•

1 year ago

The text is rendered on canvas.rt-root (you can use this search query in the inspector to find the element), which has the following rule:

.mk-map-view .rt-root {
  letter-spacing: initial;
}

But removing, or changing this live doesn't seem to fix the issue, but I'm not sure the text get re-rendered without reloading the page.

Also note that the rendering issue doesn't reproduce on Linux.

Alexandre Poirot [:ochameau]

Comment 5

•

1 year ago

Based on native method call tracing, I suspect, but I'm not 100% sure that the text is rendered around here:
https://share.firefox.dev/3TDKKL4
I don't know Canvas API enough to spot if something here would possibly render some text in the canvas.

Alexandre Poirot [:ochameau]

Comment 6

•

1 year ago

Using the debugger, I think I found where we render the text.

https://beta.maps.apple.com/static/maps-app-web-client/mapkitjs/libs/mapkit.core.1b57c0.js?mkjsVersion=5.78.83
line 1227 (pretty printed)

e is the canvas rendering context

          draw: function (e) {
            var t = this._node,
            i = this.params[t.kind];
            this._setTextProperties(e);
            var o = t.parent.annotation.darkColorScheme ? 'dark' : 'light';
            e.fillStyle = i.color[o],
            e.strokeStyle = this.params.haloColor[o],
            e.lineWidth = this.params.haloSize;
            var a = t.size.width / 2,
            n = t.size.height - i.yPadding;
            e.strokeText(t.text, a, n),
            e.fillText(t.text, a, n)
          }

t.text = "Transamerica"
a = 38.5
n = 15
e.lineWidth = 2
e.font = 600 11px "SFProRegular", "-apple-system", "BlinkMacSystemFont", "Helvetica Neue", Helvetica, Arial, sans-serif
e.textAlign = center
e.textBaseline = bottom
e.miterLimit = 2

Also interesting, in the same source.
Line 1543 (pretty printed)

e is the canvas context,
t is "Transamerica pyramid"
i is "600 11px "SFProRegular", "-apple-system", "BlinkMacSystemFont", "Helvetica Neue", Helvetica, Arial, sans-serif"

              return function (e, t, i) {
                e.font = i;
                var o = t.trim().split(/[\f\n\r\t\v\u0020\u2000-\u200b\u205f\u3000]+/),
                a = [];
                for (; o.length > 0; ) {
                  for (var n = [
                    o.shift()
                  ], s = n.join(' ').length; o.length > 0 && s <= 16; ) n.push(o.shift()),
                  s = n.join(' ').length;
                  s > 16 &&
                  n.length > 1 &&
                  o.unshift(n.pop());
                  const t = n.join(' '),
                  i = e.measureText(t),
                  l = i.width + u.labelOutlineWidth;
                  a.push([t,
                  l,
                  i])
                }
                return a

Same source, line 1135

e is "Pyramid"
t is "Title"
i is 47
a is a TextMetrics object
l is false

      function l(e, t, i, a, l) {
        o.BaseNode.call(this),
        this.text = e,
        this.kind = t,
        this._renderer = new n(this),
        this.textAscent = a.ascent ||
        0,
        this.textDescent = a.descent ||
        0,
        this.size = new s(i, this._renderer.params[this.kind].height),
        this.keyForFrozenLayer = (l ? 'dark' : 'light') + '-' + t + '#' + this.text
      }

Jonathan Kew [:jfkthame]

Assignee

Comment 7

•

1 year ago

Yeah, the "Transamerica Pyramid" label is drawn using canvas2d.fillText (unlike the other labels around the map), and that doesn't handle spacing well in Arial, which is the font we end up using on Windows.

The result looks a lot better if you remove Arial from the gfx.font_rendering.cleartype_params.force_gdi_classic_for_families pref. (Requires browser restart to see the change, I believe.)

There's a long history behind that setting, and it may be that it's time we got rid of it altogether, but any change to Windows font rendering tends to be controversial. :(

Jonathan Kew [:jfkthame]

Assignee

Comment 8

•

1 year ago

•

Edited

Note that if I render the same text "Transamerica Pyramid" using font: 600 11px Arial in HTML, the spacing is much better. The bad result here is specific to canvas2d text, which apparently doesn't handle the attempt to set the cleartype mode very well.

Jonathan Kew [:jfkthame]

Assignee

Comment 9

•

1 year ago

Attached image comparison of HTML (top) vs Canvas2d (below) rendering — Details

Jonathan Kew [:jfkthame]

Assignee

Comment 10

•

1 year ago

In the HTML-rendered text, we can see the effect of "force GDI classic" mode, e.g. making the "m" glyph wider. It looks like the canvas2d-rendered text has laid out the glyphs according to the GDI-classic metrics, but the actual rendering is not using this mode and so we get a narrower "m" and bad spacing. (Other glyphs are also affected, but the "m" is perhaps the clearest.)

Jonathan Kew [:jfkthame]

Assignee

Comment 11

•

1 year ago

We don't render the glyphs using "GDI classic" mode in canvas because we explicitly set params.allowGDI to false here. This dates back to bug 1730772.

Removing that line results in much better spacing of the "Transamerica Pyramid" text, but presumably it may regress things that bug 1730772 was aiming to fix.

Alexandre Poirot [:ochameau]

Comment 12

•

1 year ago

A quick side note to explain how I found this JS code.
I hacked something to break on any function call whose any of its arguments was equal to Transamerica pyramid.
It paused on many function calls related to the left sidebar, but ultimately paused on the right function calling the canvas API.