Skip to content

Commit

Permalink
Fixed extent lists cannot handle infinite extent
Browse files Browse the repository at this point in the history
Previously they tried to compute an integer target end index, but
integers can't represent infinity. Now we use null to represent
infinity.

Also, fix some similar issues with grids.

Fixes flutter#8398
  • Loading branch information
abarth committed Mar 20, 2017
1 parent 3fc7265 commit a281a24
Show file tree
Hide file tree
Showing 4 changed files with 78 additions and 12 deletions.
12 changes: 7 additions & 5 deletions packages/flutter/lib/src/rendering/sliver_fixed_extent_list.dart
Original file line number Diff line number Diff line change
Expand Up @@ -120,13 +120,14 @@ abstract class RenderSliverFixedExtentBoxAdaptor extends RenderSliverMultiBoxAda
);

final int firstIndex = getMinChildIndexForScrollOffset(scrollOffset, itemExtent);
final int targetLastIndex = getMaxChildIndexForScrollOffset(targetEndScrollOffset, itemExtent);
final int targetLastIndex = targetEndScrollOffset.isFinite ?
getMaxChildIndexForScrollOffset(targetEndScrollOffset, itemExtent) : null;

if (firstChild != null) {
final int oldFirstIndex = indexOf(firstChild);
final int oldLastIndex = indexOf(lastChild);
final int leadingGarbage = (firstIndex - oldFirstIndex).clamp(0, childCount);
final int trailingGarbage = (oldLastIndex - targetLastIndex).clamp(0, childCount);
final int trailingGarbage = targetLastIndex == null ? 0 : (oldLastIndex - targetLastIndex).clamp(0, childCount);
if (leadingGarbage + trailingGarbage > 0)
collectGarbage(leadingGarbage, trailingGarbage);
}
Expand Down Expand Up @@ -156,7 +157,7 @@ abstract class RenderSliverFixedExtentBoxAdaptor extends RenderSliverMultiBoxAda
trailingChildWithLayout = firstChild;
}

