Commit ee88a685 authored by Adam Barth's avatar Adam Barth

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.
parent 51d06647
...@@ -39,17 +39,17 @@ class _ScaffoldLayout extends MultiChildLayoutDelegate { ...@@ -39,17 +39,17 @@ class _ScaffoldLayout extends MultiChildLayoutDelegate {
// in this case the toolbar appears -after- the body in the stacking order, // 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. // 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; Size toolBarSize = Size.zero;
if (isChild(_Child.toolBar)) { if (isChild(_Child.toolBar)) {
toolBarSize = layoutChild(_Child.toolBar, toolBarConstraints); toolBarSize = layoutChild(_Child.toolBar, fullWidthConstraints);
positionChild(_Child.toolBar, Offset.zero); positionChild(_Child.toolBar, Offset.zero);
} }
if (isChild(_Child.body)) { if (isChild(_Child.body)) {
final double bodyHeight = size.height - toolBarSize.height; final double bodyHeight = size.height - toolBarSize.height;
final BoxConstraints bodyConstraints = toolBarConstraints.tightenHeight(bodyHeight); final BoxConstraints bodyConstraints = fullWidthConstraints.tightenHeight(bodyHeight);
layoutChild(_Child.body, bodyConstraints); layoutChild(_Child.body, bodyConstraints);
positionChild(_Child.body, new Offset(0.0, toolBarSize.height)); positionChild(_Child.body, new Offset(0.0, toolBarSize.height));
} }
...@@ -63,7 +63,6 @@ class _ScaffoldLayout extends MultiChildLayoutDelegate { ...@@ -63,7 +63,6 @@ class _ScaffoldLayout extends MultiChildLayoutDelegate {
// non-zero height then it's inset from the parent's right and bottom edges // non-zero height then it's inset from the parent's right and bottom edges
// by _kFloatingActionButtonMargin. // by _kFloatingActionButtonMargin.
final BoxConstraints fullWidthConstraints = looseConstraints.tightenWidth(size.width);
Size bottomSheetSize = Size.zero; Size bottomSheetSize = Size.zero;
Size snackBarSize = Size.zero; Size snackBarSize = Size.zero;
...@@ -89,10 +88,12 @@ class _ScaffoldLayout extends MultiChildLayoutDelegate { ...@@ -89,10 +88,12 @@ class _ScaffoldLayout extends MultiChildLayoutDelegate {
} }
if (isChild(_Child.drawer)) { if (isChild(_Child.drawer)) {
layoutChild(_Child.drawer, looseConstraints); layoutChild(_Child.drawer, new BoxConstraints.tight(size));
positionChild(_Child.drawer, Offset.zero); positionChild(_Child.drawer, Offset.zero);
} }
} }
bool shouldRelayout(MultiChildLayoutDelegate oldDelegate) => false;
} }
class Scaffold extends StatefulComponent { class Scaffold extends StatefulComponent {
......
...@@ -92,7 +92,7 @@ abstract class MultiChildLayoutDelegate { ...@@ -92,7 +92,7 @@ abstract class MultiChildLayoutDelegate {
} }
/// Override this method to return true when the children need to be laid out. /// 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 /// Layout and position all children given this widget's size and the specified
/// constraints. This method must apply layoutChild() to each child. It should /// constraints. This method must apply layoutChild() to each child. It should
......
...@@ -52,6 +52,12 @@ HSVColor debugCurrentRepaintColor = const HSVColor.fromAHSV(0.4, 60.0, 1.0, 1.0) ...@@ -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. /// The amount to increment the hue of the current repaint color.
double debugRepaintRainboxHueIncrement = 2.0; 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<String> debugDescribeTransform(Matrix4 transform) { List<String> debugDescribeTransform(Matrix4 transform) {
List<String> matrix = transform.toString().split('\n').map((String s) => ' $s').toList(); List<String> matrix = transform.toString().split('\n').map((String s) => ' $s').toList();
matrix.removeLast(); matrix.removeLast();
......
...@@ -8,6 +8,7 @@ import 'dart:ui' as ui; ...@@ -8,6 +8,7 @@ import 'dart:ui' as ui;
import 'package:flutter/gestures.dart'; import 'package:flutter/gestures.dart';
import 'package:flutter/scheduler.dart'; import 'package:flutter/scheduler.dart';
import 'package:flutter/services.dart';
import 'package:vector_math/vector_math_64.dart'; import 'package:vector_math/vector_math_64.dart';
import 'debug.dart'; import 'debug.dart';
...@@ -556,6 +557,11 @@ abstract class RenderObject extends AbstractNode implements HitTestTarget { ...@@ -556,6 +557,11 @@ abstract class RenderObject extends AbstractNode implements HitTestTarget {
} }
assert(parent == this.parent); assert(parent == this.parent);
} else { } else {
assert(() {
if (debugPrintMarkNeedsLayoutStacks)
debugPrintStack();
return true;
});
_nodesNeedingLayout.add(this); _nodesNeedingLayout.add(this);
Scheduler.instance.ensureVisualUpdate(); Scheduler.instance.ensureVisualUpdate();
} }
...@@ -950,6 +956,11 @@ abstract class RenderObject extends AbstractNode implements HitTestTarget { ...@@ -950,6 +956,11 @@ abstract class RenderObject extends AbstractNode implements HitTestTarget {
return; return;
_needsPaint = true; _needsPaint = true;
if (hasLayer) { if (hasLayer) {
assert(() {
if (debugPrintMarkNeedsPaintStacks)
debugPrintStack();
return true;
});
// If we always have our own layer, then we can just repaint // If we always have our own layer, then we can just repaint
// ourselves without involving any other nodes. // ourselves without involving any other nodes.
assert(_layer != null); assert(_layer != null);
......
...@@ -44,3 +44,11 @@ void _debugPrintTask() { ...@@ -44,3 +44,11 @@ void _debugPrintTask() {
_debugPrintStopwatch.start(); _debugPrintStopwatch.start();
} }
} }
void debugPrintStack() {
try {
throw new Exception();
} catch (e, stack) {
debugPrint(stack.toString());
}
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment