Fix: Sometimes touch is ignored when scrollable sheet reaches edge (#209

) ## Fixes / Closes (optional) Fixes #207. ## Description This PR improves floating-point value comparisons throughout the codebase to address an existing issue and prevent potential problems related to floating-point precision errors. For example, #207 was caused by infinite recursion of `SheetContentScrollPositionOwner.goBallisticWithScrollPosition` calls, triggered by `ScrollMetrics.outOfRange` always being true in `ScrollPhysics.createBallisticSimulation` due to such a floating-point precision error. ## Summary (check all that apply) - [x] Modified / added code - [x] Modified / added tests - [ ] Modified / added examples - [ ] Modified / added others (pubspec.yaml, workflows, etc...) - [ ] Updated README - [ ] Contains breaking changes - [ ] Created / updated migration guide - [x] Incremented version number - [x] Updated CHANGELOG
fujidaiti · Jul 30, 2024 · 45c1a11 · 45c1a11
1 parent f05f544
commit 45c1a11
Show file tree

Hide file tree

Showing 13 changed files with 250 additions and 72 deletions.
diff --git a/package/CHANGELOG.md b/package/CHANGELOG.md
@@ -1,5 +1,9 @@
 # Changelog
 
+## 0.9.1 Jul 30, 2024
+
+- Fix: Sometimes touch is ignored when scrollable sheet reaches edge (#209)
+
 ## 0.9.0 Jul 24, 2024
 
 This version contains some breaking changes. See the [migration guide](https://github.com/fujidaiti/smooth_sheets/blob/main/docs/migration-guide-0.9.x.md) for more details.

diff --git a/package/lib/src/foundation/sheet_context.dart b/package/lib/src/foundation/sheet_context.dart
@@ -8,6 +8,7 @@ import 'sheet_extent.dart';
 abstract class SheetContext {
   TickerProvider get vsync;
   BuildContext? get notificationContext;
+  double get devicePixelRatio;
 }
 
 @internal
@@ -20,4 +21,8 @@ mixin SheetContextStateMixin<T extends StatefulWidget>
 
   @override
   BuildContext? get notificationContext => mounted ? context : null;
+
+  @override
+  double get devicePixelRatio =>
+      MediaQuery.maybeDevicePixelRatioOf(context) ?? 1.0;
 }
diff --git a/package/lib/src/foundation/sheet_extent.dart b/package/lib/src/foundation/sheet_extent.dart
@@ -5,7 +5,7 @@ import 'package:flutter/material.dart';
 import 'package:flutter/widgets.dart';
 import 'package:meta/meta.dart';
 
-import '../internal/double_utils.dart';
+import '../internal/float_comp.dart';
 import 'sheet_activity.dart';
 import 'sheet_context.dart';
 import 'sheet_controller.dart';
@@ -175,10 +175,6 @@ abstract class SheetExtent extends ChangeNotifier
   /// A label that is used to identify this object in debug output.
   final String? debugLabel;
 
-  /// Snapshot of the current sheet's state.
-  SheetMetrics get metrics => _metrics;
-  SheetMetrics _metrics = SheetMetrics.empty;
-
   /// The current activity of the sheet.
   SheetActivity get activity => _activity!;
   SheetActivity? _activity;
@@ -190,6 +186,34 @@ abstract class SheetExtent extends ChangeNotifier
   @protected
   SheetDragController? currentDrag;
 
+  /// Snapshot of the current sheet's state.
+  SheetMetrics get metrics => _metrics;
+  SheetMetrics _metrics = SheetMetrics.empty;
+
+  /// Updates the metrics with the given values.
+  ///
+  /// Use this method instead of directly updating the metrics
+  /// to ensure that the [SheetMetrics.devicePixelRatio] is always up-to-date.
+  void _updateMetrics({
+    double? pixels,
+    double? minPixels,
+    double? maxPixels,
+    Size? contentSize,
+    Size? viewportSize,
+    EdgeInsets? viewportInsets,
+  }) {
+    _metrics = SheetMetrics(
+      pixels: pixels ?? metrics.maybePixels,
+      minPixels: minPixels ?? metrics.maybeMinPixels,
+      maxPixels: maxPixels ?? metrics.maybeMaxPixels,
+      contentSize: contentSize ?? metrics.maybeContentSize,
+      viewportSize: viewportSize ?? metrics.maybeViewportSize,
+      viewportInsets: viewportInsets ?? metrics.maybeViewportInsets,
+      // Ensure that the devicePixelRatio is always up-to-date.
+      devicePixelRatio: context.devicePixelRatio,
+    );
+  }
+
   @mustCallSuper
   void takeOver(SheetExtent other) {
     assert(currentDrag == null);
@@ -239,7 +263,7 @@ abstract class SheetExtent extends ChangeNotifier
       final oldMaxPixels = metrics.maybeMaxPixels;
       final oldMinPixels = metrics.maybeMinPixels;
       _oldContentSize = metrics.maybeContentSize;
-      _metrics = metrics.copyWith(
+      _updateMetrics(
         contentSize: contentSize,
         minPixels: minExtent.resolve(contentSize),
         maxPixels: maxExtent.resolve(contentSize),
@@ -258,7 +282,7 @@ abstract class SheetExtent extends ChangeNotifier
         metrics.maybeViewportInsets != insets) {
       _oldViewportSize = metrics.maybeViewportSize;
       _oldViewportInsets = metrics.maybeViewportInsets;
-      _metrics = metrics.copyWith(viewportSize: size, viewportInsets: insets);
+      _updateMetrics(viewportSize: size, viewportInsets: insets);
       activity.didChangeViewportDimensions(
         _oldViewportSize,
         _oldViewportInsets,
@@ -278,10 +302,7 @@ abstract class SheetExtent extends ChangeNotifier
             newMaxPixels != metrics.maybeMaxPixels) {
           final oldMinPixels = metrics.maybeMinPixels;
           final oldMaxPixels = metrics.maybeMaxPixels;
-          _metrics = metrics.copyWith(
-            minPixels: newMinPixels,
-            maxPixels: newMaxPixels,
-          );
+          _updateMetrics(minPixels: newMinPixels, maxPixels: newMaxPixels);
           activity.didChangeBoundaryConstraints(oldMinPixels, oldMaxPixels);
         }
       }
@@ -481,7 +502,7 @@ abstract class SheetExtent extends ChangeNotifier
 
   void correctPixels(double pixels) {
     if (metrics.maybePixels != pixels) {
-      _metrics = metrics.copyWith(pixels: pixels);
+      _updateMetrics(pixels: pixels);
     }
   }
 
@@ -577,6 +598,7 @@ class SheetMetrics {
     required Size? contentSize,
     required Size? viewportSize,
     required EdgeInsets? viewportInsets,
+    this.devicePixelRatio = 1.0,
   })  : maybePixels = pixels,
         maybeMinPixels = minPixels,
         maybeMaxPixels = maxPixels,
@@ -600,6 +622,10 @@ class SheetMetrics {
   final Size? maybeViewportSize;
   final EdgeInsets? maybeViewportInsets;
 
+  /// The [FlutterView.devicePixelRatio] of the view that the sheet
+  /// associated with this metrics object is drawn into.
+  final double devicePixelRatio;
+
   /// The current extent of the sheet.
   double get pixels {
     assert(_debugAssertHasProperty('pixels', maybePixels));
@@ -667,7 +693,9 @@ class SheetMetrics {
   /// Whether the sheet is within the range of [minPixels] and [maxPixels]
   /// (inclusive of both bounds).
   bool get isPixelsInBounds =>
-      hasDimensions && pixels.isInBounds(minPixels, maxPixels);
+      hasDimensions &&
+      FloatComp.distance(devicePixelRatio)
+          .isInBounds(pixels, minPixels, maxPixels);
 
   /// Whether the sheet is outside the range of [minPixels] and [maxPixels].
   bool get isPixelsOutOfBounds => !isPixelsInBounds;
@@ -690,13 +718,13 @@ class SheetMetrics {
 
   /// Creates a copy of this object with the given fields replaced.
   SheetMetrics copyWith({
-    SheetStatus? status,
     double? pixels,
     double? minPixels,
     double? maxPixels,
     Size? contentSize,
     Size? viewportSize,
     EdgeInsets? viewportInsets,
+    double? devicePixelRatio,
   }) {
     return SheetMetrics(
       pixels: pixels ?? maybePixels,
@@ -705,6 +733,7 @@ class SheetMetrics {
       contentSize: contentSize ?? maybeContentSize,
       viewportSize: viewportSize ?? maybeViewportSize,
       viewportInsets: viewportInsets ?? maybeViewportInsets,
+      devicePixelRatio: devicePixelRatio ?? this.devicePixelRatio,
     );
   }
 
@@ -718,7 +747,8 @@ class SheetMetrics {
           maybeMaxPixels == other.maxPixels &&
           maybeContentSize == other.contentSize &&
           maybeViewportSize == other.viewportSize &&
-          maybeViewportInsets == other.viewportInsets);
+          maybeViewportInsets == other.viewportInsets &&
+          devicePixelRatio == other.devicePixelRatio);
 
   @override
   int get hashCode => Object.hash(
@@ -729,6 +759,7 @@ class SheetMetrics {
         maybeContentSize,
         maybeViewportSize,
         maybeViewportInsets,
+        devicePixelRatio,
       );
 
   @override
@@ -743,5 +774,6 @@ class SheetMetrics {
         contentSize: maybeContentSize,
         viewportSize: maybeViewportSize,
         viewportInsets: maybeViewportInsets,
+        devicePixelRatio: devicePixelRatio,
       ).toString();
 }
diff --git a/package/lib/src/foundation/sheet_physics.dart b/package/lib/src/foundation/sheet_physics.dart
@@ -3,9 +3,11 @@ import 'dart:math';
 import 'dart:ui';
 
 import 'package:flutter/gestures.dart';
+import 'package:flutter/physics.dart';
 import 'package:flutter/widgets.dart';
 
 import '../internal/double_utils.dart';
+import '../internal/float_comp.dart';
 import 'sheet_extent.dart';
 
 /// The default [SpringDescription] used by [SheetPhysics] subclasses.
@@ -165,6 +167,7 @@ class InterpolationSimulation extends Simulation {
     required this.end,
     required this.curve,
     required this.durationInSeconds,
+    super.tolerance,
   })  : assert(start != end),
         assert(durationInSeconds > 0);
 
@@ -194,7 +197,7 @@ class InterpolationSimulation extends Simulation {
 
   @override
   bool isDone(double time) {
-    return x(time).isApprox(end);
+    return nearEqual(x(time), end, tolerance.distance);
   }
 }
 
@@ -214,7 +217,8 @@ mixin _SnapToNearestMixin implements SnappingSheetBehavior {
   double? findSnapPixels(double velocity, SheetMetrics metrics) {
     assert(minFlingSpeed >= 0);
 
-    if (metrics.pixels.isOutOfBounds(metrics.minPixels, metrics.maxPixels)) {
+    if (FloatComp.distance(metrics.devicePixelRatio)
+        .isOutOfBounds(metrics.pixels, metrics.minPixels, metrics.maxPixels)) {
       return null;
     }
 
@@ -258,7 +262,8 @@ class SnapToNearestEdge with _SnapToNearestMixin {
 
   @override
   (double, double) _getSnapBoundsContains(SheetMetrics metrics) {
-    assert(metrics.pixels.isInBounds(metrics.minPixels, metrics.maxPixels));
+    assert(FloatComp.distance(metrics.devicePixelRatio)
+        .isInBounds(metrics.pixels, metrics.minPixels, metrics.maxPixels));
     return (metrics.minPixels, metrics.maxPixels);
   }
 }
@@ -291,8 +296,10 @@ class SnapToNearest with _SnapToNearestMixin {
         ..sort();
 
       assert(
-        _snapTo.first.isGreaterThanOrApprox(metrics.minPixels) &&
-            _snapTo.last.isLessThanOrApprox(metrics.maxPixels),
+        FloatComp.distance(metrics.devicePixelRatio)
+                .isGreaterThanOrApprox(_snapTo.first, metrics.minPixels) &&
+            FloatComp.distance(metrics.devicePixelRatio)
+                .isLessThanOrApprox(_snapTo.last, metrics.maxPixels),
         'The snap positions must be within the range of '
         "'SheetMetrics.minPixels' and 'SheetMetrics.maxPixels'.",
       );
@@ -309,7 +316,8 @@ class SnapToNearest with _SnapToNearestMixin {
     var nearestSmaller = _snapTo[0];
     var nearestGreater = _snapTo[1];
     for (var index = 0; index < _snapTo.length - 1; index++) {
-      if (_snapTo[index].isLessThan(metrics.pixels)) {
+      if (FloatComp.distance(metrics.devicePixelRatio)
+          .isLessThan(_snapTo[index], metrics.pixels)) {
         nearestSmaller = _snapTo[index];
         nearestGreater = _snapTo[index + 1];
       } else {
@@ -349,7 +357,9 @@ class SnappingSheetPhysics extends SheetPhysics with SheetPhysicsMixin {
   @override
   Simulation? createBallisticSimulation(double velocity, SheetMetrics metrics) {
     final snapPixels = snappingBehavior.findSnapPixels(velocity, metrics);
-    if (snapPixels != null && !metrics.pixels.isApprox(snapPixels)) {
+    if (snapPixels != null &&
+        FloatComp.distance(metrics.devicePixelRatio)
+            .isNotApprox(snapPixels, metrics.pixels)) {
       return ScrollSpringSimulation(
         spring,
         metrics.pixels,
@@ -532,7 +542,8 @@ class BouncingSheetPhysics extends SheetPhysics with SheetPhysicsMixin {
       _ => 0.0,
     };
 
-    if (zeroFrictionOffset.isApprox(offset) ||
+    if (FloatComp.distance(metrics.devicePixelRatio)
+            .isApprox(zeroFrictionOffset, offset) ||
         // The friction is also not applied if the motion
         // direction is towards the content bounds.
         (currentPixels > maxPixels && offset < 0) ||

diff --git a/package/lib/src/internal/double_utils.dart b/package/lib/src/internal/double_utils.dart
@@ -1,31 +1,12 @@
 import 'dart:math';
 
-import 'package:flutter/physics.dart';
-
 extension DoubleUtils on double {
-  bool isApprox(double value) =>
-      nearEqual(this, value, Tolerance.defaultTolerance.distance);
-
-  bool isLessThan(double value) => this < value && !isApprox(value);
-
-  bool isGreaterThan(double value) => this > value && !isApprox(value);
-
-  bool isLessThanOrApprox(double value) => isLessThan(value) || isApprox(value);
-
-  bool isGreaterThanOrApprox(double value) =>
-      isGreaterThan(value) || isApprox(value);
-
-  bool isOutOfBounds(double min, double max) =>
-      isLessThan(min) || isGreaterThan(max);
-
-  bool isInBounds(double min, double max) => !isOutOfBounds(min, max);
-
   double clampAbs(double norm) => min(max(-norm, this), norm);
 
   double nearest(double a, double b) =>
       (a - this).abs() < (b - this).abs() ? a : b;
-}
 
-double inverseLerp(double min, double max, double value) {
-  return min == max ? 1.0 : (value - min) / (max - min);
+  double inverseLerp(double min, double max) {
+    return min == max ? 1.0 : (this - min) / (max - min);
+  }
 }