Closed
Bug 1490789
Opened 6 years ago
Closed 6 years ago
Scrolling Twitter in Nightly on Android shows many rendering problems
Categories
(Core :: Graphics, defect, P3)
Tracking
()
VERIFIED
FIXED
mozilla64
Tracking | Status | |
---|---|---|
firefox-esr60 | --- | unaffected |
firefox62 | --- | unaffected |
firefox63 | + | verified |
firefox64 | + | verified |
People
(Reporter: djc, Assigned: jnicol)
References
Details
(Keywords: regression, Whiteboard: [gfx-noted])
Attachments
(4 files)
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
text/x-phabricator-request
|
pascalc
:
approval-mozilla-beta+
|
Details |
See the attached screenshots from my Nexus 4 (Android 5). This regressed somewhere in the past few weeks.
Reporter | ||
Comment 1•6 years ago
|
||
Reporter | ||
Comment 2•6 years ago
|
||
Reporter | ||
Comment 3•6 years ago
|
||
Comment 4•6 years ago
|
||
Is this always reproducible or just intermittently? Are there any special steps to reproduce it besides scrolling on a twitter timeline? If possible do you have a good build date so we could bisect when the regression was introduced?
Flags: needinfo?(dirkjan)
Priority: -- → P3
Whiteboard: [gfx-noted]
Reporter | ||
Comment 5•6 years ago
|
||
It's not always reproducible. It seems it might be related to images or ads in tweets. Sorry, I don't have an exact build date, I would start with something like 8-12 weeks ago.
Flags: needinfo?(dirkjan)
Comment 6•6 years ago
|
||
Jamie, have you seen anything like this before? Or are you able to reproduce it?
Flags: needinfo?(jnicol)
Assignee | ||
Comment 7•6 years ago
|
||
Hmm, I can reproduce this very intermittently, going to be hard to get a regression range! I'll look in to it further.
Flags: needinfo?(jnicol)
Updated•6 years ago
|
Keywords: regression,
regressionwindow-wanted
![]() |
||
Updated•6 years ago
|
OS: Unspecified → Android
Updated•6 years ago
|
Assignee | ||
Comment 10•6 years ago
|
||
There's a few issues on twitter currently, so let's make this bug solely about content being rendered in the wrong place, and remaining there until another invalidation or scroll occurs. I've found the regression range: bug 1477693. This can be worked around by setting layout.display-list.flatten-transform=false. Note this is a separate issue than the sticky header bar flickering. Also, there appears to be yet another issue with content briefly flashing in the wrong place, which has been around for a long time, which I'll file a follow up for. After discussing this with Miko, I think we understand this and have a fix. In ComputeGeometryChangeForItem, we apply a shift to some rects and geometries to account for a change in scroll offset. With transform flattening, the shift is taken care of by the root transform in the item's TransformClipTree, so we no longer need to apply it manually. One of the invalidation code branches combines the old area and new area of the invalidated frame. Prior to bug 1477693, this combined area was transformed with the new transform, which takes account of the shift. Post bug 1477693, the old area is transformed by the old transform, which does not take in account the new shift. We must therefore apply the shift before combining it with the new area. Miko can correct me if the above is wrong. Patch on try currently.
Comment 11•6 years ago
|
||
Any progress here? Also, P3 seems low for twitter.com.
Flags: needinfo?(jnicol)
Updated•6 years ago
|
QA Contact: dbolter
Comment 12•6 years ago
|
||
We tried to reproduce this issue also but unfortunately, we don't have Nexus 4 and we tried with Nexus 5(Android 6.0.1) and Xiaomi Mi4i(Android 5.0.2) and we couldn't reproduce it. We will investigate more.
Updated•6 years ago
|
QA Contact: dbolter
Comment 14•6 years ago
|
||
This is how I reproduce it: 1. scroll down twitter with a momentum (with a "swipe down" gesture) 2. when it's white because of the overscroll, tap once so that the scroll stops 3. wait until the overscroll stops
Comment 15•6 years ago
|
||
(In reply to Jamie Nicol [:jnicol] from comment #10) > There's a few issues on twitter currently, so let's make this bug solely > about content being rendered in the wrong place, and remaining there until > another invalidation or scroll occurs. I've found the regression range: bug > 1477693. This can be worked around by setting > layout.display-list.flatten-transform=false. Miko, Jamie, would it be possible to set the above pref to false for 63? What would be the trade off?
Flags: needinfo?(mikokm)
Assignee | ||
Comment 16•6 years ago
|
||
Sorry for my inactivity on this bug. I have a patch which looks good on try, I'll get it landed today. The trade-off would be a performance regression, this patch is simple so it'll be better to land it instead.
Flags: needinfo?(jnicol)
Comment 17•6 years ago
|
||
(In reply to Jamie Nicol [:jnicol] from comment #16) > Sorry for my inactivity on this bug. I have a patch which looks good on try, > I'll get it landed today. > > The trade-off would be a performance regression, this patch is simple so > it'll be better to land it instead. Could your patch be ready to be uplifted this week before we enter RC week?
Flags: needinfo?(jnicol)
Assignee | ||
Comment 19•6 years ago
|
||
When calculating which regions of a layer to invalidate we usually apply a shift to the area to account for changes in scroll offset. For items within flattened transforms we do not do this, because the transform itself includes the scroll offset. However, when calculating the old area of an invalidated frame, we use the old transform. This includes the previous scroll offset rather than the current, so we must therefore still apply the shift. Not doing so was causing the incorrect region to be invalidated, and content to be rendered at the wrong location.
Updated•6 years ago
|
Flags: needinfo?(mikokm)
Comment 21•6 years ago
|
||
Pushed by jnicol@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/cf2d65b646d1 Apply shift when calculating old area of invalidated frame r=miko
Comment 22•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/cf2d65b646d1
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
Updated•6 years ago
|
Assignee: nobody → jnicol
Assignee | ||
Comment 24•6 years ago
|
||
Comment on attachment 9015509 [details] Bug 1490789 - Apply shift when calculating old area of invalidated frame [Beta/Release Uplift Approval Request] Feature/Bug causing the regression: Bug 1477693 User impact if declined: Content drawn in wrong position on some websites, including twitter Is this code covered by automated tests?: Yes Has the fix been verified in Nightly?: Yes Needs manual test from QE?: Yes If yes, steps to reproduce: List of other uplifts needed: None Risk to taking this patch: Low Why is the change risky/not risky? (and alternatives if risky): Minor change. Ensures we invalidate the correct area. String changes made/needed:
Flags: needinfo?(jnicol)
Attachment #9015509 -
Flags: approval-mozilla-beta?
Assignee | ||
Comment 25•6 years ago
|
||
I meant to say no to needs manual test, I have verified it myself. But in any case STR are scrolling quickly on twitter then stopping it. Or alternatively pull down to refresh, then as it is loading quickly tap on a tweet then the phone's back button.
Comment 26•6 years ago
|
||
Comment on attachment 9015509 [details] Bug 1490789 - Apply shift when calculating old area of invalidated frame Fix for a visible rendering issue on Android affecting at least one major site, approved for uplift to our last 63 beta. Thanks.
Attachment #9015509 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Updated•6 years ago
|
Flags: qe-verify+
Comment 27•6 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/c4fd0c5a2d5a
Updated•6 years ago
|
Blocks: 1477693
Has Regression Range: --- → yes
status-firefox62:
--- → unaffected
status-firefox-esr60:
--- → unaffected
Keywords: regressionwindow-wanted
Updated•6 years ago
|
Flags: needinfo?(fennec)
Comment 28•6 years ago
|
||
I tried the mentioned steps (scrolling quickly) but did not see anything out of place (neither text nor images) with latest 2 builds of Nightly 64.0a1(2018.10.11-12) both in browser and PWA on several devices: * Motorola Nexus 6(Android 7.1.1) * Nexus 9(Android 6.0.1) * Pixel 2 (Android 9) * Samsung S8 Edge (Android 8.0.0) I did see a slightly delay in showing the pictures content (like 2sec) like in the second uploaded picture (displays a rectangle with the color major from the picture) BUT the image is aligned and not overlapping lines nor text. That is twitter intended behavior. We will also check this at the next beta/tue
Flags: needinfo?(fennec) → needinfo?(ioana.chiorean)
Comment 29•6 years ago
|
||
Devices: - OnePlus 5T (Android 8.1.0) - Huawei P9 Lite (Android 6) Tried in the latest beta (63.0b15), everything worked as expected in both normal browsing as well as PWA. Marking as verified.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Flags: needinfo?(ioana.chiorean)
Hardware: Unspecified → ARM
You need to log in
before you can comment on or make changes to this bug.
Description
•