Unverified Commit 4953cbce authored by chunhtai's avatar chunhtai Committed by GitHub

Fix the layout calculation in sliver list where the scroll offset cor… (#59888)

parent e34c5912
......@@ -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,30 +147,15 @@ 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) {
// 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: correction - earliestScrollOffset,
scrollOffsetCorrection: -firstChildScrollOffset,
);
final SliverMultiBoxAdaptorParentData childParentData = firstChild.parentData as SliverMultiBoxAdaptorParentData;
childParentData.layoutOffset = 0.0;
return;
}
}
final SliverMultiBoxAdaptorParentData childParentData = earliestUsefulChild.parentData as SliverMultiBoxAdaptorParentData;
childParentData.layoutOffset = firstChildScrollOffset;
......@@ -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
......
......@@ -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: <Widget> [
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: <Widget> [
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 {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment