diff --git a/packages/flutter/lib/src/rendering/sliver_list.dart b/packages/flutter/lib/src/rendering/sliver_list.dart index 458a32e672..5f577b816f 100644 --- a/packages/flutter/lib/src/rendering/sliver_list.dart +++ b/packages/flutter/lib/src/rendering/sliver_list.dart @@ -120,7 +120,6 @@ class RenderSliverList extends RenderSliverMultiBoxAdaptor { earliestScrollOffset = childScrollOffset(earliestUsefulChild)) { // We have to add children before the earliestUsefulChild. earliestUsefulChild = insertAndLayoutLeadingChild(childConstraints, parentUsesSize: true); - if (earliestUsefulChild == null) { final SliverMultiBoxAdaptorParentData childParentData = firstChild.parentData as SliverMultiBoxAdaptorParentData; childParentData.layoutOffset = 0.0; @@ -148,29 +147,14 @@ class RenderSliverList extends RenderSliverMultiBoxAdaptor { final double firstChildScrollOffset = earliestScrollOffset - paintExtentOf(firstChild); // firstChildScrollOffset may contain double precision error if (firstChildScrollOffset < -precisionErrorTolerance) { - // The first child doesn't fit within the viewport (underflow) and - // there may be additional children above it. Find the real first child - // and then correct the scroll position so that there's room for all and - // so that the trailing edge of the original firstChild appears where it - // was before the scroll offset correction. - // TODO(hansmuller): do this work incrementally, instead of all at once, - // i.e. find a way to avoid visiting ALL of the children whose offset - // is < 0 before returning for the scroll correction. - double correction = 0.0; - while (earliestUsefulChild != null) { - assert(firstChild == earliestUsefulChild); - correction += paintExtentOf(firstChild); - earliestUsefulChild = insertAndLayoutLeadingChild(childConstraints, parentUsesSize: true); - } - earliestUsefulChild = firstChild; - if ((correction - earliestScrollOffset).abs() > precisionErrorTolerance) { - geometry = SliverGeometry( - scrollOffsetCorrection: correction - earliestScrollOffset, - ); - final SliverMultiBoxAdaptorParentData childParentData = firstChild.parentData as SliverMultiBoxAdaptorParentData; - childParentData.layoutOffset = 0.0; - return; - } + // Let's assume there is no child before the first child. We will + // correct it on the next layout if it is not. + geometry = SliverGeometry( + scrollOffsetCorrection: -firstChildScrollOffset, + ); + final SliverMultiBoxAdaptorParentData childParentData = firstChild.parentData as SliverMultiBoxAdaptorParentData; + childParentData.layoutOffset = 0.0; + return; } final SliverMultiBoxAdaptorParentData childParentData = earliestUsefulChild.parentData as SliverMultiBoxAdaptorParentData; @@ -180,6 +164,28 @@ class RenderSliverList extends RenderSliverMultiBoxAdaptor { trailingChildWithLayout ??= earliestUsefulChild; } + assert(childScrollOffset(firstChild) > -precisionErrorTolerance); + + // If the scroll offset is at zero, we should make sure we are + // actually at the beginning of the list. + if (scrollOffset < precisionErrorTolerance) { + if (indexOf(firstChild) > 0) { + final double earliestScrollOffset = childScrollOffset(firstChild); + // We correct one child at a time. If there are more children before + // the earliestUsefulChild, we will correct it once the scroll offset + // reach zero again. + earliestUsefulChild = insertAndLayoutLeadingChild(childConstraints, parentUsesSize: true); + assert(earliestUsefulChild != null); + final double firstChildScrollOffset = earliestScrollOffset - paintExtentOf(firstChild); + geometry = SliverGeometry( + scrollOffsetCorrection: -firstChildScrollOffset, + ); + final SliverMultiBoxAdaptorParentData childParentData = firstChild.parentData as SliverMultiBoxAdaptorParentData; + childParentData.layoutOffset = 0.0; + return; + } + } + // At this point, earliestUsefulChild is the first child, and is a child // whose scrollOffset is at or before the scrollOffset, and // leadingChildWithLayout and trailingChildWithLayout are either null or diff --git a/packages/flutter/test/widgets/slivers_test.dart b/packages/flutter/test/widgets/slivers_test.dart index 412a98650d..b47d651ba5 100644 --- a/packages/flutter/test/widgets/slivers_test.dart +++ b/packages/flutter/test/widgets/slivers_test.dart @@ -308,6 +308,109 @@ void main() { }, ); + testWidgets( + 'SliverList can handle inaccurate scroll offset due to changes in children list', + (WidgetTester tester) async { + // Regression test for https://github.com/flutter/flutter/pull/59888. + bool skip = true; + Widget _buildItem(BuildContext context, int index) { + return !skip || index.isEven + ? Card( + child: ListTile( + title: Text( + 'item$index', + style: const TextStyle(fontSize: 80), + ), + ), + ) + : Container(); + } + await tester.pumpWidget( + MaterialApp( + home: Scaffold( + body: CustomScrollView( + slivers: [ + SliverList( + delegate: SliverChildBuilderDelegate( + _buildItem, + childCount: 30, + ), + ), + ], + ) + ), + ), + ); + // Only even items 0~12 are on the screen. + expect(find.text('item0'), findsOneWidget); + expect(find.text('item12'), findsOneWidget); + expect(find.text('item14'), findsNothing); + + await tester.drag(find.byType(CustomScrollView), const Offset(0.0, -750.0)); + await tester.pump(); + // Only even items 16~28 are on the screen. + expect(find.text('item15'), findsNothing); + expect(find.text('item16'), findsOneWidget); + expect(find.text('item28'), findsOneWidget); + + skip = false; + await tester.pumpWidget( + MaterialApp( + home: Scaffold( + body: CustomScrollView( + slivers: [ + SliverList( + delegate: SliverChildBuilderDelegate( + _buildItem, + childCount: 30, + ), + ), + ], + ) + ), + ), + ); + + // Only items 12~19 are on the screen. + expect(find.text('item11'), findsNothing); + expect(find.text('item12'), findsOneWidget); + expect(find.text('item19'), findsOneWidget); + expect(find.text('item20'), findsNothing); + + await tester.drag(find.byType(CustomScrollView), const Offset(0.0, 250.0)); + await tester.pump(); + + // Only items 10~16 are on the screen. + expect(find.text('item9'), findsNothing); + expect(find.text('item10'), findsOneWidget); + expect(find.text('item16'), findsOneWidget); + expect(find.text('item17'), findsNothing); + + // The inaccurate scroll offset should reach zero at this point + await tester.drag(find.byType(CustomScrollView), const Offset(0.0, 250.0)); + await tester.pump(); + + // Only items 7~13 are on the screen. + expect(find.text('item6'), findsNothing); + expect(find.text('item7'), findsOneWidget); + expect(find.text('item13'), findsOneWidget); + expect(find.text('item14'), findsNothing); + + // It will be corrected as we scroll, so we have to drag multiple times. + await tester.drag(find.byType(CustomScrollView), const Offset(0.0, 250.0)); + await tester.pump(); + await tester.drag(find.byType(CustomScrollView), const Offset(0.0, 250.0)); + await tester.pump(); + await tester.drag(find.byType(CustomScrollView), const Offset(0.0, 250.0)); + await tester.pump(); + + // Only items 0~6 are on the screen. + expect(find.text('item0'), findsOneWidget); + expect(find.text('item6'), findsOneWidget); + expect(find.text('item7'), findsNothing); + }, + ); + testWidgets( 'SliverFixedExtentList Correctly layout children after rearranging', (WidgetTester tester) async {