From d689362dc58d8c1bc4cd2c6669bc2de55e260883 Mon Sep 17 00:00:00 2001
From: Adam Barth <abarth@chromium.org>
Date: Thu, 19 Nov 2015 12:29:36 -0800
Subject: [PATCH] Some paint functions were using canvas.save/restore around
 children

This pattern breaks when using compositing because we need to lift those
operations into the compositing tree. This patch removes all the ones I could
find and adds an assert to help prevent more from getting introduced.

Fixes #191
---
 packages/flutter/lib/src/rendering/block.dart |  9 ++++-----
 packages/flutter/lib/src/rendering/debug.dart |  3 ---
 .../lib/src/rendering/editable_paragraph.dart | 20 +++++++++----------
 packages/flutter/lib/src/rendering/flex.dart  |  6 ++----
 packages/flutter/lib/src/rendering/grid.dart  | 10 +++-------
 .../flutter/lib/src/rendering/object.dart     |  7 +------
 packages/flutter/lib/src/rendering/stack.dart |  5 +----
 7 files changed, 20 insertions(+), 40 deletions(-)

diff --git a/packages/flutter/lib/src/rendering/block.dart b/packages/flutter/lib/src/rendering/block.dart
index 275a88391f..2977e81483 100644
--- a/packages/flutter/lib/src/rendering/block.dart
+++ b/packages/flutter/lib/src/rendering/block.dart
@@ -401,18 +401,17 @@ class RenderBlockViewport extends RenderBlockBase {
     super.performLayout();
   }
 
-  void paint(PaintingContext context, Offset offset) {
-    context.canvas.save();
-
-    context.canvas.clipRect(offset & size);
+  void _paintContents(PaintingContext context, Offset offset) {
     if (isVertical)
       defaultPaint(context, offset.translate(0.0, startOffset));
     else
       defaultPaint(context, offset.translate(startOffset, 0.0));
 
     overlayPainter?.paint(context, offset);
+  }
 
-    context.canvas.restore();
+  void paint(PaintingContext context, Offset offset) {
+    context.pushClipRect(needsCompositing, offset, Point.origin & size, _paintContents);
   }
 
   void applyPaintTransform(Matrix4 transform) {
diff --git a/packages/flutter/lib/src/rendering/debug.dart b/packages/flutter/lib/src/rendering/debug.dart
index 93c304799c..28cec70fee 100644
--- a/packages/flutter/lib/src/rendering/debug.dart
+++ b/packages/flutter/lib/src/rendering/debug.dart
@@ -27,9 +27,6 @@ bool debugPaintLayerBordersEnabled = false;
 /// The color to use when painting Layer borders.
 ui.Color debugPaintLayerBordersColor = const ui.Color(0xFFFF9800);
 
-/// Causes RenderObjects to paint warnings when painting outside their bounds.
-bool debugPaintBoundsEnabled = false;
-
 /// Causes RenderBox objects to flash while they are being tapped
 bool debugPaintPointersEnabled = false;
 
diff --git a/packages/flutter/lib/src/rendering/editable_paragraph.dart b/packages/flutter/lib/src/rendering/editable_paragraph.dart
index cc208e9590..74b1451abb 100644
--- a/packages/flutter/lib/src/rendering/editable_paragraph.dart
+++ b/packages/flutter/lib/src/rendering/editable_paragraph.dart
@@ -97,15 +97,7 @@ class RenderEditableParagraph extends RenderParagraph {
     }
   }
 
-  void paint(PaintingContext context, Offset offset) {
-    layoutText(_getTextContraints(constraints));
-
-    final bool needsClipping = (_contentSize.width > size.width);
-    if (needsClipping) {
-      context.canvas.save();
-      context.canvas.clipRect(offset & size);
-    }
-
+  void _paintContents(PaintingContext context, Offset offset) {
     textPainter.paint(context.canvas, offset - _scrollOffset);
 
     if (_showCursor) {
@@ -117,9 +109,15 @@ class RenderEditableParagraph extends RenderParagraph {
       );
       context.canvas.drawRect(cursorRect, new Paint()..color = _cursorColor);
     }
+  }
 
-    if (needsClipping)
-      context.canvas.restore();
+  void paint(PaintingContext context, Offset offset) {
+    layoutText(_getTextContraints(constraints));
+    final bool hasVisualOverflow = (_contentSize.width > size.width);
+    if (hasVisualOverflow)
+      context.pushClipRect(needsCompositing, offset, Point.origin & size, _paintContents);
+    else
+      _paintContents(context, offset);
   }
 
 }
diff --git a/packages/flutter/lib/src/rendering/flex.dart b/packages/flutter/lib/src/rendering/flex.dart
index 9de75918b0..2bc79b8aad 100644
--- a/packages/flutter/lib/src/rendering/flex.dart
+++ b/packages/flutter/lib/src/rendering/flex.dart
@@ -553,10 +553,8 @@ class RenderFlex extends RenderBox with ContainerRenderObjectMixin<RenderBox, Fl
     }
 
     // We have overflow. Clip it.
-    context.canvas.save();
-    context.canvas.clipRect(offset & size);
-    defaultPaint(context, offset);
-    context.canvas.restore();
+    context.pushClipRect(needsCompositing, offset, Point.origin & size, defaultPaint);
+
     assert(() {
       // In debug mode, if you have overflow, we highlight where the
       // overflow would be by painting that area red. Since that is
diff --git a/packages/flutter/lib/src/rendering/grid.dart b/packages/flutter/lib/src/rendering/grid.dart
index b22ff83d99..68eeac96c7 100644
--- a/packages/flutter/lib/src/rendering/grid.dart
+++ b/packages/flutter/lib/src/rendering/grid.dart
@@ -139,13 +139,9 @@ class RenderGrid extends RenderBox with ContainerRenderObjectMixin<RenderBox, Gr
   }
 
   void paint(PaintingContext context, Offset offset) {
-    if (_hasVisualOverflow) {
-      context.canvas.save();
-      context.canvas.clipRect(offset & size);
+    if (_hasVisualOverflow)
+      context.pushClipRect(needsCompositing, offset, Point.origin & size, defaultPaint);
+    else
       defaultPaint(context, offset);
-      context.canvas.restore();
-    } else {
-      defaultPaint(context, offset);
-    }
   }
 }
diff --git a/packages/flutter/lib/src/rendering/object.dart b/packages/flutter/lib/src/rendering/object.dart
index 59546075c2..0fa06b6341 100644
--- a/packages/flutter/lib/src/rendering/object.dart
+++ b/packages/flutter/lib/src/rendering/object.dart
@@ -96,6 +96,7 @@ class PaintingContext {
   void _compositeChild(RenderObject child, Offset offset) {
     assert(!_isRecording);
     assert(child.hasLayer);
+    assert(_canvas == null || _canvas.getSaveCount() == 1);
 
     // Create a layer for our child, and paint the child into it.
     if (child.needsPaint) {
@@ -966,10 +967,6 @@ abstract class RenderObject extends AbstractNode implements HitTestTarget {
       _debugDoingThisPaint = true;
       debugLastActivePaint = _debugActivePaint;
       _debugActivePaint = this;
-      if (debugPaintBoundsEnabled) {
-        context.canvas.save();
-        context.canvas.clipRect(paintBounds.shift(offset));
-      }
       assert(!hasLayer || _layer != null);
       return true;
     });
@@ -983,8 +980,6 @@ abstract class RenderObject extends AbstractNode implements HitTestTarget {
     }
     assert(() {
       debugPaint(context, offset);
-      if (debugPaintBoundsEnabled)
-        context.canvas.restore();
       _debugActivePaint = debugLastActivePaint;
       _debugDoingThisPaint = false;
       return true;
diff --git a/packages/flutter/lib/src/rendering/stack.dart b/packages/flutter/lib/src/rendering/stack.dart
index 9744b99f75..946dc1cd6a 100644
--- a/packages/flutter/lib/src/rendering/stack.dart
+++ b/packages/flutter/lib/src/rendering/stack.dart
@@ -383,10 +383,7 @@ abstract class RenderStackBase extends RenderBox
 
   void paint(PaintingContext context, Offset offset) {
     if (_hasVisualOverflow) {
-      context.canvas.save();
-      context.canvas.clipRect(offset & size);
-      paintStack(context, offset);
-      context.canvas.restore();
+      context.pushClipRect(needsCompositing, offset, Point.origin & size, paintStack);
     } else {
       paintStack(context, offset);
     }