From ee88a685f8d14ccdad3422998d51ceb0b2e50c5e Mon Sep 17 00:00:00 2001 From: Adam Barth Date: Fri, 8 Jan 2016 01:57:30 -0800 Subject: [PATCH] Optimize repainting in Scaffold Previously, we triggered a layout (and hence a repaint) when sliding the draw because we gave the draw loose constraints. The drawer uses an Align to move itself to the proper side of the screen, so it can have tight constraints, which makes it a layout boundary. Also, don't trigger a layout just because the Scaffold rebuilds. There isn't any state in the scaffold custom layout, so it doesn't need to repaint just because we created a new instance of the delegate. Finally, add the debugging infrastructure I used to find these issues. --- packages/flutter/lib/src/material/scaffold.dart | 11 ++++++----- packages/flutter/lib/src/rendering/custom_layout.dart | 2 +- packages/flutter/lib/src/rendering/debug.dart | 6 ++++++ packages/flutter/lib/src/rendering/object.dart | 11 +++++++++++ packages/flutter/lib/src/services/print.dart | 8 ++++++++ 5 files changed, 32 insertions(+), 6 deletions(-) diff --git a/packages/flutter/lib/src/material/scaffold.dart b/packages/flutter/lib/src/material/scaffold.dart index 5a354076a7c..7cfd99ce579 100644 --- a/packages/flutter/lib/src/material/scaffold.dart +++ b/packages/flutter/lib/src/material/scaffold.dart @@ -39,17 +39,17 @@ class _ScaffoldLayout extends MultiChildLayoutDelegate { // in this case the toolbar appears -after- the body in the stacking order, // so the toolbar's shadow is drawn on top of the body. - final BoxConstraints toolBarConstraints = looseConstraints.tightenWidth(size.width); + final BoxConstraints fullWidthConstraints = looseConstraints.tightenWidth(size.width); Size toolBarSize = Size.zero; if (isChild(_Child.toolBar)) { - toolBarSize = layoutChild(_Child.toolBar, toolBarConstraints); + toolBarSize = layoutChild(_Child.toolBar, fullWidthConstraints); positionChild(_Child.toolBar, Offset.zero); } if (isChild(_Child.body)) { final double bodyHeight = size.height - toolBarSize.height; - final BoxConstraints bodyConstraints = toolBarConstraints.tightenHeight(bodyHeight); + final BoxConstraints bodyConstraints = fullWidthConstraints.tightenHeight(bodyHeight); layoutChild(_Child.body, bodyConstraints); positionChild(_Child.body, new Offset(0.0, toolBarSize.height)); } @@ -63,7 +63,6 @@ class _ScaffoldLayout extends MultiChildLayoutDelegate { // non-zero height then it's inset from the parent's right and bottom edges // by _kFloatingActionButtonMargin. - final BoxConstraints fullWidthConstraints = looseConstraints.tightenWidth(size.width); Size bottomSheetSize = Size.zero; Size snackBarSize = Size.zero; @@ -89,10 +88,12 @@ class _ScaffoldLayout extends MultiChildLayoutDelegate { } if (isChild(_Child.drawer)) { - layoutChild(_Child.drawer, looseConstraints); + layoutChild(_Child.drawer, new BoxConstraints.tight(size)); positionChild(_Child.drawer, Offset.zero); } } + + bool shouldRelayout(MultiChildLayoutDelegate oldDelegate) => false; } class Scaffold extends StatefulComponent { diff --git a/packages/flutter/lib/src/rendering/custom_layout.dart b/packages/flutter/lib/src/rendering/custom_layout.dart index 5e212e1389e..f4be21ebcd1 100644 --- a/packages/flutter/lib/src/rendering/custom_layout.dart +++ b/packages/flutter/lib/src/rendering/custom_layout.dart @@ -92,7 +92,7 @@ abstract class MultiChildLayoutDelegate { } /// Override this method to return true when the children need to be laid out. - bool shouldRelayout(MultiChildLayoutDelegate oldDelegate) => true; + bool shouldRelayout(MultiChildLayoutDelegate oldDelegate); /// Layout and position all children given this widget's size and the specified /// constraints. This method must apply layoutChild() to each child. It should diff --git a/packages/flutter/lib/src/rendering/debug.dart b/packages/flutter/lib/src/rendering/debug.dart index 2662962f2c2..8f6adbe2251 100644 --- a/packages/flutter/lib/src/rendering/debug.dart +++ b/packages/flutter/lib/src/rendering/debug.dart @@ -52,6 +52,12 @@ HSVColor debugCurrentRepaintColor = const HSVColor.fromAHSV(0.4, 60.0, 1.0, 1.0) /// The amount to increment the hue of the current repaint color. double debugRepaintRainboxHueIncrement = 2.0; +/// Log the call stacks that mark render objects as needing paint. +bool debugPrintMarkNeedsPaintStacks = false; + +/// Log the call stacks that mark render objects as needing layout. +bool debugPrintMarkNeedsLayoutStacks = false; + List debugDescribeTransform(Matrix4 transform) { List matrix = transform.toString().split('\n').map((String s) => ' $s').toList(); matrix.removeLast(); diff --git a/packages/flutter/lib/src/rendering/object.dart b/packages/flutter/lib/src/rendering/object.dart index ca5fd39edda..17ea8b9cbe4 100644 --- a/packages/flutter/lib/src/rendering/object.dart +++ b/packages/flutter/lib/src/rendering/object.dart @@ -8,6 +8,7 @@ import 'dart:ui' as ui; import 'package:flutter/gestures.dart'; import 'package:flutter/scheduler.dart'; +import 'package:flutter/services.dart'; import 'package:vector_math/vector_math_64.dart'; import 'debug.dart'; @@ -556,6 +557,11 @@ abstract class RenderObject extends AbstractNode implements HitTestTarget { } assert(parent == this.parent); } else { + assert(() { + if (debugPrintMarkNeedsLayoutStacks) + debugPrintStack(); + return true; + }); _nodesNeedingLayout.add(this); Scheduler.instance.ensureVisualUpdate(); } @@ -950,6 +956,11 @@ abstract class RenderObject extends AbstractNode implements HitTestTarget { return; _needsPaint = true; if (hasLayer) { + assert(() { + if (debugPrintMarkNeedsPaintStacks) + debugPrintStack(); + return true; + }); // If we always have our own layer, then we can just repaint // ourselves without involving any other nodes. assert(_layer != null); diff --git a/packages/flutter/lib/src/services/print.dart b/packages/flutter/lib/src/services/print.dart index a676eec72a2..86dc01f3869 100644 --- a/packages/flutter/lib/src/services/print.dart +++ b/packages/flutter/lib/src/services/print.dart @@ -44,3 +44,11 @@ void _debugPrintTask() { _debugPrintStopwatch.start(); } } + +void debugPrintStack() { + try { + throw new Exception(); + } catch (e, stack) { + debugPrint(stack.toString()); + } +}