Unverified Commit 3998549d authored by chunhtai's avatar chunhtai Committed by GitHub

Revert "fixes sliver list child layout offset calculation (#42703)" (#52923)

This reverts commit 11efa00f.
parent ec8448d0
...@@ -928,16 +928,13 @@ class SliverLogicalParentData extends ParentData { ...@@ -928,16 +928,13 @@ class SliverLogicalParentData extends ParentData {
/// ///
/// The number of pixels from from the zero scroll offset of the parent sliver /// The number of pixels from from the zero scroll offset of the parent sliver
/// (the line at which its [SliverConstraints.scrollOffset] is zero) to the /// (the line at which its [SliverConstraints.scrollOffset] is zero) to the
/// side of the child closest to that offset. A [layoutOffset] can be null /// side of the child closest to that offset.
/// when it cannot be determined. The value will be set after layout.
/// ///
/// In a typical list, this does not change as the parent is scrolled. /// In a typical list, this does not change as the parent is scrolled.
/// double layoutOffset = 0.0;
/// Defaults to null.
double layoutOffset;
@override @override
String toString() => 'layoutOffset=${layoutOffset == null ? 'None': layoutOffset.toStringAsFixed(1)}'; String toString() => 'layoutOffset=${layoutOffset.toStringAsFixed(1)}';
} }
/// Parent data for slivers that have multiple children and that position their /// Parent data for slivers that have multiple children and that position their
......
...@@ -91,28 +91,8 @@ class RenderSliverList extends RenderSliverMultiBoxAdaptor { ...@@ -91,28 +91,8 @@ class RenderSliverList extends RenderSliverMultiBoxAdaptor {
// it's possible for a child to get removed without notice. // it's possible for a child to get removed without notice.
RenderBox leadingChildWithLayout, trailingChildWithLayout; RenderBox leadingChildWithLayout, trailingChildWithLayout;
RenderBox earliestUsefulChild = firstChild;
// A firstChild with null layout offset is likely a result of children
// reordering.
//
// We rely on firstChild to have accurate layout offset. In the case of null
// layout offset, we have to find the first child that has valid layout
// offset.
if (childScrollOffset(firstChild) == null) {
int leadingChildrenWithoutLayoutOffset = 0;
while (childScrollOffset(earliestUsefulChild) == null) {
earliestUsefulChild = childAfter(firstChild);
leadingChildrenWithoutLayoutOffset += 1;
}
// We should be able to destroy children with null layout offset safely,
// because they are likely outside of viewport
collectGarbage(leadingChildrenWithoutLayoutOffset, 0);
assert(firstChild != null);
}
// Find the last child that is at or before the scrollOffset. // Find the last child that is at or before the scrollOffset.
earliestUsefulChild = firstChild; RenderBox earliestUsefulChild = firstChild;
for (double earliestScrollOffset = childScrollOffset(earliestUsefulChild); for (double earliestScrollOffset = childScrollOffset(earliestUsefulChild);
earliestScrollOffset > scrollOffset; earliestScrollOffset > scrollOffset;
earliestScrollOffset = childScrollOffset(earliestUsefulChild)) { earliestScrollOffset = childScrollOffset(earliestUsefulChild)) {
...@@ -160,8 +140,6 @@ class RenderSliverList extends RenderSliverMultiBoxAdaptor { ...@@ -160,8 +140,6 @@ class RenderSliverList extends RenderSliverMultiBoxAdaptor {
correction += paintExtentOf(firstChild); correction += paintExtentOf(firstChild);
earliestUsefulChild = insertAndLayoutLeadingChild(childConstraints, parentUsesSize: true); earliestUsefulChild = insertAndLayoutLeadingChild(childConstraints, parentUsesSize: true);
} }
earliestUsefulChild = firstChild;
if ((correction - earliestScrollOffset).abs() > precisionErrorTolerance) {
geometry = SliverGeometry( geometry = SliverGeometry(
scrollOffsetCorrection: correction - earliestScrollOffset, scrollOffsetCorrection: correction - earliestScrollOffset,
); );
...@@ -169,7 +147,6 @@ class RenderSliverList extends RenderSliverMultiBoxAdaptor { ...@@ -169,7 +147,6 @@ class RenderSliverList extends RenderSliverMultiBoxAdaptor {
childParentData.layoutOffset = 0.0; childParentData.layoutOffset = 0.0;
return; return;
} }
}
final SliverMultiBoxAdaptorParentData childParentData = earliestUsefulChild.parentData as SliverMultiBoxAdaptorParentData; final SliverMultiBoxAdaptorParentData childParentData = earliestUsefulChild.parentData as SliverMultiBoxAdaptorParentData;
childParentData.layoutOffset = firstChildScrollOffset; childParentData.layoutOffset = firstChildScrollOffset;
......
...@@ -578,6 +578,7 @@ abstract class RenderSliverMultiBoxAdaptor extends RenderSliver ...@@ -578,6 +578,7 @@ abstract class RenderSliverMultiBoxAdaptor extends RenderSliver
assert(child != null); assert(child != null);
assert(child.parent == this); assert(child.parent == this);
final SliverMultiBoxAdaptorParentData childParentData = child.parentData as SliverMultiBoxAdaptorParentData; final SliverMultiBoxAdaptorParentData childParentData = child.parentData as SliverMultiBoxAdaptorParentData;
assert(childParentData.layoutOffset != null);
return childParentData.layoutOffset; return childParentData.layoutOffset;
} }
......
...@@ -1069,7 +1069,6 @@ class SliverMultiBoxAdaptorElement extends RenderObjectElement implements Render ...@@ -1069,7 +1069,6 @@ class SliverMultiBoxAdaptorElement extends RenderObjectElement implements Render
assert(_currentlyUpdatingChildIndex == null); assert(_currentlyUpdatingChildIndex == null);
try { try {
final SplayTreeMap<int, Element> newChildren = SplayTreeMap<int, Element>(); final SplayTreeMap<int, Element> newChildren = SplayTreeMap<int, Element>();
final Map<int, double> indexToLayoutOffset = HashMap<int, double>();
void processElement(int index) { void processElement(int index) {
_currentlyUpdatingChildIndex = index; _currentlyUpdatingChildIndex = index;
...@@ -1081,31 +1080,17 @@ class SliverMultiBoxAdaptorElement extends RenderObjectElement implements Render ...@@ -1081,31 +1080,17 @@ class SliverMultiBoxAdaptorElement extends RenderObjectElement implements Render
if (newChild != null) { if (newChild != null) {
_childElements[index] = newChild; _childElements[index] = newChild;
final SliverMultiBoxAdaptorParentData parentData = newChild.renderObject.parentData as SliverMultiBoxAdaptorParentData; final SliverMultiBoxAdaptorParentData parentData = newChild.renderObject.parentData as SliverMultiBoxAdaptorParentData;
if (index == 0) {
parentData.layoutOffset = 0.0;
} else if (indexToLayoutOffset.containsKey(index)) {
parentData.layoutOffset = indexToLayoutOffset[index];
}
if (!parentData.keptAlive) if (!parentData.keptAlive)
_currentBeforeChild = newChild.renderObject as RenderBox; _currentBeforeChild = newChild.renderObject as RenderBox;
} else { } else {
_childElements.remove(index); _childElements.remove(index);
} }
} }
for (final int index in _childElements.keys.toList()) { for (final int index in _childElements.keys.toList()) {
final Key key = _childElements[index].widget.key; final Key key = _childElements[index].widget.key;
final int newIndex = key == null ? null : widget.delegate.findIndexByKey(key); final int newIndex = key == null ? null : widget.delegate.findIndexByKey(key);
final SliverMultiBoxAdaptorParentData childParentData =
_childElements[index].renderObject?.parentData as SliverMultiBoxAdaptorParentData;
if (childParentData != null && childParentData.layoutOffset != null)
indexToLayoutOffset[index] = childParentData.layoutOffset;
if (newIndex != null && newIndex != index) { if (newIndex != null && newIndex != index) {
// The layout offset of the child being moved is no longer accurate.
if (childParentData != null)
childParentData.layoutOffset = null;
newChildren[newIndex] = _childElements[index]; newChildren[newIndex] = _childElements[index];
// We need to make sure the original index gets processed. // We need to make sure the original index gets processed.
newChildren.putIfAbsent(index, () => null); newChildren.putIfAbsent(index, () => null);
...@@ -1324,8 +1309,7 @@ class SliverMultiBoxAdaptorElement extends RenderObjectElement implements Render ...@@ -1324,8 +1309,7 @@ class SliverMultiBoxAdaptorElement extends RenderObjectElement implements Render
break; break;
} }
return parentData.layoutOffset != null && return parentData.layoutOffset < renderObject.constraints.scrollOffset + renderObject.constraints.remainingPaintExtent &&
parentData.layoutOffset < renderObject.constraints.scrollOffset + renderObject.constraints.remainingPaintExtent &&
parentData.layoutOffset + itemExtent > renderObject.constraints.scrollOffset; parentData.layoutOffset + itemExtent > renderObject.constraints.scrollOffset;
}).forEach(visitor); }).forEach(visitor);
} }
......
...@@ -340,20 +340,18 @@ void main() { ...@@ -340,20 +340,18 @@ void main() {
final SliverMultiBoxAdaptorParentData candidate = SliverMultiBoxAdaptorParentData(); final SliverMultiBoxAdaptorParentData candidate = SliverMultiBoxAdaptorParentData();
expect(candidate.keepAlive, isFalse); expect(candidate.keepAlive, isFalse);
expect(candidate.index, isNull); expect(candidate.index, isNull);
expect(candidate.toString(), 'index=null; layoutOffset=None'); expect(candidate.toString(), 'index=null; layoutOffset=0.0');
candidate.keepAlive = null; candidate.keepAlive = null;
expect(candidate.toString(), 'index=null; layoutOffset=None'); expect(candidate.toString(), 'index=null; layoutOffset=0.0');
candidate.keepAlive = true; candidate.keepAlive = true;
expect(candidate.toString(), 'index=null; keepAlive; layoutOffset=None'); expect(candidate.toString(), 'index=null; keepAlive; layoutOffset=0.0');
candidate.keepAlive = false; candidate.keepAlive = false;
expect(candidate.toString(), 'index=null; layoutOffset=None'); expect(candidate.toString(), 'index=null; layoutOffset=0.0');
candidate.index = 0; candidate.index = 0;
expect(candidate.toString(), 'index=0; layoutOffset=None'); expect(candidate.toString(), 'index=0; layoutOffset=0.0');
candidate.index = 1; candidate.index = 1;
expect(candidate.toString(), 'index=1; layoutOffset=None'); expect(candidate.toString(), 'index=1; layoutOffset=0.0');
candidate.index = -1; candidate.index = -1;
expect(candidate.toString(), 'index=-1; layoutOffset=None'); expect(candidate.toString(), 'index=-1; layoutOffset=0.0');
candidate.layoutOffset = 100.0;
expect(candidate.toString(), 'index=-1; layoutOffset=100.0');
}); });
} }
...@@ -163,118 +163,6 @@ void main() { ...@@ -163,118 +163,6 @@ void main() {
expect(find.text('Tile 1'), findsOneWidget); expect(find.text('Tile 1'), findsOneWidget);
expect(find.text('Tile 2'), findsOneWidget); expect(find.text('Tile 2'), findsOneWidget);
}); });
testWidgets('SliverList should recalculate inaccurate layout offset case 1', (WidgetTester tester) async {
// Regression test for https://github.com/flutter/flutter/issues/42142.
final List<int> items = List<int>.generate(20, (int i) => i);
final ScrollController controller = ScrollController();
await tester.pumpWidget(
_buildSliverList(
items: List<int>.from(items),
controller: controller,
itemHeight: 50,
viewportHeight: 200,
)
);
await tester.pumpAndSettle();
await tester.drag(find.text('Tile 2'), const Offset(0.0, -1000.0));
await tester.pumpAndSettle();
// Viewport should be scrolled to the end of list.
expect(controller.offset, 800.0);
expect(find.text('Tile 15'), findsNothing);
expect(find.text('Tile 16'), findsOneWidget);
expect(find.text('Tile 17'), findsOneWidget);
expect(find.text('Tile 18'), findsOneWidget);
expect(find.text('Tile 19'), findsOneWidget);
// Prepends item to the list.
items.insert(0, -1);
await tester.pumpWidget(
_buildSliverList(
items: List<int>.from(items),
controller: controller,
itemHeight: 50,
viewportHeight: 200,
)
);
await tester.pump();
// We need second pump to ensure the scheduled animation gets run.
await tester.pumpAndSettle();
// Scroll offset should stay the same, and the items in viewport should be
// shifted by one.
expect(controller.offset, 800.0);
expect(find.text('Tile 14'), findsNothing);
expect(find.text('Tile 15'), findsOneWidget);
expect(find.text('Tile 16'), findsOneWidget);
expect(find.text('Tile 17'), findsOneWidget);
expect(find.text('Tile 18'), findsOneWidget);
expect(find.text('Tile 19'), findsNothing);
// Drags back to beginning and newly added item is visible.
await tester.drag(find.text('Tile 16'), const Offset(0.0, 1000.0));
await tester.pumpAndSettle();
expect(controller.offset, 0.0);
expect(find.text('Tile -1'), findsOneWidget);
expect(find.text('Tile 0'), findsOneWidget);
expect(find.text('Tile 1'), findsOneWidget);
expect(find.text('Tile 2'), findsOneWidget);
expect(find.text('Tile 3'), findsNothing);
});
testWidgets('SliverList should recalculate inaccurate layout offset case 2', (WidgetTester tester) async {
// Regression test for https://github.com/flutter/flutter/issues/42142.
final List<int> items = List<int>.generate(20, (int i) => i);
final ScrollController controller = ScrollController();
await tester.pumpWidget(
_buildSliverList(
items: List<int>.from(items),
controller: controller,
itemHeight: 50,
viewportHeight: 200,
)
);
await tester.pumpAndSettle();
await tester.drag(find.text('Tile 2'), const Offset(0.0, -1000.0));
await tester.pumpAndSettle();
// Viewport should be scrolled to the end of list.
expect(controller.offset, 800.0);
expect(find.text('Tile 15'), findsNothing);
expect(find.text('Tile 16'), findsOneWidget);
expect(find.text('Tile 17'), findsOneWidget);
expect(find.text('Tile 18'), findsOneWidget);
expect(find.text('Tile 19'), findsOneWidget);
// Reorders item to the front. This should make item 19 to be first child
// with layout offset = null.
final int swap = items[19];
items[19] = items[3];
items[3] = swap;
await tester.pumpWidget(
_buildSliverList(
items: List<int>.from(items),
controller: controller,
itemHeight: 50,
viewportHeight: 200,
)
);
await tester.pump();
// We need second pump to ensure the scheduled animation gets run.
await tester.pumpAndSettle();
// Scroll offset should stay the same
expect(controller.offset, 800.0);
expect(find.text('Tile 14'), findsNothing);
expect(find.text('Tile 15'), findsNothing);
expect(find.text('Tile 16'), findsOneWidget);
expect(find.text('Tile 17'), findsOneWidget);
expect(find.text('Tile 18'), findsOneWidget);
expect(find.text('Tile 3'), findsOneWidget);
});
} }
Widget _buildSliverListRenderWidgetChild(List<String> items) { Widget _buildSliverListRenderWidgetChild(List<String> items) {
...@@ -328,11 +216,6 @@ Widget _buildSliverList({ ...@@ -328,11 +216,6 @@ Widget _buildSliverList({
child: Text('Tile ${items[i]}'), child: Text('Tile ${items[i]}'),
); );
}, },
findChildIndexCallback: (Key key) {
final ValueKey<int> valueKey = key as ValueKey<int>;
final int index = items.indexOf(valueKey.value);
return index == -1 ? null : index;
},
childCount: items.length, childCount: items.length,
), ),
), ),
......
...@@ -50,8 +50,7 @@ Future<void> testSliverFixedExtentList(WidgetTester tester, List<String> items) ...@@ -50,8 +50,7 @@ Future<void> testSliverFixedExtentList(WidgetTester tester, List<String> items)
findChildIndexCallback: (Key key) { findChildIndexCallback: (Key key) {
final ValueKey<String> valueKey = key as ValueKey<String>; final ValueKey<String> valueKey = key as ValueKey<String>;
final String data = valueKey.value; final String data = valueKey.value;
final int index = items.indexOf(data); return items.indexOf(data);
return index == -1 ? null : index;
}, },
), ),
), ),
......
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