From ab01c7bf7354ac5bf22e464e97b2bea7df698a4f Mon Sep 17 00:00:00 2001 From: Hixie Date: Thu, 17 Dec 2015 13:53:34 -0800 Subject: [PATCH] Less tree walking for compositing bits updates. Use the same technique for updating compositing bits as layout and painting. This avoids walking the entire rendering tree when all you need to update is a small subtree. --- .../flutter/lib/src/rendering/binding.dart | 2 +- .../flutter/lib/src/rendering/object.dart | 93 +++++++++++++------ .../test/rendering/rendering_tester.dart | 5 +- .../flutter/test/rendering/stack_test.dart | 9 +- 4 files changed, 75 insertions(+), 34 deletions(-) diff --git a/packages/flutter/lib/src/rendering/binding.dart b/packages/flutter/lib/src/rendering/binding.dart index aa328f44e91..1edbd7df405 100644 --- a/packages/flutter/lib/src/rendering/binding.dart +++ b/packages/flutter/lib/src/rendering/binding.dart @@ -69,7 +69,7 @@ abstract class Renderer extends Scheduler void beginFrame() { assert(renderView != null); RenderObject.flushLayout(); - renderView.updateCompositingBits(); + RenderObject.flushCompositingBits(); RenderObject.flushPaint(); renderView.compositeFrame(); } diff --git a/packages/flutter/lib/src/rendering/object.dart b/packages/flutter/lib/src/rendering/object.dart index d51cbb6a4ce..643c53a0551 100644 --- a/packages/flutter/lib/src/rendering/object.dart +++ b/packages/flutter/lib/src/rendering/object.dart @@ -391,6 +391,10 @@ RenderingExceptionHandler debugRenderingExceptionHandler; /// for their children. abstract class RenderObject extends AbstractNode implements HitTestTarget { + RenderObject() { + _needsCompositing = hasLayer; + } + // LAYOUT /// Data for use by the parent render object. @@ -492,7 +496,7 @@ abstract class RenderObject extends AbstractNode implements HitTestTarget { } } - static List _nodesNeedingLayout = new List(); + static List _nodesNeedingLayout = []; bool _needsLayout = true; /// Whether this render object's layout information is dirty. bool get needsLayout => _needsLayout; @@ -597,11 +601,11 @@ abstract class RenderObject extends AbstractNode implements HitTestTarget { // TODO(ianh): assert that we're not allowing previously dirty nodes to redirty themeselves while (_nodesNeedingLayout.isNotEmpty) { List dirtyNodes = _nodesNeedingLayout; - _nodesNeedingLayout = new List(); - dirtyNodes..sort((RenderObject a, RenderObject b) => a.depth - b.depth)..forEach((RenderObject node) { + _nodesNeedingLayout = []; + for (RenderObject node in dirtyNodes..sort((RenderObject a, RenderObject b) => a.depth - b.depth)) { if (node._needsLayout && node.attached) node._layoutWithoutResize(); - }); + } } } finally { _debugDoingLayout = false; @@ -822,7 +826,8 @@ abstract class RenderObject extends AbstractNode implements HitTestTarget { static RenderObject _debugActivePaint = null; static RenderObject get debugActivePaint => _debugActivePaint; - static List _nodesNeedingPaint = new List(); + static List _nodesNeedingPaint = []; + static List _nodesNeedingCompositingBitsUpdate = []; /// Whether this render object paints using a composited layer. /// @@ -843,49 +848,79 @@ abstract class RenderObject extends AbstractNode implements HitTestTarget { return _layer; } - bool _needsCompositingBitsUpdate = true; + bool _needsCompositingBitsUpdate = false; // set to true when a child is added /// Mark the compositing state for this render object as dirty. /// - /// When the subtree is mutated, we need to recompute our [needsCompositing] - /// bit, and our ancestors need to do the same (in case ours changed). - /// Therefore, [adoptChild] and [dropChild] call - /// [markNeedsCompositingBitsUpdate]. + /// When the subtree is mutated, we need to recompute our + /// [needsCompositing] bit, and some of our ancestors need to do the + /// same (in case ours changed in a way that will change theirs). To + /// this end, [adoptChild] and [dropChild] call this method, and, as + /// necessary, this method calls the parent's, etc, walking up the + /// tree to mark all the nodes that need updating. + /// + /// This method does not schedule a rendering frame, because since + /// it cannot be the case that _only_ the compositing bits changed, + /// something else will have scheduled a frame for us. void _markNeedsCompositingBitsUpdate() { if (_needsCompositingBitsUpdate) return; _needsCompositingBitsUpdate = true; - final AbstractNode parent = this.parent; - if (parent is RenderObject) - parent._markNeedsCompositingBitsUpdate(); - assert(parent == this.parent); - } - - bool _needsCompositing = false; - /// Whether we or one of our descendants has a compositing layer. - /// - /// Only legal to call after [flushLayout] and [updateCompositingBits] have - /// been called. - bool get needsCompositing { - assert(!_needsCompositingBitsUpdate); // make sure we don't use this bit when it is dirty - return _needsCompositing; + if (parent is RenderObject) { + final RenderObject parent = this.parent; + if (parent._needsCompositingBitsUpdate) + return; + if (!hasLayer && !parent.hasLayer) { + parent._markNeedsCompositingBitsUpdate(); + return; + } + } + assert(() { + final AbstractNode parent = this.parent; + if (parent is RenderObject) + return parent._needsCompositing; + return true; + }); + // parent is fine (or there isn't one), but we are dirty + _nodesNeedingCompositingBitsUpdate.add(this); } /// Updates the [needsCompositing] bits. /// /// Called as part of the rendering pipeline after [flushLayout] and before /// [flushPaint]. - void updateCompositingBits() { + static void flushCompositingBits() { + Timeline.startSync('Compositing Bits'); + _nodesNeedingCompositingBitsUpdate.sort((RenderObject a, RenderObject b) => a.depth - b.depth); + for (RenderObject node in _nodesNeedingCompositingBitsUpdate) { + if (node.attached) + node._updateCompositingBits(); + } + _nodesNeedingCompositingBitsUpdate.clear(); + Timeline.finishSync(); + } + + bool _needsCompositing; // initialised in the constructor + /// Whether we or one of our descendants has a compositing layer. + /// + /// Only legal to call after [flushLayout] and [flushCompositingBits] have + /// been called. + bool get needsCompositing { + assert(!_needsCompositingBitsUpdate); // make sure we don't use this bit when it is dirty + return _needsCompositing; + } + + void _updateCompositingBits() { if (!_needsCompositingBitsUpdate) return; - bool didHaveCompositedDescendant = _needsCompositing; + bool oldNeedsCompositing = _needsCompositing; visitChildren((RenderObject child) { - child.updateCompositingBits(); + child._updateCompositingBits(); if (child.needsCompositing) _needsCompositing = true; }); if (hasLayer) _needsCompositing = true; - if (didHaveCompositedDescendant != _needsCompositing) + if (oldNeedsCompositing != _needsCompositing) markNeedsPaint(); _needsCompositingBitsUpdate = false; } @@ -946,7 +981,7 @@ abstract class RenderObject extends AbstractNode implements HitTestTarget { _debugDoingPaint = true; try { List dirtyNodes = _nodesNeedingPaint; - _nodesNeedingPaint = new List(); + _nodesNeedingPaint = []; // Sort the dirty nodes in reverse order (deepest first). for (RenderObject node in dirtyNodes..sort((RenderObject a, RenderObject b) => b.depth - a.depth)) { assert(node._needsPaint); diff --git a/packages/flutter/test/rendering/rendering_tester.dart b/packages/flutter/test/rendering/rendering_tester.dart index 3142c42b8bc..cc67af2c884 100644 --- a/packages/flutter/test/rendering/rendering_tester.dart +++ b/packages/flutter/test/rendering/rendering_tester.dart @@ -21,6 +21,7 @@ class TestRenderView extends RenderView { enum EnginePhase { layout, + compositingBits, paint, composite } @@ -39,7 +40,9 @@ class TestRenderingFlutterBinding extends BindingBase with Scheduler, Renderer, RenderObject.flushLayout(); if (phase == EnginePhase.layout) return; - renderer.renderView.updateCompositingBits(); + RenderObject.flushCompositingBits(); + if (phase == EnginePhase.compositingBits) + return; RenderObject.flushPaint(); if (phase == EnginePhase.paint) return; diff --git a/packages/flutter/test/rendering/stack_test.dart b/packages/flutter/test/rendering/stack_test.dart index 8ca5253a438..304a747f6d0 100644 --- a/packages/flutter/test/rendering/stack_test.dart +++ b/packages/flutter/test/rendering/stack_test.dart @@ -10,18 +10,21 @@ import 'rendering_tester.dart'; void main() { test('Stack can layout with top, right, bottom, left 0.0', () { RenderBox size = new RenderConstrainedBox( - additionalConstraints: new BoxConstraints.tight(const Size(100.0, 100.0))); + additionalConstraints: new BoxConstraints.tight(const Size(100.0, 100.0)) + ); RenderBox red = new RenderDecoratedBox( decoration: new BoxDecoration( backgroundColor: const Color(0xFFFF0000) ), - child: size); + child: size + ); RenderBox green = new RenderDecoratedBox( decoration: new BoxDecoration( backgroundColor: const Color(0xFFFF0000) - )); + ) + ); RenderBox stack = new RenderStack(children: [red, green]); (green.parentData as StackParentData)