From 5465b56b29ca0caa8bb1d2ae229fe34414b2bfcf Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Thu, 28 Oct 2021 14:28:02 -0700 Subject: [PATCH] use nested op counts to determine picture complexity for raster cache (flutter/engine#29265) --- engine/src/flutter/flow/display_list.cc | 16 +- .../flutter/flow/display_list_unittests.cc | 45 ++++ engine/src/flutter/flow/raster_cache.cc | 4 +- .../flutter/flow/raster_cache_unittests.cc | 233 +++++++++++++++++- 4 files changed, 289 insertions(+), 9 deletions(-) diff --git a/engine/src/flutter/flow/display_list.cc b/engine/src/flutter/flow/display_list.cc index fb7dfd696f..3e3806dab6 100644 --- a/engine/src/flutter/flow/display_list.cc +++ b/engine/src/flutter/flow/display_list.cc @@ -1427,14 +1427,26 @@ void DisplayListBuilder::drawPicture(const sk_sp picture, ? Push(0, 1, std::move(picture), *matrix, render_with_attributes) : Push(0, 1, std::move(picture), render_with_attributes); + // The non-nested op count accumulated in the |Push| method will include + // this call to |drawPicture| for non-nested op count metrics. + // But, for nested op count metrics we want the |drawPicture| call itself + // to be transparent. So we subtract 1 from our accumulated nested count to + // balance out against the 1 that was accumulated into the regular count. + // This behavior is identical to the way SkPicture computes nested op counts. + nested_op_count_ += picture->approximateOpCount(true) - 1; nested_bytes_ += picture->approximateBytesUsed(); - nested_op_count_ += picture->approximateOpCount(true); } void DisplayListBuilder::drawDisplayList( const sk_sp display_list) { Push(0, 1, std::move(display_list)); + // The non-nested op count accumulated in the |Push| method will include + // this call to |drawDisplayList| for non-nested op count metrics. + // But, for nested op count metrics we want the |drawDisplayList| call itself + // to be transparent. So we subtract 1 from our accumulated nested count to + // balance out against the 1 that was accumulated into the regular count. + // This behavior is identical to the way SkPicture computes nested op counts. + nested_op_count_ += display_list->op_count(true) - 1; nested_bytes_ += display_list->bytes(true); - nested_op_count_ += display_list->op_count(true); } void DisplayListBuilder::drawTextBlob(const sk_sp blob, SkScalar x, diff --git a/engine/src/flutter/flow/display_list_unittests.cc b/engine/src/flutter/flow/display_list_unittests.cc index 9e1409a8b1..8339a60c91 100644 --- a/engine/src/flutter/flow/display_list_unittests.cc +++ b/engine/src/flutter/flow/display_list_unittests.cc @@ -1072,5 +1072,50 @@ TEST(DisplayList, DisplayListSaveLayerBoundsWithAlphaFilter) { } } +TEST(DisplayList, NestedOpCountMetricsSameAsSkPicture) { + 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()); + + auto picture = outer_recorder.finishRecordingAsPicture(); + ASSERT_EQ(picture->approximateOpCount(), 1); + ASSERT_EQ(picture->approximateOpCount(true), 36); + + DisplayListBuilder builder(SkRect::MakeWH(150, 100)); + for (int y = 10; y <= 60; y += 10) { + for (int x = 10; x <= 60; x += 10) { + builder.setColor(((x + y) % 20) == 10 ? SK_ColorRED : SK_ColorBLUE); + builder.drawRect(SkRect::MakeXYWH(x, y, 80, 80)); + } + } + DisplayListBuilder outer_builder(SkRect::MakeWH(150, 100)); + outer_builder.drawDisplayList(builder.Build()); + + auto display_list = outer_builder.Build(); + ASSERT_EQ(display_list->op_count(), 1); + ASSERT_EQ(display_list->op_count(true), 36); + + ASSERT_EQ(picture->approximateOpCount(), display_list->op_count()); + ASSERT_EQ(picture->approximateOpCount(true), display_list->op_count(true)); + + DisplayListCanvasRecorder dl_recorder(SkRect::MakeWH(150, 100)); + picture->playback(&dl_recorder); + + auto sk_display_list = dl_recorder.Build(); + ASSERT_EQ(display_list->op_count(), 1); + ASSERT_EQ(display_list->op_count(true), 36); +} + } // namespace testing } // namespace flutter diff --git a/engine/src/flutter/flow/raster_cache.cc b/engine/src/flutter/flow/raster_cache.cc index f9a933edb7..c0ed5b22a7 100644 --- a/engine/src/flutter/flow/raster_cache.cc +++ b/engine/src/flutter/flow/raster_cache.cc @@ -83,7 +83,7 @@ static bool IsPictureWorthRasterizing(SkPicture* picture, // 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() > 5; + return picture->approximateOpCount(true) > 5; } static bool IsDisplayListWorthRasterizing(DisplayList* display_list, @@ -109,7 +109,7 @@ static bool IsDisplayListWorthRasterizing(DisplayList* display_list, // 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 display_list->op_count() > 5; + return display_list->op_count(true) > 5; } /// @note Procedure doesn't copy all closures. diff --git a/engine/src/flutter/flow/raster_cache_unittests.cc b/engine/src/flutter/flow/raster_cache_unittests.cc index 928a6da067..c253f073b7 100644 --- a/engine/src/flutter/flow/raster_cache_unittests.cc +++ b/engine/src/flutter/flow/raster_cache_unittests.cc @@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#include "flutter/flow/display_list.h" #include "flutter/flow/raster_cache.h" #include "flutter/flow/testing/mock_raster_cache.h" @@ -25,6 +26,44 @@ sk_sp GetSamplePicture() { return recorder.finishRecordingAsPicture(); } +sk_sp GetSampleDisplayList() { + DisplayListBuilder builder(SkRect::MakeWH(150, 100)); + builder.setColor(SK_ColorRED); + builder.drawRect(SkRect::MakeXYWH(10, 10, 80, 80)); + 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) { + for (int x = 10; x <= 60; x += 10) { + builder.setColor(((x + y) % 20) == 10 ? SK_ColorRED : SK_ColorBLUE); + builder.drawRect(SkRect::MakeXYWH(x, y, 80, 80)); + } + } + DisplayListBuilder outer_builder(SkRect::MakeWH(150, 100)); + outer_builder.drawDisplayList(builder.Build()); + return outer_builder.Build(); +} + } // namespace TEST(RasterCache, SimpleInitialization) { @@ -32,7 +71,7 @@ TEST(RasterCache, SimpleInitialization) { ASSERT_TRUE(true); } -TEST(RasterCache, ThresholdIsRespected) { +TEST(RasterCache, ThresholdIsRespectedForSkPicture) { size_t threshold = 2; flutter::RasterCache cache(threshold); @@ -65,7 +104,40 @@ TEST(RasterCache, ThresholdIsRespected) { ASSERT_TRUE(cache.Draw(*picture, dummy_canvas)); } -TEST(RasterCache, AccessThresholdOfZeroDisablesCaching) { +TEST(RasterCache, ThresholdIsRespectedForDisplayList) { + size_t threshold = 2; + flutter::RasterCache cache(threshold); + + SkMatrix matrix = SkMatrix::I(); + + auto display_list = GetSampleDisplayList(); + + SkCanvas dummy_canvas; + + PrerollContextHolder preroll_context_holder = GetSamplePrerollContextHolder(); + + ASSERT_FALSE(cache.Prepare(&preroll_context_holder.preroll_context, + display_list.get(), true, false, matrix)); + // 1st access. + ASSERT_FALSE(cache.Draw(*display_list, dummy_canvas)); + + cache.SweepAfterFrame(); + + ASSERT_FALSE(cache.Prepare(&preroll_context_holder.preroll_context, + display_list.get(), true, false, matrix)); + + // 2nd access. + ASSERT_FALSE(cache.Draw(*display_list, dummy_canvas)); + + cache.SweepAfterFrame(); + + // Now Prepare should cache it. + ASSERT_TRUE(cache.Prepare(&preroll_context_holder.preroll_context, + display_list.get(), true, false, matrix)); + ASSERT_TRUE(cache.Draw(*display_list, dummy_canvas)); +} + +TEST(RasterCache, AccessThresholdOfZeroDisablesCachingForSkPicture) { size_t threshold = 0; flutter::RasterCache cache(threshold); @@ -83,7 +155,25 @@ TEST(RasterCache, AccessThresholdOfZeroDisablesCaching) { ASSERT_FALSE(cache.Draw(*picture, dummy_canvas)); } -TEST(RasterCache, PictureCacheLimitPerFrameIsRespectedWhenZero) { +TEST(RasterCache, AccessThresholdOfZeroDisablesCachingForDisplayList) { + size_t threshold = 0; + flutter::RasterCache cache(threshold); + + SkMatrix matrix = SkMatrix::I(); + + auto display_list = GetSampleDisplayList(); + + SkCanvas dummy_canvas; + + PrerollContextHolder preroll_context_holder = GetSamplePrerollContextHolder(); + + ASSERT_FALSE(cache.Prepare(&preroll_context_holder.preroll_context, + display_list.get(), true, false, matrix)); + + 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); @@ -101,7 +191,25 @@ TEST(RasterCache, PictureCacheLimitPerFrameIsRespectedWhenZero) { ASSERT_FALSE(cache.Draw(*picture, dummy_canvas)); } -TEST(RasterCache, SweepsRemoveUnusedFrames) { +TEST(RasterCache, PictureCacheLimitPerFrameIsRespectedWhenZeroForDisplayList) { + size_t picture_cache_limit_per_frame = 0; + flutter::RasterCache cache(3, picture_cache_limit_per_frame); + + SkMatrix matrix = SkMatrix::I(); + + auto display_list = GetSampleDisplayList(); + + SkCanvas dummy_canvas; + + PrerollContextHolder preroll_context_holder = GetSamplePrerollContextHolder(); + + ASSERT_FALSE(cache.Prepare(&preroll_context_holder.preroll_context, + display_list.get(), true, false, matrix)); + + ASSERT_FALSE(cache.Draw(*display_list, dummy_canvas)); +} + +TEST(RasterCache, SweepsRemoveUnusedSkPictures) { size_t threshold = 1; flutter::RasterCache cache(threshold); @@ -129,10 +237,38 @@ TEST(RasterCache, SweepsRemoveUnusedFrames) { ASSERT_FALSE(cache.Draw(*picture, dummy_canvas)); } +TEST(RasterCache, SweepsRemoveUnusedDisplayLists) { + size_t threshold = 1; + flutter::RasterCache cache(threshold); + + SkMatrix matrix = SkMatrix::I(); + + auto display_list = GetSampleDisplayList(); + + SkCanvas dummy_canvas; + + PrerollContextHolder preroll_context_holder = GetSamplePrerollContextHolder(); + + ASSERT_FALSE(cache.Prepare(&preroll_context_holder.preroll_context, + display_list.get(), true, false, matrix)); // 1 + ASSERT_FALSE(cache.Draw(*display_list, dummy_canvas)); + + cache.SweepAfterFrame(); + + ASSERT_TRUE(cache.Prepare(&preroll_context_holder.preroll_context, + display_list.get(), true, false, matrix)); // 2 + ASSERT_TRUE(cache.Draw(*display_list, dummy_canvas)); + + cache.SweepAfterFrame(); + cache.SweepAfterFrame(); // Extra frame without a Get image access. + + 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, DeviceRectRoundOut) { +TEST(RasterCache, DeviceRectRoundOutForSkPicture) { size_t threshold = 1; flutter::RasterCache cache(threshold); @@ -154,7 +290,9 @@ TEST(RasterCache, DeviceRectRoundOut) { ASSERT_FALSE(cache.Prepare(&preroll_context_holder.preroll_context, picture.get(), true, false, ctm)); ASSERT_FALSE(cache.Draw(*picture, canvas)); + cache.SweepAfterFrame(); + ASSERT_TRUE(cache.Prepare(&preroll_context_holder.preroll_context, picture.get(), true, false, ctm)); ASSERT_TRUE(cache.Draw(*picture, canvas)); @@ -163,5 +301,90 @@ TEST(RasterCache, DeviceRectRoundOut) { ASSERT_TRUE(cache.Draw(*picture, 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, DeviceRectRoundOutForDisplayList) { + size_t threshold = 1; + flutter::RasterCache cache(threshold); + + SkRect logical_rect = SkRect::MakeLTRB(28, 0, 354.56731, 310.288); + DisplayListBuilder builder(logical_rect); + builder.setColor(SK_ColorRED); + builder.drawRect(logical_rect); + sk_sp display_list = builder.Build(); + + 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(); + + ASSERT_FALSE(cache.Prepare(&preroll_context_holder.preroll_context, + display_list.get(), true, false, ctm)); + ASSERT_FALSE(cache.Draw(*display_list, canvas)); + + cache.SweepAfterFrame(); + + ASSERT_TRUE(cache.Prepare(&preroll_context_holder.preroll_context, + display_list.get(), true, false, ctm)); + ASSERT_TRUE(cache.Draw(*display_list, canvas)); + + canvas.translate(248, 0); + 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(); + + ASSERT_FALSE(cache.Prepare(&preroll_context_holder.preroll_context, + picture.get(), false, false, matrix)); + ASSERT_FALSE(cache.Draw(*picture, dummy_canvas)); + + cache.SweepAfterFrame(); + + 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); + + SkMatrix matrix = SkMatrix::I(); + + auto display_list = GetSampleNestedDisplayList(); + ASSERT_EQ(display_list->op_count(), 1); + ASSERT_EQ(display_list->op_count(true), 36); + + SkCanvas dummy_canvas; + + PrerollContextHolder preroll_context_holder = GetSamplePrerollContextHolder(); + + ASSERT_FALSE(cache.Prepare(&preroll_context_holder.preroll_context, + display_list.get(), false, false, matrix)); + ASSERT_FALSE(cache.Draw(*display_list, dummy_canvas)); + + cache.SweepAfterFrame(); + + ASSERT_TRUE(cache.Prepare(&preroll_context_holder.preroll_context, + display_list.get(), false, false, matrix)); + ASSERT_TRUE(cache.Draw(*display_list, dummy_canvas)); +} + } // namespace testing + } // namespace flutter