1824531 - Consider removing GetSnappedBaseline[X,Y] from the text layout code

Status: RESOLVED FIXED

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

Description

[Jonathan Kew [:jfkthame]]

It's not clear that in a world of pinch-zoom and APZ, attempting to snap text baselines to device pixels in nsTextFrame::PaintText is really beneficial, and the "snapping" done here may interact poorly with rounding that happens at other levels of the rendering stack (see bug 1801098).

I suggest that we try removing the snapping calls here, and see if this results in any significant degradation of rendering. Tryserver shows a mix of a few newly-passing tests and a few newly-fuzzy ones, but nothing that looks overly concerning. However, it's possible that the result will also be affected by users' local settings regarding font rasterization/hinting, so we should watch out for reports of any changes.

Comment 1

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1824531 - Remove baseline-snapping from nsTextFrame::PaintText, as it may interact poorly with APZ. r=#layout-reviewers

In my local testing, this seems to be harmless, but we should be alert for reports of any erratic
or poorly-rasterized text that results.

Comment 2

[Jonathan Kew [:jfkthame]]

cc'ing :hiro - fyi. If we decide it's OK to do this, hopefully it'll help with the issues you've had in bug 1801098.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5f36e53be4cf
Remove baseline-snapping from nsTextFrame::PaintText, as it may interact poorly with APZ. r=emilio

Comment 4

[Cristian Tuns]

Backed out for causing reftest failures in colrv1-01.html

• Backout link
• Push with failures
• Failure Log
• Failure line: REFTEST TEST-UNEXPECTED-PASS | layout/reftests/font-face/colrv1-01.html#K == layout/reftests/font-face/colrv1-01-ref.html#K | image comparison, max difference: 255, number of differing pixels: 8752

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fcc38c48c35a
Remove baseline-snapping from nsTextFrame::PaintText, as it may interact poorly with APZ. r=emilio

Comment 6

[Serban Stanca [:SerbanS]]

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

Comment 7

[Andrej (:aglavic)]

(In reply to Cristian Tuns from comment #4)

Backed out for causing reftest failures in colrv1-01.html

• Backout link
• Push with failures
• Failure Log
• Failure line: REFTEST TEST-UNEXPECTED-PASS | layout/reftests/font-face/colrv1-01.html#K == layout/reftests/font-face/colrv1-01-ref.html#K | image comparison, max difference: 255, number of differing pixels: 8752

== Change summary for alert #38001 (as of Thu, 06 Apr 2023 16:37:27 GMT) ==

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)
62%	tscrollx	macosx1015-64-shippable-qr	e10s fission stylo webrender-sw	1.82 -> 0.70
43%	tp5o_scroll	macosx1015-64-shippable-qr	e10s fission stylo webrender-sw	2.65 -> 1.52
31%	tp5o_scroll	linux1804-64-shippable-qr	e10s fission stylo webrender-sw	3.03 -> 2.09
30%	tp5o_scroll	windows10-64-shippable-qr	e10s fission stylo webrender-sw	2.48 -> 1.73
25%	tscrollx	windows10-64-shippable-qr	e10s fission stylo webrender-sw	1.20 -> 0.90
...	...	...	...	...
4%	tscrollx	windows10-64-shippable-qr	e10s fission stylo webrender	1.00 -> 0.96

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=38001