From e85c5bd677a385a02bad1b84f33c9ae4f6ccd720 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Wed, 22 Jun 2022 09:18:18 -0700 Subject: [PATCH 1/5] Make LayerTree::Flatten use DisplayList --- flow/layers/layer_tree.cc | 17 ++++++----------- flow/layers/layer_tree.h | 2 +- lib/ui/painting/picture.cc | 20 ++++++++++---------- lib/ui/painting/picture.h | 2 +- testing/dart/observatory/tracing_test.dart | 2 +- 5 files changed, 19 insertions(+), 24 deletions(-) diff --git a/flow/layers/layer_tree.cc b/flow/layers/layer_tree.cc index 25a084294365f..5d209bf7d737f 100644 --- a/flow/layers/layer_tree.cc +++ b/flow/layers/layer_tree.cc @@ -113,15 +113,10 @@ void LayerTree::Paint(CompositorContext::ScopedFrame& frame, } } -sk_sp LayerTree::Flatten(const SkRect& bounds) { +sk_sp LayerTree::Flatten(const SkRect& bounds) { TRACE_EVENT0("flutter", "LayerTree::Flatten"); - SkPictureRecorder recorder; - auto* canvas = recorder.beginRecording(bounds); - - if (!canvas) { - return nullptr; - } + DisplayListCanvasRecorder builder(bounds); MutatorsStack unused_stack; const FixedRefreshRateStopwatch unused_stopwatch; @@ -147,14 +142,14 @@ sk_sp LayerTree::Flatten(const SkRect& bounds) { // clang-format on }; - SkISize canvas_size = canvas->getBaseLayerSize(); + SkISize canvas_size = builder.getBaseLayerSize(); SkNWayCanvas internal_nodes_canvas(canvas_size.width(), canvas_size.height()); - internal_nodes_canvas.addCanvas(canvas); + internal_nodes_canvas.addCanvas(&builder); Layer::PaintContext paint_context = { // clang-format off .internal_nodes_canvas = &internal_nodes_canvas, - .leaf_nodes_canvas = canvas, + .leaf_nodes_canvas = &builder, .gr_context = nullptr, .view_embedder = nullptr, .raster_time = unused_stopwatch, @@ -178,7 +173,7 @@ sk_sp LayerTree::Flatten(const SkRect& bounds) { } } - return recorder.finishRecordingAsPicture(); + return builder.Build(); } } // namespace flutter diff --git a/flow/layers/layer_tree.h b/flow/layers/layer_tree.h index 4ee8b0d9a4818..6b1db0bc54fd4 100644 --- a/flow/layers/layer_tree.h +++ b/flow/layers/layer_tree.h @@ -35,7 +35,7 @@ class LayerTree { void Paint(CompositorContext::ScopedFrame& frame, bool ignore_raster_cache = false) const; - sk_sp Flatten(const SkRect& bounds); + sk_sp Flatten(const SkRect& bounds); Layer* root_layer() const { return root_layer_.get(); } diff --git a/lib/ui/painting/picture.cc b/lib/ui/painting/picture.cc index 6ba11849f2047..de8ef1f544589 100644 --- a/lib/ui/painting/picture.cc +++ b/lib/ui/painting/picture.cc @@ -58,17 +58,17 @@ Dart_Handle Picture::toImage(uint32_t width, uint32_t height, Dart_Handle raw_image_callback) { if (display_list_.skia_object()) { - return RasterizeToImage( - [display_list = display_list_.skia_object()](SkCanvas* canvas) { - display_list->RenderTo(canvas); - }, - width, height, raw_image_callback); + return RasterizeToImage(display_list_.skia_object(), width, height, + raw_image_callback); } else { if (!picture_.skia_object()) { return tonic::ToDart("Picture is null"); } - return RasterizeToImage(picture_.skia_object(), width, height, - raw_image_callback); + return RasterizeToImage( + [picture = picture_.skia_object()](SkCanvas* canvas) { + canvas->drawPicture(picture); + }, + width, height, raw_image_callback); } } @@ -88,13 +88,13 @@ size_t Picture::GetAllocationSize() const { } } -Dart_Handle Picture::RasterizeToImage(sk_sp picture, +Dart_Handle Picture::RasterizeToImage(sk_sp display_list, uint32_t width, uint32_t height, Dart_Handle raw_image_callback) { return RasterizeToImage( - [picture](SkCanvas* canvas) { canvas->drawPicture(picture); }, width, - height, raw_image_callback); + [display_list](SkCanvas* canvas) { display_list->RenderTo(canvas); }, + width, height, raw_image_callback); } Dart_Handle Picture::RasterizeToImage( diff --git a/lib/ui/painting/picture.h b/lib/ui/painting/picture.h index 046168baac086..130d4f86c6c4b 100644 --- a/lib/ui/painting/picture.h +++ b/lib/ui/painting/picture.h @@ -46,7 +46,7 @@ class Picture : public RefCountedDartWrappable { static void RegisterNatives(tonic::DartLibraryNatives* natives); - static Dart_Handle RasterizeToImage(sk_sp picture, + static Dart_Handle RasterizeToImage(sk_sp display_list, uint32_t width, uint32_t height, Dart_Handle raw_image_callback); diff --git a/testing/dart/observatory/tracing_test.dart b/testing/dart/observatory/tracing_test.dart index 3257a8ef06e3c..95845bd716394 100644 --- a/testing/dart/observatory/tracing_test.dart +++ b/testing/dart/observatory/tracing_test.dart @@ -65,6 +65,6 @@ void main() { } } expect(saveLayerRecordCount, 3); - expect(saveLayerCount, 3); + expect(saveLayerCount, 6); }); } From 0824e1e9b9b3aa076d9caf6d0034a09a950594f3 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Wed, 22 Jun 2022 11:34:25 -0700 Subject: [PATCH 2/5] Drop enable_display_list flag and associated code --- common/settings.h | 3 - display_list/display_list_test_utils.cc | 31 --- display_list/display_list_test_utils.h | 4 +- flow/BUILD.gn | 3 - flow/diff_context_unittests.cc | 4 +- flow/layers/container_layer_unittests.cc | 48 ++--- flow/layers/layer.h | 2 - flow/layers/opacity_layer.h | 2 +- flow/layers/opacity_layer_unittests.cc | 4 +- flow/layers/picture_layer_unittests.cc | 26 +-- flow/raster_cache.cc | 107 +--------- flow/raster_cache.h | 58 ++---- flow/raster_cache_unittests.cc | 231 +-------------------- flow/testing/diff_context_test.cc | 15 -- flow/testing/diff_context_test.h | 9 - flow/testing/mock_raster_cache.cc | 30 +-- flow/testing/mock_raster_cache.h | 18 +- lib/ui/compositing/scene_builder.cc | 13 +- lib/ui/painting/canvas.cc | 8 +- lib/ui/painting/image_dispose_unittests.cc | 15 +- lib/ui/painting/picture.cc | 32 +-- lib/ui/painting/picture.h | 5 - lib/ui/painting/picture_recorder.cc | 22 +- lib/ui/painting/picture_recorder.h | 3 - lib/ui/ui_dart_state.cc | 6 - lib/ui/ui_dart_state.h | 4 - runtime/dart_isolate.cc | 1 - shell/common/BUILD.gn | 10 +- shell/common/persistent_cache_unittests.cc | 1 - shell/common/shell_unittests.cc | 203 +++++++++--------- shell/common/switches.cc | 10 - 31 files changed, 201 insertions(+), 727 deletions(-) diff --git a/common/settings.h b/common/settings.h index afae2b64401db..8a12ebb98f055 100644 --- a/common/settings.h +++ b/common/settings.h @@ -203,9 +203,6 @@ struct Settings { // not supported on the platform. bool enable_impeller = false; - // Selects the DisplayList for storage of rendering operations. - bool enable_display_list = true; - // Data set by platform-specific embedders for use in font initialization. uint32_t font_initialization_data = 0; diff --git a/display_list/display_list_test_utils.cc b/display_list/display_list_test_utils.cc index 3ba91c135b34f..0d5c025cdfd41 100644 --- a/display_list/display_list_test_utils.cc +++ b/display_list/display_list_test_utils.cc @@ -5,22 +5,9 @@ #include "flutter/display_list/display_list_test_utils.h" #include "flutter/display_list/display_list_builder.h" -#include "third_party/skia/include/core/SkPicture.h" -#include "third_party/skia/include/core/SkPictureRecorder.h" - namespace flutter { namespace testing { -sk_sp GetSamplePicture() { - SkPictureRecorder recorder; - recorder.beginRecording(SkRect::MakeWH(150, 100)); - SkPaint paint; - paint.setColor(SK_ColorRED); - recorder.getRecordingCanvas()->drawRect(SkRect::MakeXYWH(10, 10, 80, 80), - paint); - return recorder.finishRecordingAsPicture(); -} - sk_sp GetSampleDisplayList() { DisplayListBuilder builder(SkRect::MakeWH(150, 100)); builder.setColor(SK_ColorRED); @@ -28,24 +15,6 @@ sk_sp GetSampleDisplayList() { return builder.Build(); } -sk_sp GetSampleNestedPicture() { - SkPictureRecorder recorder; - recorder.beginRecording(SkRect::MakeWH(150, 100)); - SkCanvas* canvas = recorder.getRecordingCanvas(); - SkPaint paint; - for (int y = 10; y <= 60; y += 10) { - for (int x = 10; x <= 60; x += 10) { - paint.setColor(((x + y) % 20) == 10 ? SK_ColorRED : SK_ColorBLUE); - canvas->drawRect(SkRect::MakeXYWH(x, y, 80, 80), paint); - } - } - SkPictureRecorder outer_recorder; - outer_recorder.beginRecording(SkRect::MakeWH(150, 100)); - canvas = outer_recorder.getRecordingCanvas(); - canvas->drawPicture(recorder.finishRecordingAsPicture()); - return outer_recorder.finishRecordingAsPicture(); -} - sk_sp GetSampleNestedDisplayList() { DisplayListBuilder builder(SkRect::MakeWH(150, 100)); for (int y = 10; y <= 60; y += 10) { diff --git a/display_list/display_list_test_utils.h b/display_list/display_list_test_utils.h index c29800d05fc28..f420427ddf30a 100644 --- a/display_list/display_list_test_utils.h +++ b/display_list/display_list_test_utils.h @@ -13,11 +13,9 @@ namespace flutter { namespace testing { -sk_sp GetSamplePicture(); sk_sp GetSampleDisplayList(); -sk_sp GetSampleNestedPicture(); -sk_sp GetSampleNestedDisplayList(); sk_sp GetSampleDisplayList(int ops); +sk_sp GetSampleNestedDisplayList(); } // namespace testing } // namespace flutter diff --git a/flow/BUILD.gn b/flow/BUILD.gn index d99fb4962bd91..f8377c86b8c55 100644 --- a/flow/BUILD.gn +++ b/flow/BUILD.gn @@ -49,8 +49,6 @@ source_set("flow") { "layers/performance_overlay_layer.h", "layers/physical_shape_layer.cc", "layers/physical_shape_layer.h", - "layers/picture_layer.cc", - "layers/picture_layer.h", "layers/platform_view_layer.cc", "layers/platform_view_layer.h", "layers/shader_mask_layer.cc", @@ -154,7 +152,6 @@ if (enable_unittests) { "layers/opacity_layer_unittests.cc", "layers/performance_overlay_layer_unittests.cc", "layers/physical_shape_layer_unittests.cc", - "layers/picture_layer_unittests.cc", "layers/platform_view_layer_unittests.cc", "layers/shader_mask_layer_unittests.cc", "layers/texture_layer_unittests.cc", diff --git a/flow/diff_context_unittests.cc b/flow/diff_context_unittests.cc index 26800468d5a22..e981258ef1a03 100644 --- a/flow/diff_context_unittests.cc +++ b/flow/diff_context_unittests.cc @@ -9,8 +9,8 @@ namespace testing { TEST_F(DiffContextTest, ClipAlignment) { MockLayerTree t1; - t1.root()->Add( - CreatePictureLayer(CreatePicture(SkRect::MakeLTRB(30, 30, 50, 50), 1))); + t1.root()->Add(CreateDisplayListLayer( + CreateDisplayList(SkRect::MakeLTRB(30, 30, 50, 50), 1))); auto damage = DiffLayerTree(t1, MockLayerTree(), SkIRect::MakeEmpty(), 0, 0); EXPECT_EQ(damage.frame_damage, SkIRect::MakeLTRB(30, 30, 50, 50)); EXPECT_EQ(damage.buffer_damage, SkIRect::MakeLTRB(30, 30, 50, 50)); diff --git a/flow/layers/container_layer_unittests.cc b/flow/layers/container_layer_unittests.cc index 8573c0ee957a9..bdb2661d428c5 100644 --- a/flow/layers/container_layer_unittests.cc +++ b/flow/layers/container_layer_unittests.cc @@ -251,16 +251,16 @@ using ContainerLayerDiffTest = DiffContextTest; // Insert PictureLayer amongst container layers TEST_F(ContainerLayerDiffTest, PictureLayerInsertion) { - auto pic1 = CreatePicture(SkRect::MakeLTRB(0, 0, 50, 50), 1); - auto pic2 = CreatePicture(SkRect::MakeLTRB(100, 0, 150, 50), 1); - auto pic3 = CreatePicture(SkRect::MakeLTRB(200, 0, 250, 50), 1); + auto pic1 = CreateDisplayList(SkRect::MakeLTRB(0, 0, 50, 50), 1); + auto pic2 = CreateDisplayList(SkRect::MakeLTRB(100, 0, 150, 50), 1); + auto pic3 = CreateDisplayList(SkRect::MakeLTRB(200, 0, 250, 50), 1); MockLayerTree t1; - auto t1_c1 = CreateContainerLayer(CreatePictureLayer(pic1)); + auto t1_c1 = CreateContainerLayer(CreateDisplayListLayer(pic1)); t1.root()->Add(t1_c1); - auto t1_c2 = CreateContainerLayer(CreatePictureLayer(pic2)); + auto t1_c2 = CreateContainerLayer(CreateDisplayListLayer(pic2)); t1.root()->Add(t1_c2); auto damage = DiffLayerTree(t1, MockLayerTree()); @@ -269,13 +269,13 @@ TEST_F(ContainerLayerDiffTest, PictureLayerInsertion) { // Add in the middle MockLayerTree t2; - auto t2_c1 = CreateContainerLayer(CreatePictureLayer(pic1)); + auto t2_c1 = CreateContainerLayer(CreateDisplayListLayer(pic1)); t2_c1->AssignOldLayer(t1_c1.get()); t2.root()->Add(t2_c1); - t2.root()->Add(CreatePictureLayer(pic3)); + t2.root()->Add(CreateDisplayListLayer(pic3)); - auto t2_c2 = CreateContainerLayer(CreatePictureLayer(pic2)); + auto t2_c2 = CreateContainerLayer(CreateDisplayListLayer(pic2)); t2_c2->AssignOldLayer(t1_c2.get()); t2.root()->Add(t2_c2); @@ -285,7 +285,7 @@ TEST_F(ContainerLayerDiffTest, PictureLayerInsertion) { // Add in the beginning t2 = MockLayerTree(); - t2.root()->Add(CreatePictureLayer(pic3)); + t2.root()->Add(CreateDisplayListLayer(pic3)); t2.root()->Add(t2_c1); t2.root()->Add(t2_c2); damage = DiffLayerTree(t2, t1); @@ -296,44 +296,44 @@ TEST_F(ContainerLayerDiffTest, PictureLayerInsertion) { t2 = MockLayerTree(); t2.root()->Add(t2_c1); t2.root()->Add(t2_c2); - t2.root()->Add(CreatePictureLayer(pic3)); + t2.root()->Add(CreateDisplayListLayer(pic3)); damage = DiffLayerTree(t2, t1); EXPECT_EQ(damage.frame_damage, SkIRect::MakeLTRB(200, 0, 250, 50)); } // Insert picture layer amongst other picture layers TEST_F(ContainerLayerDiffTest, PictureInsertion) { - auto pic1 = CreatePicture(SkRect::MakeLTRB(0, 0, 50, 50), 1); - auto pic2 = CreatePicture(SkRect::MakeLTRB(100, 0, 150, 50), 1); - auto pic3 = CreatePicture(SkRect::MakeLTRB(200, 0, 250, 50), 1); + auto pic1 = CreateDisplayList(SkRect::MakeLTRB(0, 0, 50, 50), 1); + auto pic2 = CreateDisplayList(SkRect::MakeLTRB(100, 0, 150, 50), 1); + auto pic3 = CreateDisplayList(SkRect::MakeLTRB(200, 0, 250, 50), 1); MockLayerTree t1; - t1.root()->Add(CreatePictureLayer(pic1)); - t1.root()->Add(CreatePictureLayer(pic2)); + t1.root()->Add(CreateDisplayListLayer(pic1)); + t1.root()->Add(CreateDisplayListLayer(pic2)); auto damage = DiffLayerTree(t1, MockLayerTree()); EXPECT_EQ(damage.frame_damage, SkIRect::MakeLTRB(0, 0, 150, 50)); MockLayerTree t2; - t2.root()->Add(CreatePictureLayer(pic3)); - t2.root()->Add(CreatePictureLayer(pic1)); - t2.root()->Add(CreatePictureLayer(pic2)); + t2.root()->Add(CreateDisplayListLayer(pic3)); + t2.root()->Add(CreateDisplayListLayer(pic1)); + t2.root()->Add(CreateDisplayListLayer(pic2)); damage = DiffLayerTree(t2, t1); EXPECT_EQ(damage.frame_damage, SkIRect::MakeLTRB(200, 0, 250, 50)); MockLayerTree t3; - t3.root()->Add(CreatePictureLayer(pic1)); - t3.root()->Add(CreatePictureLayer(pic3)); - t3.root()->Add(CreatePictureLayer(pic2)); + t3.root()->Add(CreateDisplayListLayer(pic1)); + t3.root()->Add(CreateDisplayListLayer(pic3)); + t3.root()->Add(CreateDisplayListLayer(pic2)); damage = DiffLayerTree(t3, t1); EXPECT_EQ(damage.frame_damage, SkIRect::MakeLTRB(200, 0, 250, 50)); MockLayerTree t4; - t4.root()->Add(CreatePictureLayer(pic1)); - t4.root()->Add(CreatePictureLayer(pic2)); - t4.root()->Add(CreatePictureLayer(pic3)); + t4.root()->Add(CreateDisplayListLayer(pic1)); + t4.root()->Add(CreateDisplayListLayer(pic2)); + t4.root()->Add(CreateDisplayListLayer(pic3)); damage = DiffLayerTree(t4, t1); EXPECT_EQ(damage.frame_damage, SkIRect::MakeLTRB(200, 0, 250, 50)); diff --git a/flow/layers/layer.h b/flow/layers/layer.h index 9a7316d4c887e..898fa103152a3 100644 --- a/flow/layers/layer.h +++ b/flow/layers/layer.h @@ -102,7 +102,6 @@ struct PrerollContext { }; class ContainerLayer; -class PictureLayer; class DisplayListLayer; class PerformanceOverlayLayer; class TextureLayer; @@ -360,7 +359,6 @@ class Layer { uint64_t unique_id() const { return unique_id_; } virtual const ContainerLayer* as_container_layer() const { return nullptr; } - virtual const PictureLayer* as_picture_layer() const { return nullptr; } virtual const DisplayListLayer* as_display_list_layer() const { return nullptr; } diff --git a/flow/layers/opacity_layer.h b/flow/layers/opacity_layer.h index c9cdd0738c410..32c25c5ba1948 100644 --- a/flow/layers/opacity_layer.h +++ b/flow/layers/opacity_layer.h @@ -21,7 +21,7 @@ class OpacityLayer : public ContainerLayer { // By default, that offset is always zero, and all the offsets are handled by // some parent TransformLayers. But we allow the offset to be non-zero for // backward compatibility. If it's non-zero, the old behavior is to propage - // that offset to all the leaf layers (e.g., PictureLayer). That will make + // that offset to all the leaf layers (e.g., DisplayListLayer). That will make // the retained rendering inefficient as a small offset change could propagate // to many leaf layers. Therefore we try to capture that offset here to stop // the propagation as repainting the OpacityLayer is expensive. diff --git a/flow/layers/opacity_layer_unittests.cc b/flow/layers/opacity_layer_unittests.cc index 6c6e3dc2367d8..6ba9d754b3c85 100644 --- a/flow/layers/opacity_layer_unittests.cc +++ b/flow/layers/opacity_layer_unittests.cc @@ -663,8 +663,8 @@ TEST_F(OpacityLayerTest, OpacityInheritanceNestedWithIncompatibleChild) { using OpacityLayerDiffTest = DiffContextTest; TEST_F(OpacityLayerDiffTest, FractionalTranslation) { - auto picture = - CreatePictureLayer(CreatePicture(SkRect::MakeLTRB(10, 10, 60, 60), 1)); + auto picture = CreateDisplayListLayer( + CreateDisplayList(SkRect::MakeLTRB(10, 10, 60, 60), 1)); auto layer = CreateOpacityLater({picture}, 128, SkPoint::Make(0.5, 0.5)); MockLayerTree tree1; diff --git a/flow/layers/picture_layer_unittests.cc b/flow/layers/picture_layer_unittests.cc index 7838c84808451..896060ac7e468 100644 --- a/flow/layers/picture_layer_unittests.cc +++ b/flow/layers/picture_layer_unittests.cc @@ -158,16 +158,16 @@ TEST_F(PictureLayerTest, OpacityInheritanceUncacheablePicture) { using PictureLayerDiffTest = DiffContextTest; TEST_F(PictureLayerDiffTest, SimplePicture) { - auto picture = CreatePicture(SkRect::MakeLTRB(10, 10, 60, 60), 1); + auto picture = CreateDisplayList(SkRect::MakeLTRB(10, 10, 60, 60), 1); MockLayerTree tree1; - tree1.root()->Add(CreatePictureLayer(picture)); + tree1.root()->Add(CreateDisplayListLayer(picture)); auto damage = DiffLayerTree(tree1, MockLayerTree()); EXPECT_EQ(damage.frame_damage, SkIRect::MakeLTRB(10, 10, 60, 60)); MockLayerTree tree2; - tree2.root()->Add(CreatePictureLayer(picture)); + tree2.root()->Add(CreateDisplayListLayer(picture)); damage = DiffLayerTree(tree2, tree1); EXPECT_TRUE(damage.frame_damage.isEmpty()); @@ -178,10 +178,10 @@ TEST_F(PictureLayerDiffTest, SimplePicture) { } TEST_F(PictureLayerDiffTest, FractionalTranslation) { - auto picture = CreatePicture(SkRect::MakeLTRB(10, 10, 60, 60), 1); + auto picture = CreateDisplayList(SkRect::MakeLTRB(10, 10, 60, 60), 1); MockLayerTree tree1; - tree1.root()->Add(CreatePictureLayer(picture, SkPoint::Make(0.5, 0.5))); + tree1.root()->Add(CreateDisplayListLayer(picture, SkPoint::Make(0.5, 0.5))); auto damage = DiffLayerTree(tree1, MockLayerTree()); #ifndef SUPPORT_FRACTIONAL_TRANSLATION @@ -193,31 +193,31 @@ TEST_F(PictureLayerDiffTest, FractionalTranslation) { TEST_F(PictureLayerDiffTest, PictureCompare) { MockLayerTree tree1; - auto picture1 = CreatePicture(SkRect::MakeLTRB(10, 10, 60, 60), 1); - tree1.root()->Add(CreatePictureLayer(picture1)); + auto picture1 = CreateDisplayList(SkRect::MakeLTRB(10, 10, 60, 60), 1); + tree1.root()->Add(CreateDisplayListLayer(picture1)); auto damage = DiffLayerTree(tree1, MockLayerTree()); EXPECT_EQ(damage.frame_damage, SkIRect::MakeLTRB(10, 10, 60, 60)); MockLayerTree tree2; - auto picture2 = CreatePicture(SkRect::MakeLTRB(10, 10, 60, 60), 1); - tree2.root()->Add(CreatePictureLayer(picture2)); + auto picture2 = CreateDisplayList(SkRect::MakeLTRB(10, 10, 60, 60), 1); + tree2.root()->Add(CreateDisplayListLayer(picture2)); damage = DiffLayerTree(tree2, tree1); EXPECT_TRUE(damage.frame_damage.isEmpty()); MockLayerTree tree3; - auto picture3 = CreatePicture(SkRect::MakeLTRB(10, 10, 60, 60), 1); + auto picture3 = CreateDisplayList(SkRect::MakeLTRB(10, 10, 60, 60), 1); // add offset - tree3.root()->Add(CreatePictureLayer(picture3, SkPoint::Make(10, 10))); + tree3.root()->Add(CreateDisplayListLayer(picture3, SkPoint::Make(10, 10))); damage = DiffLayerTree(tree3, tree2); EXPECT_EQ(damage.frame_damage, SkIRect::MakeLTRB(10, 10, 70, 70)); MockLayerTree tree4; // different color - auto picture4 = CreatePicture(SkRect::MakeLTRB(10, 10, 60, 60), 2); - tree4.root()->Add(CreatePictureLayer(picture4, SkPoint::Make(10, 10))); + auto picture4 = CreateDisplayList(SkRect::MakeLTRB(10, 10, 60, 60), 2); + tree4.root()->Add(CreateDisplayListLayer(picture4, SkPoint::Make(10, 10))); damage = DiffLayerTree(tree4, tree3); EXPECT_EQ(damage.frame_damage, SkIRect::MakeLTRB(20, 20, 70, 70)); diff --git a/flow/raster_cache.cc b/flow/raster_cache.cc index e6dbf96870489..4a19864dd3d45 100644 --- a/flow/raster_cache.cc +++ b/flow/raster_cache.cc @@ -15,7 +15,6 @@ #include "third_party/skia/include/core/SkCanvas.h" #include "third_party/skia/include/core/SkColorSpace.h" #include "third_party/skia/include/core/SkImage.h" -#include "third_party/skia/include/core/SkPicture.h" #include "third_party/skia/include/core/SkSurface.h" #include "third_party/skia/include/gpu/GrDirectContext.h" @@ -41,10 +40,9 @@ void RasterCacheResult::draw(SkCanvas& canvas, const SkPaint* paint) const { } RasterCache::RasterCache(size_t access_threshold, - size_t picture_and_display_list_cache_limit_per_frame) + size_t display_list_cache_limit_per_frame) : access_threshold_(access_threshold), - picture_and_display_list_cache_limit_per_frame_( - picture_and_display_list_cache_limit_per_frame), + display_list_cache_limit_per_frame_(display_list_cache_limit_per_frame), checkerboard_images_(false) {} static bool CanRasterizeRect(const SkRect& cull_rect) { @@ -62,32 +60,6 @@ static bool CanRasterizeRect(const SkRect& cull_rect) { return true; } -static bool IsPictureWorthRasterizing(SkPicture* picture, - bool will_change, - bool is_complex) { - if (will_change) { - // If the picture is going to change in the future, there is no point in - // doing to extra work to rasterize. - return false; - } - - if (picture == nullptr || !CanRasterizeRect(picture->cullRect())) { - // No point in deciding whether the picture is worth rasterizing if it - // cannot be rasterized at all. - return false; - } - - if (is_complex) { - // The caller seems to have extra information about the picture and thinks - // the picture is always worth rasterizing. - return true; - } - - // TODO(abarth): We should find a better heuristic here that lets us avoid - // wasting memory on trivial layers that are easy to re-rasterize every frame. - return picture->approximateOpCount(true) > 5; -} - static bool IsDisplayListWorthRasterizing( DisplayList* display_list, bool will_change, @@ -157,17 +129,6 @@ static std::unique_ptr Rasterize( logical_rect, type); } -std::unique_ptr RasterCache::RasterizePicture( - SkPicture* picture, - GrDirectContext* context, - const SkMatrix& ctm, - SkColorSpace* dst_color_space, - bool checkerboard) const { - return Rasterize(context, ctm, dst_color_space, checkerboard, - picture->cullRect(), "RasterCacheFlow::SkPicture", - [=](SkCanvas* canvas) { canvas->drawPicture(picture); }); -} - std::unique_ptr RasterCache::RasterizeDisplayList( DisplayList* display_list, GrDirectContext* context, @@ -277,54 +238,6 @@ const SkRect& RasterCache::GetPaintBoundsFromLayer( } } -bool RasterCache::Prepare(PrerollContext* context, - SkPicture* picture, - bool is_complex, - bool will_change, - const SkMatrix& untranslated_matrix, - const SkPoint& offset) { - if (!GenerateNewCacheInThisFrame()) { - return false; - } - - if (!IsPictureWorthRasterizing(picture, will_change, is_complex)) { - // We only deal with pictures that are worthy of rasterization. - return false; - } - - SkMatrix transformation_matrix = untranslated_matrix; - transformation_matrix.preTranslate(offset.x(), offset.y()); - - if (!transformation_matrix.invert(nullptr)) { - // The matrix was singular. No point in going further. - return false; - } - - RasterCacheKey cache_key(picture->uniqueID(), RasterCacheKeyType::kPicture, - transformation_matrix); - - // Creates an entry, if not present prior. - Entry& entry = cache_[cache_key]; - if (entry.access_count < access_threshold_) { - // Frame threshold has not yet been reached. - return false; - } - - if (!entry.image) { - // GetIntegralTransCTM effect for matrix which only contains scale, - // translate, so it won't affect result of matrix decomposition and cache - // key. -#ifndef SUPPORT_FRACTIONAL_TRANSLATION - transformation_matrix = GetIntegralTransCTM(transformation_matrix); -#endif - entry.image = - RasterizePicture(picture, context->gr_context, transformation_matrix, - context->dst_color_space, checkerboard_images_); - picture_cached_this_frame_++; - } - return true; -} - bool RasterCache::Prepare(PrerollContext* context, DisplayList* display_list, bool is_complex, @@ -391,13 +304,6 @@ void RasterCache::Touch(Layer* layer, Touch(cache_key_optional.value()); } -void RasterCache::Touch(SkPicture* picture, - const SkMatrix& transformation_matrix) { - RasterCacheKey cache_key(picture->uniqueID(), RasterCacheKeyType::kPicture, - transformation_matrix); - Touch(cache_key); -} - void RasterCache::Touch(DisplayList* display_list, const SkMatrix& transformation_matrix) { RasterCacheKey cache_key(display_list->unique_id(), @@ -414,14 +320,6 @@ void RasterCache::Touch(const RasterCacheKey& cache_key) { } } -bool RasterCache::Draw(const SkPicture& picture, - SkCanvas& canvas, - const SkPaint* paint) const { - RasterCacheKey cache_key(picture.uniqueID(), RasterCacheKeyType::kPicture, - canvas.getTotalMatrix()); - return Draw(cache_key, canvas, paint); -} - bool RasterCache::Draw(const DisplayList& display_list, SkCanvas& canvas, const SkPaint* paint) const { @@ -464,7 +362,6 @@ bool RasterCache::Draw(const RasterCacheKey& cache_key, } void RasterCache::PrepareNewFrame() { - picture_cached_this_frame_ = 0; display_list_cached_this_frame_ = 0; } diff --git a/flow/raster_cache.h b/flow/raster_cache.h index f3dbac5b7e17f..e84acc786cc33 100644 --- a/flow/raster_cache.h +++ b/flow/raster_cache.h @@ -92,19 +92,19 @@ class RasterCache { // generated per frame. Generating too many caches in one frame may cause jank // on that frame. This limit allows us to throttle the cache and distribute // the work across multiple frames. - static constexpr int kDefaultPictureAndDispLayListCacheLimitPerFrame = 3; + static constexpr int kDefaultDispLayListCacheLimitPerFrame = 3; explicit RasterCache(size_t access_threshold = 3, - size_t picture_and_display_list_cache_limit_per_frame = - kDefaultPictureAndDispLayListCacheLimitPerFrame); + size_t display_list_cache_limit_per_frame = + kDefaultDispLayListCacheLimitPerFrame); virtual ~RasterCache() = default; /** - * @brief Rasterize a picture object and produce a RasterCacheResult + * @brief Rasterize a display list object and produce a RasterCacheResult * to be stored in the cache. * - * @param picture the SkPicture object to be cached. + * @param display_list the DisplayList object to be cached. * @param context the GrDirectContext used for rendering. * @param ctm the transformation matrix used for rendering. * @param dst_color_space the destination color space that the cached @@ -112,15 +112,9 @@ class RasterCache { * @param checkerboard a flag indicating whether or not a checkerboard * pattern should be rendered into the cached image for debug * analysis - * @return a RasterCacheResult that can draw the rendered picture into + * @return a RasterCacheResult that can draw the rendered display list into * the destination using a simple image blit */ - virtual std::unique_ptr RasterizePicture( - SkPicture* picture, - GrDirectContext* context, - const SkMatrix& ctm, - SkColorSpace* dst_color_space, - bool checkerboard) const; virtual std::unique_ptr RasterizeDisplayList( DisplayList* display_list, GrDirectContext* context, @@ -182,17 +176,11 @@ class RasterCache { // Return true if the cache is generated. // // We may return false and not generate the cache if - // 1. There are too many pictures to be cached in the current frame. - // (See also kDefaultPictureAndDispLayListCacheLimitPerFrame.) - // 2. The picture is not worth rasterizing + // 1. There are too many display lists to be cached in the current frame. + // (See also kDefaultDispLayListCacheLimitPerFrame.) + // 2. The display list is not worth rasterizing // 3. The matrix is singular - // 4. The picture is accessed too few times - bool Prepare(PrerollContext* context, - SkPicture* picture, - bool is_complex, - bool will_change, - const SkMatrix& untranslated_matrix, - const SkPoint& offset = SkPoint()); + // 4. The display list is accessed too few times bool Prepare(PrerollContext* context, DisplayList* display_list, bool is_complex, @@ -200,11 +188,10 @@ class RasterCache { const SkMatrix& untranslated_matrix, const SkPoint& offset = SkPoint()); - // If there is cache entry for this picture, display list or layer, mark it as + // If there is cache entry for this display list or layer, mark it as // used for this frame in order to not get evicted. This is needed during // partial repaint for layers that are outside of current clip and are culled // away. - void Touch(SkPicture* picture, const SkMatrix& transformation_matrix); void Touch(DisplayList* display_list, const SkMatrix& transformation_matrix); void Touch( Layer* layer, @@ -217,13 +204,6 @@ class RasterCache { const SkMatrix& ctm, RasterCacheLayerStrategy strategey = RasterCacheLayerStrategy::kLayer); - // Find the raster cache for the picture and draw it to the canvas. - // - // Return true if it's found and drawn. - bool Draw(const SkPicture& picture, - SkCanvas& canvas, - const SkPaint* paint = nullptr) const; - // Find the raster cache for the display list and draw it to the canvas. // // Return true if it's found and drawn. @@ -262,16 +242,14 @@ class RasterCache { size_t GetLayerCachedEntriesCount() const; /** - * Return the number of map entries in the picture caches (SkPicture and - * DisplayList) regardless of whether the entries have been populated with - * an image. + * Return the number of map entries in the picture cache regardless of whether + * the entries have been populated with an image. */ size_t GetPictureCachedEntriesCount() const; /** * @brief Estimate how much memory is used by picture raster cache entries in - * bytes, including cache entries in the SkPicture cache and the DisplayList - * cache. + * bytes, including cache entries in the DisplayList cache. * * Only SkImage's memory usage is counted as other objects are often much * smaller compared to SkImage. SkImageInfo::computeMinByteSize is used to @@ -318,9 +296,8 @@ class RasterCache { bool GenerateNewCacheInThisFrame() const { // Disabling caching when access_threshold is zero is historic behavior. - return access_threshold_ != 0 && - picture_cached_this_frame_ + display_list_cached_this_frame_ < - picture_and_display_list_cache_limit_per_frame_; + return access_threshold_ != 0 && display_list_cached_this_frame_ < + display_list_cache_limit_per_frame_; } std::optional TryToMakeRasterCacheKeyForLayer( @@ -333,8 +310,7 @@ class RasterCache { RasterCacheLayerStrategy strategy) const; const size_t access_threshold_; - const size_t picture_and_display_list_cache_limit_per_frame_; - size_t picture_cached_this_frame_ = 0; + const size_t display_list_cache_limit_per_frame_; size_t display_list_cached_this_frame_ = 0; RasterCacheMetrics layer_metrics_; RasterCacheMetrics picture_metrics_; diff --git a/flow/raster_cache_unittests.cc b/flow/raster_cache_unittests.cc index cfb37b87fd215..d549100230d22 100644 --- a/flow/raster_cache_unittests.cc +++ b/flow/raster_cache_unittests.cc @@ -10,8 +10,6 @@ #include "gtest/gtest.h" #include "third_party/skia/include/core/SkCanvas.h" #include "third_party/skia/include/core/SkPaint.h" -#include "third_party/skia/include/core/SkPicture.h" -#include "third_party/skia/include/core/SkPictureRecorder.h" namespace flutter { namespace testing { @@ -21,50 +19,13 @@ TEST(RasterCache, SimpleInitialization) { ASSERT_TRUE(true); } -TEST(RasterCache, ThresholdIsRespectedForSkPicture) { - size_t threshold = 2; - flutter::RasterCache cache(threshold); - - SkMatrix matrix = SkMatrix::I(); - - auto picture = GetSamplePicture(); - - SkCanvas dummy_canvas; - - PrerollContextHolder preroll_context_holder = GetSamplePrerollContextHolder(); - - cache.PrepareNewFrame(); - - ASSERT_FALSE(cache.Prepare(&preroll_context_holder.preroll_context, - picture.get(), true, false, matrix)); - // 1st access. - ASSERT_FALSE(cache.Draw(*picture, dummy_canvas)); - - cache.CleanupAfterFrame(); - cache.PrepareNewFrame(); - - ASSERT_FALSE(cache.Prepare(&preroll_context_holder.preroll_context, - picture.get(), true, false, matrix)); - - // 2nd access. - ASSERT_FALSE(cache.Draw(*picture, dummy_canvas)); - - cache.CleanupAfterFrame(); - cache.PrepareNewFrame(); - - // Now Prepare should cache it. - ASSERT_TRUE(cache.Prepare(&preroll_context_holder.preroll_context, - picture.get(), true, false, matrix)); - ASSERT_TRUE(cache.Draw(*picture, dummy_canvas)); -} - TEST(RasterCache, MetricsOmitUnpopulatedEntries) { size_t threshold = 2; flutter::RasterCache cache(threshold); SkMatrix matrix = SkMatrix::I(); - auto picture = GetSamplePicture(); + auto display_list = GetSampleDisplayList(); SkCanvas dummy_canvas; @@ -73,9 +34,9 @@ TEST(RasterCache, MetricsOmitUnpopulatedEntries) { cache.PrepareNewFrame(); ASSERT_FALSE(cache.Prepare(&preroll_context_holder.preroll_context, - picture.get(), true, false, matrix)); + display_list.get(), true, false, matrix)); // 1st access. - ASSERT_FALSE(cache.Draw(*picture, dummy_canvas)); + ASSERT_FALSE(cache.Draw(*display_list, dummy_canvas)); cache.CleanupAfterFrame(); ASSERT_EQ(cache.picture_metrics().total_count(), 0u); @@ -83,10 +44,10 @@ TEST(RasterCache, MetricsOmitUnpopulatedEntries) { cache.PrepareNewFrame(); ASSERT_FALSE(cache.Prepare(&preroll_context_holder.preroll_context, - picture.get(), true, false, matrix)); + display_list.get(), true, false, matrix)); // 2nd access. - ASSERT_FALSE(cache.Draw(*picture, dummy_canvas)); + ASSERT_FALSE(cache.Draw(*display_list, dummy_canvas)); cache.CleanupAfterFrame(); ASSERT_EQ(cache.picture_metrics().total_count(), 0u); @@ -95,13 +56,13 @@ TEST(RasterCache, MetricsOmitUnpopulatedEntries) { // Now Prepare should cache it. ASSERT_TRUE(cache.Prepare(&preroll_context_holder.preroll_context, - picture.get(), true, false, matrix)); - ASSERT_TRUE(cache.Draw(*picture, dummy_canvas)); + display_list.get(), true, false, matrix)); + ASSERT_TRUE(cache.Draw(*display_list, dummy_canvas)); cache.CleanupAfterFrame(); ASSERT_EQ(cache.picture_metrics().total_count(), 1u); // 150w * 100h * 4bpp - ASSERT_EQ(cache.picture_metrics().total_bytes(), 60000u); + ASSERT_EQ(cache.picture_metrics().total_bytes(), 25600u); } TEST(RasterCache, ThresholdIsRespectedForDisplayList) { @@ -141,26 +102,6 @@ TEST(RasterCache, ThresholdIsRespectedForDisplayList) { ASSERT_TRUE(cache.Draw(*display_list, dummy_canvas)); } -TEST(RasterCache, AccessThresholdOfZeroDisablesCachingForSkPicture) { - size_t threshold = 0; - flutter::RasterCache cache(threshold); - - SkMatrix matrix = SkMatrix::I(); - - auto picture = GetSamplePicture(); - - SkCanvas dummy_canvas; - - PrerollContextHolder preroll_context_holder = GetSamplePrerollContextHolder(); - - cache.PrepareNewFrame(); - - ASSERT_FALSE(cache.Prepare(&preroll_context_holder.preroll_context, - picture.get(), true, false, matrix)); - - ASSERT_FALSE(cache.Draw(*picture, dummy_canvas)); -} - TEST(RasterCache, AccessThresholdOfZeroDisablesCachingForDisplayList) { size_t threshold = 0; flutter::RasterCache cache(threshold); @@ -181,26 +122,6 @@ TEST(RasterCache, AccessThresholdOfZeroDisablesCachingForDisplayList) { ASSERT_FALSE(cache.Draw(*display_list, dummy_canvas)); } -TEST(RasterCache, PictureCacheLimitPerFrameIsRespectedWhenZeroForSkPicture) { - size_t picture_cache_limit_per_frame = 0; - flutter::RasterCache cache(3, picture_cache_limit_per_frame); - - SkMatrix matrix = SkMatrix::I(); - - auto picture = GetSamplePicture(); - - SkCanvas dummy_canvas; - - PrerollContextHolder preroll_context_holder = GetSamplePrerollContextHolder(); - - cache.PrepareNewFrame(); - - ASSERT_FALSE(cache.Prepare(&preroll_context_holder.preroll_context, - picture.get(), true, false, matrix)); - - ASSERT_FALSE(cache.Draw(*picture, dummy_canvas)); -} - TEST(RasterCache, PictureCacheLimitPerFrameIsRespectedWhenZeroForDisplayList) { size_t picture_cache_limit_per_frame = 0; flutter::RasterCache cache(3, picture_cache_limit_per_frame); @@ -221,41 +142,6 @@ TEST(RasterCache, PictureCacheLimitPerFrameIsRespectedWhenZeroForDisplayList) { ASSERT_FALSE(cache.Draw(*display_list, dummy_canvas)); } -TEST(RasterCache, SweepsRemoveUnusedSkPictures) { - size_t threshold = 1; - flutter::RasterCache cache(threshold); - - SkMatrix matrix = SkMatrix::I(); - - auto picture = GetSamplePicture(); - - SkCanvas dummy_canvas; - - PrerollContextHolder preroll_context_holder = GetSamplePrerollContextHolder(); - - cache.PrepareNewFrame(); - - ASSERT_FALSE(cache.Prepare(&preroll_context_holder.preroll_context, - picture.get(), true, false, matrix)); // 1 - ASSERT_FALSE(cache.Draw(*picture, dummy_canvas)); - - cache.CleanupAfterFrame(); - cache.PrepareNewFrame(); - - ASSERT_TRUE(cache.Prepare(&preroll_context_holder.preroll_context, - picture.get(), true, false, matrix)); // 2 - ASSERT_TRUE(cache.Draw(*picture, dummy_canvas)); - - cache.CleanupAfterFrame(); - - cache.PrepareNewFrame(); - cache.CleanupAfterFrame(); // Extra frame without a Get image access. - - cache.PrepareNewFrame(); - - ASSERT_FALSE(cache.Draw(*picture, dummy_canvas)); -} - TEST(RasterCache, SweepsRemoveUnusedDisplayLists) { size_t threshold = 1; flutter::RasterCache cache(threshold); @@ -291,45 +177,6 @@ TEST(RasterCache, SweepsRemoveUnusedDisplayLists) { ASSERT_FALSE(cache.Draw(*display_list, dummy_canvas)); } -// Construct a cache result whose device target rectangle rounds out to be one -// pixel wider than the cached image. Verify that it can be drawn without -// triggering any assertions. -TEST(RasterCache, DeviceRectRoundOutForSkPicture) { - size_t threshold = 1; - flutter::RasterCache cache(threshold); - - SkPictureRecorder recorder; - SkRect logical_rect = SkRect::MakeLTRB(28, 0, 354.56731, 310.288); - recorder.beginRecording(logical_rect); - SkPaint paint; - paint.setColor(SK_ColorRED); - recorder.getRecordingCanvas()->drawRect(logical_rect, paint); - sk_sp picture = recorder.finishRecordingAsPicture(); - - SkMatrix ctm = SkMatrix::MakeAll(1.3312, 0, 233, 0, 1.3312, 206, 0, 0, 1); - - SkCanvas canvas(100, 100, nullptr); - canvas.setMatrix(ctm); - - PrerollContextHolder preroll_context_holder = GetSamplePrerollContextHolder(); - - cache.PrepareNewFrame(); - - ASSERT_FALSE(cache.Prepare(&preroll_context_holder.preroll_context, - picture.get(), true, false, ctm)); - ASSERT_FALSE(cache.Draw(*picture, canvas)); - - cache.CleanupAfterFrame(); - cache.PrepareNewFrame(); - - ASSERT_TRUE(cache.Prepare(&preroll_context_holder.preroll_context, - picture.get(), true, false, ctm)); - ASSERT_TRUE(cache.Draw(*picture, canvas)); - - canvas.translate(248, 0); - ASSERT_TRUE(cache.Draw(*picture, canvas)); -} - // The device rect is pixel snapped if SUPPORT_FRACTIONAL_TRANSLATION // is disabled. TEST(RasterCache, ComputeDeviceRectBasedOnFractionalTranslation) { @@ -380,34 +227,6 @@ TEST(RasterCache, DeviceRectRoundOutForDisplayList) { ASSERT_TRUE(cache.Draw(*display_list, canvas)); } -TEST(RasterCache, NestedOpCountMetricUsedForSkPicture) { - size_t threshold = 1; - flutter::RasterCache cache(threshold); - - SkMatrix matrix = SkMatrix::I(); - - auto picture = GetSampleNestedPicture(); - ASSERT_EQ(picture->approximateOpCount(), 1); - ASSERT_EQ(picture->approximateOpCount(true), 36); - - SkCanvas dummy_canvas; - - PrerollContextHolder preroll_context_holder = GetSamplePrerollContextHolder(); - - cache.PrepareNewFrame(); - - ASSERT_FALSE(cache.Prepare(&preroll_context_holder.preroll_context, - picture.get(), false, false, matrix)); - ASSERT_FALSE(cache.Draw(*picture, dummy_canvas)); - - cache.CleanupAfterFrame(); - cache.PrepareNewFrame(); - - ASSERT_TRUE(cache.Prepare(&preroll_context_holder.preroll_context, - picture.get(), false, false, matrix)); - ASSERT_TRUE(cache.Draw(*picture, dummy_canvas)); -} - TEST(RasterCache, NestedOpCountMetricUsedForDisplayList) { size_t threshold = 1; flutter::RasterCache cache(threshold); @@ -492,40 +311,6 @@ TEST(RasterCache, NaiveComplexityScoringDisplayList) { ASSERT_TRUE(cache.Draw(*display_list, dummy_canvas)); } -TEST(RasterCache, SkPictureWithSingularMatrixIsNotCached) { - size_t threshold = 2; - flutter::RasterCache cache(threshold); - - SkMatrix matrices[] = { - SkMatrix::Scale(0, 1), - SkMatrix::Scale(1, 0), - SkMatrix::Skew(1, 1), - }; - int matrixCount = sizeof(matrices) / sizeof(matrices[0]); - - auto picture = GetSamplePicture(); - - SkCanvas dummy_canvas; - - PrerollContextHolder preroll_context_holder = GetSamplePrerollContextHolder(); - - for (int i = 0; i < 10; i++) { - cache.PrepareNewFrame(); - - for (int j = 0; j < matrixCount; j++) { - ASSERT_FALSE(cache.Prepare(&preroll_context_holder.preroll_context, - picture.get(), true, false, matrices[j])); - } - - for (int j = 0; j < matrixCount; j++) { - dummy_canvas.setMatrix(matrices[j]); - ASSERT_FALSE(cache.Draw(*picture, dummy_canvas)); - } - - cache.CleanupAfterFrame(); - } -} - TEST(RasterCache, DisplayListWithSingularMatrixIsNotCached) { size_t threshold = 2; flutter::RasterCache cache(threshold); diff --git a/flow/testing/diff_context_test.cc b/flow/testing/diff_context_test.cc index c7cf07e1f2ff8..e9d5a4b05d9e6 100644 --- a/flow/testing/diff_context_test.cc +++ b/flow/testing/diff_context_test.cc @@ -29,21 +29,6 @@ Damage DiffContextTest::DiffLayerTree(MockLayerTree& layer_tree, vertical_clip_alignment); } -sk_sp DiffContextTest::CreatePicture(const SkRect& bounds, - uint32_t color) { - SkPictureRecorder recorder; - SkCanvas* recording_canvas = recorder.beginRecording(bounds); - recording_canvas->drawRect(bounds, SkPaint(SkColor4f::FromBytes_RGBA(color))); - return recorder.finishRecordingAsPicture(); -} - -std::shared_ptr DiffContextTest::CreatePictureLayer( - sk_sp picture, - const SkPoint& offset) { - return std::make_shared( - offset, SkiaGPUObject(picture, unref_queue()), false, false); -} - sk_sp DiffContextTest::CreateDisplayList(const SkRect& bounds, SkColor color) { DisplayListBuilder builder; diff --git a/flow/testing/diff_context_test.h b/flow/testing/diff_context_test.h index ffc37f314251b..246e54817e251 100644 --- a/flow/testing/diff_context_test.h +++ b/flow/testing/diff_context_test.h @@ -5,7 +5,6 @@ #include "flutter/flow/layers/container_layer.h" #include "flutter/flow/layers/display_list_layer.h" #include "flutter/flow/layers/opacity_layer.h" -#include "flutter/flow/layers/picture_layer.h" #include "flutter/flow/testing/skia_gpu_object_layer_test.h" #include "third_party/skia/include/core/SkPicture.h" #include "third_party/skia/include/core/SkPictureRecorder.h" @@ -42,14 +41,6 @@ class DiffContextTest : public ThreadTest { int horizontal_clip_alignment = 0, int vertical_alignment = 0); - // Create picture consisting of filled rect with given color; Being able - // to specify different color is useful to test deep comparison of pictures - sk_sp CreatePicture(const SkRect& bounds, uint32_t color); - - std::shared_ptr CreatePictureLayer( - sk_sp picture, - const SkPoint& offset = SkPoint::Make(0, 0)); - // Create display list consisting of filled rect with given color; Being able // to specify different color is useful to test deep comparison of pictures sk_sp CreateDisplayList(const SkRect& bounds, uint32_t color); diff --git a/flow/testing/mock_raster_cache.cc b/flow/testing/mock_raster_cache.cc index 4f1cf2c27886d..b9907d4c77187 100644 --- a/flow/testing/mock_raster_cache.cc +++ b/flow/testing/mock_raster_cache.cc @@ -4,8 +4,6 @@ #include "flutter/flow/testing/mock_raster_cache.h" -#include "third_party/skia/include/core/SkPictureRecorder.h" - namespace flutter { namespace testing { @@ -13,18 +11,6 @@ MockRasterCacheResult::MockRasterCacheResult(SkRect device_rect) : RasterCacheResult(nullptr, SkRect::MakeEmpty(), "RasterCacheFlow::test"), device_rect_(device_rect) {} -std::unique_ptr MockRasterCache::RasterizePicture( - SkPicture* picture, - GrDirectContext* context, - const SkMatrix& ctm, - SkColorSpace* dst_color_space, - bool checkerboard) const { - SkRect logical_rect = picture->cullRect(); - SkRect cache_rect = RasterCache::GetDeviceBounds(logical_rect, ctm); - - return std::make_unique(cache_rect); -} - std::unique_ptr MockRasterCache::RasterizeDisplayList( DisplayList* display_list, GrDirectContext* context, @@ -61,21 +47,19 @@ void MockRasterCache::AddMockLayer(int width, int height) { void MockRasterCache::AddMockPicture(int width, int height) { FML_DCHECK(access_threshold() > 0); SkMatrix ctm = SkMatrix::I(); - SkPictureRecorder skp_recorder; - SkRTreeFactory rtree_factory; + DisplayListCanvasRecorder recorder( + SkRect::MakeLTRB(0, 0, 200 + width, 200 + height)); SkPath path; path.addRect(100, 100, 100 + width, 100 + height); - SkCanvas* recorder_canvas = skp_recorder.beginRecording( - SkRect::MakeLTRB(0, 0, 200 + width, 200 + height), &rtree_factory); - recorder_canvas->drawPath(path, SkPaint()); - sk_sp picture = skp_recorder.finishRecordingAsPicture(); + recorder.drawPath(path, SkPaint()); + sk_sp display_list = recorder.Build(); PrerollContextHolder holder = GetSamplePrerollContextHolder(); holder.preroll_context.dst_color_space = color_space_; for (int i = 0; i < access_threshold(); i++) { - Prepare(&holder.preroll_context, picture.get(), true, false, ctm); - Draw(*picture, mock_canvas_); + Prepare(&holder.preroll_context, display_list.get(), true, false, ctm); + Draw(*display_list, mock_canvas_); } - Prepare(&holder.preroll_context, picture.get(), true, false, ctm); + Prepare(&holder.preroll_context, display_list.get(), true, false, ctm); } PrerollContextHolder GetSamplePrerollContextHolder() { diff --git a/flow/testing/mock_raster_cache.h b/flow/testing/mock_raster_cache.h index d9921728d1798..bd99b94cb3682 100644 --- a/flow/testing/mock_raster_cache.h +++ b/flow/testing/mock_raster_cache.h @@ -12,7 +12,6 @@ #include "third_party/skia/include/core/SkColorSpace.h" #include "third_party/skia/include/core/SkColorType.h" #include "third_party/skia/include/core/SkImage.h" -#include "third_party/skia/include/core/SkPicture.h" namespace flutter { namespace testing { @@ -51,19 +50,10 @@ class MockRasterCacheResult : public RasterCacheResult { */ class MockRasterCache : public RasterCache { public: - explicit MockRasterCache( - size_t access_threshold = 3, - size_t picture_and_display_list_cache_limit_per_frame = - kDefaultPictureAndDispLayListCacheLimitPerFrame) - : RasterCache(access_threshold, - picture_and_display_list_cache_limit_per_frame) {} - - std::unique_ptr RasterizePicture( - SkPicture* picture, - GrDirectContext* context, - const SkMatrix& ctm, - SkColorSpace* dst_color_space, - bool checkerboard) const override; + explicit MockRasterCache(size_t access_threshold = 3, + size_t display_list_cache_limit_per_frame = + kDefaultDispLayListCacheLimitPerFrame) + : RasterCache(access_threshold, display_list_cache_limit_per_frame) {} std::unique_ptr RasterizeDisplayList( DisplayList* display_list, diff --git a/lib/ui/compositing/scene_builder.cc b/lib/ui/compositing/scene_builder.cc index 210d28af650df..2a2810ba99fa3 100644 --- a/lib/ui/compositing/scene_builder.cc +++ b/lib/ui/compositing/scene_builder.cc @@ -17,7 +17,6 @@ #include "flutter/flow/layers/opacity_layer.h" #include "flutter/flow/layers/performance_overlay_layer.h" #include "flutter/flow/layers/physical_shape_layer.h" -#include "flutter/flow/layers/picture_layer.h" #include "flutter/flow/layers/platform_view_layer.h" #include "flutter/flow/layers/shader_mask_layer.h" #include "flutter/flow/layers/texture_layer.h" @@ -272,15 +271,9 @@ void SceneBuilder::addPicture(double dx, return; } - // Explicitly check for both conditions, since the picture object might have - // been disposed but not collected yet, but the display list and picture are - // both null. - if (picture->picture()) { - auto layer = std::make_unique( - SkPoint::Make(dx, dy), UIDartState::CreateGPUObject(picture->picture()), - !!(hints & 1), !!(hints & 2)); - AddLayer(std::move(layer)); - } else if (picture->display_list()) { + // Explicitly check for display_list, since the picture object might have + // been disposed but not collected yet, but the display list is null. + if (picture->display_list()) { auto layer = std::make_unique( SkPoint::Make(dx, dy), UIDartState::CreateGPUObject(picture->display_list()), !!(hints & 1), diff --git a/lib/ui/painting/canvas.cc b/lib/ui/painting/canvas.cc index 4ffb112912839..a64de1cf91f91 100644 --- a/lib/ui/painting/canvas.cc +++ b/lib/ui/painting/canvas.cc @@ -589,13 +589,7 @@ void Canvas::drawPicture(Picture* picture) { ToDart("Canvas.drawPicture called with non-genuine Picture.")); return; } - if (picture->picture()) { - if (display_list_recorder_) { - builder()->drawPicture(picture->picture(), nullptr, false); - } else if (canvas_) { - canvas_->drawPicture(picture->picture().get()); - } - } else if (picture->display_list()) { + if (picture->display_list()) { if (display_list_recorder_) { builder()->drawDisplayList(picture->display_list()); } else if (canvas_) { diff --git a/lib/ui/painting/image_dispose_unittests.cc b/lib/ui/painting/image_dispose_unittests.cc index 60655d8a0c079..8ce9e595f1036 100644 --- a/lib/ui/painting/image_dispose_unittests.cc +++ b/lib/ui/painting/image_dispose_unittests.cc @@ -32,7 +32,6 @@ class ImageDisposeTest : public ShellTest { // Used to wait on Dart callbacks or Shell task runner flushing fml::AutoResetWaitableEvent message_latch_; - sk_sp current_picture_; sk_sp current_display_list_; sk_sp current_image_; }; @@ -48,13 +47,8 @@ TEST_F(ImageDisposeTest, ImageReleasedAfterFrameAndDisposePictureAndLayer) { CanvasImage* image = GetNativePeer(native_image_handle); Picture* picture = GetNativePeer(Dart_GetNativeArgument(args, 1)); ASSERT_FALSE(image->image()->unique()); - if (picture->display_list()) { - ASSERT_FALSE(picture->display_list()->unique()); - current_display_list_ = picture->display_list(); - } else { - ASSERT_FALSE(picture->picture()->unique()); - current_picture_ = picture->picture(); - } + ASSERT_FALSE(picture->display_list()->unique()); + current_display_list_ = picture->display_list(); current_image_ = image->image(); }; @@ -92,7 +86,7 @@ TEST_F(ImageDisposeTest, ImageReleasedAfterFrameAndDisposePictureAndLayer) { message_latch_.Wait(); - ASSERT_TRUE(current_display_list_ || current_picture_); + ASSERT_TRUE(current_display_list_); ASSERT_TRUE(current_image_); // Force a drain the SkiaUnrefQueue. The engine does this normally as frames @@ -107,9 +101,6 @@ TEST_F(ImageDisposeTest, ImageReleasedAfterFrameAndDisposePictureAndLayer) { if (current_display_list_) { EXPECT_TRUE(current_display_list_->unique()); current_display_list_.reset(); - } else { - EXPECT_TRUE(current_picture_->unique()); - current_picture_.reset(); } EXPECT_TRUE(current_image_->unique()); diff --git a/lib/ui/painting/picture.cc b/lib/ui/painting/picture.cc index de8ef1f544589..d9432fc6c7774 100644 --- a/lib/ui/painting/picture.cc +++ b/lib/ui/painting/picture.cc @@ -28,15 +28,6 @@ IMPLEMENT_WRAPPERTYPEINFO(ui, Picture); DART_BIND_ALL(Picture, FOR_EACH_BINDING) -fml::RefPtr Picture::Create( - Dart_Handle dart_handle, - flutter::SkiaGPUObject picture) { - auto canvas_picture = fml::MakeRefCounted(std::move(picture)); - - canvas_picture->AssociateWithDartWrapper(dart_handle); - return canvas_picture; -} - fml::RefPtr Picture::Create( Dart_Handle dart_handle, flutter::SkiaGPUObject display_list) { @@ -46,9 +37,6 @@ fml::RefPtr Picture::Create( return canvas_picture; } -Picture::Picture(flutter::SkiaGPUObject picture) - : picture_(std::move(picture)) {} - Picture::Picture(flutter::SkiaGPUObject display_list) : display_list_(std::move(display_list)) {} @@ -57,31 +45,17 @@ Picture::~Picture() = default; Dart_Handle Picture::toImage(uint32_t width, uint32_t height, Dart_Handle raw_image_callback) { - if (display_list_.skia_object()) { - return RasterizeToImage(display_list_.skia_object(), width, height, - raw_image_callback); - } else { - if (!picture_.skia_object()) { - return tonic::ToDart("Picture is null"); - } - return RasterizeToImage( - [picture = picture_.skia_object()](SkCanvas* canvas) { - canvas->drawPicture(picture); - }, - width, height, raw_image_callback); - } + return RasterizeToImage(display_list_.skia_object(), width, height, + raw_image_callback); } void Picture::dispose() { - picture_.reset(); display_list_.reset(); ClearDartWrapper(); } size_t Picture::GetAllocationSize() const { - if (auto picture = picture_.skia_object()) { - return picture->approximateBytesUsed() + sizeof(Picture); - } else if (auto display_list = display_list_.skia_object()) { + if (auto display_list = display_list_.skia_object()) { return display_list->bytes() + sizeof(Picture); } else { return sizeof(Picture); diff --git a/lib/ui/painting/picture.h b/lib/ui/painting/picture.h index 130d4f86c6c4b..33e5cb5772cf4 100644 --- a/lib/ui/painting/picture.h +++ b/lib/ui/painting/picture.h @@ -25,13 +25,10 @@ class Picture : public RefCountedDartWrappable { public: ~Picture() override; - static fml::RefPtr Create(Dart_Handle dart_handle, - flutter::SkiaGPUObject picture); static fml::RefPtr Create( Dart_Handle dart_handle, flutter::SkiaGPUObject display_list); - sk_sp picture() const { return picture_.skia_object(); } sk_sp display_list() const { return display_list_.skia_object(); } @@ -58,10 +55,8 @@ class Picture : public RefCountedDartWrappable { Dart_Handle raw_image_callback); private: - explicit Picture(flutter::SkiaGPUObject picture); explicit Picture(flutter::SkiaGPUObject display_list); - flutter::SkiaGPUObject picture_; flutter::SkiaGPUObject display_list_; }; diff --git a/lib/ui/painting/picture_recorder.cc b/lib/ui/painting/picture_recorder.cc index 32c888aef4d78..5703b45dc6ebb 100644 --- a/lib/ui/painting/picture_recorder.cc +++ b/lib/ui/painting/picture_recorder.cc @@ -40,13 +40,8 @@ PictureRecorder::PictureRecorder() {} PictureRecorder::~PictureRecorder() {} SkCanvas* PictureRecorder::BeginRecording(SkRect bounds) { - bool enable_display_list = UIDartState::Current()->enable_display_list(); - if (enable_display_list) { - display_list_recorder_ = sk_make_sp(bounds); - return display_list_recorder_.get(); - } else { - return picture_recorder_.beginRecording(bounds, &rtree_factory_); - } + display_list_recorder_ = sk_make_sp(bounds); + return display_list_recorder_.get(); } fml::RefPtr PictureRecorder::endRecording(Dart_Handle dart_picture) { @@ -56,16 +51,9 @@ fml::RefPtr PictureRecorder::endRecording(Dart_Handle dart_picture) { fml::RefPtr picture; - if (display_list_recorder_) { - picture = Picture::Create( - dart_picture, - UIDartState::CreateGPUObject(display_list_recorder_->Build())); - display_list_recorder_ = nullptr; - } else { - picture = Picture::Create( - dart_picture, UIDartState::CreateGPUObject( - picture_recorder_.finishRecordingAsPicture())); - } + picture = Picture::Create(dart_picture, UIDartState::CreateGPUObject( + display_list_recorder_->Build())); + display_list_recorder_ = nullptr; canvas_->Invalidate(); canvas_ = nullptr; diff --git a/lib/ui/painting/picture_recorder.h b/lib/ui/painting/picture_recorder.h index f1c73f4d5a7fb..52f2b450b001f 100644 --- a/lib/ui/painting/picture_recorder.h +++ b/lib/ui/painting/picture_recorder.h @@ -40,9 +40,6 @@ class PictureRecorder : public RefCountedDartWrappable { private: PictureRecorder(); - SkRTreeFactory rtree_factory_; - SkPictureRecorder picture_recorder_; - sk_sp display_list_recorder_; fml::RefPtr canvas_; diff --git a/lib/ui/ui_dart_state.cc b/lib/ui/ui_dart_state.cc index 9536c3df90375..ed60f8cbf2c5b 100644 --- a/lib/ui/ui_dart_state.cc +++ b/lib/ui/ui_dart_state.cc @@ -56,7 +56,6 @@ UIDartState::UIDartState( std::shared_ptr isolate_name_server, bool is_root_isolate, bool enable_skparagraph, - bool enable_display_list, const UIDartState::Context& context) : add_callback_(std::move(add_callback)), remove_callback_(std::move(remove_callback)), @@ -66,7 +65,6 @@ UIDartState::UIDartState( log_message_callback_(log_message_callback), isolate_name_server_(std::move(isolate_name_server)), enable_skparagraph_(enable_skparagraph), - enable_display_list_(enable_display_list), context_(std::move(context)) { AddOrRemoveTaskObserver(true /* add */); } @@ -216,8 +214,4 @@ bool UIDartState::enable_skparagraph() const { return enable_skparagraph_; } -bool UIDartState::enable_display_list() const { - return enable_display_list_; -} - } // namespace flutter diff --git a/lib/ui/ui_dart_state.h b/lib/ui/ui_dart_state.h index 1c05304a8195a..bdcb49beadf4a 100644 --- a/lib/ui/ui_dart_state.h +++ b/lib/ui/ui_dart_state.h @@ -138,8 +138,6 @@ class UIDartState : public tonic::DartState { bool enable_skparagraph() const; - bool enable_display_list() const; - template static flutter::SkiaGPUObject CreateGPUObject(sk_sp object) { if (!object) { @@ -164,7 +162,6 @@ class UIDartState : public tonic::DartState { std::shared_ptr isolate_name_server, bool is_root_isolate_, bool enable_skparagraph, - bool enable_display_list, const UIDartState::Context& context); ~UIDartState() override; @@ -189,7 +186,6 @@ class UIDartState : public tonic::DartState { LogMessageCallback log_message_callback_; const std::shared_ptr isolate_name_server_; const bool enable_skparagraph_; - const bool enable_display_list_; UIDartState::Context context_; void AddOrRemoveTaskObserver(bool add); diff --git a/runtime/dart_isolate.cc b/runtime/dart_isolate.cc index 50593813dbb6c..e12059d229e95 100644 --- a/runtime/dart_isolate.cc +++ b/runtime/dart_isolate.cc @@ -285,7 +285,6 @@ DartIsolate::DartIsolate(const Settings& settings, DartVMRef::GetIsolateNameServer(), is_root_isolate, settings.enable_skparagraph, - settings.enable_display_list, std::move(context)), may_insecurely_connect_to_all_domains_( settings.may_insecurely_connect_to_all_domains), diff --git a/shell/common/BUILD.gn b/shell/common/BUILD.gn index 5e46a60716783..2d9aa68d226b4 100644 --- a/shell/common/BUILD.gn +++ b/shell/common/BUILD.gn @@ -274,11 +274,19 @@ if (enable_unittests) { "rasterizer_unittests.cc", "resource_cache_limit_calculator_unittests.cc", "shell_unittests.cc", - "skp_shader_warmup_unittests.cc", "switches_unittests.cc", "variable_refresh_rate_display_unittests.cc", ] + if (is_fuchsia) { + sources += [ + "layers/picture_layer.cc", + "layers/picture_layer.h", + "layers/picture_layer_unittests.cc", + "skp_shader_warmup_unittests.cc", + ] + } + deps = [ ":shell_test_fixture_sources", ":shell_unittests_fixtures", diff --git a/shell/common/persistent_cache_unittests.cc b/shell/common/persistent_cache_unittests.cc index 9a1daf2a71c02..3bf8d1f9bb519 100644 --- a/shell/common/persistent_cache_unittests.cc +++ b/shell/common/persistent_cache_unittests.cc @@ -10,7 +10,6 @@ #include "flutter/flow/layers/container_layer.h" #include "flutter/flow/layers/layer.h" #include "flutter/flow/layers/physical_shape_layer.h" -#include "flutter/flow/layers/picture_layer.h" #include "flutter/fml/command_line.h" #include "flutter/fml/file.h" #include "flutter/fml/log_settings.h" diff --git a/shell/common/shell_unittests.cc b/shell/common/shell_unittests.cc index 9e942a4f7c8b2..2d3f2c6202593 100644 --- a/shell/common/shell_unittests.cc +++ b/shell/common/shell_unittests.cc @@ -12,7 +12,7 @@ #include "assets/directory_asset_bundle.h" #include "common/graphics/persistent_cache.h" -#include "flutter/flow/layers/picture_layer.h" +#include "flutter/flow/layers/display_list_layer.h" #include "flutter/flow/layers/transform_layer.h" #include "flutter/fml/command_line.h" #include "flutter/fml/dart/dart_converter.h" @@ -744,18 +744,17 @@ TEST_F(ShellTest, ExternalEmbedderNoThreadMerger) { RunEngine(shell.get(), std::move(configuration)); LayerTreeBuilder builder = [&](std::shared_ptr root) { - SkPictureRecorder recorder; - SkCanvas* recording_canvas = - recorder.beginRecording(SkRect::MakeXYWH(0, 0, 80, 80)); - recording_canvas->drawRect(SkRect::MakeXYWH(0, 0, 80, 80), - SkPaint(SkColor4f::FromColor(SK_ColorRED))); - auto sk_picture = recorder.finishRecordingAsPicture(); + DisplayListCanvasRecorder recorder(SkRect::MakeXYWH(0, 0, 80, 80)); + recorder.drawRect(SkRect::MakeXYWH(0, 0, 80, 80), + SkPaint(SkColor4f::FromColor(SK_ColorRED))); + auto display_list = recorder.Build(); fml::RefPtr queue = fml::MakeRefCounted( this->GetCurrentTaskRunner(), fml::TimeDelta::Zero()); - auto picture_layer = std::make_shared( + auto display_list_layer = std::make_shared( SkPoint::Make(10, 10), - flutter::SkiaGPUObject({sk_picture, queue}), false, false); - root->Add(picture_layer); + flutter::SkiaGPUObject({display_list, queue}), false, + false); + root->Add(display_list_layer); }; PumpOneFrame(shell.get(), 100, 100, builder); @@ -799,18 +798,17 @@ TEST_F(ShellTest, RunEngine(shell.get(), std::move(configuration)); LayerTreeBuilder builder = [&](std::shared_ptr root) { - SkPictureRecorder recorder; - SkCanvas* recording_canvas = - recorder.beginRecording(SkRect::MakeXYWH(0, 0, 80, 80)); - recording_canvas->drawRect(SkRect::MakeXYWH(0, 0, 80, 80), - SkPaint(SkColor4f::FromColor(SK_ColorRED))); - auto sk_picture = recorder.finishRecordingAsPicture(); + DisplayListCanvasRecorder recorder(SkRect::MakeXYWH(0, 0, 80, 80)); + recorder.drawRect(SkRect::MakeXYWH(0, 0, 80, 80), + SkPaint(SkColor4f::FromColor(SK_ColorRED))); + auto display_list = recorder.Build(); fml::RefPtr queue = fml::MakeRefCounted( this->GetCurrentTaskRunner(), fml::TimeDelta::Zero()); - auto picture_layer = std::make_shared( + auto display_list_layer = std::make_shared( SkPoint::Make(10, 10), - flutter::SkiaGPUObject({sk_picture, queue}), false, false); - root->Add(picture_layer); + flutter::SkiaGPUObject({display_list, queue}), false, + false); + root->Add(display_list_layer); }; PumpOneFrame(shell.get(), 100, 100, builder); @@ -852,18 +850,17 @@ TEST_F(ShellTest, RunEngine(shell.get(), std::move(configuration)); LayerTreeBuilder builder = [&](std::shared_ptr root) { - SkPictureRecorder recorder; - SkCanvas* recording_canvas = - recorder.beginRecording(SkRect::MakeXYWH(0, 0, 80, 80)); - recording_canvas->drawRect(SkRect::MakeXYWH(0, 0, 80, 80), - SkPaint(SkColor4f::FromColor(SK_ColorRED))); - auto sk_picture = recorder.finishRecordingAsPicture(); + DisplayListCanvasRecorder recorder(SkRect::MakeXYWH(0, 0, 80, 80)); + recorder.drawRect(SkRect::MakeXYWH(0, 0, 80, 80), + SkPaint(SkColor4f::FromColor(SK_ColorRED))); + auto display_list = recorder.Build(); fml::RefPtr queue = fml::MakeRefCounted( this->GetCurrentTaskRunner(), fml::TimeDelta::Zero()); - auto picture_layer = std::make_shared( + auto display_list_layer = std::make_shared( SkPoint::Make(10, 10), - flutter::SkiaGPUObject({sk_picture, queue}), false, false); - root->Add(picture_layer); + flutter::SkiaGPUObject({display_list, queue}), false, + false); + root->Add(display_list_layer); }; PumpOneFrame(shell.get(), 100, 100, builder); @@ -925,18 +922,17 @@ TEST_F(ShellTest, RunEngine(shell.get(), std::move(configuration)); LayerTreeBuilder builder = [&](std::shared_ptr root) { - SkPictureRecorder recorder; - SkCanvas* recording_canvas = - recorder.beginRecording(SkRect::MakeXYWH(0, 0, 80, 80)); - recording_canvas->drawRect(SkRect::MakeXYWH(0, 0, 80, 80), - SkPaint(SkColor4f::FromColor(SK_ColorRED))); - auto sk_picture = recorder.finishRecordingAsPicture(); + DisplayListCanvasRecorder recorder(SkRect::MakeXYWH(0, 0, 80, 80)); + recorder.drawRect(SkRect::MakeXYWH(0, 0, 80, 80), + SkPaint(SkColor4f::FromColor(SK_ColorRED))); + auto display_list = recorder.Build(); fml::RefPtr queue = fml::MakeRefCounted( this->GetCurrentTaskRunner(), fml::TimeDelta::Zero()); - auto picture_layer = std::make_shared( + auto display_list_layer = std::make_shared( SkPoint::Make(10, 10), - flutter::SkiaGPUObject({sk_picture, queue}), false, false); - root->Add(picture_layer); + flutter::SkiaGPUObject({display_list, queue}), false, + false); + root->Add(display_list_layer); }; PumpOneFrame(shell.get(), 100, 100, builder); @@ -1000,18 +996,17 @@ TEST_F(ShellTest, RunEngine(shell.get(), std::move(configuration)); LayerTreeBuilder builder = [&](std::shared_ptr root) { - SkPictureRecorder recorder; - SkCanvas* recording_canvas = - recorder.beginRecording(SkRect::MakeXYWH(0, 0, 80, 80)); - recording_canvas->drawRect(SkRect::MakeXYWH(0, 0, 80, 80), - SkPaint(SkColor4f::FromColor(SK_ColorRED))); - auto sk_picture = recorder.finishRecordingAsPicture(); + DisplayListCanvasRecorder recorder(SkRect::MakeXYWH(0, 0, 80, 80)); + recorder.drawRect(SkRect::MakeXYWH(0, 0, 80, 80), + SkPaint(SkColor4f::FromColor(SK_ColorRED))); + auto display_list = recorder.Build(); fml::RefPtr queue = fml::MakeRefCounted( this->GetCurrentTaskRunner(), fml::TimeDelta::Zero()); - auto picture_layer = std::make_shared( + auto display_list_layer = std::make_shared( SkPoint::Make(10, 10), - flutter::SkiaGPUObject({sk_picture, queue}), false, false); - root->Add(picture_layer); + flutter::SkiaGPUObject({display_list, queue}), false, + false); + root->Add(display_list_layer); }; PumpOneFrame(shell.get(), 100, 100, builder); @@ -1073,18 +1068,17 @@ TEST_F(ShellTest, RunEngine(shell.get(), std::move(configuration)); LayerTreeBuilder builder = [&](std::shared_ptr root) { - SkPictureRecorder recorder; - SkCanvas* recording_canvas = - recorder.beginRecording(SkRect::MakeXYWH(0, 0, 80, 80)); - recording_canvas->drawRect(SkRect::MakeXYWH(0, 0, 80, 80), - SkPaint(SkColor4f::FromColor(SK_ColorRED))); - auto sk_picture = recorder.finishRecordingAsPicture(); + DisplayListCanvasRecorder recorder(SkRect::MakeXYWH(0, 0, 80, 80)); + recorder.drawRect(SkRect::MakeXYWH(0, 0, 80, 80), + SkPaint(SkColor4f::FromColor(SK_ColorRED))); + auto display_list = recorder.Build(); fml::RefPtr queue = fml::MakeRefCounted( this->GetCurrentTaskRunner(), fml::TimeDelta::Zero()); - auto picture_layer = std::make_shared( + auto display_list_layer = std::make_shared( SkPoint::Make(10, 10), - flutter::SkiaGPUObject({sk_picture, queue}), false, false); - root->Add(picture_layer); + flutter::SkiaGPUObject({display_list, queue}), false, + false); + root->Add(display_list_layer); }; PumpOneFrame(shell.get(), 100, 100, builder); end_frame_latch.Wait(); @@ -1121,18 +1115,17 @@ TEST_F(ShellTest, OnPlatformViewDestroyWithoutRasterThreadMerger) { RunEngine(shell.get(), std::move(configuration)); LayerTreeBuilder builder = [&](std::shared_ptr root) { - SkPictureRecorder recorder; - SkCanvas* recording_canvas = - recorder.beginRecording(SkRect::MakeXYWH(0, 0, 80, 80)); - recording_canvas->drawRect(SkRect::MakeXYWH(0, 0, 80, 80), - SkPaint(SkColor4f::FromColor(SK_ColorRED))); - auto sk_picture = recorder.finishRecordingAsPicture(); + DisplayListCanvasRecorder recorder(SkRect::MakeXYWH(0, 0, 80, 80)); + recorder.drawRect(SkRect::MakeXYWH(0, 0, 80, 80), + SkPaint(SkColor4f::FromColor(SK_ColorRED))); + auto display_list = recorder.Build(); fml::RefPtr queue = fml::MakeRefCounted( this->GetCurrentTaskRunner(), fml::TimeDelta::Zero()); - auto picture_layer = std::make_shared( + auto display_list_layer = std::make_shared( SkPoint::Make(10, 10), - flutter::SkiaGPUObject({sk_picture, queue}), false, false); - root->Add(picture_layer); + flutter::SkiaGPUObject({display_list, queue}), false, + false); + root->Add(display_list_layer); }; PumpOneFrame(shell.get(), 100, 100, builder); @@ -1192,18 +1185,17 @@ TEST_F(ShellTest, RunEngine(shell.get(), std::move(configuration)); LayerTreeBuilder builder = [&](std::shared_ptr root) { - SkPictureRecorder recorder; - SkCanvas* recording_canvas = - recorder.beginRecording(SkRect::MakeXYWH(0, 0, 80, 80)); - recording_canvas->drawRect(SkRect::MakeXYWH(0, 0, 80, 80), - SkPaint(SkColor4f::FromColor(SK_ColorRED))); - auto sk_picture = recorder.finishRecordingAsPicture(); + DisplayListCanvasRecorder recorder(SkRect::MakeXYWH(0, 0, 80, 80)); + recorder.drawRect(SkRect::MakeXYWH(0, 0, 80, 80), + SkPaint(SkColor4f::FromColor(SK_ColorRED))); + auto display_list = recorder.Build(); fml::RefPtr queue = fml::MakeRefCounted( this->GetCurrentTaskRunner(), fml::TimeDelta::Zero()); - auto picture_layer = std::make_shared( + auto display_list_layer = std::make_shared( SkPoint::Make(10, 10), - flutter::SkiaGPUObject({sk_picture, queue}), false, false); - root->Add(picture_layer); + flutter::SkiaGPUObject({display_list, queue}), false, + false); + root->Add(display_list_layer); }; PumpOneFrame(shell.get(), 100, 100, builder); end_frame_latch.Wait(); @@ -1242,18 +1234,17 @@ TEST_F(ShellTest, GetUsedThisFrameShouldBeSetBeforeEndFrame) { RunEngine(shell.get(), std::move(configuration)); LayerTreeBuilder builder = [&](std::shared_ptr root) { - SkPictureRecorder recorder; - SkCanvas* recording_canvas = - recorder.beginRecording(SkRect::MakeXYWH(0, 0, 80, 80)); - recording_canvas->drawRect(SkRect::MakeXYWH(0, 0, 80, 80), - SkPaint(SkColor4f::FromColor(SK_ColorRED))); - auto sk_picture = recorder.finishRecordingAsPicture(); + DisplayListCanvasRecorder recorder(SkRect::MakeXYWH(0, 0, 80, 80)); + recorder.drawRect(SkRect::MakeXYWH(0, 0, 80, 80), + SkPaint(SkColor4f::FromColor(SK_ColorRED))); + auto display_list = recorder.Build(); fml::RefPtr queue = fml::MakeRefCounted( this->GetCurrentTaskRunner(), fml::TimeDelta::Zero()); - auto picture_layer = std::make_shared( + auto display_list_layer = std::make_shared( SkPoint::Make(10, 10), - flutter::SkiaGPUObject({sk_picture, queue}), false, false); - root->Add(picture_layer); + flutter::SkiaGPUObject({display_list, queue}), false, + false); + root->Add(display_list_layer); }; PumpOneFrame(shell.get(), 100, 100, builder); end_frame_latch.Wait(); @@ -2028,18 +2019,17 @@ TEST_F(ShellTest, Screenshot) { RunEngine(shell.get(), std::move(configuration)); LayerTreeBuilder builder = [&](std::shared_ptr root) { - SkPictureRecorder recorder; - SkCanvas* recording_canvas = - recorder.beginRecording(SkRect::MakeXYWH(0, 0, 80, 80)); - recording_canvas->drawRect(SkRect::MakeXYWH(0, 0, 80, 80), - SkPaint(SkColor4f::FromColor(SK_ColorRED))); - auto sk_picture = recorder.finishRecordingAsPicture(); + DisplayListCanvasRecorder recorder(SkRect::MakeXYWH(0, 0, 80, 80)); + recorder.drawRect(SkRect::MakeXYWH(0, 0, 80, 80), + SkPaint(SkColor4f::FromColor(SK_ColorRED))); + auto display_list = recorder.Build(); fml::RefPtr queue = fml::MakeRefCounted( this->GetCurrentTaskRunner(), fml::TimeDelta::Zero()); - auto picture_layer = std::make_shared( + auto display_list_layer = std::make_shared( SkPoint::Make(10, 10), - flutter::SkiaGPUObject({sk_picture, queue}), false, false); - root->Add(picture_layer); + flutter::SkiaGPUObject({display_list, queue}), false, + false); + root->Add(display_list_layer); }; PumpOneFrame(shell.get(), 100, 100, builder); @@ -2386,13 +2376,11 @@ TEST_F(ShellTest, RasterizerMakeRasterSnapshot) { DestroyShell(std::move(shell), std::move(task_runners)); } -static sk_sp MakeSizedPicture(int width, int height) { - SkPictureRecorder recorder; - SkCanvas* recording_canvas = - recorder.beginRecording(SkRect::MakeXYWH(0, 0, width, height)); - recording_canvas->drawRect(SkRect::MakeXYWH(0, 0, width, height), - SkPaint(SkColor4f::FromColor(SK_ColorRED))); - return recorder.finishRecordingAsPicture(); +static sk_sp MakeSizedDisplayList(int width, int height) { + DisplayListCanvasRecorder recorder(SkRect::MakeXYWH(0, 0, 80, 80)); + recorder.drawRect(SkRect::MakeXYWH(0, 0, 80, 80), + SkPaint(SkColor4f::FromColor(SK_ColorRED))); + return recorder.Build(); } TEST_F(ShellTest, OnServiceProtocolEstimateRasterCacheMemoryWorks) { @@ -2400,19 +2388,20 @@ TEST_F(ShellTest, OnServiceProtocolEstimateRasterCacheMemoryWorks) { std::unique_ptr shell = CreateShell(settings); // 1. Construct a picture and a picture layer to be raster cached. - sk_sp picture = MakeSizedPicture(10, 10); + sk_sp display_list = MakeSizedDisplayList(10, 10); fml::RefPtr queue = fml::MakeRefCounted( GetCurrentTaskRunner(), fml::TimeDelta::Zero()); - auto picture_layer = std::make_shared( + auto display_list_layer = std::make_shared( SkPoint::Make(0, 0), - flutter::SkiaGPUObject({MakeSizedPicture(100, 100), queue}), + flutter::SkiaGPUObject( + {MakeSizedDisplayList(100, 100), queue}), false, false); - picture_layer->set_paint_bounds(SkRect::MakeWH(100, 100)); + display_list_layer->set_paint_bounds(SkRect::MakeWH(100, 100)); // 2. Rasterize the picture and the picture layer in the raster cache. std::promise rasterized; shell->GetTaskRunners().GetRasterTaskRunner()->PostTask( - [&shell, &rasterized, &picture, &picture_layer] { + [&shell, &rasterized, &display_list, &display_list_layer] { auto* compositor_context = shell->GetRasterizer()->compositor_context(); auto& raster_cache = compositor_context->raster_cache(); @@ -2446,13 +2435,13 @@ TEST_F(ShellTest, OnServiceProtocolEstimateRasterCacheMemoryWorks) { SkMatrix matrix = SkMatrix::I(); picture_cache_generated = raster_cache.Prepare( - &preroll_context, picture.get(), true, false, matrix); - raster_cache.Draw(*picture, dummy_canvas); + &preroll_context, display_list.get(), true, false, matrix); + raster_cache.Draw(*display_list, dummy_canvas); } ASSERT_TRUE(picture_cache_generated); // 2.2. Rasterize the picture layer. - raster_cache.Prepare(&preroll_context, picture_layer.get(), + raster_cache.Prepare(&preroll_context, display_list_layer.get(), SkMatrix::I()); rasterized.set_value(true); }); @@ -2469,7 +2458,7 @@ TEST_F(ShellTest, OnServiceProtocolEstimateRasterCacheMemoryWorks) { document.Accept(writer); std::string expected_json = "{\"type\":\"EstimateRasterCacheMemory\",\"layerBytes\":40000,\"picture" - "Bytes\":400}"; + "Bytes\":25600}"; std::string actual_json = buffer.GetString(); ASSERT_EQ(actual_json, expected_json); diff --git a/shell/common/switches.cc b/shell/common/switches.cc index 4f565cd49e889..7eb3aa9568c87 100644 --- a/shell/common/switches.cc +++ b/shell/common/switches.cc @@ -444,16 +444,6 @@ Settings SettingsFromCommandLine(const fml::CommandLine& command_line) { settings.dart_flags.push_back(flag); } } - if (std::find(settings.dart_flags.begin(), settings.dart_flags.end(), - "--enable-display-list") != settings.dart_flags.end()) { - FML_LOG(ERROR) << "Manually enabling display lists"; - settings.enable_display_list = true; - } else if (std::find(settings.dart_flags.begin(), settings.dart_flags.end(), - "--no-enable-display-list") != - settings.dart_flags.end()) { - FML_LOG(ERROR) << "Manually disabling display lists"; - settings.enable_display_list = false; - } #if !FLUTTER_RELEASE command_line.GetOptionValue(FlagForSwitch(Switch::LogTag), &settings.log_tag); From c177456c7c2040bf2e338bf185470396e2be750f Mon Sep 17 00:00:00 2001 From: Dan Field Date: Wed, 22 Jun 2022 11:47:29 -0700 Subject: [PATCH 3/5] fix includes --- shell/common/BUILD.gn | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/shell/common/BUILD.gn b/shell/common/BUILD.gn index 2d9aa68d226b4..17eff77f9f360 100644 --- a/shell/common/BUILD.gn +++ b/shell/common/BUILD.gn @@ -279,10 +279,11 @@ if (enable_unittests) { ] if (is_fuchsia) { + # TODO(dnfield): Can we delete this harness? https://github.com/flutter/flutter/issues/106435 sources += [ - "layers/picture_layer.cc", - "layers/picture_layer.h", - "layers/picture_layer_unittests.cc", + "//flutter/flow/layers/picture_layer.cc", + "//flutter/flow/layers/picture_layer.h", + "//flutter/flow/layers/picture_layer_unittests.cc", "skp_shader_warmup_unittests.cc", ] } From e4a241e9f0f61c83c3b507b76b908945f84fc470 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Wed, 22 Jun 2022 12:58:24 -0700 Subject: [PATCH 4/5] Delete unused target --- ci/licenses_golden/licenses_flutter | 4 - flow/layers/picture_layer.cc | 150 ---------- flow/layers/picture_layer.h | 54 ---- flow/layers/picture_layer_unittests.cc | 214 -------------- shell/common/BUILD.gn | 10 - shell/common/skp_shader_warmup_unittests.cc | 301 -------------------- 6 files changed, 733 deletions(-) delete mode 100644 flow/layers/picture_layer.cc delete mode 100644 flow/layers/picture_layer.h delete mode 100644 flow/layers/picture_layer_unittests.cc delete mode 100644 shell/common/skp_shader_warmup_unittests.cc diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index a7b4b6aed9801..86ff647fe6a20 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -174,9 +174,6 @@ FILE: ../../../flutter/flow/layers/performance_overlay_layer_unittests.cc FILE: ../../../flutter/flow/layers/physical_shape_layer.cc FILE: ../../../flutter/flow/layers/physical_shape_layer.h FILE: ../../../flutter/flow/layers/physical_shape_layer_unittests.cc -FILE: ../../../flutter/flow/layers/picture_layer.cc -FILE: ../../../flutter/flow/layers/picture_layer.h -FILE: ../../../flutter/flow/layers/picture_layer_unittests.cc FILE: ../../../flutter/flow/layers/platform_view_layer.cc FILE: ../../../flutter/flow/layers/platform_view_layer.h FILE: ../../../flutter/flow/layers/platform_view_layer_unittests.cc @@ -1276,7 +1273,6 @@ FILE: ../../../flutter/shell/common/shell_test_platform_view_vulkan.h FILE: ../../../flutter/shell/common/shell_unittests.cc FILE: ../../../flutter/shell/common/skia_event_tracer_impl.cc FILE: ../../../flutter/shell/common/skia_event_tracer_impl.h -FILE: ../../../flutter/shell/common/skp_shader_warmup_unittests.cc FILE: ../../../flutter/shell/common/snapshot_surface_producer.h FILE: ../../../flutter/shell/common/switches.cc FILE: ../../../flutter/shell/common/switches.h diff --git a/flow/layers/picture_layer.cc b/flow/layers/picture_layer.cc deleted file mode 100644 index b3ab7143f86e0..0000000000000 --- a/flow/layers/picture_layer.cc +++ /dev/null @@ -1,150 +0,0 @@ -// Copyright 2013 The Flutter Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "flutter/flow/layers/picture_layer.h" - -#include "flutter/fml/logging.h" -#include "third_party/skia/include/core/SkSerialProcs.h" - -namespace flutter { - -PictureLayer::PictureLayer(const SkPoint& offset, - SkiaGPUObject picture, - bool is_complex, - bool will_change) - : offset_(offset), - picture_(std::move(picture)), - is_complex_(is_complex), - will_change_(will_change) {} - -bool PictureLayer::IsReplacing(DiffContext* context, const Layer* layer) const { - // Only return true for identical pictures; This way - // ContainerLayer::DiffChildren can detect when a picture layer got inserted - // between other picture layers - auto picture_layer = layer->as_picture_layer(); - return picture_layer != nullptr && offset_ == picture_layer->offset_ && - Compare(context->statistics(), this, picture_layer); -} - -void PictureLayer::Diff(DiffContext* context, const Layer* old_layer) { - DiffContext::AutoSubtreeRestore subtree(context); - if (!context->IsSubtreeDirty()) { -#ifndef NDEBUG - FML_DCHECK(old_layer); - auto prev = old_layer->as_picture_layer(); - DiffContext::Statistics dummy_statistics; - // IsReplacing has already determined that the picture is same - FML_DCHECK(prev->offset_ == offset_ && - Compare(dummy_statistics, this, prev)); -#endif - } - context->PushTransform(SkMatrix::Translate(offset_.x(), offset_.y())); - context->AddLayerBounds(picture()->cullRect()); - context->SetLayerPaintRegion(this, context->CurrentSubtreeRegion()); -} - -bool PictureLayer::Compare(DiffContext::Statistics& statistics, - const PictureLayer* l1, - const PictureLayer* l2) { - const auto& pic1 = l1->picture_.skia_object(); - const auto& pic2 = l2->picture_.skia_object(); - - if (pic1.get() == pic2.get()) { - statistics.AddSameInstancePicture(); - return true; - } - auto op_cnt_1 = pic1->approximateOpCount(); - auto op_cnt_2 = pic2->approximateOpCount(); - if (op_cnt_1 != op_cnt_2 || pic1->cullRect() != pic2->cullRect()) { - statistics.AddNewPicture(); - return false; - } - - if (op_cnt_1 > 10) { - statistics.AddPictureTooComplexToCompare(); - return false; - } - - statistics.AddDeepComparePicture(); - - // TODO(knopp) we don't actually need the data; this could be done without - // allocations by implementing stream that calculates SHA hash and - // comparing those hashes - auto d1 = l1->SerializedPicture(); - auto d2 = l2->SerializedPicture(); - auto res = d1->equals(d2.get()); - if (res) { - statistics.AddDifferentInstanceButEqualPicture(); - } else { - statistics.AddNewPicture(); - } - return res; -} - -sk_sp PictureLayer::SerializedPicture() const { - if (!cached_serialized_picture_) { - SkSerialProcs procs = { - nullptr, - nullptr, - [](SkImage* i, void* ctx) { - auto id = i->uniqueID(); - return SkData::MakeWithCopy(&id, sizeof(id)); - }, - nullptr, - [](SkTypeface* tf, void* ctx) { - auto id = tf->uniqueID(); - return SkData::MakeWithCopy(&id, sizeof(id)); - }, - nullptr, - }; - cached_serialized_picture_ = picture_.skia_object()->serialize(&procs); - } - return cached_serialized_picture_; -} - -void PictureLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) { - TRACE_EVENT0("flutter", "PictureLayer::Preroll"); - - SkPicture* sk_picture = picture(); - - SkRect bounds = sk_picture->cullRect().makeOffset(offset_.x(), offset_.y()); - - if (auto* cache = context->raster_cache) { - TRACE_EVENT0("flutter", "PictureLayer::RasterCache (Preroll)"); - if (context->cull_rect.intersects(bounds)) { - if (cache->Prepare(context, sk_picture, is_complex_, will_change_, matrix, - offset_)) { - context->subtree_can_inherit_opacity = true; - } - } else { - // Don't evict raster cache entry during partial repaint - cache->Touch(sk_picture, matrix); - } - } - - set_paint_bounds(bounds); -} - -void PictureLayer::Paint(PaintContext& context) const { - TRACE_EVENT0("flutter", "PictureLayer::Paint"); - FML_DCHECK(picture_.skia_object()); - FML_DCHECK(needs_painting(context)); - - SkAutoCanvasRestore save(context.leaf_nodes_canvas, true); - context.leaf_nodes_canvas->translate(offset_.x(), offset_.y()); - - if (context.raster_cache) { - AutoCachePaint cache_paint(context); - if (context.raster_cache->Draw(*picture(), *context.leaf_nodes_canvas, - cache_paint.paint())) { - TRACE_EVENT_INSTANT0("flutter", "raster cache hit"); - return; - } - } - - FML_DCHECK(context.inherited_opacity == SK_Scalar1); - picture()->playback(context.leaf_nodes_canvas); -} - -} // namespace flutter diff --git a/flow/layers/picture_layer.h b/flow/layers/picture_layer.h deleted file mode 100644 index 590dc72b595cd..0000000000000 --- a/flow/layers/picture_layer.h +++ /dev/null @@ -1,54 +0,0 @@ -// Copyright 2013 The Flutter Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#ifndef FLUTTER_FLOW_LAYERS_PICTURE_LAYER_H_ -#define FLUTTER_FLOW_LAYERS_PICTURE_LAYER_H_ - -#include - -#include "flutter/flow/layers/layer.h" -#include "flutter/flow/raster_cache.h" -#include "flutter/flow/skia_gpu_object.h" - -namespace flutter { - -class PictureLayer : public Layer { - public: - PictureLayer(const SkPoint& offset, - SkiaGPUObject picture, - bool is_complex, - bool will_change); - - SkPicture* picture() const { return picture_.skia_object().get(); } - - bool IsReplacing(DiffContext* context, const Layer* layer) const override; - - void Diff(DiffContext* context, const Layer* old_layer) override; - - const PictureLayer* as_picture_layer() const override { return this; } - - void Preroll(PrerollContext* frame, const SkMatrix& matrix) override; - - void Paint(PaintContext& context) const override; - - private: - SkPoint offset_; - // Even though pictures themselves are not GPU resources, they may reference - // images that have a reference to a GPU resource. - SkiaGPUObject picture_; - bool is_complex_ = false; - bool will_change_ = false; - - sk_sp SerializedPicture() const; - mutable sk_sp cached_serialized_picture_; - static bool Compare(DiffContext::Statistics& statistics, - const PictureLayer* l1, - const PictureLayer* l2); - - FML_DISALLOW_COPY_AND_ASSIGN(PictureLayer); -}; - -} // namespace flutter - -#endif // FLUTTER_FLOW_LAYERS_PICTURE_LAYER_H_ diff --git a/flow/layers/picture_layer_unittests.cc b/flow/layers/picture_layer_unittests.cc deleted file mode 100644 index 9bd5fe3aa99dd..0000000000000 --- a/flow/layers/picture_layer_unittests.cc +++ /dev/null @@ -1,214 +0,0 @@ -// Copyright 2013 The Flutter Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#define FML_USED_ON_EMBEDDER - -#include "flutter/flow/layers/picture_layer.h" - -#include "flutter/flow/testing/diff_context_test.h" -#include "flutter/flow/testing/skia_gpu_object_layer_test.h" -#include "flutter/fml/macros.h" -#include "flutter/testing/mock_canvas.h" -#include "third_party/skia/include/core/SkPicture.h" - -namespace flutter { -namespace testing { - -using PictureLayerTest = SkiaGPUObjectLayerTest; - -#ifndef NDEBUG -TEST_F(PictureLayerTest, PaintBeforePrerollInvalidPictureDies) { - const SkPoint layer_offset = SkPoint::Make(0.0f, 0.0f); - auto layer = std::make_shared( - layer_offset, SkiaGPUObject(), false, false); - - EXPECT_DEATH_IF_SUPPORTED(layer->Paint(paint_context()), - "picture_\\.skia_object\\(\\)"); -} - -TEST_F(PictureLayerTest, PaintBeforePrerollDies) { - const SkPoint layer_offset = SkPoint::Make(0.0f, 0.0f); - const SkRect picture_bounds = SkRect::MakeLTRB(5.0f, 6.0f, 20.5f, 21.5f); - auto mock_picture = SkPicture::MakePlaceholder(picture_bounds); - auto layer = std::make_shared( - layer_offset, SkiaGPUObject(mock_picture, unref_queue()), false, false); - - EXPECT_EQ(layer->paint_bounds(), SkRect::MakeEmpty()); - EXPECT_DEATH_IF_SUPPORTED(layer->Paint(paint_context()), - "needs_painting\\(context\\)"); -} - -TEST_F(PictureLayerTest, PaintingEmptyLayerDies) { - const SkPoint layer_offset = SkPoint::Make(0.0f, 0.0f); - const SkRect picture_bounds = SkRect::MakeEmpty(); - auto mock_picture = SkPicture::MakePlaceholder(picture_bounds); - auto layer = std::make_shared( - layer_offset, SkiaGPUObject(mock_picture, unref_queue()), false, false); - - layer->Preroll(preroll_context(), SkMatrix()); - EXPECT_EQ(layer->paint_bounds(), SkRect::MakeEmpty()); - EXPECT_FALSE(layer->needs_painting(paint_context())); - - EXPECT_DEATH_IF_SUPPORTED(layer->Paint(paint_context()), - "needs_painting\\(context\\)"); -} - -TEST_F(PictureLayerTest, InvalidPictureDies) { - const SkPoint layer_offset = SkPoint::Make(0.0f, 0.0f); - auto layer = std::make_shared( - layer_offset, SkiaGPUObject(), false, false); - - // Crashes reading a nullptr. - EXPECT_DEATH_IF_SUPPORTED(layer->Preroll(preroll_context(), SkMatrix()), ""); -} -#endif - -TEST_F(PictureLayerTest, SimplePicture) { - const SkPoint layer_offset = SkPoint::Make(1.5f, -0.5f); - const SkMatrix layer_offset_matrix = - SkMatrix::Translate(layer_offset.fX, layer_offset.fY); - const SkRect picture_bounds = SkRect::MakeLTRB(5.0f, 6.0f, 20.5f, 21.5f); - auto mock_picture = SkPicture::MakePlaceholder(picture_bounds); - auto layer = std::make_shared( - layer_offset, SkiaGPUObject(mock_picture, unref_queue()), false, false); - - layer->Preroll(preroll_context(), SkMatrix()); - EXPECT_EQ(layer->paint_bounds(), - picture_bounds.makeOffset(layer_offset.fX, layer_offset.fY)); - EXPECT_EQ(layer->picture(), mock_picture.get()); - EXPECT_TRUE(layer->needs_painting(paint_context())); - - layer->Paint(paint_context()); - auto expected_draw_calls = std::vector( - {MockCanvas::DrawCall{0, MockCanvas::SaveData{1}}, - MockCanvas::DrawCall{ - 1, MockCanvas::ConcatMatrixData{SkM44(layer_offset_matrix)}}, - MockCanvas::DrawCall{1, MockCanvas::RestoreData{0}}}); - EXPECT_EQ(mock_canvas().draw_calls(), expected_draw_calls); -} - -TEST_F(PictureLayerTest, OpacityInheritanceCacheablePicture) { - use_mock_raster_cache(); - const SkPoint layer_offset = SkPoint::Make(1.5f, -0.5f); - const SkRect picture_bounds = SkRect::MakeLTRB(5.0f, 6.0f, 20.5f, 21.5f); - auto mock_picture = SkPicture::MakePlaceholder(picture_bounds); - auto layer = std::make_shared( - layer_offset, SkiaGPUObject(mock_picture, unref_queue()), true, false); - - // First try, no caching, cannot support opacity - PrerollContext* context = preroll_context(); - context->subtree_can_inherit_opacity = false; - layer->Preroll(preroll_context(), SkMatrix()); - EXPECT_EQ(raster_cache()->GetPictureCachedEntriesCount(), size_t(1)); - EXPECT_EQ(raster_cache()->EstimatePictureCacheByteSize(), size_t(0)); - EXPECT_FALSE(context->subtree_can_inherit_opacity); - - // Paint it enough times to reach its cache threshold - layer->Paint(paint_context()); - layer->Paint(paint_context()); - layer->Paint(paint_context()); - - // This time it will cache and can now support opacity - context->subtree_can_inherit_opacity = false; - layer->Preroll(preroll_context(), SkMatrix()); - EXPECT_EQ(raster_cache()->GetPictureCachedEntriesCount(), size_t(1)); - EXPECT_NE(raster_cache()->EstimatePictureCacheByteSize(), size_t(0)); - EXPECT_TRUE(context->subtree_can_inherit_opacity); -} - -TEST_F(PictureLayerTest, OpacityInheritanceUncacheablePicture) { - use_mock_raster_cache(); - const SkPoint layer_offset = SkPoint::Make(1.5f, -0.5f); - const SkRect picture_bounds = SkRect::MakeLTRB(5.0f, 6.0f, 20.5f, 21.5f); - auto mock_picture = SkPicture::MakePlaceholder(picture_bounds); - auto layer = std::make_shared( - layer_offset, SkiaGPUObject(mock_picture, unref_queue()), false, true); - - PrerollContext* context = preroll_context(); - context->subtree_can_inherit_opacity = false; - layer->Preroll(preroll_context(), SkMatrix()); - EXPECT_EQ(raster_cache()->GetPictureCachedEntriesCount(), size_t(0)); - EXPECT_EQ(raster_cache()->EstimatePictureCacheByteSize(), size_t(0)); - // First try, no caching, cannot support opacity - EXPECT_FALSE(context->subtree_can_inherit_opacity); - - // Paint it enough times to reach its cache threshold - layer->Paint(paint_context()); - layer->Paint(paint_context()); - layer->Paint(paint_context()); - - context->subtree_can_inherit_opacity = false; - layer->Preroll(preroll_context(), SkMatrix()); - EXPECT_EQ(raster_cache()->GetPictureCachedEntriesCount(), size_t(0)); - EXPECT_EQ(raster_cache()->EstimatePictureCacheByteSize(), size_t(0)); - // It still is not compatible because it cannot cache - EXPECT_FALSE(context->subtree_can_inherit_opacity); -} - -using PictureLayerDiffTest = DiffContextTest; - -TEST_F(PictureLayerDiffTest, SimplePicture) { - auto picture = CreateDisplayList(SkRect::MakeLTRB(10, 10, 60, 60), 1); - - MockLayerTree tree1; - tree1.root()->Add(CreateDisplayListLayer(picture)); - - auto damage = DiffLayerTree(tree1, MockLayerTree()); - EXPECT_EQ(damage.frame_damage, SkIRect::MakeLTRB(10, 10, 60, 60)); - - MockLayerTree tree2; - tree2.root()->Add(CreateDisplayListLayer(picture)); - - damage = DiffLayerTree(tree2, tree1); - EXPECT_TRUE(damage.frame_damage.isEmpty()); - - MockLayerTree tree3; - damage = DiffLayerTree(tree3, tree2); - EXPECT_EQ(damage.frame_damage, SkIRect::MakeLTRB(10, 10, 60, 60)); -} - -TEST_F(PictureLayerDiffTest, FractionalTranslation) { - auto picture = CreateDisplayList(SkRect::MakeLTRB(10, 10, 60, 60), 1); - - MockLayerTree tree1; - tree1.root()->Add(CreateDisplayListLayer(picture, SkPoint::Make(0.5, 0.5))); - - auto damage = DiffLayerTree(tree1, MockLayerTree()); - EXPECT_EQ(damage.frame_damage, SkIRect::MakeLTRB(10, 10, 61, 61)); -} - -TEST_F(PictureLayerDiffTest, PictureCompare) { - MockLayerTree tree1; - auto picture1 = CreateDisplayList(SkRect::MakeLTRB(10, 10, 60, 60), 1); - tree1.root()->Add(CreateDisplayListLayer(picture1)); - - auto damage = DiffLayerTree(tree1, MockLayerTree()); - EXPECT_EQ(damage.frame_damage, SkIRect::MakeLTRB(10, 10, 60, 60)); - - MockLayerTree tree2; - auto picture2 = CreateDisplayList(SkRect::MakeLTRB(10, 10, 60, 60), 1); - tree2.root()->Add(CreateDisplayListLayer(picture2)); - - damage = DiffLayerTree(tree2, tree1); - EXPECT_TRUE(damage.frame_damage.isEmpty()); - - MockLayerTree tree3; - auto picture3 = CreateDisplayList(SkRect::MakeLTRB(10, 10, 60, 60), 1); - // add offset - tree3.root()->Add(CreateDisplayListLayer(picture3, SkPoint::Make(10, 10))); - - damage = DiffLayerTree(tree3, tree2); - EXPECT_EQ(damage.frame_damage, SkIRect::MakeLTRB(10, 10, 70, 70)); - - MockLayerTree tree4; - // different color - auto picture4 = CreateDisplayList(SkRect::MakeLTRB(10, 10, 60, 60), 2); - tree4.root()->Add(CreateDisplayListLayer(picture4, SkPoint::Make(10, 10))); - - damage = DiffLayerTree(tree4, tree3); - EXPECT_EQ(damage.frame_damage, SkIRect::MakeLTRB(20, 20, 70, 70)); -} - -} // namespace testing -} // namespace flutter diff --git a/shell/common/BUILD.gn b/shell/common/BUILD.gn index 17eff77f9f360..bda5dd56e9325 100644 --- a/shell/common/BUILD.gn +++ b/shell/common/BUILD.gn @@ -278,16 +278,6 @@ if (enable_unittests) { "variable_refresh_rate_display_unittests.cc", ] - if (is_fuchsia) { - # TODO(dnfield): Can we delete this harness? https://github.com/flutter/flutter/issues/106435 - sources += [ - "//flutter/flow/layers/picture_layer.cc", - "//flutter/flow/layers/picture_layer.h", - "//flutter/flow/layers/picture_layer_unittests.cc", - "skp_shader_warmup_unittests.cc", - ] - } - deps = [ ":shell_test_fixture_sources", ":shell_unittests_fixtures", diff --git a/shell/common/skp_shader_warmup_unittests.cc b/shell/common/skp_shader_warmup_unittests.cc deleted file mode 100644 index 7790e66667fdc..0000000000000 --- a/shell/common/skp_shader_warmup_unittests.cc +++ /dev/null @@ -1,301 +0,0 @@ -// Copyright 2013 The Flutter Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include - -#include "flutter/assets/directory_asset_bundle.h" -#include "flutter/common/graphics/persistent_cache.h" -#include "flutter/flow/layers/container_layer.h" -#include "flutter/flow/layers/layer.h" -#include "flutter/flow/layers/physical_shape_layer.h" -#include "flutter/flow/layers/picture_layer.h" -#include "flutter/fml/command_line.h" -#include "flutter/fml/file.h" -#include "flutter/fml/log_settings.h" -#include "flutter/fml/unique_fd.h" -#include "flutter/shell/common/serialization_callbacks.h" -#include "flutter/shell/common/shell_test.h" -#include "flutter/shell/common/switches.h" -#include "flutter/shell/version/version.h" -#include "flutter/testing/testing.h" -#include "include/core/SkFont.h" -#include "include/core/SkPicture.h" -#include "include/core/SkPictureRecorder.h" -#include "include/core/SkSerialProcs.h" -#include "include/core/SkTextBlob.h" - -#if defined(OS_FUCHSIA) -#include "lib/sys/cpp/component_context.h" -#include "third_party/skia/include/ports/SkFontMgr_fuchsia.h" - -namespace flutter { -namespace testing { - -static void WaitForIO(Shell* shell) { - std::promise io_task_finished; - shell->GetTaskRunners().GetIOTaskRunner()->PostTask( - [&io_task_finished]() { io_task_finished.set_value(true); }); - io_task_finished.get_future().wait(); -} - -class SkpWarmupTest : public ShellTest { - public: - SkpWarmupTest() {} - - void TestWarmup(const SkISize& draw_size, const LayerTreeBuilder& builder) { - // Create a temp dir to store the persistent cache - fml::ScopedTemporaryDirectory dir; - PersistentCache::SetCacheDirectoryPath(dir.path()); - PersistentCache::ResetCacheForProcess(); - - auto settings = CreateSettingsForFixture(); - settings.cache_sksl = true; - settings.dump_skp_on_shader_compilation = true; - - fml::AutoResetWaitableEvent firstFrameLatch; - settings.frame_rasterized_callback = - [&firstFrameLatch](const FrameTiming& t) { firstFrameLatch.Signal(); }; - - auto config = RunConfiguration::InferFromSettings(settings); - config.SetEntrypoint("emptyMain"); - std::unique_ptr shell = CreateShell(settings); - PlatformViewNotifyCreated(shell.get()); - RunEngine(shell.get(), std::move(config)); - - // Initially, we should have no SkSL cache - auto cache = PersistentCache::GetCacheForProcess()->LoadSkSLs(); - ASSERT_EQ(cache.size(), 0u); - - PumpOneFrame(shell.get(), draw_size.width(), draw_size.height(), builder); - firstFrameLatch.Wait(); - WaitForIO(shell.get()); - - // Count the number of shaders this builder generated. We use this as a - // proxy for whether new shaders were generated, since skia will dump an skp - // any time a new shader is compiled. - int skp_count = 0; - fml::FileVisitor skp_count_visitor = [&skp_count]( - const fml::UniqueFD& directory, - const std::string& filename) { - if (filename.size() >= 4 && - filename.substr(filename.size() - 4, 4) == ".skp") { - skp_count += 1; - } - return true; - }; - fml::VisitFilesRecursively(dir.fd(), skp_count_visitor); - int first_skp_count = skp_count; - skp_count = 0; - ASSERT_GT(first_skp_count, 0); - - // Deserialize all skps into memory - std::vector> pictures; - fml::FileVisitor skp_deserialize_visitor = - [&pictures](const fml::UniqueFD& directory, - const std::string& filename) { - if (filename.size() >= 4 && - filename.substr(filename.size() - 4, 4) == ".skp") { - auto fd = fml::OpenFileReadOnly(directory, filename.c_str()); - if (fd.get() < 0) { - FML_LOG(ERROR) << "Failed to open " << filename; - return true; - } - // Deserialize - sk_sp data = SkData::MakeFromFD(fd.get()); - std::unique_ptr stream = SkMemoryStream::Make(data); - - SkDeserialProcs procs = {0}; - procs.fImageProc = DeserializeImageWithoutData; - procs.fTypefaceProc = DeserializeTypefaceWithoutData; - sk_sp picture = - SkPicture::MakeFromStream(stream.get(), &procs); - if (!picture) { - FML_LOG(ERROR) << "Failed to deserialize " << filename; - return true; - } - pictures.push_back(std::move(picture)); - fd.reset(); - } - return true; - }; - fml::VisitFilesRecursively(dir.fd(), skp_deserialize_visitor); - ASSERT_GT(pictures.size(), 0ul); - - // Reinitialize shell with clean cache and verify that drawing again dumps - // the same number of shaders - fml::RemoveFilesInDirectory(dir.fd()); - PersistentCache::ResetCacheForProcess(); - DestroyShell(std::move(shell)); - auto config2 = RunConfiguration::InferFromSettings(settings); - config2.SetEntrypoint("emptyMain"); - shell = CreateShell(settings); - PlatformViewNotifyCreated(shell.get()); - RunEngine(shell.get(), std::move(config2)); - firstFrameLatch.Reset(); - PumpOneFrame(shell.get(), draw_size.width(), draw_size.height(), builder); - firstFrameLatch.Wait(); - WaitForIO(shell.get()); - - // Verify same number of shaders dumped - fml::VisitFilesRecursively(dir.fd(), skp_count_visitor); - int second_skp_count = skp_count; - skp_count = 0; - ASSERT_EQ(second_skp_count, first_skp_count); - - // Reinitialize shell and draw deserialized skps to warm up shaders - fml::RemoveFilesInDirectory(dir.fd()); - PersistentCache::ResetCacheForProcess(); - DestroyShell(std::move(shell)); - auto config3 = RunConfiguration::InferFromSettings(settings); - config3.SetEntrypoint("emptyMain"); - shell = CreateShell(settings); - PlatformViewNotifyCreated(shell.get()); - RunEngine(shell.get(), std::move(config3)); - firstFrameLatch.Reset(); - - for (auto& picture : pictures) { - fml::RefPtr queue = fml::MakeRefCounted( - this->GetCurrentTaskRunner(), fml::TimeDelta::FromSeconds(0)); - LayerTreeBuilder picture_builder = - [picture, queue](std::shared_ptr root) { - auto picture_layer = std::make_shared( - SkPoint::Make(0, 0), SkiaGPUObject(picture, queue), - /* is_complex */ false, - /* will_change */ false); - root->Add(picture_layer); - }; - PumpOneFrame(shell.get(), picture->cullRect().width(), - picture->cullRect().height(), picture_builder); - } - firstFrameLatch.Wait(); - WaitForIO(shell.get()); - - // Verify same number of shaders dumped - fml::VisitFilesRecursively(dir.fd(), skp_count_visitor); - int third_skp_count = skp_count; - skp_count = 0; - ASSERT_EQ(third_skp_count, first_skp_count); - - // Remove files generated - fml::RemoveFilesInDirectory(dir.fd()); - - // Draw orignal material again - firstFrameLatch.Reset(); - PumpOneFrame(shell.get(), draw_size.width(), draw_size.height(), builder); - - firstFrameLatch.Wait(); - WaitForIO(shell.get()); - - // Verify no new shaders dumped - fml::VisitFilesRecursively(dir.fd(), skp_count_visitor); - int fourth_skp_count = skp_count; - skp_count = 0; - ASSERT_EQ(fourth_skp_count, 0); - - // Clean Up - fml::RemoveFilesInDirectory(dir.fd()); - } -}; - -TEST_F(SkpWarmupTest, Basic) { - SkISize draw_size = SkISize::Make(100, 100); - // Draw something to trigger shader compilations. - LayerTreeBuilder builder = - [&draw_size](std::shared_ptr root) { - SkPath path; - path.addCircle(draw_size.width() / 2, draw_size.height() / 2, 20); - auto physical_shape_layer = std::make_shared( - SK_ColorRED, SK_ColorBLUE, 1.0f, path, Clip::antiAlias); - root->Add(physical_shape_layer); - }; - TestWarmup(draw_size, builder); -} - -TEST_F(SkpWarmupTest, Image) { - SkISize draw_size = SkISize::Make(100, 100); - // We reuse this builder to draw the same content sever times in this test - LayerTreeBuilder builder = [&draw_size, - this](std::shared_ptr root) { - SkPictureRecorder recorder; - auto canvas = - recorder.beginRecording(draw_size.width(), draw_size.height()); - - // include an image so we can test that the warmup works even with image - // data excluded from the skp - auto image_size = - SkISize::Make(draw_size.width() / 2, draw_size.height() / 2); - auto color_space = SkColorSpace::MakeSRGB(); - auto info = - SkImageInfo::Make(image_size, SkColorType::kRGBA_8888_SkColorType, - SkAlphaType::kPremul_SkAlphaType, color_space); - sk_sp image_data = - SkData::MakeUninitialized(image_size.width() * image_size.height() * 4); - memset(image_data->writable_data(), 0x0f, image_data->size()); - sk_sp image = - SkImage::MakeRasterData(info, image_data, image_size.width() * 4); - - canvas->drawImage(image, image_size.width(), image_size.height()); - - auto picture = recorder.finishRecordingAsPicture(); - - fml::RefPtr queue = fml::MakeRefCounted( - this->GetCurrentTaskRunner(), fml::TimeDelta::FromSeconds(0)); - auto picture_layer = std::make_shared( - SkPoint::Make(0, 0), SkiaGPUObject(picture, queue), - /* is_complex */ false, - /* will_change */ false); - root->Add(picture_layer); - }; - - TestWarmup(draw_size, builder); -} - -// Re-enable once https://bugs.chromium.org/p/skia/issues/detail?id=10404 -// is fixed and integrated, or a workaround is found. -TEST_F(SkpWarmupTest, DISABLED_Text) { - auto context = sys::ComponentContext::Create(); - fuchsia::fonts::ProviderSyncPtr sync_font_provider; - context->svc()->Connect(sync_font_provider.NewRequest()); - auto font_mgr = SkFontMgr_New_Fuchsia(std::move(sync_font_provider)); - auto raw_typeface = - font_mgr->matchFamilyStyle(nullptr, SkFontStyle::Normal()); - auto typeface = sk_sp(raw_typeface); - - SkFont font(typeface, 12); - auto text_blob = - SkTextBlob::MakeFromString("test", font, SkTextEncoding::kUTF8); - - SkISize draw_size = - SkISize::Make(text_blob->bounds().width(), text_blob->bounds().height()); - // We reuse this builder to draw the same content sever times in this test - LayerTreeBuilder builder = [&draw_size, &text_blob, - this](std::shared_ptr root) { - SkPictureRecorder recorder; - - auto canvas = - recorder.beginRecording(draw_size.width(), draw_size.height()); - - auto color_space = SkColorSpace::MakeSRGB(); - auto paint = SkPaint(SkColors::kWhite, color_space.get()); - canvas->drawTextBlob(text_blob, draw_size.width() / 2, - draw_size.height() / 2, paint); - - auto picture = recorder.finishRecordingAsPicture(); - - fml::RefPtr queue = fml::MakeRefCounted( - this->GetCurrentTaskRunner(), fml::TimeDelta::FromSeconds(0)); - auto picture_layer = std::make_shared( - SkPoint::Make(0, 0), SkiaGPUObject(picture, queue), - /* is_complex */ false, - /* will_change */ false); - root->Add(picture_layer); - }; - - TestWarmup(draw_size, builder); -} - -} // namespace testing -} // namespace flutter - -#endif // defined(OS_FUCHSIA) From 6bb08b50c41cea1abcb548f8a54a0a9f61103b79 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Wed, 22 Jun 2022 15:54:43 -0700 Subject: [PATCH 5/5] jim review --- lib/ui/painting/canvas.cc | 158 ++------------------------------ shell/common/shell_unittests.cc | 106 ++++++++------------- 2 files changed, 44 insertions(+), 220 deletions(-) diff --git a/lib/ui/painting/canvas.cc b/lib/ui/painting/canvas.cc index a64de1cf91f91..068c21a782bcc 100644 --- a/lib/ui/painting/canvas.cc +++ b/lib/ui/painting/canvas.cc @@ -111,8 +111,6 @@ Canvas::~Canvas() {} void Canvas::save() { if (display_list_recorder_) { builder()->save(); - } else if (canvas_) { - canvas_->save(); } } @@ -125,10 +123,6 @@ void Canvas::saveLayerWithoutBounds(const Paint& paint, FML_DCHECK(restore_with_paint); TRACE_EVENT0("flutter", "ui.Canvas::saveLayer (Recorded)"); builder()->saveLayer(nullptr, restore_with_paint); - } else if (canvas_) { - SkPaint sk_paint; - TRACE_EVENT0("flutter", "ui.Canvas::saveLayer (Recorded)"); - canvas_->saveLayer(nullptr, paint.paint(sk_paint)); } } @@ -146,26 +140,18 @@ void Canvas::saveLayer(double left, FML_DCHECK(restore_with_paint); TRACE_EVENT0("flutter", "ui.Canvas::saveLayer (Recorded)"); builder()->saveLayer(&bounds, restore_with_paint); - } else if (canvas_) { - SkPaint sk_paint; - TRACE_EVENT0("flutter", "ui.Canvas::saveLayer (Recorded)"); - canvas_->saveLayer(&bounds, paint.paint(sk_paint)); } } void Canvas::restore() { if (display_list_recorder_) { builder()->restore(); - } else if (canvas_) { - canvas_->restore(); } } int Canvas::getSaveCount() { if (display_list_recorder_) { return builder()->getSaveCount(); - } else if (canvas_) { - return canvas_->getSaveCount(); } else { return 0; } @@ -174,32 +160,24 @@ int Canvas::getSaveCount() { void Canvas::translate(double dx, double dy) { if (display_list_recorder_) { builder()->translate(dx, dy); - } else if (canvas_) { - canvas_->translate(dx, dy); } } void Canvas::scale(double sx, double sy) { if (display_list_recorder_) { builder()->scale(sx, sy); - } else if (canvas_) { - canvas_->scale(sx, sy); } } void Canvas::rotate(double radians) { if (display_list_recorder_) { builder()->rotate(radians * 180.0 / M_PI); - } else if (canvas_) { - canvas_->rotate(radians * 180.0 / M_PI); } } void Canvas::skew(double sx, double sy) { if (display_list_recorder_) { builder()->skew(sx, sy); - } else if (canvas_) { - canvas_->skew(sx, sy); } } @@ -214,11 +192,6 @@ void Canvas::transform(const tonic::Float64List& matrix4) { matrix4[ 2], matrix4[ 6], matrix4[10], matrix4[14], matrix4[ 3], matrix4[ 7], matrix4[11], matrix4[15]); // clang-format on - } else if (canvas_) { - canvas_->concat(SkM44(matrix4[0], matrix4[4], matrix4[8], matrix4[12], - matrix4[1], matrix4[5], matrix4[9], matrix4[13], - matrix4[2], matrix4[6], matrix4[10], matrix4[14], - matrix4[3], matrix4[7], matrix4[11], matrix4[15])); } } @@ -244,17 +217,12 @@ void Canvas::clipRect(double left, if (display_list_recorder_) { builder()->clipRect(SkRect::MakeLTRB(left, top, right, bottom), clipOp, doAntiAlias); - } else if (canvas_) { - canvas_->clipRect(SkRect::MakeLTRB(left, top, right, bottom), clipOp, - doAntiAlias); } } void Canvas::clipRRect(const RRect& rrect, bool doAntiAlias) { if (display_list_recorder_) { builder()->clipRRect(rrect.sk_rrect, SkClipOp::kIntersect, doAntiAlias); - } else if (canvas_) { - canvas_->clipRRect(rrect.sk_rrect, doAntiAlias); } } @@ -266,8 +234,6 @@ void Canvas::clipPath(const CanvasPath* path, bool doAntiAlias) { } if (display_list_recorder_) { builder()->clipPath(path->path(), SkClipOp::kIntersect, doAntiAlias); - } else if (canvas_) { - canvas_->clipPath(path->path(), doAntiAlias); } } @@ -278,8 +244,12 @@ void Canvas::getDestinationClipBounds(tonic::Float64List& rect) { rect[1] = bounds.fTop; rect[2] = bounds.fRight; rect[3] = bounds.fBottom; - } else { - SkIRect bounds = canvas_->getDeviceClipBounds(); + } +} + +void Canvas::getLocalClipBounds(tonic::Float64List& rect) { + if (display_list_recorder_) { + SkRect bounds = display_list_recorder_->builder()->getLocalClipBounds(); rect[0] = bounds.fLeft; rect[1] = bounds.fTop; rect[2] = bounds.fRight; @@ -287,21 +257,9 @@ void Canvas::getDestinationClipBounds(tonic::Float64List& rect) { } } -void Canvas::getLocalClipBounds(tonic::Float64List& rect) { - SkRect bounds = display_list_recorder_ - ? display_list_recorder_->builder()->getLocalClipBounds() - : canvas_->getLocalClipBounds(); - rect[0] = bounds.fLeft; - rect[1] = bounds.fTop; - rect[2] = bounds.fRight; - rect[3] = bounds.fBottom; -} - void Canvas::drawColor(SkColor color, DlBlendMode blend_mode) { if (display_list_recorder_) { builder()->drawColor(color, blend_mode); - } else if (canvas_) { - canvas_->drawColor(color, ToSk(blend_mode)); } } @@ -315,9 +273,6 @@ void Canvas::drawLine(double x1, if (display_list_recorder_) { paint.sync_to(builder(), kDrawLineFlags); builder()->drawLine(SkPoint::Make(x1, y1), SkPoint::Make(x2, y2)); - } else if (canvas_) { - SkPaint sk_paint; - canvas_->drawLine(x1, y1, x2, y2, *paint.paint(sk_paint)); } } @@ -332,16 +287,6 @@ void Canvas::drawPaint(const Paint& paint, const PaintData& paint_data) { TRACE_EVENT0("flutter", "ui.Canvas::saveLayer (Recorded)"); } builder()->drawPaint(); - } else if (canvas_) { - SkPaint sk_paint; - paint.paint(sk_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(sk_paint); } } @@ -355,10 +300,6 @@ void Canvas::drawRect(double left, if (display_list_recorder_) { paint.sync_to(builder(), kDrawRectFlags); builder()->drawRect(SkRect::MakeLTRB(left, top, right, bottom)); - } else if (canvas_) { - SkPaint sk_paint; - canvas_->drawRect(SkRect::MakeLTRB(left, top, right, bottom), - *paint.paint(sk_paint)); } } @@ -369,9 +310,6 @@ void Canvas::drawRRect(const RRect& rrect, if (display_list_recorder_) { paint.sync_to(builder(), kDrawRRectFlags); builder()->drawRRect(rrect.sk_rrect); - } else if (canvas_) { - SkPaint sk_paint; - canvas_->drawRRect(rrect.sk_rrect, *paint.paint(sk_paint)); } } @@ -383,9 +321,6 @@ void Canvas::drawDRRect(const RRect& outer, if (display_list_recorder_) { paint.sync_to(builder(), kDrawDRRectFlags); builder()->drawDRRect(outer.sk_rrect, inner.sk_rrect); - } else if (canvas_) { - SkPaint sk_paint; - canvas_->drawDRRect(outer.sk_rrect, inner.sk_rrect, *paint.paint(sk_paint)); } } @@ -399,10 +334,6 @@ void Canvas::drawOval(double left, if (display_list_recorder_) { paint.sync_to(builder(), kDrawOvalFlags); builder()->drawOval(SkRect::MakeLTRB(left, top, right, bottom)); - } else if (canvas_) { - SkPaint sk_paint; - canvas_->drawOval(SkRect::MakeLTRB(left, top, right, bottom), - *paint.paint(sk_paint)); } } @@ -415,9 +346,6 @@ void Canvas::drawCircle(double x, if (display_list_recorder_) { paint.sync_to(builder(), kDrawCircleFlags); builder()->drawCircle(SkPoint::Make(x, y), radius); - } else if (canvas_) { - SkPaint sk_paint; - canvas_->drawCircle(x, y, radius, *paint.paint(sk_paint)); } } @@ -439,11 +367,6 @@ void Canvas::drawArc(double left, builder()->drawArc(SkRect::MakeLTRB(left, top, right, bottom), startAngle * 180.0 / M_PI, sweepAngle * 180.0 / M_PI, useCenter); - } else if (canvas_) { - SkPaint sk_paint; - canvas_->drawArc(SkRect::MakeLTRB(left, top, right, bottom), - startAngle * 180.0 / M_PI, sweepAngle * 180.0 / M_PI, - useCenter, *paint.paint(sk_paint)); } } @@ -459,9 +382,6 @@ void Canvas::drawPath(const CanvasPath* path, if (display_list_recorder_) { paint.sync_to(builder(), kDrawPathFlags); builder()->drawPath(path->path()); - } else if (canvas_) { - SkPaint sk_paint; - canvas_->drawPath(path->path(), *paint.paint(sk_paint)); } } @@ -482,17 +402,6 @@ void Canvas::drawImage(const CanvasImage* image, bool with_attributes = paint.sync_to(builder(), kDrawImageWithPaintFlags); builder()->drawImage(image->image(), SkPoint::Make(x, y), sampling, with_attributes); - } else if (canvas_) { - auto dl_image = image->image(); - if (!dl_image) { - return; - } - auto sk_image = dl_image->skia_image(); - if (!sk_image) { - return; - } - SkPaint sk_paint; - canvas_->drawImage(sk_image, x, y, ToSk(sampling), paint.paint(sk_paint)); } } @@ -523,19 +432,6 @@ void Canvas::drawImageRect(const CanvasImage* image, builder()->drawImageRect(image->image(), src, dst, sampling, with_attributes, SkCanvas::kFast_SrcRectConstraint); - } else if (canvas_) { - auto dl_image = image->image(); - if (!dl_image) { - return; - } - auto sk_image = dl_image->skia_image(); - if (!sk_image) { - return; - } - SkPaint sk_paint; - canvas_->drawImageRect(sk_image, src, dst, ToSk(sampling), - paint.paint(sk_paint), - SkCanvas::kFast_SrcRectConstraint); } } @@ -568,18 +464,6 @@ void Canvas::drawImageNine(const CanvasImage* image, paint.sync_to(builder(), kDrawImageNineWithPaintFlags); builder()->drawImageNine(image->image(), icenter, dst, filter, with_attributes); - } else if (canvas_) { - auto dl_image = image->image(); - if (!dl_image) { - return; - } - auto sk_image = dl_image->skia_image(); - if (!sk_image) { - return; - } - SkPaint sk_paint; - canvas_->drawImageNine(sk_image.get(), icenter, dst, ToSk(filter), - paint.paint(sk_paint)); } } @@ -623,12 +507,6 @@ void Canvas::drawPoints(const Paint& paint, builder()->drawPoints(point_mode, points.num_elements() / 2, // SkPoints have 2 floats reinterpret_cast(points.data())); - } else if (canvas_) { - SkPaint sk_paint; - canvas_->drawPoints(point_mode, - points.num_elements() / 2, // SkPoints have 2 floats - reinterpret_cast(points.data()), - *paint.paint(sk_paint)); } } @@ -645,10 +523,6 @@ void Canvas::drawVertices(const Vertices* vertices, if (display_list_recorder_) { paint.sync_to(builder(), kDrawVerticesFlags); builder()->drawVertices(vertices->vertices(), blend_mode); - } else if (canvas_) { - SkPaint sk_paint; - canvas_->drawVertices(vertices->vertices()->skia_object(), ToSk(blend_mode), - *paint.paint(sk_paint)); } } @@ -687,23 +561,6 @@ void Canvas::drawAtlas(const Paint& paint, rects.num_elements() / 4, // SkRect have four floats. blend_mode, sampling, reinterpret_cast(cull_rect.data()), with_attributes); - } else if (canvas_) { - if (!dl_image) { - return; - } - auto sk_image = dl_image->skia_image(); - if (!sk_image) { - return; - } - SkPaint sk_paint; - canvas_->drawAtlas(sk_image.get(), - reinterpret_cast(transforms.data()), - reinterpret_cast(rects.data()), - reinterpret_cast(colors.data()), - rects.num_elements() / 4, // SkRect have four floats. - ToSk(blend_mode), ToSk(sampling), - reinterpret_cast(cull_rect.data()), - paint.paint(sk_paint)); } } @@ -731,9 +588,6 @@ void Canvas::drawShadow(const CanvasPath* path, // See: https://bugs.chromium.org/p/skia/issues/detail?id=12125 builder()->drawShadow(path->path(), color, elevation, transparentOccluder, dpr); - } else if (canvas_) { - DisplayListCanvasDispatcher::DrawShadow( - canvas_, path->path(), color, elevation, transparentOccluder, dpr); } } diff --git a/shell/common/shell_unittests.cc b/shell/common/shell_unittests.cc index 2d3f2c6202593..18737e57f6d6f 100644 --- a/shell/common/shell_unittests.cc +++ b/shell/common/shell_unittests.cc @@ -270,6 +270,13 @@ static void PostSync(const fml::RefPtr& task_runner, latch.Wait(); } +static sk_sp MakeSizedDisplayList(int width, int height) { + DisplayListCanvasRecorder recorder(SkRect::MakeXYWH(0, 0, width, height)); + recorder.drawRect(SkRect::MakeXYWH(0, 0, width, height), + SkPaint(SkColor4f::FromColor(SK_ColorRED))); + return recorder.Build(); +} + TEST_F(ShellTest, InitializeWithInvalidThreads) { ASSERT_FALSE(DartVMRef::IsInstanceRunning()); Settings settings = CreateSettingsForFixture(); @@ -744,16 +751,13 @@ TEST_F(ShellTest, ExternalEmbedderNoThreadMerger) { RunEngine(shell.get(), std::move(configuration)); LayerTreeBuilder builder = [&](std::shared_ptr root) { - DisplayListCanvasRecorder recorder(SkRect::MakeXYWH(0, 0, 80, 80)); - recorder.drawRect(SkRect::MakeXYWH(0, 0, 80, 80), - SkPaint(SkColor4f::FromColor(SK_ColorRED))); - auto display_list = recorder.Build(); fml::RefPtr queue = fml::MakeRefCounted( this->GetCurrentTaskRunner(), fml::TimeDelta::Zero()); auto display_list_layer = std::make_shared( SkPoint::Make(10, 10), - flutter::SkiaGPUObject({display_list, queue}), false, - false); + flutter::SkiaGPUObject( + {MakeSizedDisplayList(80, 80), queue}), + false, false); root->Add(display_list_layer); }; @@ -798,16 +802,13 @@ TEST_F(ShellTest, RunEngine(shell.get(), std::move(configuration)); LayerTreeBuilder builder = [&](std::shared_ptr root) { - DisplayListCanvasRecorder recorder(SkRect::MakeXYWH(0, 0, 80, 80)); - recorder.drawRect(SkRect::MakeXYWH(0, 0, 80, 80), - SkPaint(SkColor4f::FromColor(SK_ColorRED))); - auto display_list = recorder.Build(); fml::RefPtr queue = fml::MakeRefCounted( this->GetCurrentTaskRunner(), fml::TimeDelta::Zero()); auto display_list_layer = std::make_shared( SkPoint::Make(10, 10), - flutter::SkiaGPUObject({display_list, queue}), false, - false); + flutter::SkiaGPUObject( + {MakeSizedDisplayList(80, 80), queue}), + false, false); root->Add(display_list_layer); }; @@ -850,16 +851,13 @@ TEST_F(ShellTest, RunEngine(shell.get(), std::move(configuration)); LayerTreeBuilder builder = [&](std::shared_ptr root) { - DisplayListCanvasRecorder recorder(SkRect::MakeXYWH(0, 0, 80, 80)); - recorder.drawRect(SkRect::MakeXYWH(0, 0, 80, 80), - SkPaint(SkColor4f::FromColor(SK_ColorRED))); - auto display_list = recorder.Build(); fml::RefPtr queue = fml::MakeRefCounted( this->GetCurrentTaskRunner(), fml::TimeDelta::Zero()); auto display_list_layer = std::make_shared( SkPoint::Make(10, 10), - flutter::SkiaGPUObject({display_list, queue}), false, - false); + flutter::SkiaGPUObject( + {MakeSizedDisplayList(80, 80), queue}), + false, false); root->Add(display_list_layer); }; @@ -922,16 +920,13 @@ TEST_F(ShellTest, RunEngine(shell.get(), std::move(configuration)); LayerTreeBuilder builder = [&](std::shared_ptr root) { - DisplayListCanvasRecorder recorder(SkRect::MakeXYWH(0, 0, 80, 80)); - recorder.drawRect(SkRect::MakeXYWH(0, 0, 80, 80), - SkPaint(SkColor4f::FromColor(SK_ColorRED))); - auto display_list = recorder.Build(); fml::RefPtr queue = fml::MakeRefCounted( this->GetCurrentTaskRunner(), fml::TimeDelta::Zero()); auto display_list_layer = std::make_shared( SkPoint::Make(10, 10), - flutter::SkiaGPUObject({display_list, queue}), false, - false); + flutter::SkiaGPUObject( + {MakeSizedDisplayList(80, 80), queue}), + false, false); root->Add(display_list_layer); }; @@ -996,16 +991,13 @@ TEST_F(ShellTest, RunEngine(shell.get(), std::move(configuration)); LayerTreeBuilder builder = [&](std::shared_ptr root) { - DisplayListCanvasRecorder recorder(SkRect::MakeXYWH(0, 0, 80, 80)); - recorder.drawRect(SkRect::MakeXYWH(0, 0, 80, 80), - SkPaint(SkColor4f::FromColor(SK_ColorRED))); - auto display_list = recorder.Build(); fml::RefPtr queue = fml::MakeRefCounted( this->GetCurrentTaskRunner(), fml::TimeDelta::Zero()); auto display_list_layer = std::make_shared( SkPoint::Make(10, 10), - flutter::SkiaGPUObject({display_list, queue}), false, - false); + flutter::SkiaGPUObject( + {MakeSizedDisplayList(80, 80), queue}), + false, false); root->Add(display_list_layer); }; @@ -1068,16 +1060,13 @@ TEST_F(ShellTest, RunEngine(shell.get(), std::move(configuration)); LayerTreeBuilder builder = [&](std::shared_ptr root) { - DisplayListCanvasRecorder recorder(SkRect::MakeXYWH(0, 0, 80, 80)); - recorder.drawRect(SkRect::MakeXYWH(0, 0, 80, 80), - SkPaint(SkColor4f::FromColor(SK_ColorRED))); - auto display_list = recorder.Build(); fml::RefPtr queue = fml::MakeRefCounted( this->GetCurrentTaskRunner(), fml::TimeDelta::Zero()); auto display_list_layer = std::make_shared( SkPoint::Make(10, 10), - flutter::SkiaGPUObject({display_list, queue}), false, - false); + flutter::SkiaGPUObject( + {MakeSizedDisplayList(80, 80), queue}), + false, false); root->Add(display_list_layer); }; PumpOneFrame(shell.get(), 100, 100, builder); @@ -1115,16 +1104,13 @@ TEST_F(ShellTest, OnPlatformViewDestroyWithoutRasterThreadMerger) { RunEngine(shell.get(), std::move(configuration)); LayerTreeBuilder builder = [&](std::shared_ptr root) { - DisplayListCanvasRecorder recorder(SkRect::MakeXYWH(0, 0, 80, 80)); - recorder.drawRect(SkRect::MakeXYWH(0, 0, 80, 80), - SkPaint(SkColor4f::FromColor(SK_ColorRED))); - auto display_list = recorder.Build(); fml::RefPtr queue = fml::MakeRefCounted( this->GetCurrentTaskRunner(), fml::TimeDelta::Zero()); auto display_list_layer = std::make_shared( SkPoint::Make(10, 10), - flutter::SkiaGPUObject({display_list, queue}), false, - false); + flutter::SkiaGPUObject( + {MakeSizedDisplayList(80, 80), queue}), + false, false); root->Add(display_list_layer); }; PumpOneFrame(shell.get(), 100, 100, builder); @@ -1185,16 +1171,13 @@ TEST_F(ShellTest, RunEngine(shell.get(), std::move(configuration)); LayerTreeBuilder builder = [&](std::shared_ptr root) { - DisplayListCanvasRecorder recorder(SkRect::MakeXYWH(0, 0, 80, 80)); - recorder.drawRect(SkRect::MakeXYWH(0, 0, 80, 80), - SkPaint(SkColor4f::FromColor(SK_ColorRED))); - auto display_list = recorder.Build(); fml::RefPtr queue = fml::MakeRefCounted( this->GetCurrentTaskRunner(), fml::TimeDelta::Zero()); auto display_list_layer = std::make_shared( SkPoint::Make(10, 10), - flutter::SkiaGPUObject({display_list, queue}), false, - false); + flutter::SkiaGPUObject( + {MakeSizedDisplayList(80, 80), queue}), + false, false); root->Add(display_list_layer); }; PumpOneFrame(shell.get(), 100, 100, builder); @@ -1234,16 +1217,13 @@ TEST_F(ShellTest, GetUsedThisFrameShouldBeSetBeforeEndFrame) { RunEngine(shell.get(), std::move(configuration)); LayerTreeBuilder builder = [&](std::shared_ptr root) { - DisplayListCanvasRecorder recorder(SkRect::MakeXYWH(0, 0, 80, 80)); - recorder.drawRect(SkRect::MakeXYWH(0, 0, 80, 80), - SkPaint(SkColor4f::FromColor(SK_ColorRED))); - auto display_list = recorder.Build(); fml::RefPtr queue = fml::MakeRefCounted( this->GetCurrentTaskRunner(), fml::TimeDelta::Zero()); auto display_list_layer = std::make_shared( SkPoint::Make(10, 10), - flutter::SkiaGPUObject({display_list, queue}), false, - false); + flutter::SkiaGPUObject( + {MakeSizedDisplayList(80, 80), queue}), + false, false); root->Add(display_list_layer); }; PumpOneFrame(shell.get(), 100, 100, builder); @@ -2019,16 +1999,13 @@ TEST_F(ShellTest, Screenshot) { RunEngine(shell.get(), std::move(configuration)); LayerTreeBuilder builder = [&](std::shared_ptr root) { - DisplayListCanvasRecorder recorder(SkRect::MakeXYWH(0, 0, 80, 80)); - recorder.drawRect(SkRect::MakeXYWH(0, 0, 80, 80), - SkPaint(SkColor4f::FromColor(SK_ColorRED))); - auto display_list = recorder.Build(); fml::RefPtr queue = fml::MakeRefCounted( this->GetCurrentTaskRunner(), fml::TimeDelta::Zero()); auto display_list_layer = std::make_shared( SkPoint::Make(10, 10), - flutter::SkiaGPUObject({display_list, queue}), false, - false); + flutter::SkiaGPUObject( + {MakeSizedDisplayList(80, 80), queue}), + false, false); root->Add(display_list_layer); }; @@ -2376,13 +2353,6 @@ TEST_F(ShellTest, RasterizerMakeRasterSnapshot) { DestroyShell(std::move(shell), std::move(task_runners)); } -static sk_sp MakeSizedDisplayList(int width, int height) { - DisplayListCanvasRecorder recorder(SkRect::MakeXYWH(0, 0, 80, 80)); - recorder.drawRect(SkRect::MakeXYWH(0, 0, 80, 80), - SkPaint(SkColor4f::FromColor(SK_ColorRED))); - return recorder.Build(); -} - TEST_F(ShellTest, OnServiceProtocolEstimateRasterCacheMemoryWorks) { Settings settings = CreateSettingsForFixture(); std::unique_ptr shell = CreateShell(settings); @@ -2458,7 +2428,7 @@ TEST_F(ShellTest, OnServiceProtocolEstimateRasterCacheMemoryWorks) { document.Accept(writer); std::string expected_json = "{\"type\":\"EstimateRasterCacheMemory\",\"layerBytes\":40000,\"picture" - "Bytes\":25600}"; + "Bytes\":400}"; std::string actual_json = buffer.GetString(); ASSERT_EQ(actual_json, expected_json);