Closed Bug 1144534 (fuzzy-text) Opened 6 years ago Closed 6 years ago

fuzzy/bad looking text depending on scroll position, or fuzziness coming and going

Categories

(Core :: Layout, defect)

43 Branch
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla44
Tracking Status
firefox44 --- fixed
fennec Nightly+ ---

People

(Reporter: tnikkel, Assigned: mstange)

References

()

Details

(Whiteboard: layer-tiles)

Attachments

(2 files)

Load http://www.winnipegfreepress.com/arts-and-life/entertainment/TV/hockey-wives-stickhandle-lives-and-work-off-the-ice-296530101.html?device=mobile scroll. The text will alternately look fine, bad, and worse, and then fine again. What's weird is that other articles on the same site do not have this problem. I tested in Firefox Beta latest as of today.
There's a slew of bugs like these on file; see bugzil.la/whiteboard:layer-tiles

If you can consistently reproduce on nightly it would be good to investigate as we've had trouble finding reproducible scenarios and it's a high visibility issue.
See Also: → 1085076
Whiteboard: layer-tiles
Yes, consistently reproduces for me on nightly. mstange said he could reproduce too.
In that case please investigate, it would be good to at least figure out what's causing it even if the fix turns out to be nontrivial.
So it appears as though when we draw the text blurry we decide to render a transform that contains the test as active, when the transform is rendered as inactive the text looks fine. Dissecting the page on desktop the transform seems to be a translate only, and it doesn't seem to be changing at all. So not sure why it becomes active and then inactive and the active again. Also odd is that the layer dump for the active transform case has a prescale of 1,4 (scalex=1, scaley=4). I dont see any transform on the page at all with a scale, nevermind unequal x and y scales.
It looks like when choosing the scale in FrameLayerBuilder we are exceeding the max texture size so the code in RestrictScaleToMaxLayerSize shrinks our value to be under the max texture size and then scales us up by that factor.
We should be able to just ignore the max texture size if tiling is enabled, I think.
I think we have to make sure that the layer in question will be tiled too.
tracking-fennec: --- → +
tracking-fennec: + → 41+
Assignee: nobody → tnikkel
The root cause behind this bug (and all the dupes) is somewhat described in bug 1159564 comment 11. Copying it here:

This has something to do with wrapping preserve-3d transforms, I think. When I traced through the code, after starting the video the childrenVisible rect at [1] becomes ginormous, causing the aVisibleRect passed to ChooseScaleAndSetTransform to also become ginormous. This exceeds the max layer size and then causes the pre-scale to be applied.

Tracing backwards from there, the big childrenVisible rect was coming from an nsDisplayTransform which was coming from [2]. It looks like to me that |aTemp| there collects a lot of items before it finally gets flushed, and so the nsDisplayTransform that holds all of those items has a super-tall childrenVisible rect. I stuck in the attached wonderful hack which seemed to fix the problem, confirming my diagnosis. I don't know enough about this code to know if this is the root-cause or not, though. From a cursory reading of this code it doesn't like it would be a problem to split one nsDisplayTransform into a bunch but I don't really know. Somebody who knows the preserve-3d code should look into this.

[1] http://mxr.mozilla.org/mozilla-central/source/layout/base/FrameLayerBuilder.cpp?rev=9da538675a2d#4755
[2] http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsFrame.cpp?rev=9da538675a2d#1799
Alias: fuzzy-text
Summary: bad looking text depending on scroll → fuzzy/bad looking text depending on scroll position, or fuzziness coming and going
Attached patch Ugly ugly hackSplinter Review
Here's the patch that I posted on bug 1159564 (not landable at all). Matt, do you have any suggestions on how to fix this, since you probably know more about this preserve-3d stuff?
Flags: needinfo?(matt.woodrow)
I don't think tn is actively working on this either, so unassigning.
Assignee: tnikkel → nobody
Bug 1097464 is going to remove WrapPreserve3DList entirely, so that will hopefully fix this issue (or move it to a different location).
Depends on: 1097464
Flags: needinfo?(matt.woodrow)
tracking-fennec: 41+ → Nightly+
This is showing up on B2G (bug 1203971) as well as desktop (bug 1203109) - both of those are on medium.com articles. Moving this to layout and making it cross-platform.
Component: Graphics, Panning and Zooming → Layout
OS: Mac OS X → All
Product: Firefox for Android → Core
Hardware: x86 → All
Version: unspecified → 43 Branch
Duplicate of this bug: 1195526
Bug 1144534 - If we have tiling, don't reduce layer resolution for large transforms. r?mattwoodrow

With bug 1207590 we can be sure that large PaintedLayers will be tiled whenever
tiling is available. Large intermediate surfaces might still be a problem, but
those should be required much less often once we have bug 1021845.
Attachment #8664862 - Flags: review?(matt.woodrow)
Comment on attachment 8664862 [details]
MozReview Request: Bug 1144534 - If we have tiling, don't reduce layer resolution for large transforms. r?mattwoodrow

https://reviewboard.mozilla.org/r/20043/#review18063
Attachment #8664862 - Flags: review?(matt.woodrow) → review+
https://hg.mozilla.org/mozilla-central/rev/99f8738f9169
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
Depends on: 1219493
No longer depends on: 1219493
Duplicate of this bug: 1224599
See Also: → 1621007
You need to log in before you can comment on or make changes to this bug.