From aaa2875ca95eea37090630aec563cb863ff872e0 Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Tue, 19 Mar 2024 13:49:10 +0100 Subject: [PATCH 1/2] fix: SuperSliverList exceeding max layout cycles Happens when scrolling at the end of list when items are smaller than estimation --- lib/src/render_object.dart | 6 +++++- test/super_sliver_list_test.dart | 34 ++++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/lib/src/render_object.dart b/lib/src/render_object.dart index 5213f91..999ec8a 100644 --- a/lib/src/render_object.dart +++ b/lib/src/render_object.dart @@ -734,9 +734,12 @@ class RenderSuperSliverList extends RenderSliverMultiBoxAdaptor childScrollOffset(firstVisible)! - firstVisibleLayoutOffset!; } + // Start offset with applied scroll correction. + var correctedStartOffset = startOffset + scrollCorrection; + // Adding preceding children. while ((indexOf(firstChild!) > 0 && - childScrollOffset(firstChild!)! > startOffset + scrollCorrection) || + childScrollOffset(firstChild!)! > correctedStartOffset) || (_childScrollOffsetEstimation != null && indexOf(firstChild!) > _childScrollOffsetEstimation!.index)) { final prevOffset = childScrollOffset(firstChild!)!; @@ -761,6 +764,7 @@ class RenderSuperSliverList extends RenderSliverMultiBoxAdaptor constraints.viewportMainAxisExtent) { scrollCorrection += correction; } + correctedStartOffset += correction; } // Additional correction: first child is not at the very beginning diff --git a/test/super_sliver_list_test.dart b/test/super_sliver_list_test.dart index 32439d1..30e2742 100644 --- a/test/super_sliver_list_test.dart +++ b/test/super_sliver_list_test.dart @@ -516,6 +516,7 @@ void main() async { // Failing this likely means the child manager is not aware of underflow. expect(find.text("Tile 0:0"), findsOneWidget); }); + testWidgets("shrinkWrap", (tester) async { final list = SliverListConfiguration.generate( slivers: 3, @@ -534,6 +535,7 @@ void main() async { final frames = await tester.pumpAndSettle(); expect(frames, 1); }); + testWidgets("visible range", (tester) async { final list = SliverListConfiguration.generate( slivers: 3, @@ -617,6 +619,7 @@ void main() async { expect(list.slivers[1].listController.unobstructedVisibleRange, equals((1, 4))); }); + testWidgets("kept alive widgets are laid out", (tester) async { final key1 = GlobalKey(); final key2 = GlobalKey(); @@ -696,6 +699,7 @@ void main() async { expect(position, equals(const Offset(0, -920))); } }); + testWidgets("delay populating cache area enabled", (tester) async { final keys0 = List.generate(50, (index) => GlobalKey()); final keys1 = List.generate(1, (index) => GlobalKey()); @@ -778,6 +782,7 @@ void main() async { controller.dispose(); }); + testWidgets("delay populating cache area disabled", (tester) async { final keys = List.generate(50, (index) => GlobalKey()); final controller = ScrollController(); @@ -834,6 +839,31 @@ void main() async { expect(keys[25].currentContext, isNotNull); controller.dispose(); }); + + testWidgets("does not exceed layout cycles", (tester) async { + final configuration = SliverListConfiguration.generate( + slivers: 1, + itemsPerSliver: (_) => 50, + itemHeight: (_, __) => 30, + viewportHeight: 500, + ); + final ScrollController controller = + ScrollController(initialScrollOffset: 0); + addTearDown(controller.dispose); + + await tester.pumpWidget(_buildSliverList( + configuration, + preciseLayout: false, + controller: controller, + extentEstimation: (_, __) => 200, + delayPopulatingCacheArea: false, + )); + await tester.pumpAndSettle(); + + // This must not cause maximum layout cycle exceeded exception. + controller.jumpTo(controller.position.maxScrollExtent); + await tester.pumpAndSettle(); + }); }); group("Fuzzer", () { testWidgets("layout multiple slivers scrolling down", (tester) async { @@ -1287,6 +1317,8 @@ Widget _buildSliverList( ScrollController? controller, ListController? listController, bool shrinkWrap = false, + ExtentEstimationProvider? extentEstimation, + bool delayPopulatingCacheArea = true, }) { return Directionality( textDirection: TextDirection.ltr, @@ -1321,7 +1353,9 @@ Widget _buildSliverList( extentPrecalculationPolicy: _SimpleExtentPrecalculatePolicy( precalculate: preciseLayout, ), + extentEstimation: extentEstimation, listController: listController ?? sliver.listController, + delayPopulatingCacheArea: delayPopulatingCacheArea, delegate: SliverChildBuilderDelegate( (BuildContext context, int i) { return SizedBox( From 969fdceb28b84c7bbf6ccad8699270e07c05aaab Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Tue, 19 Mar 2024 14:40:53 +0100 Subject: [PATCH 2/2] ensure cache area reaches first child --- lib/src/render_object.dart | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/lib/src/render_object.dart b/lib/src/render_object.dart index 999ec8a..16a38e6 100644 --- a/lib/src/render_object.dart +++ b/lib/src/render_object.dart @@ -754,6 +754,7 @@ class RenderSuperSliverList extends RenderSliverMultiBoxAdaptor final correction = paintExtentOf(box) - previousExtent; _log.finest( "Adding preceding child with index ${indexOf(box)} " + "extent: ${paintExtentOf(box)} " "(${correction.format()} correction)", ); _shiftLayoutOffsets(childAfter(box), correction); @@ -764,7 +765,16 @@ class RenderSuperSliverList extends RenderSliverMultiBoxAdaptor constraints.viewportMainAxisExtent) { scrollCorrection += correction; } - correctedStartOffset += correction; + + // If start offset is 0, we must end up at initial item, so do not correct the + // start offset. + // If start offset > 0 we care about filling up the cache area, so correct the + // offset depending on the extent of the child. TODO(knopp): It might be enough + // to just ensure that we have a single completely hidden child in the cache + // area. See https://github.com/flutter/flutter/issues/128601 + if (startOffset > 0) { + correctedStartOffset += correction; + } } // Additional correction: first child is not at the very beginning