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.

packages/flutter/lib/src/rendering/sliver.dart

@@ -364,6 +364,7 @@ class SliverGeometry {
     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 @@ class SliverGeometry {
   /// 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 @@ class SliverGeometry {
     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 @@ class SliverGeometry {
       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 @@ class RenderViewport2 extends RenderBox with ContainerRenderObjectMixin<RenderSl
   double _minScrollExtent;
   double _maxScrollExtent;
   double _shrinkWrapExtent;
+  bool _hasVisualOverflow = false;
 
   @override
   void performLayout() {
@@ -1321,6 +1333,7 @@ class RenderViewport2 extends RenderBox with ContainerRenderObjectMixin<RenderSl
       _minScrollExtent = 0.0;
       _maxScrollExtent = 0.0;
       _shrinkWrapExtent = 0.0;
+      _hasVisualOverflow = false;
       offset.applyContentDimensions(0.0, 0.0);
       return;
     }
@@ -1416,6 +1429,7 @@ class RenderViewport2 extends RenderBox with ContainerRenderObjectMixin<RenderSl
     _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 @@ class RenderViewport2 extends RenderBox with ContainerRenderObjectMixin<RenderSl
       }
       _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 @@ class RenderViewport2 extends RenderBox with ContainerRenderObjectMixin<RenderSl
     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;
@@ -1589,6 +1600,20 @@ class RenderViewport2 extends RenderBox with ContainerRenderObjectMixin<RenderSl
     }
   }
 
+  @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(() {
@@ -1781,6 +1806,7 @@ class RenderSliverToBoxAdapter extends RenderSliver with RenderObjectWithChildMi
       paintExtent: paintedChildSize,
       maxPaintExtent: childExtent,
       hitTestExtent: paintedChildSize,
+      hasVisualOverflow: childExtent > constraints.remainingPaintExtent || constraints.scrollOffset > 0.0,
     );
 
     final SliverPhysicalParentData childParentData = child.parentData;

packages/flutter/lib/src/rendering/sliver_app_bar.dart

@@ -210,6 +210,7 @@ abstract class RenderSliverScrollingAppBar extends RenderSliverAppBar {
       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 @@ abstract class RenderSliverPinnedAppBar extends RenderSliverAppBar {
       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 @@ abstract class RenderSliverFloatingAppBar extends RenderSliverAppBar {
       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;

packages/flutter/lib/src/rendering/sliver_block.dart

@@ -497,6 +497,8 @@ abstract class RenderSliverBlock extends RenderSliver
       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);

packages/flutter/lib/src/rendering/sliver_padding.dart

@@ -232,6 +232,7 @@ class RenderSliverPadding extends RenderSliver with RenderObjectWithChildMixin<R
         mainAxisPaddingPaintExtent + childLayoutGeometry.paintExtent,
         beforePaddingPaintExtent + childLayoutGeometry.hitTestExtent,
       ),
+      hasVisualOverflow: childLayoutGeometry.hasVisualOverflow,
     );
 
     final SliverPhysicalParentData childParentData = child.parentData;

packages/flutter/test/rendering/mock_canvas.dart

@@ -107,6 +107,18 @@ abstract class PaintPattern {
   /// 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
@@ -227,6 +239,11 @@ class _TestRecordingCanvasPatternMatcher extends Matcher implements PaintPattern
     _predicates.add(new _SaveRestorePairPaintPredicate());
   }
 
+  @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));
@@ -399,6 +416,14 @@ class _TestRecordingPaintingContext implements PaintingContext {
     child.paint(this, offset);
   }
 
+  @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) {
   }

packages/flutter/test/widgets/slivers_block_test.dart

@@ -6,6 +6,8 @@ import 'package:flutter_test/flutter_test.dart';
 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 @@ void main() {
       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());
+  });
 }