flutter · dnfield · Apr 10, 2019 · Mar 28, 2019 · Mar 29, 2019 · Mar 29, 2019
diff --git a/packages/flutter/lib/src/rendering/binding.dart b/packages/flutter/lib/src/rendering/binding.dart
@@ -87,6 +87,17 @@ mixin RendererBinding on BindingBase, ServicesBinding, SchedulerBinding, Gesture
           return Future<void>.value();
         },
       );
+      registerBoolServiceExtension(
+        name: 'debugCheckElevationsEnabled',
+        getter: () async => debugCheckElevationsEnabled,
+        setter: (bool value) {
+          if (debugCheckElevationsEnabled == value) {
+            return Future<void>.value();
+          }
+          debugCheckElevationsEnabled = value;
+          return _forceRepaint();
+        }
+      );
       registerSignalServiceExtension(
         name: 'debugDumpLayerTree',
         callback: () {

diff --git a/packages/flutter/lib/src/rendering/debug.dart b/packages/flutter/lib/src/rendering/debug.dart
@@ -50,6 +50,44 @@ bool debugRepaintRainbowEnabled = false;
 /// Overlay a rotating set of colors when repainting text in checked mode.
 bool debugRepaintTextRainbowEnabled = false;
 
+/// Causes [PhysicalModelLayer]s to paint a red rectangle around themselves if
+/// they are overlapping and painted out of order with regard to their elevation.
+///
+/// Android and iOS will show the last painted layer on top, whereas Fuchsia
+/// will show the layer with the highest elevation on top.
+///
+/// For example, a rectangular elevation at 3.0 that is painted before an
+/// overlapping rectangular elevation at 2.0 would render this way on Android
+/// and iOS (with fake shadows):
+/// ```
+/// ┌───────────────────┐
+/// │                   │
+/// │      3.0          │
+/// │            ┌───────────────────┐
+/// │            │                   │
+/// └────────────│                   │
+///              │        2.0        │
+///              │                   │
+///              └───────────────────┘
+/// ```
+///
+/// But this way on Fuchsia (with real shadows):
+/// ```
+/// ┌───────────────────┐
+/// │                   │
+/// │      3.0          │
+/// │                   │────────────┐
+/// │                   │            │
+/// └───────────────────┘            │
+///              │         2.0       │
+///              │                   │
+///              └───────────────────┘
+/// ```
+///
+/// This check helps developers that want a consistent look and feel detect
+/// where this inconsistency would occur.
+bool debugCheckElevationsEnabled = false;
+
 /// The current color to overlay when repainting a layer.
 ///
 /// This is used by painting debug code that implements

diff --git a/packages/flutter/lib/src/rendering/layer.dart b/packages/flutter/lib/src/rendering/layer.dart
@@ -4,7 +4,8 @@
 
 import 'dart:async';
 import 'dart:collection';
-import 'dart:ui' as ui show EngineLayer, Image, ImageFilter, Picture, Scene, SceneBuilder;
+import 'dart:ui' as ui show EngineLayer, Image, ImageFilter, PathMetric,
+                            Picture, PictureRecorder, Scene, SceneBuilder;
 
 import 'package:flutter/foundation.dart';
 import 'package:flutter/painting.dart';
@@ -503,6 +504,100 @@ class ContainerLayer extends Layer {
     return child == equals;
   }
 
+  PictureLayer _highlightConflictingLayer(PhysicalModelLayer child) {
+    final ui.PictureRecorder recorder = ui.PictureRecorder();
+    final Canvas canvas = Canvas(recorder);
+    canvas.drawPath(
+      child.clipPath,
+      Paint()
+        ..color = const Color(0xFFAA0000)
+        ..style = PaintingStyle.stroke
+        // The elevation may be 0 or otherwise too small to notice.
+        // Adding 10 to it makes it more visually obvious.
+        ..strokeWidth = child.elevation + 10.0,
+    );
+    final PictureLayer pictureLayer = PictureLayer(child.clipPath.getBounds())
+      ..picture = recorder.endRecording()
+      ..debugCreator = child;
+    child.append(pictureLayer);
+    return pictureLayer;
+  }
+
+  List<PictureLayer> _processConflictingPhysicalLayers(PhysicalModelLayer predecessor, PhysicalModelLayer child) {
+    FlutterError.reportError(FlutterErrorDetails(
+      exception: FlutterError('Painting order is out of order with respect to elevation.\n'
+                              'See https://api.flutter.dev/flutter/rendering/debugCheckElevations.html '
+                              'for more details.'),
+      library: 'rendering library',
+      context: 'during compositing',
+      informationCollector: (StringBuffer buffer) {
+        buffer.writeln('Attempted to composite layer:');
+        buffer.writeln(child);
+        buffer.writeln('after layer:');
+        buffer.writeln(predecessor);
+        buffer.writeln('which occupies the same area at a higher elevation.');
+      }
+    ));
+    return <PictureLayer>[
+      _highlightConflictingLayer(predecessor),
+      _highlightConflictingLayer(child),
+    ];
+  }
+
+  /// Checks that no [PhysicalModelLayer] would paint after another overlapping
+  /// [PhysicalModelLayer] that has a higher elevation.
+  ///
+  /// Returns a list of [PictureLayer] objects it added to the tree to highlight
+  /// bad nodes. These layers should be removed from the tree after building the
+  /// [Scene].
+  List<PictureLayer> _debugCheckElevations() {
+    final List<PhysicalModelLayer> physicalModelLayers = depthFirstIterateChildren().whereType<PhysicalModelLayer>().toList();
+    final List<PictureLayer> addedLayers = <PictureLayer>[];
+
+    for (int i = 0; i < physicalModelLayers.length; i++) {
+      final PhysicalModelLayer physicalModelLayer = physicalModelLayers[i];
+      assert(
+        physicalModelLayer.lastChild?.debugCreator != physicalModelLayer,
+        'debugCheckElevations has either already visited this layer or failed '
+        'to remove the added picture from it.',
+      );
+      double accumulatedElevation = physicalModelLayer.elevation;
+      Layer ancestor = physicalModelLayer.parent;
+      while (ancestor != null) {
+        if (ancestor is PhysicalModelLayer) {
+          accumulatedElevation += ancestor.elevation;
+        }
+        ancestor = ancestor.parent;
+      }
+      for (int j = 0; j <= i; j++) {
+        final PhysicalModelLayer predecessor = physicalModelLayers[j];
+        double predecessorAccumulatedElevation = predecessor.elevation;
+        ancestor = predecessor.parent;
+        while (ancestor != null) {
+          if (ancestor == predecessor) {
+            continue;
+          }
+          if (ancestor is PhysicalModelLayer) {
+            predecessorAccumulatedElevation += ancestor.elevation;
+          }
+          ancestor = ancestor.parent;
+        }
+        if (predecessorAccumulatedElevation <= accumulatedElevation) {
+          continue;
+        }
+        final Path intersection = Path.combine(
+          PathOperation.intersect,
+          predecessor._debugTransformedClipPath,
+          physicalModelLayer._debugTransformedClipPath,
+        );
+        if (intersection != null && intersection.computeMetrics().any((ui.PathMetric metric) => metric.length > 0)) {
+          addedLayers.addAll(_processConflictingPhysicalLayers(predecessor, physicalModelLayer));
+        }
+      }
+    }
+    return addedLayers;
+  }
+
   @override
   void updateSubtreeNeedsAddToScene() {
     super.updateSubtreeNeedsAddToScene();
@@ -679,6 +774,23 @@ class ContainerLayer extends Layer {
     assert(transform != null);
   }
 
+  /// Returns the descendants of this layer in depth first order.
+  @visibleForTesting
+  List<Layer> depthFirstIterateChildren() {
+    if (firstChild == null)
+      return <Layer>[];
+    final List<Layer> children = <Layer>[];
+    Layer child = firstChild;
+    while(child != null) {
+      children.add(child);
+      if (child is ContainerLayer) {
+        children.addAll(child.depthFirstIterateChildren());
+      }
+      child = child.nextSibling;
+    }
+    return children;
+  }
+
   @override
   List<DiagnosticsNode> debugDescribeChildren() {
     final List<DiagnosticsNode> children = <DiagnosticsNode>[];
@@ -744,9 +856,29 @@ class OffsetLayer extends ContainerLayer {
   /// Consider this layer as the root and build a scene (a tree of layers)
   /// in the engine.
   ui.Scene buildScene(ui.SceneBuilder builder) {
+    List<PictureLayer> temporaryLayers;
+    assert(() {
+      if (debugCheckElevationsEnabled) {
+        temporaryLayers = _debugCheckElevations();
+      }
+      return true;
+    }());
     updateSubtreeNeedsAddToScene();
     addToScene(builder);
-    return builder.build();
+    final ui.Scene scene = builder.build();
+    assert(() {
+      // We should remove any layers that got added to highlight the incorrect
+      // PhysicalModelLayers. If we don't, we'll end up adding duplicate layers
+      // or potentially leaving a physical model that is now correct highlighted
+      // in red.
+      if (temporaryLayers != null) {
+        for (PictureLayer temporaryLayer in temporaryLayers) {
+          temporaryLayer.remove();
+        }
+      }
+      return true;
+    }());
+    return scene;
   }
 
   @override
@@ -1090,7 +1222,12 @@ class TransformLayer extends OffsetLayer {
   void applyTransform(Layer child, Matrix4 transform) {
     assert(child != null);
     assert(transform != null);
-    transform.multiply(_lastEffectiveTransform);
+    assert(_lastEffectiveTransform != null || this.transform != null);
+    if (_lastEffectiveTransform == null) {
+      transform.multiply(this.transform);
+    } else {
+      transform.multiply(_lastEffectiveTransform);
+    }
   }
 
   @override
@@ -1309,6 +1446,16 @@ class PhysicalModelLayer extends ContainerLayer {
     }
   }
 
+  Path get _debugTransformedClipPath {
+    ContainerLayer ancestor = parent;
+    final Matrix4 matrix = Matrix4.identity();
+    while (ancestor != null && ancestor.parent != null) {
+      ancestor.applyTransform(this, matrix);
+      ancestor = ancestor.parent;
+    }
+    return clipPath.transform(matrix.storage);
+  }
+
   /// {@macro flutter.widgets.Clip}
   Clip get clipBehavior => _clipBehavior;
   Clip _clipBehavior;

diff --git a/packages/flutter/lib/src/rendering/proxy_box.dart b/packages/flutter/lib/src/rendering/proxy_box.dart
@@ -1709,6 +1709,10 @@ class RenderPhysicalModel extends _RenderPhysicalModelBase<RRect> {
         color: color,
         shadowColor: shadowColor,
       );
+      assert(() {
+        physicalModel.debugCreator = debugCreator;
+        return true;
+      }());
       context.pushLayer(physicalModel, super.paint, offset, childPaintBounds: offsetBounds);
     }
   }
@@ -1799,6 +1803,10 @@ class RenderPhysicalShape extends _RenderPhysicalModelBase<Path> {
         color: color,
         shadowColor: shadowColor,
       );
+      assert(() {
+        physicalModel.debugCreator = debugCreator;
+        return true;
+      }());
       context.pushLayer(physicalModel, super.paint, offset, childPaintBounds: offsetBounds);
     }
   }

diff --git a/packages/flutter/test/foundation/service_extensions_test.dart b/packages/flutter/test/foundation/service_extensions_test.dart
@@ -117,7 +117,7 @@ Future<Map<String, dynamic>> hasReassemble(Future<Map<String, dynamic>> pendingR
 void main() {
   final List<String> console = <String>[];
 
-  test('Service extensions - pretest', () async {
+  setUpAll(() async {
     binding = TestServiceExtensionsBinding();
     expect(binding.frameScheduled, isTrue);
 
@@ -142,10 +142,25 @@ void main() {
     };
   });
 
+  tearDownAll(() async {
+    // See widget_inspector_test.dart for tests of the ext.flutter.inspector
+    // service extensions included in this count.
+    int widgetInspectorExtensionCount = 15;
+    if (WidgetInspectorService.instance.isWidgetCreationTracked()) {
+      // Some inspector extensions are only exposed if widget creation locations
+      // are tracked.
+      widgetInspectorExtensionCount += 2;
+    }
+
+    // If you add a service extension... TEST IT! :-)
+    // ...then increment this number.
+    expect(binding.extensions.length, 26 + widgetInspectorExtensionCount);
+
+    expect(console, isEmpty);
+    debugPrint = debugPrintThrottled;
+  });
+
   // The following list is alphabetical, one test per extension.
-  //
-  // The order doesn't really matter except that the pretest and posttest tests
-  // must be first and last respectively.
 
   test('Service extensions - debugAllowBanner', () async {
     Map<String, dynamic> result;
@@ -170,6 +185,34 @@ void main() {
     expect(binding.frameScheduled, isFalse);
   });
 
+  test('Service extensions - debugCheckElevationsEnabled', () async {
+    expect(binding.frameScheduled, isFalse);
+    expect(debugCheckElevationsEnabled, false);
+
+    bool lastValue = false;
+    Future<void> _updateAndCheck(bool newValue) async {
+      Map<String, dynamic> result;
+      binding.testExtension(
+        'debugCheckElevationsEnabled',
+        <String, String>{'enabled': '$newValue'}
+      ).then((Map<String, dynamic> answer) => result = answer);
+      await binding.flushMicrotasks();
+      expect(binding.frameScheduled, lastValue != newValue);
+      await binding.doFrame();
+      await binding.flushMicrotasks();
+      expect(result, <String, String>{'enabled': '$newValue'});
+      expect(debugCheckElevationsEnabled, newValue);
+      lastValue = newValue;
+    }
+
+    await _updateAndCheck(false);
+    await _updateAndCheck(true);
+    await _updateAndCheck(true);
+    await _updateAndCheck(false);
+    await _updateAndCheck(false);
+    expect(binding.frameScheduled, isFalse);
+  });
+
   test('Service extensions - debugDumpApp', () async {
     Map<String, dynamic> result;
 
@@ -617,22 +660,4 @@ void main() {
     expect(trace, contains('package:test_api/test_api.dart,::,test\n'));
     expect(trace, contains('service_extensions_test.dart,::,main\n'));
   });
-
-  test('Service extensions - posttest', () async {
-    // See widget_inspector_test.dart for tests of the ext.flutter.inspector
-    // service extensions included in this count.
-    int widgetInspectorExtensionCount = 15;
-    if (WidgetInspectorService.instance.isWidgetCreationTracked()) {
-      // Some inspector extensions are only exposed if widget creation locations
-      // are tracked.
-      widgetInspectorExtensionCount += 2;
-    }
-
-    // If you add a service extension... TEST IT! :-)
-    // ...then increment this number.
-    expect(binding.extensions.length, 25 + widgetInspectorExtensionCount);
-
-    expect(console, isEmpty);
-    debugPrint = debugPrintThrottled;
-  });
 }