-
Notifications
You must be signed in to change notification settings - Fork 27.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Flutter Scrolling does not match iOS; inadvertent scrolling when user lifts up finger #97761
Flutter Scrolling does not match iOS; inadvertent scrolling when user lifts up finger #97761
Comments
@chipweinberger I although don't have said OS and device, but I verified this behavior using latest master and stable (2.10.0) on iPhone 6s (OS 14.4.1) but didn't notice the unusual scrolling: RPReplay-Final1643964993.MP4stable, master flutter doctor -v
Can you upgrade to latest versions and try again to see if behavior still persists or not ? |
@darshankawar theres some technique to it. I find it happens more if your thumb is flatter against the screen when you lift up. If your thumb is too “pointy”, it’s harder to repro. Are there any considerations when switching Flutter versions? I don’t want to screw up my dev environment. |
Oh I didn’t know 2.10 just came out! I’ll upgrade and report back. |
I can repro on 2.10. |
Ok, I tried this behavior again on iPhone 12 mini and on iPhone 12 Pro max simulators and could see the scroll happening when I lift the pointer up: 97661.movBut I was unable to replicate this on a physical device (iPhone 6s) as mentioned earlier. As I observed, the extra scrolling doesn't occur everytime I try to scroll but occurs sometimes. I am keeping it open and labeling it for further insights from the team on this behavior. stable, master flutter doctor -v
/cc @Piinks |
It’s really fascinating that you can reproduce this in the simulator! I would have thought it was finicky touch events causing this. |
Any update on this? I can reproduce this behavior using a real device on iOS 16.12 iPhone 12 Pro. This happens on safari too. |
I can still repro in 3.10 |
The bug is very simple to understand - Therefore, if you move your finger quickly & (important) stop suddenly with no extra movement, the last 3 samples will all be > 0 dy, and you will get positive estimated velocity when you lift your finger up. Logs from velocity_tracker.dart:
There are 2 options to fix it:
|
This is a simple fix that completely solves the problem. velocity_tracker.dart class VelocityTracker {
+ final Stopwatch sinceLastSample = Stopwatch();
void addPosition(Duration time, Offset position) {
+ sinceLastSample.start();
}
VelocityEstimate? getVelocityEstimate() {
+ // there has been no user movement
+ if (sinceLastSample.elapsedMilliseconds > 60) {
+ return const VelocityEstimate(
+ pixelsPerSecond: Offset.zero,
+ confidence: 1.0,
+ duration: Duration.zero,
+ offset: Offset.zero,
+ );
} class IOSScrollViewFlingVelocityTracker extends VelocityTracker {
@override
void addPosition(Duration time, Offset position) {
+ sinceLastSample.start();
VelocityEstimate? getVelocityEstimate() {
+ // there has been no user movement
+ if (sinceLastSample.elapsedMilliseconds > 60) {
+ return const VelocityEstimate(
+ pixelsPerSecond: Offset.zero,
+ confidence: 1.0,
+ duration: Duration.zero,
+ offset: Offset.zero,
+ ); class MacOSScrollViewFlingVelocityTracker extends IOSScrollViewFlingVelocityTracker {
VelocityEstimate? getVelocityEstimate() {
+ // there has been no user movement
+ if (sinceLastSample.elapsedMilliseconds > 60) {
+ return const VelocityEstimate(
+ pixelsPerSecond: Offset.zero,
+ confidence: 1.0,
+ duration: Duration.zero,
+ offset: Offset.zero,
+ ); |
… iOS; inadvertent scrolling when user lifts up finger (#132291) ## Issue **Issue:** #97761 https://github.com/flutter/flutter/assets/1863934/53c5e0df-b85a-483c-a17d-bddd18db3aa9 ## The Cause: The bug is very simple to understand - `velocity_tracker.dart` **only adds new samples while your finger is moving**. **Therefore**, if you move your finger quickly & (important) stop suddenly with no extra movement, the last 3 samples will all be > 0 dy. Regardless of how long you wait, you will get movement when you lift up your finger. **Logs from velocity_tracker.dart:** Notice: all 3 `_previousVelocityAt` are `dy > 0` despite a 2 second delay since the last scroll ``` // start moving finger flutter: addPosition dy:-464.0 flutter: addPosition dy:-465.0 flutter: addPosition dy:-466.0 flutter: addPosition dy:-467.0 flutter: addPosition dy:-468.0 flutter: addPosition dy:-469.0 flutter: addPosition dy:-470.0 // stop moving finger here, keep it still for 2 seconds & lift it up flutter: _previousVelocityAt(-2) samples(-467.0, -468.0)) dy:-176.772140710624 flutter: _previousVelocityAt(-1) samples(-468.0, -469.0)) dy:-375.0937734433609 flutter: _previousVelocityAt(0) samples(-469.0, -470.0)) dy:-175.71604287471447 flutter: primaryVelocity DragEndDetails(Velocity(0.0, -305.5)).primaryVelocity flutter: createBallisticSimulation pixels 464.16666666666663 velocity 305.4699824197211 ``` ## The Fix **There are 3 options to fix it:** A. sample uniformly *per unit time* (a larger more risky change, hurts battery life) B. consider elapsed time since the last sample. If greater than X, assume no more velocity. (easy & just as valid) C. similar to B, but instead add "ghost samples" of velocity zero, and run calculations as normal (a bit tricker, of dubious benefit imo) **For Option B I considered two approaches:** 1. _get the current timestamp and compare to event timestamp._ This is tricky because events are documented to use an arbitrary timescale & I wasn't able to find the code that generates the timestamps. This approach could be considered more. 2. _get a new timestamp using Stopwatch and compare now vs when the last sample was added._ This is the solution implemented here. There is a limitation in that we don't know when addSamples is called relative to the event. But, this estimation is already on a very low latency path & still it gives us a *minimum* time bound which is sufficient for comparison. **This PR chooses the simplest of the all solutions. Please try it our yourself, it completely solves the problem ð���** Option _B.1_ would be a nice alternative as well, if we can define and access the same timesource as the pointer tracker in a maintainable simple way. ## After Fix https://github.com/flutter/flutter/assets/1863934/be50d8e7-d5da-495a-a4af-c71bc541cbe3
… iOS; inadvertent scrolling when user lifts up finger (flutter#132291) ## Issue **Issue:** flutter#97761 https://github.com/flutter/flutter/assets/1863934/53c5e0df-b85a-483c-a17d-bddd18db3aa9 ## The Cause: The bug is very simple to understand - `velocity_tracker.dart` **only adds new samples while your finger is moving**. **Therefore**, if you move your finger quickly & (important) stop suddenly with no extra movement, the last 3 samples will all be > 0 dy. Regardless of how long you wait, you will get movement when you lift up your finger. **Logs from velocity_tracker.dart:** Notice: all 3 `_previousVelocityAt` are `dy > 0` despite a 2 second delay since the last scroll ``` // start moving finger flutter: addPosition dy:-464.0 flutter: addPosition dy:-465.0 flutter: addPosition dy:-466.0 flutter: addPosition dy:-467.0 flutter: addPosition dy:-468.0 flutter: addPosition dy:-469.0 flutter: addPosition dy:-470.0 // stop moving finger here, keep it still for 2 seconds & lift it up flutter: _previousVelocityAt(-2) samples(-467.0, -468.0)) dy:-176.772140710624 flutter: _previousVelocityAt(-1) samples(-468.0, -469.0)) dy:-375.0937734433609 flutter: _previousVelocityAt(0) samples(-469.0, -470.0)) dy:-175.71604287471447 flutter: primaryVelocity DragEndDetails(Velocity(0.0, -305.5)).primaryVelocity flutter: createBallisticSimulation pixels 464.16666666666663 velocity 305.4699824197211 ``` ## The Fix **There are 3 options to fix it:** A. sample uniformly *per unit time* (a larger more risky change, hurts battery life) B. consider elapsed time since the last sample. If greater than X, assume no more velocity. (easy & just as valid) C. similar to B, but instead add "ghost samples" of velocity zero, and run calculations as normal (a bit tricker, of dubious benefit imo) **For Option B I considered two approaches:** 1. _get the current timestamp and compare to event timestamp._ This is tricky because events are documented to use an arbitrary timescale & I wasn't able to find the code that generates the timestamps. This approach could be considered more. 2. _get a new timestamp using Stopwatch and compare now vs when the last sample was added._ This is the solution implemented here. There is a limitation in that we don't know when addSamples is called relative to the event. But, this estimation is already on a very low latency path & still it gives us a *minimum* time bound which is sufficient for comparison. **This PR chooses the simplest of the all solutions. Please try it our yourself, it completely solves the problem ð���** Option _B.1_ would be a nice alternative as well, if we can define and access the same timesource as the pointer tracker in a maintainable simple way. ## After Fix https://github.com/flutter/flutter/assets/1863934/be50d8e7-d5da-495a-a4af-c71bc541cbe3
This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of |
The change that resolved this issue is being reverted because it introduced a bunch of flakes in the framework. See #135728 |
This updates the implementation to use the stopwatch from the Clock object and piping it through to the TestWidgetsFlutterBinding so it will be kept in sync with FakeAsync. Relands #132291 Fixes #97761 The change was reverted due to flakiness it introduced in tests that use fling gestures. * #135728
Reverts #137381 Initiated by: Piinks This change reverts the following previous change: Original Description: This updates the implementation to use the stopwatch from the Clock object and piping it through to the TestWidgetsFlutterBinding so it will be kept in sync with FakeAsync. Relands #132291 Fixes #97761 The change was reverted due to flakiness it introduced in tests that use fling gestures. * #135728
This updates the implementation to use the stopwatch from the Clock object and piping it through to the TestWidgetsFlutterBinding so it will be kept in sync with FakeAsync. Relands #137381 which attempted to reland #132291 Fixes #97761 The original change was reverted due to flakiness it introduced in tests that use fling gestures. * #135728 It was reverted again due to a change in the leak tracking tests that broke it.
This updates the implementation to use the stopwatch from the Clock object and pipes it through to the TestWidgetsFlutterBinding so it will be kept in sync with FakeAsync. Relands #138843 attempted to reland #137381 which attempted to reland #132291 Fixes #97761 1. The original change was reverted due to flakiness it introduced in tests that use fling gestures. * Using a mocked clock through the test binding fixes this now 2. It was reverted a second time because a change at tip of tree broke it, exposing memory leaks, but it was not rebased before landing. * These leaks are now fixed 3. It was reverted a third time, because we were so excellently quick to revert those other times, that we did not notice the broken benchmark that only runs in postsubmit. * The benchmark is now fixed
This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of |
This updates the implementation to use the stopwatch from the Clock object and piping it through to the TestWidgetsFlutterBinding so it will be kept in sync with FakeAsync. Relands flutter#137381 which attempted to reland flutter#132291 Fixes flutter#97761 The original change was reverted due to flakiness it introduced in tests that use fling gestures. * flutter#135728 It was reverted again due to a change in the leak tracking tests that broke it.
This updates the implementation to use the stopwatch from the Clock object and pipes it through to the TestWidgetsFlutterBinding so it will be kept in sync with FakeAsync. Relands flutter#138843 attempted to reland flutter#137381 which attempted to reland flutter#132291 Fixes flutter#97761 1. The original change was reverted due to flakiness it introduced in tests that use fling gestures. * Using a mocked clock through the test binding fixes this now 2. It was reverted a second time because a change at tip of tree broke it, exposing memory leaks, but it was not rebased before landing. * These leaks are now fixed 3. It was reverted a third time, because we were so excellently quick to revert those other times, that we did not notice the broken benchmark that only runs in postsubmit. * The benchmark is now fixed
Flutter 2.8.1
iOS 15.3
iPhone 12 Mini
Problem: Doing the exact same gesture on both Flutter and the iOS Settings App, its far easier to accidentally scroll when you lift your finger up. It's actually kind of hard not to hit this behavior.
Given how important scrolling is, this seems really important to fix for users. It also looks pretty bad to developers because this is the kind of issue that React Native, and Native just gets right by design.
I think what is happening: we are detecting a 'Fling' gesture when we should not be? If you move your finger slowly this behavior does not happen, because we never confuse it for a fling perhaps. Maybe the 'Fling' logic is too simplistic and needs to take the last ~200 milliseconds of events into greater account?
inadvertant.scrolling.small.mov
The text was updated successfully, but these errors were encountered: