Cached DisplayList opacity inheritance fix (#39690)

* only indicate opacity inheritance when DL is actually cached

* unit test

* use CacheInfo struct to simplify return values

Author: flar

display_list/display_list.cc

@@ -186,16 +186,15 @@ static bool CompareOps(uint8_t* ptrA,
   return true;
 }
 
-void DisplayList::RenderTo(DisplayListBuilder* builder,
-                           SkScalar opacity) const {
-  // TODO(100983): Opacity is not respected and attributes are not reset.
+void DisplayList::RenderTo(DisplayListBuilder* builder) const {
   if (!builder) {
     return;
   }
   Dispatch(*builder);
 }
 
 void DisplayList::RenderTo(SkCanvas* canvas, SkScalar opacity) const {
+  FML_DCHECK(can_apply_group_opacity() || opacity >= SK_Scalar1);
   DisplayListCanvasDispatcher dispatcher(canvas, opacity);
   Dispatch(dispatcher);
 }

display_list/display_list.h

@@ -229,8 +229,7 @@ class DisplayList : public SkRefCnt {
     Dispatch(ctx, ptr, ptr + byte_count_);
   }
 
-  void RenderTo(DisplayListBuilder* builder,
-                SkScalar opacity = SK_Scalar1) const;
+  void RenderTo(DisplayListBuilder* builder) const;
 
   void RenderTo(SkCanvas* canvas, SkScalar opacity = SK_Scalar1) const;
 

flow/layers/display_list_layer_unittests.cc

@@ -7,6 +7,7 @@
 #include "flutter/flow/layers/display_list_layer.h"
 
 #include "flutter/display_list/display_list_builder.h"
+#include "flutter/flow/layers/layer_tree.h"
 #include "flutter/flow/testing/diff_context_test.h"
 #include "flutter/flow/testing/skia_gpu_object_layer_test.h"
 #include "flutter/fml/macros.h"
@@ -249,6 +250,8 @@ TEST_F(DisplayListLayerTest, CachedIncompatibleDisplayListOpacityInheritance) {
   display_list_layer->Preroll(preroll_context());
   display_list_layer->Preroll(preroll_context());
   display_list_layer->Preroll(preroll_context());
+  LayerTree::TryToRasterCache(*preroll_context()->raster_cached_entries,
+                              &paint_context(), false);
 
   int opacity_alpha = 0x7F;
   SkPoint opacity_offset = SkPoint::Make(10, 10);
@@ -516,5 +519,69 @@ TEST_F(DisplayListLayerTest, DisplayListAccessCountDependsOnVisibility) {
   ASSERT_TRUE(raster_cache_item->Draw(paint_context(), nullptr));
 }
 
+TEST_F(DisplayListLayerTest, OverflowCachedDisplayListOpacityInheritance) {
+  use_mock_raster_cache();
+  PrerollContext* context = preroll_context();
+  int per_frame =
+      RasterCacheUtil::kDefaultPictureAndDispLayListCacheLimitPerFrame;
+  int layer_count = per_frame + 1;
+  SkPoint opacity_offset = {10, 10};
+  auto opacity_layer = std::make_shared<OpacityLayer>(0.5f, opacity_offset);
+  std::shared_ptr<DisplayListLayer> layers[layer_count];
+  for (int i = 0; i < layer_count; i++) {
+    DisplayListBuilder builder(false);
+    builder.drawRect({0, 0, 100, 100});
+    builder.drawRect({50, 50, 100, 100});
+    auto display_list = builder.Build();
+    ASSERT_FALSE(display_list->can_apply_group_opacity());
+    SkPoint offset = {i * 200.0f, 0};
+
+    layers[i] = std::make_shared<DisplayListLayer>(
+        offset, SkiaGPUObject(display_list, unref_queue()), true, false);
+    opacity_layer->Add(layers[i]);
+  }
+  for (size_t j = 0; j < context->raster_cache->access_threshold(); j++) {
+    context->raster_cache->BeginFrame();
+    for (int i = 0; i < layer_count; i++) {
+      context->renderable_state_flags = 0;
+      layers[i]->Preroll(context);
+      ASSERT_EQ(context->renderable_state_flags, 0) << "pass " << (j + 1);
+    }
+  }
+  opacity_layer->Preroll(context);
+  ASSERT_FALSE(opacity_layer->children_can_accept_opacity());
+  LayerTree::TryToRasterCache(*context->raster_cached_entries, &paint_context(),
+                              false);
+  context->raster_cached_entries->clear();
+  context->raster_cache->BeginFrame();
+  for (int i = 0; i < per_frame; i++) {
+    context->renderable_state_flags = 0;
+    layers[i]->Preroll(context);
+    ASSERT_EQ(context->renderable_state_flags,
+              LayerStateStack::kCallerCanApplyOpacity)
+        << "layer " << (i + 1);
+  }
+  for (int i = per_frame; i < layer_count; i++) {
+    context->renderable_state_flags = 0;
+    layers[i]->Preroll(context);
+    ASSERT_EQ(context->renderable_state_flags, 0) << "layer " << (i + 1);
+  }
+  opacity_layer->Preroll(context);
+  ASSERT_FALSE(opacity_layer->children_can_accept_opacity());
+  LayerTree::TryToRasterCache(*context->raster_cached_entries, &paint_context(),
+                              false);
+  context->raster_cached_entries->clear();
+  context->raster_cache->BeginFrame();
+  for (int i = 0; i < layer_count; i++) {
+    context->renderable_state_flags = 0;
+    layers[i]->Preroll(context);
+    ASSERT_EQ(context->renderable_state_flags,
+              LayerStateStack::kCallerCanApplyOpacity)
+        << "layer " << (i + 1);
+  }
+  opacity_layer->Preroll(context);
+  ASSERT_TRUE(opacity_layer->children_can_accept_opacity());
+}
+
 }  // namespace testing
 }  // namespace flutter

flow/layers/display_list_raster_cache_item.cc

@@ -109,11 +109,16 @@ void DisplayListRasterCacheItem::PrerollFinalize(PrerollContext* context,
   // it great than the threshold. Otherwise we only increase the entry
   // access_count.
   bool visible = !context->state_stack.content_culled(bounds);
-  int accesses = raster_cache->MarkSeen(key_id_, matrix, visible);
-  if (!visible || accesses <= raster_cache->access_threshold()) {
+  RasterCache::CacheInfo cache_info =
+      raster_cache->MarkSeen(key_id_, matrix, visible);
+  if (!visible ||
+      cache_info.accesses_since_visible <= raster_cache->access_threshold()) {
     cache_state_ = kNone;
   } else {
-    context->renderable_state_flags |= LayerStateStack::kCallerCanApplyOpacity;
+    if (cache_info.has_image) {
+      context->renderable_state_flags |=
+          LayerStateStack::kCallerCanApplyOpacity;
+    }
     cache_state_ = kCurrent;
   }
   return;

flow/raster_cache.cc

@@ -109,17 +109,17 @@ bool RasterCache::UpdateCacheEntry(
   return entry.image != nullptr;
 }
 
-int RasterCache::MarkSeen(const RasterCacheKeyID& id,
-                          const SkMatrix& matrix,
-                          bool visible) const {
+RasterCache::CacheInfo RasterCache::MarkSeen(const RasterCacheKeyID& id,
+                                             const SkMatrix& matrix,
+                                             bool visible) const {
   RasterCacheKey key = RasterCacheKey(id, matrix);
   Entry& entry = cache_[key];
   entry.encountered_this_frame = true;
   entry.visible_this_frame = visible;
   if (visible || entry.accesses_since_visible > 0) {
     entry.accesses_since_visible++;
   }
-  return entry.accesses_since_visible;
+  return {entry.accesses_since_visible, entry.image != nullptr};
 }
 
 int RasterCache::GetAccessCount(const RasterCacheKeyID& id,

flow/raster_cache.h

@@ -121,6 +121,10 @@ class RasterCache {
     const SkRect& logical_rect;
     const char* flow_type;
   };
+  struct CacheInfo {
+    const size_t accesses_since_visible;
+    const bool has_image;
+  };
 
   std::unique_ptr<RasterCacheResult> Rasterize(
       const RasterCache::Context& context,
@@ -203,7 +207,7 @@ class RasterCache {
    * If the number is one, then it must be prepared and drawn on 1 frame
    * and it will then be cached on the next frame if it is prepared.
    */
-  int access_threshold() const { return access_threshold_; }
+  size_t access_threshold() const { return access_threshold_; }
 
   bool GenerateNewCacheInThisFrame() const {
     // Disabling caching when access_threshold is zero is historic behavior.
@@ -221,9 +225,9 @@ class RasterCache {
    * @return the number of times the entry has been hit since it was created.
    * For a new entry that will be 1 if it is visible, or zero if non-visible.
    */
-  int MarkSeen(const RasterCacheKeyID& id,
-               const SkMatrix& matrix,
-               bool visible) const;
+  CacheInfo MarkSeen(const RasterCacheKeyID& id,
+                     const SkMatrix& matrix,
+                     bool visible) const;
 
   /**
    * Returns the access count (i.e. accesses_since_visible) for the given

flow/testing/mock_raster_cache.cc

@@ -65,7 +65,7 @@ void MockRasterCache::AddMockPicture(int width, int height) {
 
   DisplayListRasterCacheItem display_list_item(display_list.get(), SkPoint(),
                                                true, false);
-  for (int i = 0; i < access_threshold(); i++) {
+  for (size_t i = 0; i < access_threshold(); i++) {
     AutoCache(&display_list_item, &preroll_context_, ctm);
   }
   RasterCache::Context r_context = {