Fix ScrollPosition overscroll precision error #127321

bleroux · 2023-05-22T14:01:01Z

Description

This PR fixes a precision error in ~~ClampingScrollPhysics~~ ScrollPosition that leads to StretchingOverscrollIndicator stretching its content unexpectedly in some devices (see #126561 where this is visible in TabBarView and the test added in this PR where it reproduces with a PageView).

~~This PR also contains a change to nested_scroll_view.dart because the first change (the one in ClampingScrollPhysics) breaks the precision error test added by #87801.~~

Related Issue

Fixes #126561

Tests

Adds 1 test.

packages/flutter/lib/src/widgets/nested_scroll_view.dart

Piinks

I'd like to run this through some extra testing, great find by the way. I was surprised to see this was in ClampingScrollPhysics, originally I thought this would have to do with PageScrollPhysics. cc'd @nt4f04uNd since you noticed this affected a change they made a while back. Thanks for diving into NestedScrollView to prevent a regression here.

packages/flutter/test/widgets/page_view_test.dart

nt4f04uNd · 2023-05-23T18:13:28Z

packages/flutter/lib/src/widgets/scroll_physics.dart

I have a gut feeling that this is not right.

We are losing here even more precision - the caller of this relies on that to update the scroll position.

We already saw that this breaks the NestedScrollView test, which possibly might be just one case we have covered where it breaks something.

I feel like we should rather change the overscroll != 0.0 condition to something like overscroll.abs() > precisionErrorTolerance. This seems like a much safer alternative to chaning the core logic inside ClampingScrollPhysics, and also definitely covers all the cases of precision errors.

cc @Hixie
IIRC he's the one who is very familiar with the core logic of scroll phyics

Thanks, I agree that overscroll.abs() > precisionErrorTolerance is a safer and simpler alternative.

nt4f04uNd · 2023-05-23T19:16:32Z

packages/flutter/lib/src/widgets/scroll_physics.dart

Another thing that I'm wondering, why is overflowing by such a small value of 1.1368683772161603e-13 gives us the stetch of 1.440112717630772 and the stretch does not recede?

There might be another deeper problem with the StretchingOverscrollIndicator

@Piinks what do you think?

…in nested_scroll_view_test.dart

Piinks · 2023-05-24T22:27:23Z

packages/flutter/lib/src/widgets/scroll_position.dart

        didUpdateScrollPositionBy(pixels - oldPixels);
      }
