2070683 - Parts of mathematical expressions inside popups on Wikipedia are blurry (particularly on a non-HiDPI display)

Status: RESOLVED FIXED

(Core :: SVG, defect, P3)

Description

[Viktor Jägersküpper]

OS: Arch Linux

Steps to reproduce with latest Nightly:

• Go to https://en.wikipedia.org/wiki/Basel_problem#Euler's_approach
• Hover the mouse pointer over the link to reference [5] so that a popup is displayed
  -> Some parts of the mathematical expressions inside the popup are blurry. By scrolling to the right/left in small steps inside the popup, blurry parts become sharp and formerly sharp parts become blurry.

Running mozregression gives:
Last good revision: 2a21129467270b93cf6d9584dfece592776bfe52
First bad revision: 35e727d8fda22f3ccfcb1e620484f041002a63c0
Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=2a21129467270b93cf6d9584dfece592776bfe52&tochange=35e727d8fda22f3ccfcb1e620484f041002a63c0

This points to bug 2048146 which enabled layout.disable-pixel-alignment. As in bug 2063386 I manually arrived at the following regression windows by enabling layout.scroll.disable-pixel-alignment:
Last good revision: 8e959a7ded5f111e711c06a6728f76d3bd660699 (Nightly build: 20231203092644)
First bad revision: 369922cc5c4efad6c87b13600325eb5a989cde81 (Nightly build: 20231204091942)
Pushlog:
https://hg-edge.mozilla.org/mozilla-central/pushloghtml?fromchange=8e959a7ded5f111e711c06a6728f76d3bd660699&tochange=369922cc5c4efad6c87b13600325eb5a989cde81

So this issue has likely been there since layout.scroll.disable-pixel-alignment was added in bug 1867425.

Comment 1

[Hiroyuki Ikezoe (:hiro)]

CCing Glenn, just in case. It's totally unclear to me whether the underlying issue is in SVG layout or in WebRender.

Comment 2

[Daniel Holbert [:dholbert]]

-> Some parts of the mathematical expressions inside the popup are blurry. By scrolling to the right/left in small steps inside the popup, blurry parts become sharp and formerly sharp parts become blurry.

Could you capture & attach screenshots showing the different renderings here to demonstrate the issue? When I try to reproduce locally (in Nightly on Ubuntu, with layout.disable-pixel-alignment default-enabled), I don't think I'm seeing any blurriness.

Comment 3

[Viktor Jägersküpper]

Attachment: screenshot_01.png

Compare the two sin(x) renderings

Comment 4

[Viktor Jägersküpper]

Attachment: screenshot_02.png

Compare the pi renderings here with those in screenshot 01

Comment 5

[Viktor Jägersküpper]

My display is pretty standard: 1920x1080 resolution, no scaling (High DPI), no HDR

Comment 6

[Daniel Holbert [:dholbert]]

Attachment: 10x-zoomed screenshot comparing 'pi' renderings from previous two screenshots

Thanks. Here's a screenshot (zoomed in 10x, scaling up the pixels) to compare those renderings.

Both of the pi renderings look pretty fuzzy to me (particularly when zoomed up like this), but the first one definitely has a nicer dark stroke along the center of the lines.

I suspect this is just a case where fractional scroll positions are triggering a different fractional pixel-offset for this particular SVG shape, which results in different antialiasing behavior when we draw it.

