Implement rational overflow clipping in SPv2

third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp

Index: third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp
diff --git a/third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp b/third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp
index 4430b0150fea03eddcb2864a791bd0fb96c5f401..e3076b4029788e45f8953e3328081ceb0f63210a 100644
--- a/third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp
+++ b/third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp
@@ -583,12 +583,23 @@ void PaintPropertyTreeBuilder::UpdateEffect(
         output_clip = properties.MaskClip();
 
         // TODO(crbug.com/683425): PaintArtifactCompositor does not handle
-        // grouping (i.e. descendant-dependent compositing reason) properly
-        // yet. This forces masked subtree always create a layer for now.
+        // grouping (i.e. descendant-dependent compositing reason) properly yet.
+        // This forces masked subtree always create a layer for now.
         compositing_reasons |= kCompositingReasonIsolateCompositedDescendants;

[trchen, 2017/05/03 21:13:22]

(Unrelated) I think this no longer applies. I shall try remove this hack in a
separate CL.

       } else {
         if (auto* properties = object.GetMutableForPainting().PaintProperties())
           force_subtree_update |= properties->ClearMaskClip();
+
+        bool has_filter = object.IsBoxModelObject() &&
+                          object.StyleRef().HasFilter() &&
+                          ToLayoutBoxModelObject(object).Layer();
+        if (!has_filter) {
+          // [fx-containment, step 1]: push clip of any contained content below
+          // the clip of an effect that commutes with clips.
+          // context.fixed_position.clip is the "highest" in the clip tree that
+          // such a contained clip could be.
+          output_clip = context.fixed_position.clip;

[trchen, 2017/05/03 21:13:22]

fixed_position.clip is not necessarily shallower than in-flow clip, due to CSS
clip. Why is this needed though? The default from line #560 is as shallow as you
could get.

It may be beneficial to push it further down, e.g. allocating a smaller buffer
with canvas->saveLayer, but I think PaintChunksToCcLayer is in a better position
for it (for composited effects, cc already does it when computing render surface
bounds).

[chrishtr, 2017/05/03 21:20:59]

It's required to implement fx-containment, step 1 I think - no clips may escape
effects.

I don't get why fixed_position.clip might be deeper than in-flow clip. Can you
post an example?

+        }
       }
 
       SkBlendMode blend_mode =
@@ -717,6 +728,9 @@ void PaintPropertyTreeBuilder::UpdateFilter(
     // TODO(trchen): Change input clip to expansion hint once implemented.
     const ClipPaintPropertyNode* input_clip =
         properties->Filter()->OutputClip();
+    // [fx-containment, step 1]: clips may not escape effects which
+    // move pixels. This only needs to happen if HasFilterThatMovesPixels(),
+    // but we do it for all filters to match SPv1 heuristics (see above).
     context.input_clip_of_current_effect = context.current.clip =
         context.absolute_position.clip = context.fixed_position.clip =
             input_clip;