Implement PageView using SliverLayoutBuilder, Deprecate RenderSliverFillViewport (#37024)

packages/flutter/lib/src/rendering/sliver_fill.dart

@@ -27,6 +27,7 @@ import 'sliver_multi_box_adaptor.dart';
 ///  * [RenderSliverFixedExtentList], which has a configurable [itemExtent].
 ///  * [RenderSliverList], which does not require its children to have the same
 ///    extent in the main axis.
+@Deprecated('Use SliverLayoutBuilder instead.')
 class RenderSliverFillViewport extends RenderSliverFixedExtentBoxAdaptor {
   /// Creates a sliver that contains multiple box children that each fill the
   /// viewport.
@@ -105,8 +106,6 @@ class RenderSliverFillViewport extends RenderSliverFixedExtentBoxAdaptor {
 ///
 /// See also:
 ///
-///  * [RenderSliverFillViewport], which sizes its children based on the
-///    size of the viewport, regardless of what else is in the scroll view.
 ///  * [RenderSliverList], which shows a list of variable-sized children in a
 ///    viewport.
 class RenderSliverFillRemaining extends RenderSliverSingleBoxAdapter {

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

@@ -930,8 +930,8 @@ class RawGestureDetectorState extends State<RawGestureDetector> {
     }
   }
 
-  /// This method can be called outside of the build phase to filter the list of
+  /// This method can be called to filter the list of available semantic actions,
-  /// available semantic actions.
+  /// after the render object was created.
   ///
   /// The actual filtering is happening in the next frame and a frame will be
   /// scheduled if non is pending.
@@ -942,20 +942,21 @@ class RawGestureDetectorState extends State<RawGestureDetector> {
   /// If this is never called, then the actions are not filtered. If the list of
   /// actions to filter changes, it must be called again.
   void replaceSemanticsActions(Set<SemanticsAction> actions) {
+    if (widget.excludeFromSemantics)
+      return;
+
+    final RenderSemanticsGestureHandler semanticsGestureHandler = context.findRenderObject();
     assert(() {
-      final Element element = context;
+      if (semanticsGestureHandler == null) {
-      if (element.owner.debugBuilding) {
         throw FlutterError(
           'Unexpected call to replaceSemanticsActions() method of RawGestureDetectorState.\n'
-          'The replaceSemanticsActions() method can only be called outside of the build phase.'
+          'The replaceSemanticsActions() method can only be called after the RenderSemanticsGestureHandler has been created.'
         );
       }
       return true;
     }());
-    if (!widget.excludeFromSemantics) {
+
-      final RenderSemanticsGestureHandler semanticsGestureHandler = context.findRenderObject();
+    semanticsGestureHandler.validActions = actions; // will call _markNeedsSemanticsUpdate(), if required.
-      semanticsGestureHandler.validActions = actions; // will call _markNeedsSemanticsUpdate(), if required.
-    }
   }
 
   @override

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

@@ -368,8 +368,16 @@ class _PagePosition extends ScrollPositionWithSingleContext implements PageMetri
       forcePixels(getPixelsFromPage(oldPage));
   }
 
+  // The amount of offset that will be added to [minScrollExtent] and subtracted
+  // from [maxScrollExtent], such that every page will properly snap to the center
+  // of the viewport when viewportFraction is greater than 1.
+  //
+  // The value is 0 if viewportFraction is less than or equal to 1, larger than 0
+  // otherwise.
+  double get _initialPageOffset => math.max(0, viewportDimension * (viewportFraction - 1) / 2);
+
   double getPageFromPixels(double pixels, double viewportDimension) {
-    final double actual = math.max(0.0, pixels) / math.max(1.0, viewportDimension * viewportFraction);
+    final double actual = math.max(0, pixels - _initialPageOffset) / math.max(1.0, viewportDimension * viewportFraction);
     final double round = actual.roundToDouble();
     if ((actual - round).abs() < precisionErrorTolerance) {
       return round;
@@ -378,7 +386,7 @@ class _PagePosition extends ScrollPositionWithSingleContext implements PageMetri
   }
 
   double getPixelsFromPage(double page) {
-    return page * viewportDimension * viewportFraction;
+    return page * viewportDimension * viewportFraction + _initialPageOffset;
   }
 
   @override
@@ -420,6 +428,15 @@ class _PagePosition extends ScrollPositionWithSingleContext implements PageMetri
     return result;
   }
 
+  @override
+  bool applyContentDimensions(double minScrollExtent, double maxScrollExtent) {
+    final double newMinScrollExtent = minScrollExtent + _initialPageOffset;
+    return super.applyContentDimensions(
+      newMinScrollExtent,
+      math.max(newMinScrollExtent, maxScrollExtent - _initialPageOffset),
+    );
+  }
+
   @override
   PageMetrics copyWith({
     double minScrollExtent,

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

@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 import 'dart:collection' show SplayTreeMap, HashMap;
+import 'dart:math' as math show max;
 
 import 'package:flutter/foundation.dart';
 import 'package:flutter/rendering.dart';
@@ -10,6 +11,7 @@ import 'package:flutter/rendering.dart';
 import 'automatic_keep_alive.dart';
 import 'basic.dart';
 import 'framework.dart';
+import 'sliver_layout_builder.dart';
 
 export 'package:flutter/rendering.dart' show
   SliverGridDelegate,
@@ -717,6 +719,7 @@ abstract class SliverMultiBoxAdaptorWidget extends SliverWithKeepAliveWidget {
   }) : assert(delegate != null),
        super(key: key);
 
+  /// {@template flutter.widgets.sliverChildDelegate}
   /// The delegate that provides the children for this widget.
   ///
   /// The children are constructed lazily using this widget to avoid creating
@@ -727,6 +730,7 @@ abstract class SliverMultiBoxAdaptorWidget extends SliverWithKeepAliveWidget {
   ///  * [SliverChildBuilderDelegate] and [SliverChildListDelegate], which are
   ///    commonly used subclasses of [SliverChildDelegate] that use a builder
   ///    callback and an explicit child list, respectively.
+  /// {@endtemplate}
   final SliverChildDelegate delegate;
 
   @override
@@ -1030,15 +1034,16 @@ class SliverGrid extends SliverMultiBoxAdaptorWidget {
 ///    the main axis extent of each item.
 ///  * [SliverList], which does not require its children to have the same
 ///    extent in the main axis.
-class SliverFillViewport extends SliverMultiBoxAdaptorWidget {
+class SliverFillViewport extends StatelessWidget {
   /// Creates a sliver whose box children that each fill the viewport.
   const SliverFillViewport({
     Key key,
-    @required SliverChildDelegate delegate,
+    @required this.delegate,
     this.viewportFraction = 1.0,
   }) : assert(viewportFraction != null),
        assert(viewportFraction > 0.0),
-       super(key: key, delegate: delegate);
+       assert(delegate != null),
+       super(key: key);
 
   /// The fraction of the viewport that each child should fill in the main axis.
   ///
@@ -1047,15 +1052,34 @@ class SliverFillViewport extends SliverMultiBoxAdaptorWidget {
   /// the viewport in the main axis.
   final double viewportFraction;
 
-  @override
+  /// {@macro flutter.widgets.sliverChildDelegate}
-  RenderSliverFillViewport createRenderObject(BuildContext context) {
+  final SliverChildDelegate delegate;
-    final SliverMultiBoxAdaptorElement element = context;
-    return RenderSliverFillViewport(childManager: element, viewportFraction: viewportFraction);
-  }
 
   @override
-  void updateRenderObject(BuildContext context, RenderSliverFillViewport renderObject) {
+  Widget build(BuildContext context) {
-    renderObject.viewportFraction = viewportFraction;
+    return SliverLayoutBuilder(
+      builder: (BuildContext context, SliverConstraints constraints) {
+        final double fixedExtent = constraints.viewportMainAxisExtent * viewportFraction;
+        final double padding = math.max(0, constraints.viewportMainAxisExtent - fixedExtent) / 2;
+
+        EdgeInsets sliverPaddingValue;
+        switch (constraints.axis) {
+          case Axis.horizontal:
+            sliverPaddingValue = EdgeInsets.symmetric(horizontal: padding);
+            break;
+          case Axis.vertical:
+            sliverPaddingValue = EdgeInsets.symmetric(vertical: padding);
+        }
+
+        return SliverPadding(
+          padding: sliverPaddingValue,
+          sliver: SliverFixedExtentList(
+            delegate: delegate,
+            itemExtent: fixedExtent,
+          ),
+        );
+      }
+    );
   }
 }
 

packages/flutter/test/rendering/sliver_fixed_extent_layout_test.dart

@@ -24,7 +24,7 @@ void main() {
       offset: ViewportOffset.zero(),
       cacheExtent: 0,
       children: <RenderSliver>[
-        childManager.createRenderSliverFillViewport(),
+        childManager.createRenderSliverFixedExtentList(),
       ],
     );
     layout(root);
@@ -52,10 +52,11 @@ class TestRenderSliverBoxChildManager extends RenderSliverBoxChildManager {
   RenderSliverMultiBoxAdaptor _renderObject;
   List<RenderBox> children;
 
-  RenderSliverFillViewport createRenderSliverFillViewport() {
+  RenderSliverFixedExtentList createRenderSliverFixedExtentList() {
     assert(_renderObject == null);
-    _renderObject = RenderSliverFillViewport(
+    _renderObject = RenderSliverFixedExtentList(
       childManager: this,
+      itemExtent: 600,
     );
     return _renderObject;
   }

packages/flutter/test/widgets/page_view_test.dart

@@ -394,8 +394,8 @@ void main() {
             return Container(
               height: 200.0,
               color: index % 2 == 0
-                  ? const Color(0xFF0000FF)
+                ? const Color(0xFF0000FF)
-                  : const Color(0xFF00FF00),
+                : const Color(0xFF00FF00),
               child: Text(kStates[index]),
             );
           },
@@ -500,8 +500,8 @@ void main() {
             return Container(
               height: 200.0,
               color: index % 2 == 0
-                  ? const Color(0xFF0000FF)
+                ? const Color(0xFF0000FF)
-                  : const Color(0xFF00FF00),
+                : const Color(0xFF00FF00),
               child: Text(kStates[index]),
             );
           },
@@ -545,8 +545,8 @@ void main() {
             return Container(
               height: 200.0,
               color: index % 2 == 0
-                  ? const Color(0xFF0000FF)
+                ? const Color(0xFF0000FF)
-                  : const Color(0xFF00FF00),
+                : const Color(0xFF00FF00),
               child: Text(kStates[index]),
             );
           },
@@ -565,6 +565,88 @@ void main() {
     expect(tester.getTopLeft(find.text('Hawaii')), const Offset(-100.0, 0.0));
   });
 
+  testWidgets(
+    'Updating PageView large viewportFraction',
+    (WidgetTester tester) async {
+      Widget build(PageController controller) {
+        return Directionality(
+          textDirection: TextDirection.ltr,
+          child: PageView.builder(
+            controller: controller,
+            itemCount: kStates.length,
+            itemBuilder: (BuildContext context, int index) {
+              return Container(
+                height: 200.0,
+                color: index % 2 == 0
+                  ? const Color(0xFF0000FF)
+                  : const Color(0xFF00FF00),
+                child: Text(kStates[index]),
+              );
+            },
+          ),
+        );
+      }
+
+      final PageController oldController = PageController(viewportFraction: 5/4);
+      await tester.pumpWidget(build(oldController));
+
+      expect(tester.getTopLeft(find.text('Alabama')), const Offset(-100, 0));
+      expect(tester.getBottomRight(find.text('Alabama')), const Offset(900.0, 600.0));
+
+      final PageController newController = PageController(viewportFraction: 4);
+      await tester.pumpWidget(build(newController));
+      newController.jumpToPage(10);
+      await tester.pump();
+
+      expect(tester.getTopLeft(find.text('Hawaii')), const Offset(-(4 - 1) * 800 / 2, 0));
+  });
+
+  testWidgets(
+    'All visible pages are able to receive touch events',
+    (WidgetTester tester) async {
+      final PageController controller = PageController(viewportFraction: 1/4, initialPage: 0);
+      int tappedIndex;
+
+      Widget build() {
+        return Directionality(
+          textDirection: TextDirection.ltr,
+          child: PageView.builder(
+            controller: controller,
+            itemCount: 20,
+            itemBuilder: (BuildContext context, int index) {
+              return GestureDetector(
+                onTap: () => tappedIndex = index,
+                child: SizedBox.expand(child: Text('$index')),
+              );
+            },
+          ),
+        );
+      }
+
+      Iterable<int> visiblePages = const <int> [0, 1, 2];
+      await tester.pumpWidget(build());
+
+      // The first 3 items should be visible and tappable.
+      for (int index in visiblePages) {
+        expect(find.text(index.toString()), findsOneWidget);
+        // The center of page 2's x-coordinate is 800, so we have to manually
+        // offset it a bit to make sure the tap lands within the screen.
+        final Offset center = tester.getCenter(find.text('$index')) - const Offset(3, 0);
+        await tester.tapAt(center);
+        expect(tappedIndex, index);
+      }
+
+      controller.jumpToPage(19);
+      await tester.pump();
+      // The last 3 items should be visible and tappable.
+      visiblePages = const <int> [17, 18, 19];
+      for (int index in visiblePages) {
+        expect(find.text('$index'), findsOneWidget);
+        await tester.tap(find.text('$index'));
+        expect(tappedIndex, index);
+      }
+  });
+
   testWidgets('PageView does not report page changed on overscroll', (WidgetTester tester) async {
     final PageController controller = PageController(
       initialPage: kStates.length - 1,

packages/flutter/test/widgets/sliver_fill_viewport_test.dart

@@ -64,7 +64,7 @@ void main() {
     expect(
       viewport.toStringDeep(minLevel: DiagnosticLevel.info),
       equalsIgnoringHashCodes(
-        'RenderSliverFillViewport#00000 relayoutBoundary=up1\n'
+        '_RenderSliverLayoutBuilder#00000 relayoutBoundary=up1\n'
         ' │ needs compositing\n'
         ' │ parentData: paintOffset=Offset(0.0, 0.0) (can use size)\n'
         ' │ constraints: SliverConstraints(AxisDirection.down,\n'
@@ -76,58 +76,86 @@ void main() {
         ' │ geometry: SliverGeometry(scrollExtent: 12000.0, paintExtent:\n'
         ' │   600.0, maxPaintExtent: 12000.0, hasVisualOverflow: true,\n'
         ' │   cacheExtent: 850.0)\n'
-        ' │ currently live children: 0 to 1\n'
         ' │\n'
-        ' ├─child with index 0: RenderRepaintBoundary#00000\n'
+        ' └─child: RenderSliverPadding#00000 relayoutBoundary=up2\n'
-        ' │ │ needs compositing\n'
-        ' │ │ parentData: index=0; layoutOffset=0.0\n'
-        ' │ │ constraints: BoxConstraints(w=800.0, h=600.0)\n'
-        ' │ │ layer: OffsetLayer#00000\n'
-        ' │ │ size: Size(800.0, 600.0)\n'
-        ' │ │ metrics: 66.7% useful (1 bad vs 2 good)\n'
-        ' │ │ diagnosis: insufficient data to draw conclusion (less than five\n'
-        ' │ │   repaints)\n'
-        ' │ │\n'
-        ' │ └─child: RenderParagraph#00000\n'
-        ' │   │ parentData: <none> (can use size)\n'
-        ' │   │ constraints: BoxConstraints(w=800.0, h=600.0)\n'
-        ' │   │ semantics node: SemanticsNode#2\n'
-        ' │   │ size: Size(800.0, 600.0)\n'
-        ' │   │ textAlign: start\n'
-        ' │   │ textDirection: ltr\n'
-        ' │   │ softWrap: wrapping at box width\n'
-        ' │   │ overflow: clip\n'
-        ' │   │ maxLines: unlimited\n'
-        ' │   ╘═╦══ text ═══\n'
-        ' │     ║ TextSpan:\n'
-        ' │     ║   <all styles inherited>\n'
-        ' │     ║   "0"\n'
-        ' │     ╚═══════════\n'
-        ' └─child with index 1: RenderRepaintBoundary#00000\n'
         '   │ needs compositing\n'
-        '   │ parentData: index=1; layoutOffset=600.0\n'
+        '   │ parentData: <none> (can use size)\n'
-        '   │ constraints: BoxConstraints(w=800.0, h=600.0)\n'
+        '   │ constraints: SliverConstraints(AxisDirection.down,\n'
-        '   │ layer: OffsetLayer#00000 DETACHED\n'
+        '   │   GrowthDirection.forward, ScrollDirection.idle, scrollOffset:\n'
-        '   │ size: Size(800.0, 600.0)\n'
+        '   │   0.0, remainingPaintExtent: 600.0, crossAxisExtent: 800.0,\n'
-        '   │ metrics: 50.0% useful (1 bad vs 1 good)\n'
+        '   │   crossAxisDirection: AxisDirection.right,\n'
-        '   │ diagnosis: insufficient data to draw conclusion (less than five\n'
+        '   │   viewportMainAxisExtent: 600.0, remainingCacheExtent: 850.0\n'
-        '   │   repaints)\n'
+        '   │   cacheOrigin: 0.0 )\n'
+        '   │ geometry: SliverGeometry(scrollExtent: 12000.0, paintExtent:\n'
+        '   │   600.0, maxPaintExtent: 12000.0, hasVisualOverflow: true,\n'
+        '   │   cacheExtent: 850.0)\n'
+        '   │ padding: EdgeInsets.zero\n'
+        '   │ textDirection: ltr\n'
         '   │\n'
-        '   └─child: RenderParagraph#00000\n'
+        '   └─child: RenderSliverFixedExtentList#00000 relayoutBoundary=up3\n'
-        '     │ parentData: <none> (can use size)\n'
+        '     │ needs compositing\n'
-        '     │ constraints: BoxConstraints(w=800.0, h=600.0)\n'
+        '     │ parentData: paintOffset=Offset(0.0, 0.0) (can use size)\n'
-        '     │ semantics node: SemanticsNode#3\n'
+        '     │ constraints: SliverConstraints(AxisDirection.down,\n'
-        '     │ size: Size(800.0, 600.0)\n'
+        '     │   GrowthDirection.forward, ScrollDirection.idle, scrollOffset:\n'
-        '     │ textAlign: start\n'
+        '     │   0.0, remainingPaintExtent: 600.0, crossAxisExtent: 800.0,\n'
-        '     │ textDirection: ltr\n'
+        '     │   crossAxisDirection: AxisDirection.right,\n'
-        '     │ softWrap: wrapping at box width\n'
+        '     │   viewportMainAxisExtent: 600.0, remainingCacheExtent: 850.0\n'
-        '     │ overflow: clip\n'
+        '     │   cacheOrigin: 0.0 )\n'
-        '     │ maxLines: unlimited\n'
+        '     │ geometry: SliverGeometry(scrollExtent: 12000.0, paintExtent:\n'
-        '     ╘═╦══ text ═══\n'
+        '     │   600.0, maxPaintExtent: 12000.0, hasVisualOverflow: true,\n'
-        '       ║ TextSpan:\n'
+        '     │   cacheExtent: 850.0)\n'
-        '       ║   <all styles inherited>\n'
+        '     │ currently live children: 0 to 1\n'
-        '       ║   "1"\n'
+        '     │\n'
-        '       ╚═══════════\n'
+        '     ├─child with index 0: RenderRepaintBoundary#00000\n'
+        '     │ │ needs compositing\n'
+        '     │ │ parentData: index=0; layoutOffset=0.0\n'
+        '     │ │ constraints: BoxConstraints(w=800.0, h=600.0)\n'
+        '     │ │ layer: OffsetLayer#00000\n'
+        '     │ │ size: Size(800.0, 600.0)\n'
+        '     │ │ metrics: 66.7% useful (1 bad vs 2 good)\n'
+        '     │ │ diagnosis: insufficient data to draw conclusion (less than five\n'
+        '     │ │   repaints)\n'
+        '     │ │\n'
+        '     │ └─child: RenderParagraph#00000\n'
+        '     │   │ parentData: <none> (can use size)\n'
+        '     │   │ constraints: BoxConstraints(w=800.0, h=600.0)\n'
+        '     │   │ semantics node: SemanticsNode#2\n'
+        '     │   │ size: Size(800.0, 600.0)\n'
+        '     │   │ textAlign: start\n'
+        '     │   │ textDirection: ltr\n'
+        '     │   │ softWrap: wrapping at box width\n'
+        '     │   │ overflow: clip\n'
+        '     │   │ maxLines: unlimited\n'
+        '     │   ╘═╦══ text ═══\n'
+        '     │     ║ TextSpan:\n'
+        '     │     ║   <all styles inherited>\n'
+        '     │     ║   "0"\n'
+        '     │     ╚═══════════\n'
+        '     └─child with index 1: RenderRepaintBoundary#00000\n'
+        '       │ needs compositing\n'
+        '       │ parentData: index=1; layoutOffset=600.0\n'
+        '       │ constraints: BoxConstraints(w=800.0, h=600.0)\n'
+        '       │ layer: OffsetLayer#00000 DETACHED\n'
+        '       │ size: Size(800.0, 600.0)\n'
+        '       │ metrics: 50.0% useful (1 bad vs 1 good)\n'
+        '       │ diagnosis: insufficient data to draw conclusion (less than five\n'
+        '       │   repaints)\n'
+        '       │\n'
+        '       └─child: RenderParagraph#00000\n'
+        '         │ parentData: <none> (can use size)\n'
+        '         │ constraints: BoxConstraints(w=800.0, h=600.0)\n'
+        '         │ semantics node: SemanticsNode#3\n'
+        '         │ size: Size(800.0, 600.0)\n'
+        '         │ textAlign: start\n'
+        '         │ textDirection: ltr\n'
+        '         │ softWrap: wrapping at box width\n'
+        '         │ overflow: clip\n'
+        '         │ maxLines: unlimited\n'
+        '         ╘═╦══ text ═══\n'
+        '           ║ TextSpan:\n'
+        '           ║   <all styles inherited>\n'
+        '           ║   "1"\n'
+        '           ╚═══════════\n'
         ''
       ),
     );

packages/flutter/test/widgets/slivers_keepalive_test.dart

@@ -462,7 +462,7 @@ class _SwitchingChildBuilderTest extends State<SwitchingChildBuilderTest> {
                   childCount: children.length,
                   findChildIndexCallback: (Key key) => _mapKeyToIndex[key] ?? -1,
                 ),
-              )
+              ),
             ],
           ),
         ),