[CompositeAfterPaint] Allow non-atomic effects in multicol

This is not fully correct, but we have to accept it in our current
multicol painting based on FragmentClips.

Actually we have already allowed non-atomic effects in multicol within
composited layers since SlimmingPaintV175. A fragmented element with
effect applies the effect on each fragment separately. We have to do
this because each fragment has its own fragment clip, and its own paint
properties because they may depend on the fragment clip.

This CL enables non-atomic composited effects for CompositeAfterPaint.
This is not an issue in pre-CompositeAfterPaint which doesn't allow
fragmentation of composited layers (which is incorrect).

The root issue will be fully fixed by LayoutNG block fragmentation
which doesn't need FragmentClips.

Bug: 1064341
Change-Id: Id6ae4c263381f24d037f3c2bc493d73cd050598e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2124340
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#754166}

third_party/blink/renderer/core/paint/paint_property_tree_builder.cc

@@ -251,6 +251,12 @@ class FragmentPaintPropertyTreeBuilder {
     full_context_.clip_changed |= cleared;
   }
 
+  CompositorElementId GetCompositorElementId(
+      CompositorElementIdNamespace namespace_id) const {
+    return CompositorElementIdFromUniqueObjectId(fragment_data_.UniqueId(),
+                                                 namespace_id);
+  }
+
   const LayoutObject& object_;
   // The tree builder context for the whole object.
   PaintPropertyTreeBuilderContext& full_context_;
@@ -535,8 +541,7 @@ void FragmentPaintPropertyTreeBuilder::UpdateStickyTranslation() {
       const auto& box_model = ToLayoutBoxModelObject(object_);
       TransformPaintPropertyNode::State state{
           FloatSize(box_model.StickyPositionOffset())};
-      state.compositor_element_id = CompositorElementIdFromUniqueObjectId(
-          box_model.UniqueId(),
+      state.compositor_element_id = GetCompositorElementId(
           CompositorElementIdNamespace::kStickyTranslation);
 
       auto* layer = box_model.Layer();
@@ -802,8 +807,8 @@ void FragmentPaintPropertyTreeBuilder::UpdateTransform() {
           object_.HasHiddenBackface()
               ? TransformPaintPropertyNode::BackfaceVisibility::kHidden
               : TransformPaintPropertyNode::BackfaceVisibility::kVisible;
-      state.compositor_element_id = CompositorElementIdFromUniqueObjectId(
-          object_.UniqueId(), CompositorElementIdNamespace::kPrimaryTransform);
+      state.compositor_element_id = GetCompositorElementId(
+          CompositorElementIdNamespace::kPrimaryTransform);
 
       TransformPaintPropertyNode::AnimationState animation_state;
       animation_state.is_running_animation_on_compositor =
@@ -1046,8 +1051,8 @@ void FragmentPaintPropertyTreeBuilder::UpdateEffect() {
 
       CompositorElementId mask_compositor_element_id;
       if (mask_clip || has_spv1_composited_clip_path) {
-        mask_compositor_element_id = CompositorElementIdFromUniqueObjectId(
-            object_.UniqueId(), CompositorElementIdNamespace::kEffectMask);
+        mask_compositor_element_id =
+            GetCompositorElementId(CompositorElementIdNamespace::kEffectMask);
       }
 
       EffectPaintPropertyNode::State state;
@@ -1084,15 +1089,15 @@ void FragmentPaintPropertyTreeBuilder::UpdateEffect() {
           full_context_.direct_compositing_reasons &
           CompositingReasonsForEffectProperty();
       if (state.direct_compositing_reasons) {
-        state.compositor_element_id = CompositorElementIdFromUniqueObjectId(
-            object_.UniqueId(), CompositorElementIdNamespace::kPrimaryEffect);
+        state.compositor_element_id = GetCompositorElementId(
+            CompositorElementIdNamespace::kPrimaryEffect);
       } else {
         // The effect node CompositorElementId is used to uniquely identify
         // renderpasses so even if we don't need one for animations we still
         // need to set an id. Using kPrimary avoids confusing cc::Animation
         // into thinking the element has been composited for animations.
-        state.compositor_element_id = CompositorElementIdFromUniqueObjectId(
-            object_.UniqueId(), CompositorElementIdNamespace::kPrimary);
+        state.compositor_element_id =
+            GetCompositorElementId(CompositorElementIdNamespace::kPrimary);
       }
 
       // TODO(crbug.com/900241): Remove these setters when we can use
@@ -1149,9 +1154,7 @@ void FragmentPaintPropertyTreeBuilder::UpdateEffect() {
         clip_path_state.local_transform_space = context_.current.transform;
         clip_path_state.output_clip = output_clip;
         clip_path_state.blend_mode = SkBlendMode::kDstIn;
-        clip_path_state.compositor_element_id =
-            CompositorElementIdFromUniqueObjectId(
-                object_.UniqueId(),
+        clip_path_state.compositor_element_id = GetCompositorElementId(
             CompositorElementIdNamespace::kEffectClipPath);
         OnUpdate(
             properties_->UpdateClipPath(parent, std::move(clip_path_state)));
@@ -1266,8 +1269,8 @@ void FragmentPaintPropertyTreeBuilder::UpdateFilter() {
       state.direct_compositing_reasons =
           full_context_.direct_compositing_reasons &
           CompositingReasonsForFilterProperty();
-      state.compositor_element_id = CompositorElementIdFromUniqueObjectId(
-          object_.UniqueId(), CompositorElementIdNamespace::kEffectFilter);
+      state.compositor_element_id =
+          GetCompositorElementId(CompositorElementIdNamespace::kEffectFilter);
 
       // TODO(crbug.com/900241): Remove the setter when we can use
       // state.direct_compositing_reasons to check for active animations.

third_party/blink/renderer/platform/graphics/compositing/property_tree_manager.cc

@@ -1049,11 +1049,20 @@ void PropertyTreeManager::BuildEffectNodesRecursively(
   BuildEffectNodesRecursively(*next_effect.Parent());
   DCHECK_EQ(&next_effect.Parent()->Unalias(), current_.effect);
 
-#if DCHECK_IS_ON()
-  DCHECK(!GetEffectTree().Node(next_effect.CcNodeId(new_sequence_number_)))
-      << "Malformed paint artifact. Paint chunks under the same effect should "
-         "be contiguous.";
-#endif
+  bool has_multiple_groups = false;
+  if (GetEffectTree().Node(next_effect.CcNodeId(new_sequence_number_))) {
+    if (RuntimeEnabledFeatures::CompositeAfterPaintEnabled()) {
+      // TODO(crbug.com/1064341): We have to allow one blink effect node to
+      // apply to multiple groups in block fragments (multicol, etc.) due to
+      // the current FragmentClip implementation. This can only be fixed by
+      // LayoutNG block fragments. For now we'll create multiple cc effect
+      // nodes in the case.
+      has_multiple_groups = true;
+    } else {
+      NOTREACHED() << "Malformed paint artifact. Paint chunks under the same"
+                      " effect should be contiguous.";
+    }
+  }
 
   auto backdrop_effect_state = kNoBackdropEffect;
   int output_clip_id = 0;
@@ -1080,6 +1089,8 @@ void PropertyTreeManager::BuildEffectNodesRecursively(
   int effect_node_id =
       GetEffectTree().Insert(cc::EffectNode(), current_.effect_id);
   auto& effect_node = *GetEffectTree().Node(effect_node_id);
+
+  if (!has_multiple_groups)
     next_effect.SetCcNodeId(new_sequence_number_, effect_node_id);
 
   PopulateCcEffectNode(effect_node, next_effect, output_clip_id,
@@ -1087,7 +1098,7 @@ void PropertyTreeManager::BuildEffectNodesRecursively(
 
   CompositorElementId compositor_element_id =
       next_effect.GetCompositorElementId();
-  if (compositor_element_id) {
+  if (compositor_element_id && !has_multiple_groups) {
     DCHECK(!property_trees_.element_id_to_effect_node_index.contains(
         compositor_element_id));
     property_trees_.element_id_to_effect_node_index[compositor_element_id] =

third_party/blink/web_tests/FlagExpectations/composite-after-paint

@@ -105,16 +105,8 @@ http/tests/devtools/layers/layer-compositing-reasons.js [ Failure ]
 # Missing WheelEventHandler
 http/tests/devtools/layers/layer-scroll-rects-get.js [ Failure ]
 
-# Crash on non-contiguous effect on multiple columns
-crbug.com/1064341 fast/multicol/composited-layer-will-change.html [ Crash ]
-crbug.com/1064341 paint/clipath/change-mask-clip-path-multicol-crash.html [ Crash ]
-crbug.com/1064341 fast/multicol/composited-layer-multiple-fragments-translated.html [ Crash ]
-crbug.com/1064341 fast/multicol/composited-layer-multiple-fragments.html [ Crash ]
-crbug.com/1064341 fast/multicol/composited-layer-nested.html [ Crash ]
-
-# Outline paints incorrectly with columns. For now this is hidden by crbug.com/1064341.
-# crbug.com/1047358 paint/pagination/composited-paginated-outlined-box.html [ Failure ]
-crbug.com/1064341 paint/pagination/composited-paginated-outlined-box.html [ Crash ]
+# Outline paints incorrectly with columns.
+crbug.com/1047358 paint/pagination/composited-paginated-outlined-box.html [ Failure ]
 
 # Crash on weird clip hierarchy in multiple columns
 compositing/geometry/composited-in-columns.html [ Crash ]