DisplayList forward opacity incompatibility through deferred saves (#…

…53078) The previous reorg of the DisplayListBuilder save/restore code placed the code that forwards layer flags inside the code that processes a non-deferred save call, but it needs to be processed regardless of the deferred save.
flutter · May 29, 2024 · 21f1d14 · 21f1d14
1 parent f90aa52
commit 21f1d14
Show file tree

Hide file tree

Showing 3 changed files with 49 additions and 23 deletions.
diff --git a/display_list/display_list_unittests.cc b/display_list/display_list_unittests.cc
@@ -4047,5 +4047,30 @@ TEST_F(DisplayListTest, TransformingFilterSaveLayerFloodedContentBounds) {
   EXPECT_EQ(dl->bounds(), SkRect::MakeLTRB(100.0f, 100.0f, 200.0f, 200.0f));
 }
 
+TEST_F(DisplayListTest, OpacityIncompatibleRenderOpInsideDeferredSave) {
+  {
+    // Without deferred save
+    DisplayListBuilder builder;
+    builder.DrawRect(SkRect::MakeLTRB(0, 0, 10, 10),
+                     DlPaint().setBlendMode(DlBlendMode::kClear));
+    EXPECT_FALSE(builder.Build()->can_apply_group_opacity());
+  }
+
+  {
+    // With deferred save
+    DisplayListBuilder builder;
+    builder.Save();
+    {
+      // Nothing to trigger the deferred save...
+      builder.DrawRect(SkRect::MakeLTRB(0, 0, 10, 10),
+                       DlPaint().setBlendMode(DlBlendMode::kClear));
+    }
+    // Deferred save was not triggered, did it forward the incompatibility
+    // flags?
+    builder.Restore();
+    EXPECT_FALSE(builder.Build()->can_apply_group_opacity());
+  }
+}
+
 }  // namespace testing
 }  // namespace flutter
diff --git a/display_list/dl_builder.cc b/display_list/dl_builder.cc
@@ -610,30 +610,32 @@ void DisplayListBuilder::Restore() {
 
       op->restore_index = op_index_;
       op->total_content_depth = depth_ - current_info.save_depth;
+    }
 
-      if (current_info.is_save_layer) {
-        RestoreLayer(current_info, parent_info(), op);
-      } else {
-        // No need to propagate bounds as we do with layers...
+    if (current_info.is_save_layer) {
+      RestoreLayer(current_info, parent_info());
+    } else {
+      // No need to propagate bounds as we do with layers...
 
-        // global accumulator is either the same object or both nullptr
-        FML_DCHECK(current_info.global_space_accumulator.get() ==
-                   parent_info().global_space_accumulator.get());
+      // global accumulator is either the same object or both nullptr
+      FML_DCHECK(current_info.global_space_accumulator.get() ==
+                 parent_info().global_space_accumulator.get());
 
-        // layer accumulators are both the same object
-        FML_DCHECK(current_info.layer_local_accumulator.get() ==
-                   parent_info().layer_local_accumulator.get());
-        FML_DCHECK(current_info.layer_local_accumulator.get() != nullptr);
+      // layer accumulators are both the same object
+      FML_DCHECK(current_info.layer_local_accumulator.get() ==
+                 parent_info().layer_local_accumulator.get());
+      FML_DCHECK(current_info.layer_local_accumulator.get() != nullptr);
 
-        // We only propagate these values through a regular save()
-        if (current_info.opacity_incompatible_op_detected) {
-          parent_info().opacity_incompatible_op_detected = true;
-        }
+      // We only propagate these values through a regular save()
+      if (current_info.opacity_incompatible_op_detected) {
+        parent_info().opacity_incompatible_op_detected = true;
       }
+    }
 
-      // Wait until all outgoing bounds information for the saveLayer is
-      // recorded before pushing the record to the buffer so that any rtree
-      // bounds will be attributed to the op_index of the restore op.
+    // Wait until all outgoing bounds information for the saveLayer is
+    // recorded before pushing the record to the buffer so that any rtree
+    // bounds will be attributed to the op_index of the restore op.
+    if (!current_info.has_deferred_save_op) {
       Push<RestoreOp>(0);
     } else {
       FML_DCHECK(!current_info.is_save_layer);
@@ -644,8 +646,7 @@ void DisplayListBuilder::Restore() {
 }
 
 void DisplayListBuilder::RestoreLayer(const SaveInfo& current_info,
-                                      SaveInfo& parent_info,
-                                      void* base_op) {
+                                      SaveInfo& parent_info) {
   FML_DCHECK(save_stack_.size() > 1);
   FML_DCHECK(!current_info.has_deferred_save_op);
 
@@ -656,7 +657,8 @@ void DisplayListBuilder::RestoreLayer(const SaveInfo& current_info,
 
   SkRect content_bounds = current_info.layer_local_accumulator->bounds();
 
-  SaveLayerOpBase* layer_op = reinterpret_cast<SaveLayerOpBase*>(base_op);
+  SaveLayerOpBase* layer_op = reinterpret_cast<SaveLayerOpBase*>(
+      storage_.get() + current_info.save_offset);
   FML_DCHECK(layer_op->type == DisplayListOpType::kSaveLayer ||
              layer_op->type == DisplayListOpType::kSaveLayerBackdrop);
 

diff --git a/display_list/dl_builder.h b/display_list/dl_builder.h
@@ -665,8 +665,7 @@ class DisplayListBuilder final : public virtual DlCanvas,
   }
 
   void RestoreLayer(const SaveInfo& current_info,
-                    SaveInfo& parent_info,
-                    void* base_op);
+                    SaveInfo& parent_info);
   void TransferLayerBounds(const SaveInfo& current_info,
                            SaveInfo& parent_info,
                            const SkRect& content_bounds);