-      if (overscroll != 0.0) {
+      if (overscroll.abs() > precisionErrorTolerance) {


Ah yes, this is a much clearer culprit. 👍 Thank you both for looking into this!

Piinks

LGTM!

rydmike · 2023-05-25T08:45:27Z

Wow, nice work all of you 👍🏻 💙

This was quite a peculiar looking bug, with an effect that looked really strange, making it hard to even first believe it when it was discovered. When @chanan first showed it to me on Twitter, I was like naah, that can be possible, until I tried it, lol.

Nice to see a speedy resolution, and thanks @bleroux for digging into it so quickly.

…r#127321)

Roll Flutter from 216605b to ee162e4 (53 revisions) flutter/flutter@216605b...ee162e4 2023-05-26 engine-flutter-autoroll@skia.org Roll Packages from 995bfc5 to 9f8dcc5 (4 revisions) (flutter/flutter#127671) 2023-05-26 engine-flutter-autoroll@skia.org Roll Flutter Engine from 474fcbbe445e to 8573f3b63a1f (1 revision) (flutter/flutter#127647) 2023-05-26 engine-flutter-autoroll@skia.org Roll Flutter Engine from f63fcf5a274e to 474fcbbe445e (1 revision) (flutter/flutter#127644) 2023-05-26 engine-flutter-autoroll@skia.org Roll Flutter Engine from b6fcbe31ba1a to f63fcf5a274e (1 revision) (flutter/flutter#127642) 2023-05-26 engine-flutter-autoroll@skia.org Roll Flutter Engine from bec5da5bff7c to b6fcbe31ba1a (1 revision) (flutter/flutter#127641) 2023-05-26 engine-flutter-autoroll@skia.org Roll Flutter Engine from 593d4d6b1f9b to bec5da5bff7c (2 revisions) (flutter/flutter#127638) 2023-05-26 engine-flutter-autoroll@skia.org Roll Flutter Engine from f4dc96aef71b to 593d4d6b1f9b (1 revision) (flutter/flutter#127635) 2023-05-26 engine-flutter-autoroll@skia.org Roll Flutter Engine from ff04d2fdd12b to f4dc96aef71b (2 revisions) (flutter/flutter#127633) 2023-05-26 31859944+LongCatIsLooong@users.noreply.github.com Remove a bad assert from tooltip implementation (flutter/flutter#127629) 2023-05-26 31859944+LongCatIsLooong@users.noreply.github.com Remove rounding from TextPainter (flutter/flutter#127099) 2023-05-26 109253501+pdblasi-google@users.noreply.github.com Adds `TestDisplay` API for testing `Display` features (flutter/flutter#127525) 2023-05-25 engine-flutter-autoroll@skia.org Roll Flutter Engine from 6a4e675ab0b4 to ff04d2fdd12b (3 revisions) (flutter/flutter#127630) 2023-05-25 engine-flutter-autoroll@skia.org Roll Flutter Engine from 99f2653cbb79 to 6a4e675ab0b4 (2 revisions) (flutter/flutter#127625) 2023-05-25 54558023+keyonghan@users.noreply.github.com Append $flutter/osx sdk property to existing platforms/targets relying on xcode (flutter/flutter#127537) 2023-05-25 chillers@google.com Indent markdown value on CP issue template (flutter/flutter#127623) 2023-05-25 engine-flutter-autoroll@skia.org Roll Flutter Engine from 6ff02c17268f to 99f2653cbb79 (6 revisions) (flutter/flutter#127619) 2023-05-25 42216813+eliasyishak@users.noreply.github.com `ProcessResultMatcher` created and used in test (flutter/flutter#127414) 2023-05-25 chillers@google.com Fix yaml lint issue (flutter/flutter#127600) 2023-05-25 engine-flutter-autoroll@skia.org Roll Flutter Engine from 515a4bfc4b6e to 6ff02c17268f (1 revision) (flutter/flutter#127609) 2023-05-25 36861262+QuncCccccc@users.noreply.github.com `SearchBar` should not be impacted by overall `InputDecorationTheme` (flutter/flutter#127465) 2023-05-25 engine-flutter-autoroll@skia.org Roll Flutter Engine from 195009b91abd to 515a4bfc4b6e (4 revisions) (flutter/flutter#127601) 2023-05-25 engine-flutter-autoroll@skia.org Roll Flutter Engine from fb463217d1f3 to 195009b91abd (1 revision) (flutter/flutter#127591) 2023-05-25 dnfield@google.com avoid incorrect usage of TimelineTask (flutter/flutter#127527) 2023-05-25 tessertaha@gmail.com Add `ScrollNotificationObserver` sample (flutter/flutter#127023) 2023-05-25 engine-flutter-autoroll@skia.org Roll Packages from fba97fa to 995bfc5 (3 revisions) (flutter/flutter#127590) 2023-05-25 engine-flutter-autoroll@skia.org Roll Flutter Engine from a74d9d1f4776 to fb463217d1f3 (1 revision) (flutter/flutter#127577) 2023-05-25 engine-flutter-autoroll@skia.org Roll Flutter Engine from a7f026b03e35 to a74d9d1f4776 (1 revision) (flutter/flutter#127565) 2023-05-25 engine-flutter-autoroll@skia.org Roll Flutter Engine from fb38bb6b1bc1 to a7f026b03e35 (1 revision) (flutter/flutter#127563) 2023-05-25 engine-flutter-autoroll@skia.org Roll Flutter Engine from 2be736c390ee to fb38bb6b1bc1 (1 revision) (flutter/flutter#127560) 2023-05-25 31859944+LongCatIsLooong@users.noreply.github.com Remove Tooltip mouse tracker listener & update hovering/MouseRegion logic & animation (flutter/flutter#119199) 2023-05-25 engine-flutter-autoroll@skia.org Roll Flutter Engine from 37ebad2d5c27 to 2be736c390ee (2 revisions) (flutter/flutter#127554) 2023-05-25 engine-flutter-autoroll@skia.org Roll Flutter Engine from 62a83490ee60 to 37ebad2d5c27 (1 revision) (flutter/flutter#127551) 2023-05-25 engine-flutter-autoroll@skia.org Roll Flutter Engine from 1bed04a4375a to 62a83490ee60 (3 revisions) (flutter/flutter#127546) 2023-05-25 engine-flutter-autoroll@skia.org Roll Flutter Engine from 6a361090a367 to 1bed04a4375a (2 revisions) (flutter/flutter#127544) 2023-05-25 engine-flutter-autoroll@skia.org Roll Flutter Engine from 9ba461efd3fe to 6a361090a367 (3 revisions) (flutter/flutter#127542) 2023-05-24 chillers@google.com Revert "Update labeler action wildcards" (flutter/flutter#127541) 2023-05-24 mdebbar@google.com [web] ui.platformViewRegistry => ui_web.platformViewRegistry (flutter/flutter#127493) 2023-05-24 engine-flutter-autoroll@skia.org Roll Flutter Engine from c641f6307f08 to 9ba461efd3fe (6 revisions) (flutter/flutter#127536) 2023-05-24 lsaudon@gmail.com Add missing parameters in `TextFormField` (flutter/flutter#127020) 2023-05-24 chillers@google.com Update labeler action wildcards (flutter/flutter#127524) 2023-05-24 31859944+LongCatIsLooong@users.noreply.github.com Improve `TextPainter.layout` caching (flutter/flutter#118128) 2023-05-24 49699333+dependabot[bot]@users.noreply.github.com Bump actions/labeler from 2713f7303c96cb1e69627957ec16eea0fd7f94a4 to 0776a679364a9a16110aac8d0f40f5e11009e327 (flutter/flutter#127533) 2023-05-24 leroux_bruno@yahoo.fr Fix ScrollPosition overscroll precision error (flutter/flutter#127321) 2023-05-24 dleyba042@gmail.com `Slider.onChangeStart` & `Slider.onChangeEnd` are not called on keyboard shortcuts (flutter/flutter#126896) 2023-05-24 bdero@google.com Manually roll Flutter Engine from 0c41b02cd5a6 to c641f6307f08 (flutter/flutter#127514) 2023-05-24 chillers@google.com Update CP template to include timeline and changelog (flutter/flutter#127507) ...

bleroux · 2023-06-02T06:30:11Z

@Piinks FYI, there is another report #127912.
I'm not sure if this PR can be considered for CP?

Piinks · 2023-06-02T17:02:57Z

Since this wasn't a regression in the release I don't know that we should cherry pick it. Working its way through the release process helps us catch any potential issues with it or other regressions it may introduce, but that's my opinion. I would reach out on discord to see if anyone feels strongly that this should be CP'd. Thanks!

absar · 2023-08-10T23:00:45Z

Its been two months since the fix landed in master but it is still not in stable. Can you please cherry pick it, since it's giving end users very bad UX, the screen stretches and does not come back to original shape. Even if the new flutter stable 3.13/3.14 is being released we cannot risk using that for production until that gains stability

…r#127321)

flutter-dashboard bot added f: scrolling Viewports, list views, slivers, etc. framework flutter/packages/flutter repository. See also f: labels. labels May 22, 2023

bleroux requested a review from Piinks May 22, 2023 14:54

bleroux commented May 22, 2023

View reviewed changes

packages/flutter/lib/src/widgets/nested_scroll_view.dart Outdated Show resolved Hide resolved

Piinks requested a review from nt4f04uNd May 22, 2023 23:17

Piinks reviewed May 22, 2023

View reviewed changes

nt4f04uNd reviewed May 23, 2023

View reviewed changes

bleroux added 3 commits May 24, 2023 09:36

Fix ScrollPhysics double precision error leads to unexpected overscroll

db24857

Fix for "NestedScrollView doesn't crash due to precision error" test …

9d32e02

…in nested_scroll_view_test.dart

Move the precision check to ScrollPosition

1db9c8d

bleroux force-pushed the fix_scrollphysics_precision_error branch from e02fe0c to 1db9c8d Compare May 24, 2023 07:55

github-actions bot removed f: scrolling Viewports, list views, slivers, etc. framework flutter/packages/flutter repository. See also f: labels. labels May 24, 2023

flutter-dashboard bot added f: scrolling Viewports, list views, slivers, etc. framework flutter/packages/flutter repository. See also f: labels. labels May 24, 2023

nt4f04uNd approved these changes May 24, 2023

View reviewed changes

bleroux requested a review from Piinks May 24, 2023 12:54

Piinks reviewed May 24, 2023

View reviewed changes

Piinks changed the title ~~Fix ClampingScrollPhysics precision error~~ Fix ScrollPosition overscroll precision error May 24, 2023

Piinks approved these changes May 24, 2023

View reviewed changes

Piinks added the autosubmit Merge PR when tree becomes green via auto submit App label May 24, 2023

auto-submit bot merged commit 2b105ac into flutter:master May 24, 2023

bleroux deleted the fix_scrollphysics_precision_error branch May 25, 2023 07:09

engine-flutter-autoroll mentioned this pull request May 25, 2023

Roll Flutter from 216605b60f67 to 20e164e60cb2 (27 revisions) flutter/packages#4077

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 25, 2023

2b105ac Fix ScrollPosition overscroll precision error (flutter/flutte…

fe6981d

…r#127321)

engine-flutter-autoroll mentioned this pull request May 25, 2023

Roll Flutter from 216605b60f67 to 60a64a5a5c7f (28 revisions) flutter/packages#4078

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 25, 2023

2b105ac Fix ScrollPosition overscroll precision error (flutter/flutte…

d90b098

…r#127321)

engine-flutter-autoroll mentioned this pull request May 25, 2023

Roll Flutter from 216605b60f67 to 3e610f7333f6 (29 revisions) flutter/packages#4079

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 25, 2023

2b105ac Fix ScrollPosition overscroll precision error (flutter/flutte…

3297486

…r#127321)

engine-flutter-autoroll mentioned this pull request May 25, 2023

Roll Flutter from 216605b60f67 to c05a05e6fcc0 (30 revisions) flutter/packages#4080

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 25, 2023

2b105ac Fix ScrollPosition overscroll precision error (flutter/flutte…

c6bfe49

…r#127321)

engine-flutter-autoroll mentioned this pull request May 25, 2023

Roll Flutter from 216605b60f67 to a21f33e7ad2f (31 revisions) flutter/packages#4081

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 25, 2023

2b105ac Fix ScrollPosition overscroll precision error (flutter/flutte…

a29e2d8

…r#127321)

engine-flutter-autoroll mentioned this pull request May 25, 2023

Roll Flutter from 216605b60f67 to e583ba4cf0b1 (32 revisions) flutter/packages#4082

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 25, 2023

2b105ac Fix ScrollPosition overscroll precision error (flutter/flutte…

8071c64

…r#127321)

engine-flutter-autoroll mentioned this pull request May 25, 2023

Roll Flutter from 216605b60f67 to f71075db2eba (33 revisions) flutter/packages#4083

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 25, 2023

2b105ac Fix ScrollPosition overscroll precision error (flutter/flutte…

3cee727

…r#127321)

engine-flutter-autoroll mentioned this pull request May 25, 2023

Manual roll Flutter from 216605b60f67 to 08266bdfcf11 (38 revisions) flutter/packages#4084

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 25, 2023

2b105ac Fix ScrollPosition overscroll precision error (flutter/flutte…

dd49377

…r#127321)

reidbaker mentioned this pull request May 25, 2023

Support minifcation for apps that depend on AGP 8 and integration_test #127628

Merged

8 tasks

engine-flutter-autoroll mentioned this pull request May 26, 2023

Roll Flutter from 216605b60f67 to 369192ae5049 (52 revisions) flutter/packages#4088

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 26, 2023

2b105ac Fix ScrollPosition overscroll precision error (flutter/flutte…

e64209d

…r#127321)

engine-flutter-autoroll mentioned this pull request May 26, 2023

Roll Flutter from 216605b60f67 to ee162e4b7f4f (53 revisions) flutter/packages#4090

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 26, 2023

2b105ac Fix ScrollPosition overscroll precision error (flutter/flutte…

cb21bab

…r#127321)

reidbaker mentioned this pull request May 26, 2023

i127682 android api 33 test support #127689

Merged

8 tasks

HansMuller mentioned this pull request May 26, 2023

Flutter Framework Team: Notable Commits #121415

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 16, 2023

2b105ac Fix ScrollPosition overscroll precision error (flutter/flutte…

f92b06d

…r#127321)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 17, 2023

2b105ac Fix ScrollPosition overscroll precision error (flutter/flutte…

bb6a8a6

…r#127321)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 17, 2023

2b105ac Fix ScrollPosition overscroll precision error (flutter/flutte…

0ae6209

…r#127321)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 17, 2023

2b105ac Fix ScrollPosition overscroll precision error (flutter/flutte…

e84b9fd

…r#127321)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix ScrollPosition overscroll precision error #127321

Fix ScrollPosition overscroll precision error #127321

Uh oh!

bleroux commented May 22, 2023 •

edited

Loading

Uh oh!

Uh oh!

Piinks left a comment

Uh oh!

Uh oh!

nt4f04uNd May 23, 2023 •

edited

Loading

Uh oh!

bleroux May 24, 2023

Uh oh!

nt4f04uNd May 23, 2023

Uh oh!

Piinks May 24, 2023

Uh oh!

Piinks left a comment

Uh oh!

rydmike commented May 25, 2023

Uh oh!

bleroux commented Jun 2, 2023

Uh oh!

Piinks commented Jun 2, 2023

Uh oh!

absar commented Aug 10, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Fix ScrollPosition overscroll precision error #127321

Fix ScrollPosition overscroll precision error #127321

Uh oh!

Conversation

bleroux commented May 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issue

Tests

Uh oh!

Uh oh!

Piinks left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nt4f04uNd May 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bleroux May 24, 2023

Choose a reason for hiding this comment

Uh oh!

nt4f04uNd May 23, 2023

Choose a reason for hiding this comment

Uh oh!

Piinks May 24, 2023

Choose a reason for hiding this comment

Uh oh!

Piinks left a comment

Choose a reason for hiding this comment

Uh oh!

rydmike commented May 25, 2023

Uh oh!

bleroux commented Jun 2, 2023

Uh oh!

Piinks commented Jun 2, 2023

Uh oh!

absar commented Aug 10, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

bleroux commented May 22, 2023 •

edited

Loading

nt4f04uNd May 23, 2023 •

edited

Loading