Make rendering use PaintingNodes for increased efficiency.

sky/sdk/lib/rendering/object.dart

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 import 'dart:math' as math;
 import 'dart:sky' as sky;
 import 'dart:sky' show Point, Offset, Size, Rect, Color, Paint, Path;
 
 import '../base/hit_test.dart';
 import '../base/node.dart';
 import '../base/scheduler.dart' as scheduler;
 
 export 'dart:sky' show Point, Offset, Size, Rect, Color, Paint, Path;
 export '../base/hit_test.dart' show HitTestTarget, HitTestEntry, HitTestResult;
 
 
 class ParentData {
   void detach() {
     detachSiblings();
   }
   void detachSiblings() { } // workaround for lack of inter-class mixins in Dart
   void merge(ParentData other) {
     // override this in subclasses to merge in data from other into this
     assert(other.runtimeType == this.runtimeType);
   }
   String toString() => '<none>';
 }
 
 class PaintingCanvas extends sky.Canvas {
-  PaintingCanvas(sky.PictureRecorder recorder, Size bounds) : super(recorder, bounds);
+  PaintingCanvas(sky.PictureRecorder recorder, Rect bounds) : super(recorder, bounds);
 
+  List<RenderObject> _descendentsWithPaintingCanvases; // used by RenderObject._updatePaintingCanvas() to find out which RenderObjects to ask to paint
   void paintChild(RenderObject child, Point point) {
-    child.paint(this, point.toOffset());
+    if (child.createNewDisplayList) {
+      if (_descendentsWithPaintingCanvases == null)
+        _descendentsWithPaintingCanvases = new List<RenderObject>();

[abarth-chromium, 2015/07/06 20:50:58]

You can simplify this pattern by writing:

final List<RenderObject> _descendentsWithPaintingCanvases = new
List<RenderObject>();

The list will be created lazily when you first access
_descendentsWithPaintingCanvases.

[iansf, 2015/07/07 20:36:47]

Done.

+      assert(!_descendentsWithPaintingCanvases.contains(child));
+      _descendentsWithPaintingCanvases.add(child);
+      translate(point.x, point.y);
+      drawPaintingNode(child._paintingNode);
+      translate(-point.x, -point.y);

[abarth-chromium, 2015/07/06 20:50:58]

It looks like drawPaintingNode should take a Point so we don't need to do this
matrix operations from Dart.  The Skia team is adding offsets to drawDrawable so
we can refactor the C++ to not use translate once we have that patch.

[iansf, 2015/07/07 20:36:47]

Done.

+    } else {
+      child._paintOnCanvas(this, point.toOffset());
+    }
   }
 }
 
 abstract class Constraints {
   const Constraints();
   bool get isTight;
 }
 
 abstract class RenderObject extends AbstractNode implements HitTestTarget {
 
@@ skipping 87 matching lines @@
       _cleanRelayoutSubtreeRootChildren();
     }
   }
   void _cleanRelayoutSubtreeRootChildren() { } // workaround for lack of inter-class mixins in Dart
   void scheduleInitialLayout() {
     assert(attached);
     assert(parent == null);
     assert(_relayoutSubtreeRoot == null);
     _relayoutSubtreeRoot = this;
     _nodesNeedingLayout.add(this);
+    _nodesNeedingPaint.add(this);
     scheduler.ensureVisualUpdate();
   }
   static void flushLayout() {
     sky.tracing.begin('RenderObject.flushLayout');
     _debugDoingLayout = true;
     try {
       List<RenderObject> dirtyNodes = _nodesNeedingLayout;
       _nodesNeedingLayout = new List<RenderObject>();
       dirtyNodes..sort((a, b) => a.depth - b.depth)..forEach((node) {
         if (node._needsLayout && node.attached)
@@ skipping 80 matching lines @@
 
   void rotate({
     int oldAngle, // 0..3
     int newAngle, // 0..3
     Duration time
   }) { }
 
 
   // PAINTING
 
-  static bool debugDoingPaint = false;
+  static List<RenderObject> _nodesNeedingPaint = new List<RenderObject>();
+  static bool _debugDoingPaint = false;
+  static bool get debugDoingPaint => _debugDoingPaint;
+
+  sky.PaintingNode _paintingNode = new sky.PaintingNode(); // only set if createNewDisplayList is true

[abarth-chromium, 2015/07/06 20:50:58]

Without |final|, this call to new will happen all the time.  Maybe leave it with
a default value of |null| ?

[iansf, 2015/07/07 21:11:22]

I made this final -- it's never reassigned.

+  sky.PaintingNode get paintingNode => _paintingNode;
+  bool _needsPaint = true;
+  bool get needsPaint => _needsPaint;
+  bool get createNewDisplayList => true;

[abarth-chromium, 2015/07/06 20:50:58]

I'm not sure we want the default behavior to be to create a new display list. 
Most RenderObjects don't paint anything at all, so that seems very wasteful.

[iansf, 2015/07/07 21:11:23]

Done.

+
   void markNeedsPaint() {
     assert(!debugDoingPaint);
-    scheduler.ensureVisualUpdate();
+    if (_needsPaint) return;
+    if (createNewDisplayList) {
+      _needsPaint = true;
+      _nodesNeedingPaint.add(this);
+      scheduler.ensureVisualUpdate();
+    } else if (parent != null) {
+      if (parent is RenderObject) {
+        (parent as RenderObject).markNeedsPaint(); // TODO(ianh): remove the cast once the analyzer is cleverer
+      }
+    }
   }
+
+  static void flushPaint() {
+    _debugDoingPaint = true;
+    List<RenderObject> dirtyNodes = _nodesNeedingPaint;
+    _nodesNeedingPaint = new List<RenderObject>();
+    dirtyNodes..sort((a, b) => a.depth - b.depth)..forEach((node) {
+      if (node._needsPaint && node.attached)
+        node._updatePaintingCanvas();
+    });
+    assert(_nodesNeedingPaint.length == 0);
+    _debugDoingPaint = false;

[abarth-chromium, 2015/07/06 20:50:58]

We should wrap the body of this function in a try .. finally block to make sure
we always set _debugDoingPaint = false on the way out of this function if we hit
an exception.  If we don't set _debugDoingPaint = false, we'll be in a very
corrupt state.

[iansf, 2015/07/07 20:36:47]

Done.

+  }
+
+  void _updatePaintingCanvas() {
+    assert(!_needsLayout);
+    assert(createNewDisplayList);
+    sky.PictureRecorder recorder = new sky.PictureRecorder();
+    PaintingCanvas canvas = new PaintingCanvas(recorder, paintBounds);
+    _needsPaint = false;
+    try {
+      _paintOnCanvas(canvas, Offset.zero);
+    } catch (e, stack) {

[abarth-chromium, 2015/07/06 20:50:58]

Capturing the stack here is going to be too expensive.  Can we just capture |e|
alone?

[Hixie, 2015/07/06 21:13:42]

If you change this make sure the equivalent code in layout is kept in sync.

[iansf, 2015/07/07 20:36:47]

Done.

[iansf, 2015/07/07 20:36:47]

Done.

+      print('Exception raised during _updatePaintingCanvas:\n${e}\nContext:\n${this}');
+      print(stack);
+      return;
+    }
+    assert(!_needsLayout); // check that the paint() method didn't mark us dirty again
+    assert(!_needsPaint); // check that the paint() method didn't mark us dirty again
+    _paintingNode.setBackingDrawable(recorder.endRecordingAsDrawable());
+
+    if (canvas._descendentsWithPaintingCanvases != null) {
+      canvas._descendentsWithPaintingCanvases.forEach((node) {

[abarth-chromium, 2015/07/06 20:50:57]

We should use a for-each loop rather than calling |forEach|.  Calling the
forEach function is more expensive because we need to allocate a stack from for
each entry in the list:

for (RenderObject node in canvas._descendentsWithPaintingCanvases) {
  ...
}

[iansf, 2015/07/07 20:36:47]

Done.

+        assert(node.attached == attached);
+        if (node._needsPaint)
+          node._updatePaintingCanvas();
+      });
+    }
+
+  }
+
+  void _paintOnCanvas(PaintingCanvas canvas, Offset offset) {
+    _needsPaint = false;

[abarth-chromium, 2015/07/06 20:50:58]

Why do we set _needsPaint = false both here and on line 294?  It seems like we
can remove the set on line 294.

[iansf, 2015/07/07 20:36:47]

Done.

+    paint(canvas, offset);
+    assert(!_needsPaint);
+  }
+
+  Rect get paintBounds;
+
   void paint(PaintingCanvas canvas, Offset offset) { }
 
 
   // EVENTS
 
   void handleEvent(sky.Event event, HitTestEntry entry) {
     // override this if you have a client, to hand it to the client
     // override this if you want to do anything with the event
   }
 
@@ skipping 273 matching lines @@
     int count = 1;
     ChildType child = _firstChild;
     while (child != null) {
       result += '${prefix}child ${count}: ${child.toString(prefix)}';
       count += 1;
       child = child.parentData.nextSibling;
     }
     return result;
   }
 }