Closed Bug 1522026 Opened 4 years ago Closed 4 years ago

sluggish scrolling due to the implementation of scroll anchoring

Categories

(Core :: Layout: Scrolling and Overflow, defect, P2)

66 Branch
Unspecified
Android
defect

Tracking

()

RESOLVED FIXED
mozilla67
Tracking Status
firefox-esr60 --- unaffected
firefox64 --- unaffected
firefox65 --- unaffected
firefox66 --- fixed
firefox67 --- fixed

People

(Reporter: karlcow, Assigned: rhunt)

References

()

Details

(Keywords: regression, Whiteboard: [webcompat] )

Attachments

(1 file)

This was reported initially on https://webcompat.com/issues/24330

Steps to reproduce:
0. with a mobile device or Firefox Desktop with RDM + mobile UA.

  1. Go to https://www.phoronix.com/forums/forum/phoronix/latest-phoronix-articles/1073431-fedora-decides-to-not-allow-ssplv1-licensed-software-into-its-repositories
  2. wait for the ads to load
  3. scroll up and down

Actual:
The scroll is sluggish, choppy.

Expected:
smooth scroll.

This is a regression introduced by Bug 1305957
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=fbe6548db11ded24b5221180719ce66e785dc3c6&tochange=ad851d4345c08f7e0e5d5578652004194a6e667f

Flags: webcompat?

Possibly the same as bug 1521579?

Component: Layout → Layout: Scrolling and Overflow
Flags: needinfo?(rhunt)

Likely (hopefully)

Priority: P3 → P2
Keywords: regression
Whiteboard: [webcompat] [regression] → [webcompat]

I'm not sure this is the same issue as bug 1521579.

I can't seem to reproduce poor scrolling performance, that is where we checkerboard more with scroll anchoring enabled.

I was able to intermittently hit a case where I would fling or maybe try to start a fling and not have Firefox respond. This could be related to bug 1474196 then.

Flags: needinfo?(rhunt)
Logging from async pan zoom controller log

