Viewport2 should clip visual overflow (#7641)
When you put a box in a Viewport2, it might paint outside its allocated bounds.
This patch teaches Viewport2 to clip when that happens.
diff --git a/packages/flutter/lib/src/rendering/sliver.dart b/packages/flutter/lib/src/rendering/sliver.dart
index 15d7708..6db86cc 100644
--- a/packages/flutter/lib/src/rendering/sliver.dart
+++ b/packages/flutter/lib/src/rendering/sliver.dart
@@ -364,6 +364,7 @@
this.maxPaintExtent: 0.0,
double hitTestExtent,
bool visible,
+ this.hasVisualOverflow: false,
this.scrollOffsetCorrection: 0.0
}) : layoutExtent = layoutExtent ?? paintExtent,
hitTestExtent = hitTestExtent ?? paintExtent,
@@ -416,6 +417,13 @@
/// false if [paintExtent] is zero.
final bool visible;
+ /// Whether this sliver has visual overflow.
+ ///
+ /// By default, this is false, which means the viewport does not need to clip
+ /// its children. If any slivers have visual overflow, the viewport will apply
+ /// a clip to its children.
+ final bool hasVisualOverflow;
+
/// If this is non-zero after [RenderSliver.performLayout] returns, the scroll
/// offset will be adjusted by the parent and then the entire layout of the
/// parent will be rerun.
@@ -451,6 +459,7 @@
assert(hitTestExtent != null);
assert(hitTestExtent >= 0.0);
assert(visible != null);
+ assert(hasVisualOverflow != null);
assert(scrollOffsetCorrection != null);
assert(scrollOffsetCorrection == 0.0);
return true;
@@ -481,6 +490,8 @@
buffer.write('maxPaintExtent: ${maxPaintExtent.toStringAsFixed(1)}, ');
if (hitTestExtent != paintExtent)
buffer.write('hitTestExtent: ${hitTestExtent.toStringAsFixed(1)}, ');
+ if (hasVisualOverflow)
+ buffer.write('hasVisualOverflow: true, ');
buffer.write('scrollOffsetCorrection: ${scrollOffsetCorrection.toStringAsFixed(1)}');
buffer.write(')');
return buffer.toString();
@@ -1301,6 +1312,7 @@
double _minScrollExtent;
double _maxScrollExtent;
double _shrinkWrapExtent;
+ bool _hasVisualOverflow = false;
@override
void performLayout() {
@@ -1321,6 +1333,7 @@
_minScrollExtent = 0.0;
_maxScrollExtent = 0.0;
_shrinkWrapExtent = 0.0;
+ _hasVisualOverflow = false;
offset.applyContentDimensions(0.0, 0.0);
return;
}
@@ -1416,6 +1429,7 @@
_minScrollExtent = 0.0;
_maxScrollExtent = 0.0;
_shrinkWrapExtent = 0.0;
+ _hasVisualOverflow = false;
// centerOffset is the offset from the leading edge of the RenderViewport2
// to the zero scroll offset (the line between the forward slivers and the
@@ -1526,6 +1540,9 @@
}
_shrinkWrapExtent += childLayoutGeometry.maxPaintExtent;
+ if (childLayoutGeometry.hasVisualOverflow)
+ _hasVisualOverflow = true;
+
// move on to the next child
assert(child.parentData == childParentData);
child = advance(child);
@@ -1559,17 +1576,11 @@
childParentData.applyPaintTransform(transform);
}
- @override
- void paint(PaintingContext context, Offset offset) {
- if (center == null) {
- assert(firstChild == null);
- return;
- }
+ void _paintContents(PaintingContext context, Offset offset) {
assert(center.parent == this);
assert(firstChild != null);
- RenderSliver child;
- // TODO(ianh): if we have content beyond our max extents, clip
- child = firstChild;
+
+ RenderSliver child = firstChild;
while (child != center) {
if (child.geometry.visible) {
final SliverPhysicalParentData childParentData = child.parentData;
@@ -1590,6 +1601,20 @@
}
@override
+ void paint(PaintingContext context, Offset offset) {
+ if (center == null) {
+ assert(firstChild == null);
+ return;
+ }
+
+ if (_hasVisualOverflow) {
+ context.pushClipRect(needsCompositing, offset, Point.origin & size, _paintContents);
+ } else {
+ _paintContents(context, offset);
+ }
+ }
+
+ @override
void debugPaintSize(PaintingContext context, Offset offset) {
assert(() {
super.debugPaintSize(context, offset);
@@ -1781,6 +1806,7 @@
paintExtent: paintedChildSize,
maxPaintExtent: childExtent,
hitTestExtent: paintedChildSize,
+ hasVisualOverflow: childExtent > constraints.remainingPaintExtent || constraints.scrollOffset > 0.0,
);
final SliverPhysicalParentData childParentData = child.parentData;
diff --git a/packages/flutter/lib/src/rendering/sliver_app_bar.dart b/packages/flutter/lib/src/rendering/sliver_app_bar.dart
index d8efac1..b2230fe 100644
--- a/packages/flutter/lib/src/rendering/sliver_app_bar.dart
+++ b/packages/flutter/lib/src/rendering/sliver_app_bar.dart
@@ -210,6 +210,7 @@
scrollExtent: maxExtent,
paintExtent: paintExtent.clamp(0.0, constraints.remainingPaintExtent),
maxPaintExtent: maxExtent,
+ hasVisualOverflow: true, // Conservatively say we do have overflow to avoid complexity.
);
_childPosition = math.min(0.0, paintExtent - childExtent);
}
@@ -240,6 +241,7 @@
paintExtent: math.min(constraints.overlap + childExtent, constraints.remainingPaintExtent),
layoutExtent: (maxExtent - constraints.scrollOffset).clamp(0.0, constraints.remainingPaintExtent),
maxPaintExtent: constraints.overlap + maxExtent,
+ hasVisualOverflow: true, // Conservatively say we do have overflow to avoid complexity.
);
}
@@ -289,6 +291,7 @@
paintExtent: paintExtent.clamp(0.0, constraints.remainingPaintExtent),
layoutExtent: layoutExtent,
maxPaintExtent: maxExtent,
+ hasVisualOverflow: true, // Conservatively say we do have overflow to avoid complexity.
);
_childPosition = math.min(0.0, paintExtent - childExtent);
_lastActualScrollOffset = constraints.scrollOffset;
diff --git a/packages/flutter/lib/src/rendering/sliver_block.dart b/packages/flutter/lib/src/rendering/sliver_block.dart
index e89fbda..cf6de77 100644
--- a/packages/flutter/lib/src/rendering/sliver_block.dart
+++ b/packages/flutter/lib/src/rendering/sliver_block.dart
@@ -497,6 +497,8 @@
scrollExtent: estimatedTotalExtent,
paintExtent: paintedExtent,
maxPaintExtent: estimatedTotalExtent,
+ // Conservative to avoid flickering away the clip during scroll.
+ hasVisualOverflow: endScrollOffset > targetEndScrollOffset || constraints.scrollOffset > 0.0,
);
assert(_currentlyUpdatingChildIndex == null);
diff --git a/packages/flutter/lib/src/rendering/sliver_padding.dart b/packages/flutter/lib/src/rendering/sliver_padding.dart
index a85b1d6..6d3731a 100644
--- a/packages/flutter/lib/src/rendering/sliver_padding.dart
+++ b/packages/flutter/lib/src/rendering/sliver_padding.dart
@@ -232,6 +232,7 @@
mainAxisPaddingPaintExtent + childLayoutGeometry.paintExtent,
beforePaddingPaintExtent + childLayoutGeometry.hitTestExtent,
),
+ hasVisualOverflow: childLayoutGeometry.hasVisualOverflow,
);
final SliverPhysicalParentData childParentData = child.parentData;
diff --git a/packages/flutter/test/rendering/mock_canvas.dart b/packages/flutter/test/rendering/mock_canvas.dart
index f3c512c..6919bc0 100644
--- a/packages/flutter/test/rendering/mock_canvas.dart
+++ b/packages/flutter/test/rendering/mock_canvas.dart
@@ -107,6 +107,18 @@
/// See also: [save], [restore].
void saveRestore();
+ /// Indicates that a rectangular clip.
+ ///
+ /// The next rectangular clip is examined. Any arguments that are passed to
+ /// this method are compared to the actual [Canvas.clipRect] call's argument
+ /// and any mismatches result in failure.
+ ///
+ /// If no call to [Canvas.clipRect] was made, then this results in failure.
+ ///
+ /// Any calls made between the last matched call (if any) and the
+ /// [Canvas.clipRect] call are ignored.
+ void clipRect({ Rect rect });
+
/// Indicates that a rectangle is expected next.
///
/// The next rectangle is examined. Any arguments that are passed to this
@@ -228,6 +240,11 @@
}
@override
+ void clipRect({ Rect rect }) {
+ _predicates.add(new _FunctionPaintPredicate(#clipRect, <dynamic>[rect]));
+ }
+
+ @override
void rect({ Rect rect, Color color, bool hasMaskFilter, PaintingStyle style }) {
_predicates.add(new _RectPaintPredicate(rect: rect, color: color, hasMaskFilter: hasMaskFilter, style: style));
}
@@ -400,6 +417,14 @@
}
@override
+ void pushClipRect(bool needsCompositing, Offset offset, Rect clipRect, PaintingContextCallback painter) {
+ canvas.save();
+ canvas.clipRect(clipRect.shift(offset));
+ painter(this, offset);
+ canvas.restore();
+ }
+
+ @override
void noSuchMethod(Invocation invocation) {
}
}
diff --git a/packages/flutter/test/widgets/slivers_block_test.dart b/packages/flutter/test/widgets/slivers_block_test.dart
index a379249..40cf371 100644
--- a/packages/flutter/test/widgets/slivers_block_test.dart
+++ b/packages/flutter/test/widgets/slivers_block_test.dart
@@ -6,6 +6,8 @@
import 'package:flutter/rendering.dart';
import 'package:flutter/widgets.dart';
+import '../rendering/mock_canvas.dart';
+
Future<Null> test(WidgetTester tester, double offset) {
return tester.pumpWidget(new Viewport2(
offset: new ViewportOffset.fixed(offset),
@@ -156,4 +158,104 @@
const Point(0.0, 504.0),
], 'acb');
});
+
+ testWidgets('Viewport2 overflow clipping of SliverToBoxAdapter', (WidgetTester tester) async {
+ await tester.pumpWidget(new Viewport2(
+ offset: new ViewportOffset.zero(),
+ children: <Widget>[
+ new SliverToBoxAdapter(
+ child: new SizedBox(height: 400.0, child: new Text('a')),
+ ),
+ ],
+ ));
+
+ expect(find.byType(Viewport2), isNot(paints..clipRect()));
+
+ await tester.pumpWidget(new Viewport2(
+ offset: new ViewportOffset.fixed(100.0),
+ children: <Widget>[
+ new SliverToBoxAdapter(
+ child: new SizedBox(height: 400.0, child: new Text('a')),
+ ),
+ ],
+ ));
+
+ expect(find.byType(Viewport2), paints..clipRect());
+
+ await tester.pumpWidget(new Viewport2(
+ offset: new ViewportOffset.fixed(100.0),
+ children: <Widget>[
+ new SliverToBoxAdapter(
+ child: new SizedBox(height: 4000.0, child: new Text('a')),
+ ),
+ ],
+ ));
+
+ expect(find.byType(Viewport2), paints..clipRect());
+
+ await tester.pumpWidget(new Viewport2(
+ offset: new ViewportOffset.zero(),
+ children: <Widget>[
+ new SliverToBoxAdapter(
+ child: new SizedBox(height: 4000.0, child: new Text('a')),
+ ),
+ ],
+ ));
+
+ expect(find.byType(Viewport2), paints..clipRect());
+ });
+
+ testWidgets('Viewport2 overflow clipping of SliverBlock', (WidgetTester tester) async {
+ await tester.pumpWidget(new Viewport2(
+ offset: new ViewportOffset.zero(),
+ children: <Widget>[
+ new SliverBlock(
+ delegate: new SliverBlockChildListDelegate(<Widget>[
+ new SizedBox(height: 400.0, child: new Text('a')),
+ ]),
+ ),
+ ],
+ ));
+
+ expect(find.byType(Viewport2), isNot(paints..clipRect()));
+
+ await tester.pumpWidget(new Viewport2(
+ offset: new ViewportOffset.fixed(100.0),
+ children: <Widget>[
+ new SliverBlock(
+ delegate: new SliverBlockChildListDelegate(<Widget>[
+ new SizedBox(height: 400.0, child: new Text('a')),
+ ]),
+ ),
+ ],
+ ));
+
+ expect(find.byType(Viewport2), paints..clipRect());
+
+ await tester.pumpWidget(new Viewport2(
+ offset: new ViewportOffset.fixed(100.0),
+ children: <Widget>[
+ new SliverBlock(
+ delegate: new SliverBlockChildListDelegate(<Widget>[
+ new SizedBox(height: 4000.0, child: new Text('a')),
+ ]),
+ ),
+ ],
+ ));
+
+ expect(find.byType(Viewport2), paints..clipRect());
+
+ await tester.pumpWidget(new Viewport2(
+ offset: new ViewportOffset.zero(),
+ children: <Widget>[
+ new SliverBlock(
+ delegate: new SliverBlockChildListDelegate(<Widget>[
+ new SizedBox(height: 4000.0, child: new Text('a')),
+ ]),
+ ),
+ ],
+ ));
+
+ expect(find.byType(Viewport2), paints..clipRect());
+ });
}
