1640523 - Text is Unreadable in Any Webpage with an extremely large Drop Shadow

Status: VERIFIED FIXED

(Core :: Graphics: WebRender, defect, P3)

Description

[u658110]

Attachment: Quality Browsing.html

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:76.0) Gecko/20100101 Firefox/76.0

Steps to reproduce:

See attached HTML file.
1. The page is relatively long.
2. The style contains a drop shadow.
3. Attempt to read the text half way down the page.
FF Version: 76.0.1, Windows, 64-bit

Actual results:

The text is distorted beyond recognition. The effect occurs strongest at the vertical center of the page. The effect is persistent.

Expected results:

The text should be legible.

Comment 1

[Timothy Nikkel (:tnikkel)]

Testcase starts crashing with this regression range

https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=07b5def477f77911c23ca458daf5aa7d649c0452&tochange=e3f5c752e57fb82c03a9e8fb288907599b6d9489

-> bug 1617050 maybe?

The crash gets fixed and the rendering becomes blurry with this range

https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=5390cddc67103d922061a5257c0d5c215af03660&tochange=af66b6bd4aa714a47d1e44f3ac5baddfe9e7344f

-> bug 1626827

Comment 2

[Dzmitry Malyshau [:kvark]]

The crash fix could make it blurry only if the WR local-space picture scaling doesn't work correctly. Routing to Bert to check this.

Comment 3

[Bert Peers [:bpeers]]

My understanding of the problem:

• the text surface does not establish a raster root;
• the text surface is 1494 x 65555; (surface_rect in post_update);
• in take_context, we ask for a 1494 x 30264 when scrolling halfway;
• since it's not a raster root, this will crash trying to allocate a giant rendertarget;
• the crash was fixed by always scaling within limits, even when not a raster root;
• thus we scale to 25% to fit 30264 < 8192, text is effectively 374 pixels wide and upsampled;

So the code is doing exactly what we need it to, to not crash. It just doesn't look great.
The only fix I see is to avoid asking for a 30264 pixels-high canvas when the screen is only 2K pixels tall, but that's not related to the scaling/crash changes per se.
And even so, you can switch from blur to perspective and get similar problems where there's no clear fix like that (Bug 1634162).

In short I don't think it's a code bug or math oopsie, but a deeper question of how we switched from screen based to surface/RT based rendering? Am I wrong? Thoughts? Thanks :)

Comment 4

[Bert Peers [:bpeers]]

PS: that said, it's not normal for the text to get bigger as well - it should be the same size, just blurrier. I'll take a look at that.

Edit: this is perhaps also due to the text not being a raster root, so it has no raster_config, which is how we pass the scale factor to update_font_instance. The crash fix might need an alternative way to route that scale. However cbrewster is also changing how this works with more scaling factors folded directly into the surface.DPI, maybe that could be an alternative?

Comment 5

[James Teh [:Jamie]]

Not really relevant for a11y triage, since this impacts all sighted users with equal severity; it doesn't disproportionately affect low vision users, for example.

Comment 6

[Jessie [:jbonisteel] pls NI]

Test case is attached to this bug

Comment 7

[Dzmitry Malyshau [:kvark]]

I suppose we can also consider switching the local raster root ON for the case where scaling is enforced (and the picture is otherwise not in the local raster root). Glenn, please have a look and tell what you think.

Comment 8

[Glenn Watson [:gw]]

It's not totally clear to me why we need to scale this in the case where we don't create a raster root (as the clip-to-screen functionality in this case should result in us not creating a huge target). But I haven't looked in any detail, I'm probably just missing some context - I'll take a look at this today to understand better what's going on.

Comment 9

[Glenn Watson [:gw]]

The problem appears to be in https://searchfox.org/mozilla-central/source/gfx/wr/webrender/src/picture.rs#4826 (which was introduced in https://phabricator.services.mozilla.com/D68230).

In this case, the clip_inflation value is calculated to be ~21,000 is incorrect. This makes WR think the size of the target we need to allocate is much bigger than what we actually need.

There's two potential issues here:

• The code to calculate clip_inflation appears to have a bug in it.
• Looking more closely at the code, it's not clear to me why clip_inflation is even needed (although it made sense at the time). It seems like clipped shouldn't need to include any inflation as this is handled later in the specific blur / drop-shadow filter match branches.

Comment 10

[Glenn Watson [:gw]]

Attachment: Bug 1640523 - Fix render quality on pages with very large drop shadows.

The logic for expanding the drop shadow clip rect was calling the
map_vector method in the ScaleOffset struct.

This method was incorrect (it was mapping the value as a point,
including the offset). This is fixed by modifying the map_vector
method to exclude the offset.

However, the text run snapping code was also calling map_vector
but expecting it to treat the value there as a point, so also
introduce a map_point method and switch the text run code
to use that.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f77b09a15a11
Fix render quality on pages with very large drop shadows. r=nical,aosmond

Comment 12

[Razvan Maries]

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

Comment 13

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:gw, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Comment 14

[Glenn Watson [:gw]]

Comment on attachment 9152602 [details]
Bug 1640523 - Fix render quality on pages with very large drop shadows.

Beta/Release Uplift Approval Request

• User impact if declined: Major text quality issues in this specific case (I suspect is very uncommon on real content, but it's hard to know for sure).
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: As per attached test case in bugzilla.
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Small patch, easy to back out if needed and only affects a small part of WR.
• String changes made/needed:

Comment 15

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9152602 [details]
Bug 1640523 - Fix render quality on pages with very large drop shadows.

Fixes a pretty bad text rendering regression. Approved for 78.0b3.

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/92fde967002c

Comment 17

[Cristian Baica [:cbaica], Release Desktop QA]

I have managed to reproduce the issue using Fx78.0a1 buildID: 20200524212021.
The issue is verified fixed using Fx79.0a1 and Fx78.0b3 on windows 10, ubuntu 18.04 and macOS 10.13. Now the entire text in the document is readable without issues.