while (indexOf(trailingChildWithLayout) < targetLastIndex) {
while (targetLastIndex == null || indexOf(trailingChildWithLayout) < targetLastIndex) {
RenderBox child = childAfter(trailingChildWithLayout);
if (child == null) {
child = insertAndLayoutChild(childConstraints, after: trailingChildWithLayout);
Expand All @@ -180,7 +181,7 @@ abstract class RenderSliverFixedExtentBoxAdaptor extends RenderSliverMultiBoxAda
assert(firstIndex == 0 || childScrollOffset(firstChild) <= scrollOffset);
assert(debugAssertChildListIsNonEmptyAndContiguous());
assert(indexOf(firstChild) == firstIndex);
assert(lastIndex <= targetLastIndex);
assert(targetLastIndex == null || lastIndex <= targetLastIndex);

final double estimatedMaxScrollOffset = estimateMaxScrollOffset(
constraints,
Expand All @@ -201,7 +202,8 @@ abstract class RenderSliverFixedExtentBoxAdaptor extends RenderSliverMultiBoxAda
paintExtent: paintExtent,
maxPaintExtent: estimatedMaxScrollOffset,
// Conservative to avoid flickering away the clip during scroll.
hasVisualOverflow: lastIndex >= targetLastIndex || constraints.scrollOffset > 0.0,
hasVisualOverflow: (targetLastIndex != null && lastIndex >= targetLastIndex)
|| constraints.scrollOffset > 0.0,
);

assert(childManager.debugAssertChildListLocked());
Expand Down
18 changes: 11 additions & 7 deletions packages/flutter/lib/src/rendering/sliver_grid.dart
Original file line number Diff line number Diff line change
Expand Up @@ -172,13 +172,16 @@ class SliverGridRegularTileLayout extends SliverGridLayout {

@override
int getMinChildIndexForScrollOffset(double scrollOffset) {
return crossAxisCount * (scrollOffset ~/ mainAxisStride);
return mainAxisStride > 0.0 ? crossAxisCount * (scrollOffset ~/ mainAxisStride) : 0;
}

@override
int getMaxChildIndexForScrollOffset(double scrollOffset) {
final int mainAxisCount = (scrollOffset / mainAxisStride).ceil();
return math.max(0, crossAxisCount * mainAxisCount - 1);
if (mainAxisStride > 0.0) {
final int mainAxisCount = (scrollOffset / mainAxisStride).ceil();
return math.max(0, crossAxisCount * mainAxisCount - 1);
}
return 0;
}

@override
Expand Down Expand Up @@ -486,13 +489,14 @@ class RenderSliverGrid extends RenderSliverMultiBoxAdaptor {
final SliverGridLayout layout = _gridDelegate.getLayout(constraints);

final int firstIndex = layout.getMinChildIndexForScrollOffset(scrollOffset);
final int targetLastIndex = layout.getMaxChildIndexForScrollOffset(targetEndScrollOffset);
final int targetLastIndex = targetEndScrollOffset.isFinite ?
layout.getMaxChildIndexForScrollOffset(targetEndScrollOffset) : null;

if (firstChild != null) {
final int oldFirstIndex = indexOf(firstChild);
final int oldLastIndex = indexOf(lastChild);
final int leadingGarbage = (firstIndex - oldFirstIndex).clamp(0, childCount);
final int trailingGarbage = (oldLastIndex - targetLastIndex).clamp(0, childCount);
final int trailingGarbage = targetLastIndex == null ? 0 : (oldLastIndex - targetLastIndex).clamp(0, childCount);
if (leadingGarbage + trailingGarbage > 0)
collectGarbage(leadingGarbage, trailingGarbage);
}
Expand Down Expand Up @@ -532,7 +536,7 @@ class RenderSliverGrid extends RenderSliverMultiBoxAdaptor {
trailingChildWithLayout = firstChild;
}

for (int index = indexOf(trailingChildWithLayout) + 1; index <= targetLastIndex; ++index) {
for (int index = indexOf(trailingChildWithLayout) + 1; targetLastIndex == null || index <= targetLastIndex; ++index) {
final SliverGridGeometry gridGeometry = layout.getGeometryForChildIndex(index);
final BoxConstraints childConstraints = gridGeometry.getBoxConstraints(constraints);
RenderBox child = childAfter(trailingChildWithLayout);
Expand All @@ -559,7 +563,7 @@ class RenderSliverGrid extends RenderSliverMultiBoxAdaptor {
assert(childScrollOffset(firstChild) <= scrollOffset);
assert(debugAssertChildListIsNonEmptyAndContiguous());
assert(indexOf(firstChild) == firstIndex);
assert(lastIndex <= targetLastIndex);
assert(targetLastIndex == null || lastIndex <= targetLastIndex);

final double estimatedTotalExtent = childManager.estimateMaxScrollOffset(
constraints,
Expand Down
41 changes: 41 additions & 0 deletions packages/flutter/test/widgets/grid_view_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,47 @@ void main() {
expect(find.byType(GridView), isNot(paints..rect(color: green)..rect(color: green)..rect(color: green)));
});

testWidgets('GridView in zero context', (WidgetTester tester) async {
await tester.pumpWidget(
new Center(
child: new SizedBox(
width: 0.0,
height: 0.0,
child: new GridView.count(
crossAxisCount: 4,
children: new List<Widget>.generate(20, (int i) {
return new Container(
child: new Text('$i'),
);
}),
),
),
),
);

expect(find.text('0'), findsOneWidget);
expect(find.text('1'), findsNothing);
});

testWidgets('GridView in unbounded context', (WidgetTester tester) async {
await tester.pumpWidget(
new SingleChildScrollView(
child: new GridView.count(
crossAxisCount: 4,
shrinkWrap: true,
children: new List<Widget>.generate(20, (int i) {
return new Container(
child: new Text('$i'),
);
}),
),
),
);

expect(find.text('0'), findsOneWidget);
expect(find.text('19'), findsOneWidget);
});

// TODO(ianh): can you tap a grid cell that is slightly off the bottom of the screen?
// (try it with the flutter_gallery Grid demo)
}
19 changes: 19 additions & 0 deletions packages/flutter/test/widgets/list_view_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -144,4 +144,23 @@ void main() {
expect(find.text('4'), findsOneWidget);
expect(find.text('5'), findsNothing);
});

testWidgets('ListView with itemExtent in unbounded context', (WidgetTester tester) async {
await tester.pumpWidget(
new SingleChildScrollView(
child: new ListView(
itemExtent: 100.0,
shrinkWrap: true,
children: new List<Widget>.generate(20, (int i) {
return new Container(
child: new Text('$i'),
);
}),
),
),
);

expect(find.text('0'), findsOneWidget);
expect(find.text('19'), findsOneWidget);
});
}

0 comments on commit a281a24

Please sign in to comment.