diff --git a/packages/flutter/lib/src/rendering/layer.dart b/packages/flutter/lib/src/rendering/layer.dart index f9466b0f50..80276f2418 100644 --- a/packages/flutter/lib/src/rendering/layer.dart +++ b/packages/flutter/lib/src/rendering/layer.dart @@ -100,10 +100,14 @@ abstract class Layer extends AbstractNode with TreeDiagnosticsMixin { /// that created this layer. Used in debug messages. dynamic debugCreator; + @override + String toString() => '${super.toString()}${ owner == null ? " DETACHED" : ""}'; + @override void debugFillDescription(List description) { super.debugFillDescription(description); - description.add('${ owner != null ? "owner: $owner" : "DETACHED" }'); + if (parent == null && owner != null) + description.add('owner: $owner'); if (debugCreator != null) description.add('creator: $debugCreator'); } diff --git a/packages/flutter/lib/src/rendering/object.dart b/packages/flutter/lib/src/rendering/object.dart index 0e83e49dad..44b6fdb322 100644 --- a/packages/flutter/lib/src/rendering/object.dart +++ b/packages/flutter/lib/src/rendering/object.dart @@ -67,18 +67,28 @@ class PaintingContext { /// Repaint the given render object. /// - /// 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. + /// The render object must be attached to a [PipelineOwner], must have a + /// composited layer, and must be in need of painting. The render object's + /// layer, if any, is re-used, along with any layers in the subtree that don't + /// need to be repainted. static void repaintCompositedChild(RenderObject child, { bool debugAlsoPaintedParent: false }) { assert(child.isRepaintBoundary); assert(child._needsPaint); assert(() { - child.debugRegisterRepaintBoundaryPaint(includedParent: debugAlsoPaintedParent, includedChild: true); + // register the call for RepaintBoundary metrics + child.debugRegisterRepaintBoundaryPaint( + includedParent: debugAlsoPaintedParent, + includedChild: true, + ); return true; }); - child._layer ??= new OffsetLayer(); - child._layer.removeAllChildren(); + if (child._layer == null) { + assert(debugAlsoPaintedParent); + child._layer = new OffsetLayer(); + } else { + assert(debugAlsoPaintedParent || child._layer.attached); + child._layer.removeAllChildren(); + } assert(() { child._layer.debugCreator = child.debugCreator ?? child.runtimeType; return true; @@ -125,7 +135,11 @@ class PaintingContext { } else { assert(child._layer != null); assert(() { - child.debugRegisterRepaintBoundaryPaint(includedParent: true, includedChild: false); + // register the call for RepaintBoundary metrics + child.debugRegisterRepaintBoundaryPaint( + includedParent: true, + includedChild: false, + ); child._layer.debugCreator = child.debugCreator ?? child; return true; }); @@ -1096,8 +1110,14 @@ class PipelineOwner { _nodesNeedingPaint = []; // Sort the dirty nodes in reverse order (deepest first). for (RenderObject node in dirtyNodes..sort((RenderObject a, RenderObject b) => b.depth - a.depth)) { - if (node._needsPaint && node.owner == this) - PaintingContext.repaintCompositedChild(node); + assert(node._layer != null); + if (node._needsPaint && node.owner == this) { + if (node._layer.attached) { + PaintingContext.repaintCompositedChild(node); + } else { + node._skippedPaintingOnLayer(); + } + } } assert(_nodesNeedingPaint.isEmpty); } finally { @@ -2132,6 +2152,31 @@ abstract class RenderObject extends AbstractNode implements HitTestTarget { } } + // Called when flushPaint() tries to make us paint but our layer is detached. + // To make sure that our subtree is repainted when it's finally reattached, + // even in the case where some ancestor layer is itself never marked dirty, we + // have to mark our entire detached subtree as dirty and needing to be + // repainted. That way, we'll eventually be repainted. + void _skippedPaintingOnLayer() { + assert(attached); + assert(isRepaintBoundary); + assert(_needsPaint); + assert(_layer != null); + assert(!_layer.attached); + AbstractNode ancestor = parent; + while (ancestor is RenderObject) { + final RenderObject node = ancestor; + if (node.isRepaintBoundary) { + if (node._layer == null) + break; // looks like the subtree here has never been painted. let it handle itself. + if (node._layer.attached) + break; // it's the one that detached us, so it's the one that will decide to repaint us. + node._needsPaint = true; + } + ancestor = node.parent; + } + } + /// Bootstrap the rendering pipeline by scheduling the very first paint. /// /// Requires that this render object is attached, is the root of the render @@ -2549,6 +2594,8 @@ abstract class RenderObject extends AbstractNode implements HitTestTarget { } if (_needsLayout) header += ' NEEDS-LAYOUT'; + if (_needsPaint) + header += ' NEEDS-PAINT'; if (!attached) header += ' DETACHED'; return header; @@ -2606,6 +2653,8 @@ abstract class RenderObject extends AbstractNode implements HitTestTarget { description.add('creator: $debugCreator'); description.add('parentData: $parentData'); description.add('constraints: $constraints'); + if (_layer != null) // don't access it via the "layer" getter since that's only valid when we don't need paint + description.add('layer: $_layer'); } /// Returns a string describing the current node's descendants. Each line of diff --git a/packages/flutter/lib/src/widgets/framework.dart b/packages/flutter/lib/src/widgets/framework.dart index ae53fdf936..10f0b35f9e 100644 --- a/packages/flutter/lib/src/widgets/framework.dart +++ b/packages/flutter/lib/src/widgets/framework.dart @@ -15,7 +15,7 @@ export 'dart:ui' show hashValues, hashList; export 'package:flutter/foundation.dart' show FlutterError, debugPrint, debugPrintStack; export 'package:flutter/foundation.dart' show VoidCallback, ValueChanged, ValueGetter, ValueSetter; -export 'package:flutter/rendering.dart' show RenderObject, RenderBox, debugDumpRenderTree; +export 'package:flutter/rendering.dart' show RenderObject, RenderBox, debugDumpRenderTree, debugDumpLayerTree; // KEYS diff --git a/packages/flutter/test/widgets/navigator_and_layers_test.dart b/packages/flutter/test/widgets/navigator_and_layers_test.dart new file mode 100644 index 0000000000..263dc209c7 --- /dev/null +++ b/packages/flutter/test/widgets/navigator_and_layers_test.dart @@ -0,0 +1,88 @@ +// 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 'package:flutter_test/flutter_test.dart'; +import 'package:flutter/material.dart'; + +import 'test_widgets.dart'; + +class TestCustomPainter extends CustomPainter { + TestCustomPainter({ this.log, this.name }); + + final List log; + final String name; + + @override + void paint(Canvas canvas, Size size) { + log.add(name); + } + + @override + bool shouldRepaint(TestCustomPainter oldPainter) { + return name != oldPainter.name + || log != oldPainter.log; + } +} + +void main() { + testWidgets('Do we paint when coming back from a navigation', (WidgetTester tester) async { + final List log = []; + log.add('0'); + await tester.pumpWidget( + new MaterialApp( + routes: { + '/': (BuildContext context) => new RepaintBoundary( + child: new Container( + child: new RepaintBoundary( + child: new FlipWidget( + left: new CustomPaint( + painter: new TestCustomPainter( + log: log, + name: 'left' + ), + ), + right: new CustomPaint( + painter: new TestCustomPainter( + log: log, + name: 'right' + ), + ), + ), + ), + ), + ), + '/second': (BuildContext context) => new Container(), + }, + ), + ); + log.add('1'); + final NavigatorState navigator = tester.state(find.byType(Navigator)); + navigator.pushNamed('/second'); + log.add('2'); + expect(await tester.pumpAndSettle(const Duration(minutes: 1)), 2); + log.add('3'); + flipStatefulWidget(tester, skipOffstage: false); + log.add('4'); + navigator.pop(); + log.add('5'); + expect(await tester.pumpAndSettle(const Duration(minutes: 1)), 2); + log.add('6'); + flipStatefulWidget(tester); + expect(await tester.pumpAndSettle(), 1); + log.add('7'); + expect(log, [ + '0', + 'left', + '1', + '2', + '3', + '4', + '5', + 'right', + '6', + 'left', + '7', + ]); + }); +} diff --git a/packages/flutter/test/widgets/test_widgets.dart b/packages/flutter/test/widgets/test_widgets.dart index 739044348c..a0d1b4badd 100644 --- a/packages/flutter/test/widgets/test_widgets.dart +++ b/packages/flutter/test/widgets/test_widgets.dart @@ -54,6 +54,6 @@ class FlipWidgetState extends State { } } -void flipStatefulWidget(WidgetTester tester) { - tester.state(find.byType(FlipWidget)).flip(); +void flipStatefulWidget(WidgetTester tester, { bool skipOffstage: true }) { + tester.state(find.byType(FlipWidget, skipOffstage: skipOffstage)).flip(); }