Merge pull request #2798 from Hixie/repaint

Track metrics for RepaintBoundary.

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

@@ -63,9 +63,13 @@ class PaintingContext {
   /// The render object must have a composited layer and must be in need of
   /// painting. The render object's layer is re-used, along with any layers in
   /// the subtree that don't need to be repainted.
-  static void repaintCompositedChild(RenderObject child) {
+  static void repaintCompositedChild(RenderObject child, { bool debugAlsoPaintedParent: false }) {
     assert(child.isRepaintBoundary);
     assert(child.needsPaint);
+    assert(() {
+      child.debugRegisterRepaintBoundaryPaint(includedParent: debugAlsoPaintedParent, includedChild: true);
+      return true;
+    });
     child._layer ??= new OffsetLayer();
     child._layer.removeAllChildren();
     assert(() {
@@ -98,9 +102,13 @@ class PaintingContext {
 
     // Create a layer for our child, and paint the child into it.
     if (child.needsPaint) {
-      repaintCompositedChild(child);
+      repaintCompositedChild(child, debugAlsoPaintedParent: true);
     } else {
       assert(child._layer != null);
+      assert(() {
+        child.debugRegisterRepaintBoundaryPaint(includedParent: true, includedChild: false);
+        return true;
+      });
       child._layer.detach();
       assert(() {
         child._layer.debugOwner = child.debugOwner ?? child.runtimeType;
@@ -1190,6 +1198,13 @@ abstract class RenderObject extends AbstractNode implements HitTestTarget {
   /// Warning: This getter must not change value over the lifetime of this object.
   bool get isRepaintBoundary => false;
 
+  /// Called, in checked mode, if [isRepaintBoundary] is true, when either the
+  /// this render object or its parent attempt to paint.
+  ///
+  /// This can be used to record metrics about whether the node should actually
+  /// be a repaint boundary.
+  void debugRegisterRepaintBoundaryPaint({ bool includedParent: true, bool includedChild: false }) { }
+
   /// Whether this render object always needs compositing.
   ///
   /// Override this in subclasses to indicate that your paint function always

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

@@ -1480,11 +1480,113 @@ class RenderPointerListener extends RenderProxyBoxWithHitTestBehavior {
 /// previously. Similarly, when the child repaints but the surround tree does
 /// not, we can re-record its display list without re-recording the display list
 /// for the surround tree.
+///
+/// In some cases, it is necessary to place _two_ (or more) repaint boundaries
+/// to get a useful effect. Consider, for example, an e-mail application that
+/// shows an unread count and a list of e-mails. Whenever a new e-mail comes in,
+/// the list would update, but so would the unread count. If only one of these
+/// two parts of the application was behind a repaint boundary, the entire
+/// application would repaint each time. On the other hand, if both were behind
+/// a repaint boundary, a new e-mail would only change those two parts of the
+/// application and the rest of the application would not repaint.
+///
+/// To tell if a particular RenderRepaintBoundary is useful, run your
+/// application in checked mode, interacting with it in typical ways, and then
+/// call [debugDumpRenderTree]. Each RenderRepaintBoundary will include the
+/// ratio of cases where the repaint boundary was useful vs the cases where it
+/// was not. These counts can also be inspected programmatically using
+/// [debugAsymmetricPaintCount] and [debugSymmetricPaintCount] respectively.
 class RenderRepaintBoundary extends RenderProxyBox {
   RenderRepaintBoundary({ RenderBox child }) : super(child);
 
   @override
   bool get isRepaintBoundary => true;
+
+  /// The number of times that this render object repainted at the same time as
+  /// its parent. Repaint boundaries are only useful when the parent and child
+  /// paint at different times. When both paint at the same time, the repaint
+  /// boundary is redundant, and may be actually making performance worse.
+  ///
+  /// Only valid in checked mode. In release builds, always returns zero.
+  ///
+  /// Can be reset using [debugResetMetrics]. See [debugAsymmetricPaintCount]
+  /// for the corresponding count of times where only the parent or only the
+  /// child painted.
+  int get debugSymmetricPaintCount => _debugSymmetricPaintCount;
+  int _debugSymmetricPaintCount = 0;
+
+  /// The number of times that either this render object repainted without the
+  /// parent being painted, or the parent repainted without this object being
+  /// painted. When a repaint boundary is used at a seam in the render tree
+  /// where the parent tends to repaint at entirely different times than the
+  /// child, it can improve performance by reducing the number of paint
+  /// operations that have to be recorded each frame.
+  ///
+  /// Only valid in checked mode. In release builds, always returns zero.
+  ///
+  /// Can be reset using [debugResetMetrics]. See [debugSymmetricPaintCount] for
+  /// the corresponding count of times where both the parent and the child
+  /// painted together.
+  int get debugAsymmetricPaintCount => _debugAsymmetricPaintCount;
+  int _debugAsymmetricPaintCount = 0;
+
+  /// Resets the [debugSymmetricPaintCount] and [debugAsymmetricPaintCount]
+  /// counts to zero.
+  ///
+  /// Only valid in checked mode. Does nothing in release builds.
+  void debugResetMetrics() {
+    assert(() {
+      _debugSymmetricPaintCount = 0;
+      _debugAsymmetricPaintCount = 0;
+      return true;
+    });
+  }
+
+  @override
+  void debugRegisterRepaintBoundaryPaint({ bool includedParent: true, bool includedChild: false }) {
+    assert(() {
+      if (includedParent && includedChild)
+        _debugSymmetricPaintCount += 1;
+      else
+        _debugAsymmetricPaintCount += 1;
+      return true;
+    });
+  }
+
+  @override
+  void debugFillDescription(List<String> description) {
+    super.debugFillDescription(description);
+    bool inReleaseMode = true;
+    assert(() {
+      inReleaseMode = false;
+      if (debugSymmetricPaintCount + debugAsymmetricPaintCount == 0) {
+        description.add('usefulness ratio: no metrics collected yet (never painted)');
+      } else {
+        double percentage = 100.0 * debugAsymmetricPaintCount / (debugSymmetricPaintCount + debugAsymmetricPaintCount);
+        String diagnosis;
+        if (debugSymmetricPaintCount + debugAsymmetricPaintCount < 5) {
+          diagnosis = 'insufficient data to draw conclusion (less than five repaints)';
+        } else if (percentage > 90.0) {
+          diagnosis = 'this is an outstandingly useful repaint boundary and should definitely be kept';
+        } else if (percentage > 50.0) {
+          diagnosis = 'this is a useful repaint boundary and should be kept';
+        } else if (percentage > 30.0) {
+          diagnosis = 'this repaint boundary is probably useful, but maybe it would be more useful in tandem with adding more repaint boundaries elsewhere';
+        } else if (percentage > 10.0) {
+          diagnosis = 'this repaint boundary does sometimes show value, though currently not that often';
+        } else if (debugAsymmetricPaintCount == 0) {
+          diagnosis = 'this repaint boundary is astoundingly ineffectual and should be removed';
+        } else {
+          diagnosis = 'this repaint boundary is not very effective and should probably be removed';
+        }
+        description.add('metrics: ${percentage.toStringAsFixed(1)}% useful ($debugSymmetricPaintCount bad vs $debugAsymmetricPaintCount good)');
+        description.add('diagnosis: $diagnosis');
+      }
+      return true;
+    });
+    if (inReleaseMode)
+      description.add('(run in checked mode to collect repaint boundary statistics)');
+  }
 }
 
 /// Is invisible during hit testing.