From 370d76c8d0811db0442785a010d1642a0380caed Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Mon, 8 Nov 2021 16:01:25 -0800 Subject: [PATCH 1/2] separate record and execute saveLayer events and trace more saveLayer calls --- flow/display_list_canvas.cc | 10 ++++++++-- flow/layers/layer.cc | 2 ++ lib/ui/painting/canvas.cc | 4 ++-- testing/dart/observatory/tracing_test.dart | 11 +++++++++-- 4 files changed, 21 insertions(+), 6 deletions(-) diff --git a/flow/display_list_canvas.cc b/flow/display_list_canvas.cc index 5b44c97d6071a..d87275cb19f51 100644 --- a/flow/display_list_canvas.cc +++ b/flow/display_list_canvas.cc @@ -19,6 +19,7 @@ void DisplayListCanvasDispatcher::restore() { } void DisplayListCanvasDispatcher::saveLayer(const SkRect* bounds, bool restore_with_paint) { + TRACE_EVENT0("flutter", "Canvas::saveLayer"); canvas_->saveLayer(bounds, restore_with_paint ? &paint() : nullptr); } @@ -170,8 +171,13 @@ void DisplayListCanvasDispatcher::drawAtlas(const sk_sp atlas, void DisplayListCanvasDispatcher::drawPicture(const sk_sp picture, const SkMatrix* matrix, bool render_with_attributes) { - canvas_->drawPicture(picture, matrix, - render_with_attributes ? &paint() : nullptr); + if (render_with_attributes) { + // drawPicture does an implicit saveLayer if an SkPaint is supplied. + TRACE_EVENT0("flutter", "Canvas::saveLayer"); + canvas_->drawPicture(picture, matrix, &paint()); + } else { + canvas_->drawPicture(picture, matrix, nullptr); + } } void DisplayListCanvasDispatcher::drawDisplayList( const sk_sp display_list) { diff --git a/flow/layers/layer.cc b/flow/layers/layer.cc index e66359857409a..8a34b775ef766 100644 --- a/flow/layers/layer.cc +++ b/flow/layers/layer.cc @@ -65,6 +65,7 @@ Layer::AutoSaveLayer::AutoSaveLayer(const PaintContext& paint_context, canvas_(save_mode == SaveMode::kInternalNodesCanvas ? *(paint_context.internal_nodes_canvas) : *(paint_context.leaf_nodes_canvas)) { + TRACE_EVENT0("flutter", "Canvas::saveLayer"); canvas_.saveLayer(bounds_, paint); } @@ -76,6 +77,7 @@ Layer::AutoSaveLayer::AutoSaveLayer(const PaintContext& paint_context, canvas_(save_mode == SaveMode::kInternalNodesCanvas ? *(paint_context.internal_nodes_canvas) : *(paint_context.leaf_nodes_canvas)) { + TRACE_EVENT0("flutter", "Canvas::saveLayer"); canvas_.saveLayer(layer_rec); } diff --git a/lib/ui/painting/canvas.cc b/lib/ui/painting/canvas.cc index 14270e994137f..9d2001fa89014 100644 --- a/lib/ui/painting/canvas.cc +++ b/lib/ui/painting/canvas.cc @@ -105,7 +105,7 @@ void Canvas::saveLayerWithoutBounds(const Paint& paint, if (!canvas_) { return; } - TRACE_EVENT0("flutter", "Canvas::saveLayer"); + TRACE_EVENT0("flutter", "ui.Canvas::saveLayer (Recorded)"); canvas_->saveLayer(nullptr, paint.paint()); } @@ -118,7 +118,7 @@ void Canvas::saveLayer(double left, if (!canvas_) { return; } - TRACE_EVENT0("flutter", "Canvas::saveLayer"); + TRACE_EVENT0("flutter", "ui.Canvas::saveLayer (Recorded)"); SkRect bounds = SkRect::MakeLTRB(left, top, right, bottom); canvas_->saveLayer(&bounds, paint.paint()); } diff --git a/testing/dart/observatory/tracing_test.dart b/testing/dart/observatory/tracing_test.dart index 50274db5bcfa8..ac11e4959f386 100644 --- a/testing/dart/observatory/tracing_test.dart +++ b/testing/dart/observatory/tracing_test.dart @@ -47,13 +47,20 @@ void main() { final vms.Timeline timeline = await vmService.getVMTimeline(); await vmService.dispose(); + int saveLayerRecordCount = 0; int saveLayerCount = 0; for (final vms.TimelineEvent event in timeline.traceEvents!) { final Map json = event.json!; - if (json['name'] == 'Canvas::saveLayer' && json['ph'] == 'B') { - saveLayerCount += 1; + if (json['ph'] == 'B') { + if (json['name'] == 'ui.Canvas::saveLayer (Recorded)') { + saveLayerRecordCount += 1; + } + if (json['name'] == 'Canvas::saveLayer') { + saveLayerCount += 1; + } } } + expect(saveLayerRecordCount, 2); expect(saveLayerCount, 2); }); } From 4fb97e9174918bfb43fbbef1958aebc14aa6ca3f Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Tue, 9 Nov 2021 11:26:48 -0800 Subject: [PATCH 2/2] review feedback, trace savePaint calls that use saveLayer --- flow/display_list_canvas.cc | 9 ++++++++- lib/ui/painting/canvas.cc | 7 +++++++ testing/dart/observatory/tracing_test.dart | 10 ++++++---- 3 files changed, 21 insertions(+), 5 deletions(-) diff --git a/flow/display_list_canvas.cc b/flow/display_list_canvas.cc index d87275cb19f51..72b8574d3c9f1 100644 --- a/flow/display_list_canvas.cc +++ b/flow/display_list_canvas.cc @@ -77,7 +77,14 @@ void DisplayListCanvasDispatcher::clipPath(const SkPath& path, } void DisplayListCanvasDispatcher::drawPaint() { - canvas_->drawPaint(paint()); + const SkPaint& sk_paint = paint(); + SkImageFilter* filter = sk_paint.getImageFilter(); + if (filter && !filter->asColorFilter(nullptr)) { + // drawPaint does an implicit saveLayer if an SkImageFilter is + // present that cannot be replaced by an SkColorFilter. + TRACE_EVENT0("flutter", "Canvas::saveLayer"); + } + canvas_->drawPaint(sk_paint); } void DisplayListCanvasDispatcher::drawColor(SkColor color, SkBlendMode mode) { canvas_->drawColor(color, mode); diff --git a/lib/ui/painting/canvas.cc b/lib/ui/painting/canvas.cc index 9d2001fa89014..a6075d054d68e 100644 --- a/lib/ui/painting/canvas.cc +++ b/lib/ui/painting/canvas.cc @@ -230,6 +230,13 @@ void Canvas::drawPaint(const Paint& paint, const PaintData& paint_data) { if (!canvas_) { return; } + const SkPaint* sk_paint = paint.paint(); + SkImageFilter* filter = sk_paint->getImageFilter(); + if (filter && !filter->asColorFilter(nullptr)) { + // drawPaint does an implicit saveLayer if an SkImageFilter is + // present that cannot be replaced by an SkColorFilter. + TRACE_EVENT0("flutter", "ui.Canvas::saveLayer (Recorded)"); + } canvas_->drawPaint(*paint.paint()); } diff --git a/testing/dart/observatory/tracing_test.dart b/testing/dart/observatory/tracing_test.dart index ac11e4959f386..597d436d2eb16 100644 --- a/testing/dart/observatory/tracing_test.dart +++ b/testing/dart/observatory/tracing_test.dart @@ -23,9 +23,11 @@ void main() { ); final Completer completer = Completer(); - window.onBeginFrame = (Duration timeStamp) { + window.onBeginFrame = (Duration timeStamp) async { final PictureRecorder recorder = PictureRecorder(); final Canvas canvas = Canvas(recorder); + canvas.drawColor(const Color(0xff0000ff), BlendMode.srcOut); + canvas.drawPaint(Paint()..imageFilter = ImageFilter.blur(sigmaX: 2, sigmaY: 3)); canvas.saveLayer(null, Paint()); canvas.saveLayer(const Rect.fromLTWH(0, 0, 100, 100), Paint()); canvas.drawRect(const Rect.fromLTRB(10, 10, 20, 20), Paint()); @@ -37,7 +39,7 @@ void main() { builder.addPicture(Offset.zero, picture); final Scene scene = builder.build(); - window.render(scene); + await scene.toImage(100, 100); scene.dispose(); completer.complete(); }; @@ -60,7 +62,7 @@ void main() { } } } - expect(saveLayerRecordCount, 2); - expect(saveLayerCount, 2); + expect(saveLayerRecordCount, 3); + expect(saveLayerCount, 3); }); }