Skip to content

Scroll protocol's physical "ballistic" invariant isn't documented #120341

Description

@gnprice

I wrote the following just now at #120338:

"""
The framework's approach to scrolling assumes that each ScrollPhysics implementation is restartable, or ballistic. The simulation is frequently restarted, and the ScrollPhysics is responsible for not letting that cause problems.

This is a lot like how the framework may frequently rebuild widgets, and if that affects behavior then that's a bug in the widget.

In the case of scrolling, this assumption is very natural because it will automatically be satisfied by any scroll physics so long as it obeys a reasonable ("ballistic") physical metaphor:

  • The thing that's scrolling is a physical object in motion.
  • Its velocity changes due to physical forces on it, according to Newton's second law.
  • Those forces depend only on (a) its current velocity and (b) where it is in its environment.

This metaphor works great for a wide variety of springs, frictional forces, clamping barriers, and other forms of scrolling physics one could dream up. Modulo small bugs (#109675 and #120340), it works great for all of the framework's scroll physics…

… all, that is, except one. The trouble is that one scroll physics, ClampingScrollPhysics, breaks this assumption. [Then that's the issue #120338 is about.]
"""

and added that this assumption doesn't seem to be clearly documented.

As discussed at #120338 (comment) , we might change this assumption in the future, and we've had previous efforts at doing so.

But for now this assumption is very much there. Fundamentally it stems from the fact that ScrollPhysics.createBallisticAnimation takes only (a) a velocity and (b) a ScrollMetrics, where the latter encodes the current position plus some facts like ScrollMetrics.maxScrollExtent that describe the physical environment. In particular, that method isn't provided the old simulation, or any further information from the old simulation like how long it had been running. So removing this assumption will be a significant change, and I think almost certainly a breaking one.

So, even if we eventually remove this assumption, we should first document it.

I plan to write up a PR to make this change. I think the documentation on ScrollMetrics.createBallisticAnimation may be the one place that needs to mention this requirement.

Activity

  1. added
    in triagePresently being triaged by the triage team
    frameworkflutter/packages/flutter repository. See also f: labels.
    f: scrollingViewports, list views, slivers, etc.
    d: api docsIssues with https://api.flutter.dev/
    and removed
    in triagePresently being triaged by the triage team
    on Feb 9, 2023
  2. added
    P2Important issues not at the top of the work list
    on Feb 14, 2023
  3. added a commit that references this issue on Feb 16, 2023
    09ad9f3
  4. github-actions commented on Mar 3, 2023

    @github-actions

    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 flutter doctor -v and a minimal reproduction of the issue.

  5. locked as resolved and limited conversation to collaborators on Mar 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    P2Important issues not at the top of the work listd: api docsIssues with https://api.flutter.dev/f: scrollingViewports, list views, slivers, etc.frameworkflutter/packages/flutter repository. See also f: labels.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions