Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: SuperSliverList exceeding max layout cycles #47

Merged
merged 2 commits into from
Mar 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion lib/src/render_object.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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!)!;
Expand All @@ -751,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);
Expand All @@ -761,6 +765,16 @@ class RenderSuperSliverList extends RenderSliverMultiBoxAdaptor
constraints.viewportMainAxisExtent) {
scrollCorrection += 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
Expand Down
34 changes: 34 additions & 0 deletions test/super_sliver_list_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -1287,6 +1317,8 @@ Widget _buildSliverList(
ScrollController? controller,
ListController? listController,
bool shrinkWrap = false,
ExtentEstimationProvider? extentEstimation,
bool delayPopulatingCacheArea = true,
}) {
return Directionality(
textDirection: TextDirection.ltr,
Expand Down Expand Up @@ -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(
Expand Down