New issue
Advanced search Search tips

Issue 866517 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Closed: Aug 20
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 1
Type: Bug

Blocking:
issue 497851



Sign in to add a comment

[css-scroll-snap] Touch scrolling causes subtle oscillation

Project Member Reported by majidvp@chromium.org, Jul 23

Issue description

Repro steps:
1. Load https://snap.glitch.me/carousel.html
2. Enable "Touch input" using devtools > sensors
3. Scroll using touch 


What happens?
When scroll ends at the snap point, there is a subtle oscillating behavior. This should not happen. I am not sure if touch emulation has anything to do with it but in any case we should find out what is happening and if it affect regular touch scrolling then try to get fix this for M69.

See attached video.

Version tested:

Chromium	69.0.3494.0 (Developer Build) (64-bit)
Revision	d059788bbef3a57d5d4832945f5d25121cb05867-refs/heads/master@{#575248}
OS	Linux

 
snap-bug-shivering-2018.07.23-12-00-01.webm
436 KB View Download
After an initial investigation, I feel like the bug is caused by the integer rounding of the ScrollOffset somewhere. When debugging, look for GetScrollDelta() and UpdateCurrentOffset() in SnapFlingCurve. Especially watch for the values of current_displacement_ and new_displacement.
Project Member

Comment 2 by bugdroid1@chromium.org, Aug 17

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e86044fc6f467d5384d523c244235f2f4750ca88

commit e86044fc6f467d5384d523c244235f2f4750ca88
Author: Sandra Sun <sunyunjia@chromium.org>
Date: Fri Aug 17 17:53:37 2018

End the snap fling within 1 pixel of destination.

The scroll offsets returned from blink are only integers. However,
if the target snap offset is fractional, it will never reach the
destination and will oscillate between the two closest integers until it
reaches the maximum time duration.

In this patch, we end the snap fling if we've reached the 1-pixel range
of the destination.

Bug:  866517 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I185fd4267c626e1f4e561e9bd92a962c8f289d38
Reviewed-on: https://chromium-review.googlesource.com/1159088
Reviewed-by: David Bokan <bokan@chromium.org>
Reviewed-by: Majid Valipour <majidvp@chromium.org>
Commit-Queue: Sandra Sun <sunyunjia@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584119}
[modify] https://crrev.com/e86044fc6f467d5384d523c244235f2f4750ca88/cc/input/snap_fling_curve.cc
[modify] https://crrev.com/e86044fc6f467d5384d523c244235f2f4750ca88/cc/input/snap_fling_curve.h
[modify] https://crrev.com/e86044fc6f467d5384d523c244235f2f4750ca88/cc/input/snap_fling_curve_unittest.cc

Labels: -Pri-2 Merge-Request-69 OS-Android OS-Linux OS-Mac Pri-1
Status: Verified (was: Assigned)
Snap points will be shipped in M69 but this bug seriously affects its functionality, especially on Android. Now the bug is fixed, please approve to merge it in M69. Thanks!
Project Member

Comment 4 by sheriffbot@chromium.org, Aug 20

Labels: -Merge-Request-69 Merge-Review-69 Hotlist-Merge-Review
This bug requires manual review: M69 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), kariahda@(iOS), cindyb@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
sunyunjia@ I assume this also affects Windows and Chrome platforms too or am I missing something?
Labels: OS-Chrome OS-Windows
Adding them as well.
How is the change listed at #2 looking in canary? Is it safe to merge now?
It is working well in canary now and should be safe to merge.
Cc: benmason@chromium.org
Labels: -Merge-Review-69 Merge-Approved-69
Approving merge to M69 branch 3497 based on comment #3 and #8. Please merge ASAP,Thank you.
Project Member

Comment 10 by bugdroid1@chromium.org, Aug 20

Labels: -merge-approved-69 merge-merged-3497
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/69ccb2fba9844d837fbc4e325b5630114b68a700

commit 69ccb2fba9844d837fbc4e325b5630114b68a700
Author: Sandra Sun <sunyunjia@chromium.org>
Date: Mon Aug 20 15:38:35 2018

End the snap fling within 1 pixel of destination.

The scroll offsets returned from blink are only integers. However,
if the target snap offset is fractional, it will never reach the
destination and will oscillate between the two closest integers until it
reaches the maximum time duration.

In this patch, we end the snap fling if we've reached the 1-pixel range
of the destination.

Bug:  866517 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I185fd4267c626e1f4e561e9bd92a962c8f289d38
Reviewed-on: https://chromium-review.googlesource.com/1159088
Reviewed-by: David Bokan <bokan@chromium.org>
Reviewed-by: Majid Valipour <majidvp@chromium.org>
Commit-Queue: Sandra Sun <sunyunjia@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#584119}(cherry picked from commit e86044fc6f467d5384d523c244235f2f4750ca88)
Reviewed-on: https://chromium-review.googlesource.com/1181282
Reviewed-by: Sandra Sun <sunyunjia@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#709}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/69ccb2fba9844d837fbc4e325b5630114b68a700/cc/input/snap_fling_curve.cc
[modify] https://crrev.com/69ccb2fba9844d837fbc4e325b5630114b68a700/cc/input/snap_fling_curve.h
[modify] https://crrev.com/69ccb2fba9844d837fbc4e325b5630114b68a700/cc/input/snap_fling_curve_unittest.cc

Sign in to add a comment