01-26 14:50:21.560 15653 15827 I Gecko   : APZC: 0xc6f5c000 relative updating scroll offset from (0,3057.01) by (0,0)
01-26 14:50:21.560 15653 15827 I Gecko   : APZC: 0xc6f5c000 running CancelAnimation(0x0) in state 0
01-26 14:50:21.560 15653 15827 I Gecko   : APZC: 0xc6f5c000 changing from state 0 to 0
01-26 14:50:21.677 15653 15653 I Gecko   : APZC: 0xc8a0c000 flushing repaint for new input block
01-26 14:50:21.677 15653 15653 I Gecko   : APZC: 0xc6f5c000 flushing repaint for new input block
01-26 14:50:21.677 15653 15653 I Gecko   : APZC: 0xc6f5c000 running CancelAnimation(0x3) in state 0
01-26 14:50:21.677 15653 15653 I Gecko   : APZC: 0xc6f5c000 changing from state 0 to 0
01-26 14:50:21.677 15653 15653 I Gecko   : APZC: 0xc6f5c000 scroll snapping near (0,3057.01)
01-26 14:50:21.677 15653 15653 I Gecko   : APZC: 0xc8a0c000 running CancelAnimation(0x3) in state 0
01-26 14:50:21.677 15653 15653 I Gecko   : APZC: 0xc8a0c000 changing from state 0 to 0
01-26 14:50:21.677 15653 15653 I Gecko   : APZC: 0xc8a0c000 scroll snapping near (0,0)
01-26 14:50:21.677 15653 15653 I Gecko   : APZC: 0xc6f5c000 got a touch-start in state 0
01-26 14:50:21.678 15653 15653 I Gecko   : APZC: 0xc6f5c000 changing from state 0 to 2
01-26 14:50:21.713 15653 15653 I Gecko   : APZC: 0xc6f5c000 got a touch-move in state 2
01-26 14:50:21.730 15653 15653 I Gecko   : APZC: 0xc6f5c000 got a touch-move in state 2
01-26 14:50:21.730 15653 15653 I Gecko   : APZC: 0xc6f5c000 changing from state 2 to 5
01-26 14:50:21.746 15653 15827 I Gecko   : APZC: 0xc8a0c000 NotifyLayersUpdated short-circuit
01-26 14:50:21.747 15653 15827 I Gecko   : APZC: c6f5c000 got a NotifyLayersUpdated with aIsFirstPaint=0, aThisLayerTreeUpdated=1:{ [cb=(x=0.000000, y=0.000000, w=1080.000000, h=1584.000000)] [sr=(x=0.000000, y=0.000000, w=415.149994, h=7722.250000)] [s=(0,3057.08)] [dp=(x=0.000000, y=-2615.483398, w=415.149994, h=5888.000000)] [cdp=(x=0.000000, y=-407.483337, w=415.149994, h=1472.000000) [dpm=(l=1.499992, t=985.261108, r=1.499992, b=984.866211)] [rcs=(w=414.000000, h=607.199951)] [v=(x=0.000000, y=3057.083252, w=414.000000, h=607.200012)] [z=(ld=2.609 r=1.000 cr=1 z=2.61 er=1)] [u=(1 0 1382)] [i=(6 4 1)] }
01-26 14:50:21.747 15653 15827 I Gecko   : APZC: 0xc6f5c000 relative updating scroll offset from (0,3057.01) by (0,0)
01-26 14:50:21.747 15653 15827 I Gecko   : APZC: 0xc6f5c000 running CancelAnimation(0x0) in state 5
01-26 14:50:21.747 15653 15827 I Gecko   : APZC: 0xc6f5c000 changing from state 5 to 0
01-26 14:50:21.748 15653 15653 I Gecko   : APZC: 0xc6f5c000 got a touch-move in state 0
01-26 14:50:21.760 15653 15797 I Gecko   : APZC: Calculated displayport as (x=-0.574997, y=-377.607727, w=415.149994, h=1362.415405) from velocity (0,0) paint time 50.000000 metrics:{ [cb=(x=0.000000, y=0.000000, w=1080.000000, h=1584.000000)] [sr=(x=0.000000, y=0.000000, w=415.149994, h=7722.250000)] [s=(0,3057.01)] [dp=(x=0.000000, y=0.000000, w=415.149994, h=607.200012)] [cdp=(x=0.000000, y=0.000000, w=415.149994, h=607.200012) [dpm=(l=1.499992, t=985.063660, r=1.499992, b=985.063660)] [rcs=(w=414.000000, h=607.199951)] [v=(x=0.000000, y=3057.007568, w=414.000000, h=607.200012)] [z=(ld=2.609 r=1.000 cr=1 z=2.61 er=1)] [u=(0 0 1382)] [i=(6 4 1)] }
01-26 14:50:21.760 15653 15797 I Gecko   : APZC: c6f5c000 requesting content repaint:{ [cb=(x=0.000000, y=0.000000, w=1080.000000, h=1584.000000)] [sr=(x=0.000000, y=0.000000, w=415.149994, h=7722.250000)] [s=(0,3057.01)] [dp=(x=0.000000, y=0.000000, w=415.149994, h=607.200012)] [cdp=(x=0.000000, y=0.000000, w=415.149994, h=607.200012) [dpm=(l=1.499992, t=985.063660, r=1.499992, b=985.063660)] [rcs=(w=414.000000, h=607.199951)] [v=(x=0.000000, y=3057.007568, w=414.000000, h=607.200012)] [z=(ld=2.609 r=1.000 cr=1 z=2.61 er=1)] [u=(0 0 1382)] [i=(6 4 1)] }
01-26 14:50:21.763 15653 15653 I Gecko   : APZC: 0xc6f5c000 got a touch-move in state 0
01-26 14:50:21.775 15653 15653 I Gecko   : APZC: 0xc6f5c000 got a touch-move in state 0
01-26 14:50:21.776 15653 15653 I Gecko   : APZC: 0xc6f5c000 got a touch-end in state 0

Logging from scroll anchor log

01-26 14:56:26.054 16033 16111 I Gecko   : ANCHOR: Anchor has moved from -1353 to 867.
01-26 14:56:26.054 16033 16111 I Gecko   : ANCHOR: Applying anchor adjustment of 2220 in h-ltr for 0xbc627224 and anchor 0xbcb86910.
01-26 14:56:26.184 16033 16111 I Gecko   : ANCHOR: Ignoring post-reflow (anchor=0x0, dirty=1, container=0xb7034f8c).
01-26 14:56:26.185 16033 16111 I Gecko   : ANCHOR: Anchor has moved from -1364 to -3584.
01-26 14:56:26.185 16033 16111 I Gecko   : ANCHOR: Applying anchor adjustment of -2220 in h-ltr for 0xbc627224 and anchor 0xbcb86910.
01-26 14:56:26.359 16033 16111 I Gecko   : ANCHOR: Ignoring post-reflow (anchor=0x0, dirty=1, container=0xb7034f8c).
01-26 14:56:26.359 16033 16111 I Gecko   : ANCHOR: Anchor has moved from -1353 to 867.
01-26 14:56:26.359 16033 16111 I Gecko   : ANCHOR: Applying anchor adjustment of 2220 in h-ltr for 0xbc627224 and anchor 0xbcb86910.
01-26 14:56:26.496 16033 16111 I Gecko   : ANCHOR: Ignoring post-reflow (anchor=0x0, dirty=1, container=0xb7034f8c).
01-26 14:56:26.497 16033 16111 I Gecko   : ANCHOR: Anchor has moved from -1364 to -3584.
01-26 14:56:26.497 16033 16111 I Gecko   : ANCHOR: Applying anchor adjustment of -2220 in h-ltr for 0xbc627224 and anchor 0xbcb86910.
01-26 14:56:26.668 16033 16111 I Gecko   : ANCHOR: Ignoring post-reflow (anchor=0x0, dirty=1, container=0xb7034f8c).
01-26 14:56:26.668 16033 16111 I Gecko   : ANCHOR: Anchor has moved from -1353 to 867.
01-26 14:56:26.668 16033 16111 I Gecko   : ANCHOR: Applying anchor adjustment of 2220 in h-ltr for 0xbc627224 and anchor 0xbcb86910.
01-26 14:56:26.819 16033 16111 I Gecko   : ANCHOR: Ignoring post-reflow (anchor=0x0, dirty=1, container=0xb7034f8c).
01-26 14:56:26.820 16033 16111 I Gecko   : ANCHOR: Anchor has moved from -1364 to -3584.
01-26 14:56:26.820 16033 16111 I Gecko   : ANCHOR: Applying anchor adjustment of -2220 in h-ltr for 0xbc627224 and anchor 0xbcb86910.
01-26 14:56:27.127 16033 16111 I Gecko   : ANCHOR: Ignoring post-reflow (anchor=0x0, dirty=1, container=0xb7034f8c).
01-26 14:56:27.128 16033 16111 I Gecko   : ANCHOR: Anchor has moved from -1353 to 867.
01-26 14:56:27.128 16033 16111 I Gecko   : ANCHOR: Applying anchor adjustment of 2220 in h-ltr for 0xbc627224 and anchor 0xbcb86910.

The page seems to be in an adjustment loop with different scroll offsets. The adjustments seem to make a complete loop by the end of the refresh tick, and so APZ only sees a relative update of (0, 0). This update though causes us to cancel animations and ignore inputs.

I think we can special case a relative update of (0, 0) here.

Assignee: nobody → rhunt
Pushed by rhunt@eqrion.net:
https://hg.mozilla.org/integration/autoland/rev/fae409184e4a
Don't cancel animations over relative updates with no offset. r=botond
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla67

I was able to reproduce this issue following the steps from the description on the latest Nightly build 67.0a1 with a Google Pixel 3XL Android P.

For more details please check the following attachments: video, logcat.
Note that you can see once the ads are loaded, while scrolling down the scrolling is sluggish, choppy.
Based on my comment I will re-open this issue.

Status: RESOLVED → REOPENED
Flags: qe-verify+
Resolution: FIXED → ---
Target Milestone: mozilla67 → ---

:rhunt, are you taking another run at this one? should we still track this for 66?

Flags: needinfo?(rhunt)

I just took a look at the attached video, and while the scrolling may be sluggish it doesn't appear to be an issue with scroll anchoring.

The issue that this patch fixed caused flings to be ignored or canceled midway through, so we'd see attempts to scroll fail or scrolling that got canceled. I can't seem to see this in that video, but I could be missing it.

Status: REOPENED → RESOLVED
Closed: 4 years ago4 years ago
Flags: needinfo?(rhunt)
Resolution: --- → FIXED
Target Milestone: --- → mozilla67

Comment on attachment 9039360 [details]
Bug 1522026 - Don't cancel animations over relative updates with no offset. r?botond

Beta/Release Uplift Approval Request

Feature/Bug causing the regression

Scroll Anchoring

User impact if declined

Users may have their fling scrolls canceled on certain pages giving a poor scrolling experience.

Is this code covered by automated tests?

No

Has the fix been verified in Nightly?

Yes

Needs manual test from QE?

No

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)

This code has been baking on nightly for a while with no apparent regressions, and is a very simple patch.

String changes made/needed

Attachment #9039360 - Flags: approval-mozilla-beta?

Comment on attachment 9039360 [details]
Bug 1522026 - Don't cancel animations over relative updates with no offset. r?botond

Tweak to improve scrolling, has had time on Nightly.
Ok to uplift for beta 8.

Attachment #9039360 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.