use nested op counts to determine picture complexity for raster cache (flutter/engine#29265)

engine/src/flutter/flow/display_list.cc

@@ -1427,14 +1427,26 @@ void DisplayListBuilder::drawPicture(const sk_sp<SkPicture> picture,
       ? Push<DrawSkPictureMatrixOp>(0, 1, std::move(picture), *matrix,
                                     render_with_attributes)
       : Push<DrawSkPictureOp>(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<DisplayList> display_list) {
   Push<DrawDisplayListOp>(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<SkTextBlob> blob,
                                       SkScalar x,

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

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.

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<SkPicture> GetSamplePicture() {
   return recorder.finishRecordingAsPicture();
 }
 
+sk_sp<DisplayList> GetSampleDisplayList() {
+  DisplayListBuilder builder(SkRect::MakeWH(150, 100));
+  builder.setColor(SK_ColorRED);
+  builder.drawRect(SkRect::MakeXYWH(10, 10, 80, 80));
+  return builder.Build();
+}
+
+sk_sp<SkPicture> 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<DisplayList> 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<DisplayList> 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