Unverified Commit 7f26dbe0 authored by Jonah Williams's avatar Jonah Williams Committed by GitHub

[flutter] remove elevation checker (#86837)

parent 84c3b568
......@@ -100,17 +100,6 @@ 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();
},
);
registerServiceExtension(
name: 'debugDumpLayerTree',
callback: (Map<String, String> parameters) async {
......
......@@ -47,49 +47,6 @@ 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.
///
/// This check assumes that elevations on [PhysicalModelLayer] objects have
/// been set to non-null values before the scene is built. If this assumption
/// is violated, the check will throw exceptions. (The scene building would
/// also fail in that case, however.)
bool debugCheckElevationsEnabled = false;
/// The current color to overlay when repainting a layer.
///
/// This is used by painting debug code that implements
......
......@@ -937,13 +937,6 @@ class ContainerLayer extends Layer {
// both to render the whole layer tree (e.g. a normal application frame) and
// to render a subtree (e.g. `OffsetLayer.toImage`).
ui.Scene buildScene(ui.SceneBuilder builder) {
List<PictureLayer>? temporaryLayers;
assert(() {
if (debugCheckElevationsEnabled) {
temporaryLayers = _debugCheckElevations();
}
return true;
}());
updateSubtreeNeedsAddToScene();
addToScene(builder);
// Clearing the flag _after_ calling `addToScene`, not _before_. This is
......@@ -951,17 +944,6 @@ class ContainerLayer extends Layer {
// mark this layer as dirty.
_needsAddToScene = false;
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 continuing to render stale outlines.
if (temporaryLayers != null) {
for (final PictureLayer temporaryLayer in temporaryLayers!) {
temporaryLayer.remove();
}
}
return true;
}());
return scene;
}
......@@ -985,102 +967,6 @@ 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/debugCheckElevationsEnabled.html '
'for more details.',
),
library: 'rendering library',
context: ErrorDescription('during compositing'),
informationCollector: () {
return <DiagnosticsNode>[
child.toDiagnosticsNode(name: 'Attempted to composite layer', style: DiagnosticsTreeStyle.errorProperty),
predecessor.toDiagnosticsNode(name: 'after layer', style: DiagnosticsTreeStyle.errorProperty),
ErrorDescription('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 dispose() {
removeAllChildren();
......@@ -2122,16 +2008,6 @@ 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.material.Material.clipBehavior}
Clip get clipBehavior => _clipBehavior;
Clip _clipBehavior;
......
......@@ -167,7 +167,7 @@ void main() {
const int disabledExtensions = kIsWeb ? 2 : 0;
// If you add a service extension... TEST IT! :-)
// ...then increment this number.
expect(binding.extensions.length, 31 + widgetInspectorExtensionCount - disabledExtensions);
expect(binding.extensions.length, 30 + widgetInspectorExtensionCount - disabledExtensions);
expect(console, isEmpty);
debugPrint = debugPrintThrottled;
......@@ -198,34 +198,6 @@ 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 {
final Map<String, dynamic> result = await binding.testExtension('debugDumpApp', <String, String>{});
......
......@@ -418,172 +418,6 @@ void main() {
});
});
group('PhysicalModelLayer checks elevations', () {
/// Adds the layers to a container where A paints before B.
///
/// Expects there to be `expectedErrorCount` errors. Checking elevations is
/// enabled by default.
void _testConflicts(
PhysicalModelLayer layerA,
PhysicalModelLayer layerB, {
required int expectedErrorCount,
bool enableCheck = true,
}) {
assert(expectedErrorCount != null);
assert(enableCheck || expectedErrorCount == 0, 'Cannot disable check and expect non-zero error count.');
final OffsetLayer container = OffsetLayer();
container.append(layerA);
container.append(layerB);
debugCheckElevationsEnabled = enableCheck;
debugDisableShadows = false;
int errors = 0;
if (enableCheck) {
FlutterError.onError = (FlutterErrorDetails details) {
errors++;
};
}
container.buildScene(SceneBuilder());
expect(errors, expectedErrorCount);
debugCheckElevationsEnabled = false;
}
// Tests:
//
// ───────────── (LayerA, paints first)
// │ ───────────── (LayerB, paints second)
// │ │
// ───────────────────────────
test('Overlapping layers at wrong elevation', () {
final PhysicalModelLayer layerA = PhysicalModelLayer(
clipPath: Path()..addRect(const Rect.fromLTWH(0, 0, 20, 20)),
elevation: 3.0,
color: const Color(0x00000000),
shadowColor: const Color(0x00000000),
);
final PhysicalModelLayer layerB =PhysicalModelLayer(
clipPath: Path()..addRect(const Rect.fromLTWH(10, 10, 20, 20)),
elevation: 2.0,
color: const Color(0x00000000),
shadowColor: const Color(0x00000000),
);
_testConflicts(layerA, layerB, expectedErrorCount: 1);
}, skip: isBrowser); // https://github.com/flutter/flutter/issues/44572
// Tests:
//
// ───────────── (LayerA, paints first)
// │ ───────────── (LayerB, paints second)
// │ │
// ───────────────────────────
//
// Causes no error if check is disabled.
test('Overlapping layers at wrong elevation, check disabled', () {
final PhysicalModelLayer layerA = PhysicalModelLayer(
clipPath: Path()..addRect(const Rect.fromLTWH(0, 0, 20, 20)),
elevation: 3.0,
color: const Color(0x00000000),
shadowColor: const Color(0x00000000),
);
final PhysicalModelLayer layerB =PhysicalModelLayer(
clipPath: Path()..addRect(const Rect.fromLTWH(10, 10, 20, 20)),
elevation: 2.0,
color: const Color(0x00000000),
shadowColor: const Color(0x00000000),
);
_testConflicts(layerA, layerB, expectedErrorCount: 0, enableCheck: false);
});
// Tests:
//
// ────────── (LayerA, paints first)
// │ ─────────── (LayerB, paints second)
// │ │
// ────────────────────────────
test('Non-overlapping layers at wrong elevation', () {
final PhysicalModelLayer layerA = PhysicalModelLayer(
clipPath: Path()..addRect(const Rect.fromLTWH(0, 0, 20, 20)),
elevation: 3.0,
color: const Color(0x00000000),
shadowColor: const Color(0x00000000),
);
final PhysicalModelLayer layerB =PhysicalModelLayer(
clipPath: Path()..addRect(const Rect.fromLTWH(20, 20, 20, 20)),
elevation: 2.0,
color: const Color(0x00000000),
shadowColor: const Color(0x00000000),
);
_testConflicts(layerA, layerB, expectedErrorCount: 0);
}, skip: isBrowser); // https://github.com/flutter/flutter/issues/44572
// Tests:
//
// ─────── (Child of A, paints second)
// │
// ─────────── (LayerA, paints first)
// │ ──────────── (LayerB, paints third)
// │ │
// ────────────────────────────
test('Non-overlapping layers at wrong elevation, child at lower elevation', () {
final PhysicalModelLayer layerA = PhysicalModelLayer(
clipPath: Path()..addRect(const Rect.fromLTWH(0, 0, 20, 20)),
elevation: 3.0,
color: const Color(0x00000000),
shadowColor: const Color(0x00000000),
);
layerA.append(PhysicalModelLayer(
clipPath: Path()..addRect(const Rect.fromLTWH(2, 2, 10, 10)),
elevation: 1.0,
color: const Color(0x00000000),
shadowColor: const Color(0x00000000),
));
final PhysicalModelLayer layerB =PhysicalModelLayer(
clipPath: Path()..addRect(const Rect.fromLTWH(20, 20, 20, 20)),
elevation: 2.0,
color: const Color(0x00000000),
shadowColor: const Color(0x00000000),
);
_testConflicts(layerA, layerB, expectedErrorCount: 0);
}, skip: isBrowser); // https://github.com/flutter/flutter/issues/44572
// Tests:
//
// ─────────── (Child of A, paints second, overflows)
// │ ──────────── (LayerB, paints third)
// ─────────── │ (LayerA, paints first)
// │ │
// │ │
// ────────────────────────────
//
// Which fails because the overflowing child overlaps something that paints
// after it at a lower elevation.
test('Child overflows parent and overlaps another physical layer', () {
final PhysicalModelLayer layerA = PhysicalModelLayer(
clipPath: Path()..addRect(const Rect.fromLTWH(0, 0, 20, 20)),
elevation: 3.0,
color: const Color(0x00000000),
shadowColor: const Color(0x00000000),
);
layerA.append(PhysicalModelLayer(
clipPath: Path()..addRect(const Rect.fromLTWH(15, 15, 25, 25)),
elevation: 2.0,
color: const Color(0x00000000),
shadowColor: const Color(0x00000000),
));
final PhysicalModelLayer layerB =PhysicalModelLayer(
clipPath: Path()..addRect(const Rect.fromLTWH(20, 20, 20, 20)),
elevation: 4.0,
color: const Color(0x00000000),
shadowColor: const Color(0x00000000),
);
_testConflicts(layerA, layerB, expectedErrorCount: 1);
}, skip: isBrowser); // https://github.com/flutter/flutter/issues/44572
});
test('ContainerLayer.toImage can render interior layer', () {
final OffsetLayer parent = OffsetLayer();
final OffsetLayer child = OffsetLayer();
......
......@@ -167,12 +167,6 @@ class CommandHelp {
'debugDumpApp',
);
late final CommandHelpOption z = _makeOption(
'z',
'Toggle elevation checker.',
'debugCheckElevationsEnabled',
);
// When updating the list above, see the notes above the list regarding order
// and tests.
......
......@@ -738,22 +738,6 @@ abstract class ResidentHandlers {
return true;
}
/// Toggle the "elevation check" debugging feature.
Future<bool> debugToggleDebugCheckElevationsEnabled() async {
if (!supportsServiceProtocol) {
return false;
}
for (final FlutterDevice device in flutterDevices) {
final List<FlutterView> views = await device.vmService.getFlutterViews();
for (final FlutterView view in views) {
await device.vmService.flutterToggleDebugCheckElevationsEnabled(
isolateId: view.uiIsolate.id,
);
}
}
return true;
}
/// Toggle the performance overlay.
///
/// This is not supported in web mode.
......@@ -1426,7 +1410,6 @@ abstract class ResidentRunner extends ResidentHandlers {
commandHelp.I.print();
commandHelp.o.print();
commandHelp.b.print();
commandHelp.z.print();
} else {
commandHelp.S.print();
commandHelp.U.print();
......@@ -1659,9 +1642,6 @@ class TerminalHandler {
case 'w':
case 'W':
return residentRunner.debugDumpApp();
case 'z':
case 'Z':
return residentRunner.debugToggleDebugCheckElevationsEnabled();
}
return false;
}
......
......@@ -613,10 +613,6 @@ class FlutterVmService {
@required String isolateId,
}) => _flutterToggle('debugPaint', isolateId: isolateId);
Future<Map<String, dynamic>> flutterToggleDebugCheckElevationsEnabled({
@required String isolateId,
}) => _flutterToggle('debugCheckElevationsEnabled', isolateId: isolateId);
Future<Map<String, dynamic>> flutterTogglePerformanceOverlayOverride({
@required String isolateId,
}) => _flutterToggle('showPerformanceOverlay', isolateId: isolateId);
......
......@@ -72,7 +72,6 @@ void _testMessageLength({
expect(commandHelp.s.toString().length, lessThanOrEqualTo(expectedWidth));
expect(commandHelp.t.toString().length, lessThanOrEqualTo(expectedWidth));
expect(commandHelp.w.toString().length, lessThanOrEqualTo(expectedWidth));
expect(commandHelp.z.toString().length, lessThanOrEqualTo(expectedWidth));
}
void main() {
......@@ -123,7 +122,6 @@ void main() {
expect(commandHelp.s.toString(), startsWith('\x1B[1ms\x1B[22m'));
expect(commandHelp.t.toString(), startsWith('\x1B[1mt\x1B[22m'));
expect(commandHelp.w.toString(), startsWith('\x1B[1mw\x1B[22m'));
expect(commandHelp.z.toString(), startsWith('\x1B[1mz\x1B[22m'));
});
testWithoutContext('commands that should have a grey bolden parenthetical text', () {
......@@ -144,7 +142,6 @@ void main() {
expect(commandHelp.p.toString(), endsWith('\x1B[90m(debugPaintSizeEnabled)\x1B[39m\x1B[22m'));
expect(commandHelp.t.toString(), endsWith('\x1B[90m(debugDumpRenderTree)\x1B[39m\x1B[22m'));
expect(commandHelp.w.toString(), endsWith('\x1B[90m(debugDumpApp)\x1B[39m\x1B[22m'));
expect(commandHelp.z.toString(), endsWith('\x1B[90m(debugCheckElevationsEnabled)\x1B[39m\x1B[22m'));
});
testWithoutContext('should not create a help text longer than maxLineWidth without ansi support', () {
......@@ -208,7 +205,6 @@ void main() {
expect(commandHelp.t.toString(), equals('\x1B[1mt\x1B[22m Dump rendering tree to the console. \x1B[90m(debugDumpRenderTree)\x1B[39m\x1B[22m'));
expect(commandHelp.v.toString(), equals('\x1B[1mv\x1B[22m Open Flutter DevTools.'));
expect(commandHelp.w.toString(), equals('\x1B[1mw\x1B[22m Dump widget hierarchy to the console. \x1B[90m(debugDumpApp)\x1B[39m\x1B[22m'));
expect(commandHelp.z.toString(), equals('\x1B[1mz\x1B[22m Toggle elevation checker. \x1B[90m(debugCheckElevationsEnabled)\x1B[39m\x1B[22m'));
});
testWithoutContext('should create the correct help text without ansi support', () {
......@@ -240,7 +236,6 @@ void main() {
expect(commandHelp.t.toString(), equals('t Dump rendering tree to the console. (debugDumpRenderTree)'));
expect(commandHelp.v.toString(), equals('v Open Flutter DevTools.'));
expect(commandHelp.w.toString(), equals('w Dump widget hierarchy to the console. (debugDumpApp)'));
expect(commandHelp.z.toString(), equals('z Toggle elevation checker. (debugCheckElevationsEnabled)'));
});
});
});
......
......@@ -1239,7 +1239,6 @@ void main() {
commandHelp.I,
commandHelp.o,
commandHelp.b,
commandHelp.z,
commandHelp.P,
commandHelp.a,
commandHelp.M,
......
......@@ -600,7 +600,6 @@ void main() {
'I Toggle oversized image inversion. (debugInvertOversizedImages)',
'o Simulate different operating systems. (defaultTargetPlatform)',
'b Toggle platform brightness (dark and light mode). (debugBrightnessOverride)',
'z Toggle elevation checker. (debugCheckElevationsEnabled)',
'P Toggle performance overlay. (WidgetsApp.showPerformanceOverlay)',
'a Toggle timeline events for all widget build methods. (debugProfileWidgetBuilds)',
'M Write SkSL shaders to a unique file in the project directory.',
......
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