For actual text, this theoretically shouldn't happen, because the font typically has special "hinting" behavior to try to snap solid lines to pixel boundaries, to avoid this fuzziness. But this isn't actual text; this is an SVG <path>, which just-so-happens to be the shape of a pi glyph. (The SVG image in question is https://wikimedia.org/api/rest_v1/media/math/render/svg/4b293c53c9eb2d3e0b86b00c51c8d0cbc48a48bb )

My display is pretty standard: 1920x1080 resolution, no scaling (High DPI), no HDR

The "no scaling" is likely an important requirement to see the bad outcome here. For users who do have High DPI, they'll be more likely to get a crisper-looking shape here, regardless of the fractional-css-pixel position.

Comment 7

[Daniel Holbert [:dholbert]]

But given that this is an image, I wonder why/how the antialiasing behavior is changing (with the scroll position) here... I thought we snap images to display pixels when painting them, so I wouldn't think that going between fractional scroll positions would be able to change the antialiasing behavior of images...

Unless the fractional scroll position is resulting in a different pixel-snapped size, somehow? And that's making the SVG viewport ever-so-slightly wider/skinnier (sort of similar to what was happening in bug 2063903). If so, that could impact the way the image is painted.

Comment 8

[BugBot [:suhaib / :marco]]

Set release status flags based on info from the regressing bug 2048146

Comment 9

[Glenn Watson [:gw]]

This is a rather unfortunate hack / workaround that fixes the reported bug. It's a reasonable change for now, though I would like to revisit the whole reference frame snapping setup we now have, and make a more principled fix when I have time. Try run is green.

Comment 10

[Glenn Watson [:gw]]

Attachment: Bug 2070683 - Snap reference-frame origins, and pick image raster sizes at the snapped phase

An image's raster size comes from rounding its destination rect, so it has to
be rounded at the sub-pixel phase that rect is snapped at. Two things shift
that phase, and either alone makes WebRender resample the whole image to close
a one-pixel gap.

ShouldSnapToGrid leaves reference-frame origins for WebRender to round under
layout.disable-pixel-alignment, but WebRender rounds one only when the frame
asks to be snapped, which only zoom, fixed and scrollbar layers did. Ask for it
on the translate-only reference frame, and add snap_origin for the frames that
carry a transform as well: there, rounding the offset the frame composes to
would round the transform's own translation with it, so the origin has to be
rounded before the transform is applied. That is what Gecko itself assumes -
GetTransform still rounds the origin, so hit testing and the non-WebRender
paint path already place content there.

The display list is also normalized by the external scroll offset in whole app
units while WebRender re-applies it in whole device pixels, so apply that
offset before choosing the size. Read it back from the builder rather than
re-summing it, so the two cannot drift apart.

One reftest per phase shift; each fails without its own half. The transform
half is covered by gfx/tests/reftest/1501195.html, which asserts that adding
an identity translate3d changes nothing: it snapped the origin-only reference
frame Gecko creates for the stripped identity transform, but not the same
origin arriving on a transformed frame.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/88ae0340c4b1
https://hg.mozilla.org/integration/autoland/rev/973c9268a4c9
Snap reference-frame origins, and pick image raster sizes at the snapped phase r=gfx-reviewers,nical

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/6433953dde21
https://hg.mozilla.org/integration/autoland/rev/c0973a7f8e25
Revert "Bug 2070683 - Snap reference-frame origins, and pick image raster sizes at the snapped phase r=gfx-reviewers,nical" for causing wr failures at backface-visibility-hidden-animated-002.html

Comment 13

[rperta]

Backed out for causing wr failures at backface-visibility-hidden-animated-002.html
Backout link
Push with failures
Failure log(s) wr
Failure log(s) R

Comment 14

[Glenn Watson [:gw]]

Attachment: Bug 2070683 - Draw an image texture rasterized for its rect 1:1 when the snapped rect is a device pixel off, instead of stretching it

WebRender snaps a primitive rect to the device grid edge by edge, so a rect
with a fractional device extent is a pixel wider or taller depending on where
it sits. The size an image is rasterized at is chosen by the producer from the
same rect, rounded through a transform that does not see where WebRender
places it: under layout.disable-pixel-alignment the external scroll offset and
every reference-frame origin above the item shift that phase, and for an
animated frame the producer cannot know it at all. When the two roundings
disagree the texture is stretched by a pixel to fit, which resamples the whole
image and blurs it.

Neither side can round its way out of this: snapping the extent instead of the
edges keeps the size but opens seams against neighbours that snap by edges,
and predicting the phase in the producer fails for animated frames. So keep
the geometry as it is, and stop the size disagreement from driving a resample.
When a texture is within one device pixel of the snapped rect on both axes,
for an image drawn once in a space where device pixels can be counted, situate
it 1:1 from the snapped origin: a surplus row or column is clipped by the
bounds, and a shortfall samples past the texture's edge, where the sampler's
clamp repeats the edge texel. Nothing moves, so nothing seams.

The wrench reftest pins both cases against a reference built from the same
texture at 1:1, plus the repeated column and the clipped column. The Gecko
reftests put an image at a fractional reference-frame origin and under a
fractional scroll offset, where Gecko rasterizes it a row taller than the rect
it snaps to, and compare against that raster on the grid, clipped to the
snapped height.

Comment 15

[Glenn Watson [:gw]]

Abandoned the previous patch and pushed a better patch to work around this. Still want a more principled fix long term.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/c60a50ce57f4
https://hg.mozilla.org/integration/autoland/rev/a2cb23057511
Draw an image texture rasterized for its rect 1:1 when the snapped rect is a device pixel off, instead of stretching it r=gfx-reviewers,nical

Comment 17

[Sandor Molnar[:smolnar]]

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

Comment 18

[BugBot [:suhaib / :marco]]

The patch landed in nightly and beta is affected.
:gw, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
  • See https://wiki.mozilla.org/Release_Management/Requesting_an_Uplift for documentation on how to request an uplift.
• If no, please set status-firefox157 to wontfix.

For more information, please visit BugBot documentation.