Reland "Remove dependency of paint offset translation on compositing"

This reverts commit 19192c4. Reason for revert: Re-landing since broken test does not reproduce. Original change's description: > Revert "Remove dependency of paint offset translation on compositing" > > This reverts commit 6b69ef4. > > Reason for revert: https://ci.chromium.org/p/chromium/builders/ci/Mac10.13%20Tests%20%28dbg%29/19163 > > Original change's description: > > Remove dependency of paint offset translation on compositing > > > > Before this CL, some non-direct compositing reasons induced a > > paint offset translation for descendant reasons - in particular: > > CompositingReason::kComboCompositedDescendants > > CompositingReason::kNegativeZIndexChildren > > CompositingReason::kRoot > > > > and we depended on the computation of these reasons in > > CompositingRequirementsUpdater. > > > > With this CL, we only do so for kComboCompositedDescendants, and > > remove the dependency on CompositingRequirementsUpdater. > > > > Bug: 1100711 > > > > Change-Id: I8b9948cfae01b0973623aba84a1fdf84ae0a9611 > > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2276521 > > Commit-Queue: Chris Harrelson <chrishtr@chromium.org> > > Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org> > > Cr-Commit-Position: refs/heads/master@{#784695} > > TBR=wangxianzhu@chromium.org,chrishtr@chromium.org > > # Not skipping CQ checks because original CL landed > 1 day ago. > > Bug: 1100711,1101917 > Change-Id: I41164b5235575a5e56e14472b1dd2bf9d32da8fe > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2281106 > Reviewed-by: Stephen Chenney <schenney@chromium.org> > Commit-Queue: Stephen Chenney <schenney@chromium.org> > Cr-Commit-Position: refs/heads/master@{#785145} TBR=wangxianzhu@chromium.org,chrishtr@chromium.org,schenney@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 1100711, 1101917 Change-Id: Ic857f8dda802095ef3a4b1ad364ff2dc4c4566ef Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2283731 Reviewed-by: Chris Harrelson <chrishtr@chromium.org> Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org> Commit-Queue: Chris Harrelson <chrishtr@chromium.org> Cr-Commit-Position: refs/heads/master@{#785603}
chromium · Jul 7, 2020 · 8aa1953 · 8aa1953
1 parent 2f8e6f8
commit 8aa1953
Show file tree

Hide file tree

Showing 24 changed files with 217 additions and 230 deletions.
diff --git a/third_party/blink/renderer/core/paint/compositing/compositing_inputs_updater.cc b/third_party/blink/renderer/core/paint/compositing/compositing_inputs_updater.cc
@@ -229,6 +229,45 @@ void CompositingInputsUpdater::UpdateSelfAndDescendantsRecursively(
   }
 
   compositor->ClearCompositingInputsRoot();
+
+  DisableCompositingQueryAsserts disabler;
+
+  bool previously_needed_paint_offset_translation =
+      layer->NeedsPaintOffsetTranslationForCompositing();
+
+  layer->SetNeedsPaintOffsetTranslationForCompositing(
+      NeedsPaintOffsetTranslationForCompositing(layer));
+
+  // Invalidate if needed to affect NeedsPaintOffsetTranslation().
+  if (previously_needed_paint_offset_translation !=
+      layer->NeedsPaintOffsetTranslationForCompositing())
+    layout_object.SetNeedsPaintPropertyUpdate();
+}
+
+bool CompositingInputsUpdater::NeedsPaintOffsetTranslationForCompositing(
+    PaintLayer* layer) {
+  PaintLayerCompositor* compositor =
+      layer->GetLayoutObject().View()->Compositor();
+
+  /// Allocate when the developer indicated compositing via a direct
+  // method.
+  if ((compositor->CanBeComposited(layer) &&
+       layer->DirectCompositingReasons()) ||
+      layer->NeedsCompositedScrolling())
+    return true;
+
+  // Allocate when there is a need for a cc effect that applies to
+  // descendants.
+  // TODO(chrishtr): this should not be necessary, but currently at least
+  // cc mask layers don't apply correctly otherwise.
+  // compositing/clip-path-with-composited-descendants.html is one test
+  // that demonstrates this.
+  if ((layer->PotentialCompositingReasonsFromStyle() &
+       CompositingReason::kComboCompositedDescendants) &&
+      layer->DescendantHasDirectOrScrollingCompositingReason())
+    return true;
+
+  return false;
 }
 
 void CompositingInputsUpdater::UpdateAncestorInfo(PaintLayer* const layer,

diff --git a/third_party/blink/renderer/core/paint/compositing/compositing_inputs_updater.h b/third_party/blink/renderer/core/paint/compositing/compositing_inputs_updater.h
@@ -86,6 +86,8 @@ class CompositingInputsUpdater {
   // current value of AncestorInfo.
   void UpdateAncestorInfo(PaintLayer* const, UpdateType&, AncestorInfo&);
 
+  bool NeedsPaintOffsetTranslationForCompositing(PaintLayer*);
+
   LayoutGeometryMap geometry_map_;
   PaintLayer* root_layer_;
   PaintLayer* compositing_inputs_root_;

diff --git a/third_party/blink/renderer/core/paint/compositing/paint_layer_compositor.cc b/third_party/blink/renderer/core/paint/compositing/paint_layer_compositor.cc
@@ -459,10 +459,6 @@ bool PaintLayerCompositor::AllocateOrClearCompositedLayerMapping(
 
   layer->ClearClipRects(kPaintingClipRects);
 
-  // Compositing state affects whether to create paint offset translation of
-  // this layer, and amount of paint offset translation of descendants.
-  layer->GetLayoutObject().SetNeedsPaintPropertyUpdate();
-
   return true;
 }
 

diff --git a/third_party/blink/renderer/core/paint/paint_layer.cc b/third_party/blink/renderer/core/paint/paint_layer.cc
@@ -183,6 +183,7 @@ PaintLayer::PaintLayer(LayoutBoxModelObject& layout_object)
       needs_reorder_overlay_overflow_controls_(false),
       static_inline_edge_(InlineEdge::kInlineStart),
       static_block_edge_(BlockEdge::kBlockStart),
+      needs_paint_offset_translation_for_compositing_(false),
 #if DCHECK_IS_ON()
       layer_list_mutation_allowed_(true),
 #endif

diff --git a/third_party/blink/renderer/core/paint/paint_layer.h b/third_party/blink/renderer/core/paint/paint_layer.h
@@ -1121,6 +1121,16 @@ class CORE_EXPORT PaintLayer : public DisplayItemClient {
                       : PhysicalOffset();
   }
 
+  bool NeedsPaintOffsetTranslationForCompositing() const {
+    DCHECK(!RuntimeEnabledFeatures::CompositeAfterPaintEnabled());
+    DCHECK(IsAllowedToQueryCompositingState());
+    return needs_paint_offset_translation_for_compositing_;
+  }
+
+  void SetNeedsPaintOffsetTranslationForCompositing(bool b) {
+    needs_paint_offset_translation_for_compositing_ = b;
+  }
+
  private:
   void SetNeedsCompositingInputsUpdateInternal();
 
@@ -1368,6 +1378,8 @@ class CORE_EXPORT PaintLayer : public DisplayItemClient {
   unsigned static_inline_edge_ : 2;
   unsigned static_block_edge_ : 2;
 
+  unsigned needs_paint_offset_translation_for_compositing_ : 1;
+
 #if DCHECK_IS_ON()
   mutable unsigned layer_list_mutation_allowed_ : 1;
 #endif

diff --git a/third_party/blink/renderer/core/paint/paint_property_tree_builder.cc b/third_party/blink/renderer/core/paint/paint_property_tree_builder.cc
@@ -419,8 +419,8 @@ static bool NeedsPaintOffsetTranslation(
        object.HasReflection()))
     return true;
 
-  // Don't let paint offset cross composited layer boundaries, to avoid
-  // unnecessary full layer paint/raster invalidation when paint offset in
+  // Don't let paint offset cross composited layer boundaries when possible, to
+  // avoid unnecessary full layer paint/raster invalidation when paint offset in
   // ancestor transform node changes which should not affect the descendants
   // of the composited layer. For now because of crbug.com/780242, this is
   // limited to LayoutBlocks and LayoutReplaceds that won't be escaped by
@@ -442,11 +442,9 @@ static bool NeedsPaintOffsetTranslation(
             EBackfaceVisibility::kHidden)
           return true;
       } else {
-        if (layer->GetCompositingReasons() &
-            ~(CompositingReason::kComboSquashableReasons |
-              CompositingReason::kSquashingDisallowed)) {
+        if (layer->NeedsPaintOffsetTranslationForCompositing())
           return true;
-        }
+
         if (is_affected_by_outer_viewport_bounds_delta)
           return true;
       }

diff --git a/.../blink/web_tests/compositing/layer-creation/overlap-transformed-preserved-3d-expected.txt b/.../blink/web_tests/compositing/layer-creation/overlap-transformed-preserved-3d-expected.txt
@@ -15,9 +15,9 @@ Before:
     },
     {
       "name": "LayoutNGBlockFlow (positioned) DIV id='green-box' class='center box-container'",
+      "position": [108, 108],
       "contentsOpaque": true,
-      "drawsContent": false,
-      "transform": 1
+      "drawsContent": false
     },
     {
       "name": "LayoutNGBlockFlow DIV id='camera' class='rotate-3d-start'",
@@ -68,20 +68,21 @@ Before:
       "transform": [
         [1, 0, 0, 0],
         [0, 1, 0, 0],
-        [0, 0, 1, 0],
-        [108, 108, 0, 1]
-      ]
+        [0, 0, 1, -0.005],
+        [0, 0, 0, 1]
+      ],
+      "origin": [158, 158]
     },
     {
       "id": 2,
       "parent": 1,
       "transform": [
         [1, 0, 0, 0],
         [0, 1, 0, 0],
-        [0, 0, 1, -0.005],
-        [0, 0, 0, 1]
+        [0, 0, 1, 0],
+        [108, 108, 0, 1]
       ],
-      "origin": [50, 50]
+      "flattenInheritedTransform": false
     },
     {
       "id": 3,
@@ -223,9 +224,9 @@ After:
     },
     {
       "name": "LayoutNGBlockFlow (positioned) DIV id='green-box' class='center box-container'",
+      "position": [108, 108],
       "contentsOpaque": true,
-      "drawsContent": false,
-      "transform": 1
+      "drawsContent": false
     },
     {
       "name": "LayoutNGBlockFlow DIV id='camera' class='rotate-3d-start rotate-3d-end'",
@@ -288,20 +289,21 @@ After:
       "transform": [
         [1, 0, 0, 0],
         [0, 1, 0, 0],
-        [0, 0, 1, 0],
-        [108, 108, 0, 1]
-      ]
+        [0, 0, 1, -0.005],
+        [0, 0, 0, 1]
+      ],
+      "origin": [158, 158]
     },
     {
       "id": 2,
       "parent": 1,
       "transform": [
         [1, 0, 0, 0],
         [0, 1, 0, 0],
-        [0, 0, 1, -0.005],
-        [0, 0, 0, 1]
+        [0, 0, 1, 0],
+        [108, 108, 0, 1]
       ],
-      "origin": [50, 50]
+      "flattenInheritedTransform": false
     },
     {
       "id": 3,

diff --git a/third_party/blink/web_tests/compositing/squashing/squashing-inside-perspective-expected.txt b/third_party/blink/web_tests/compositing/squashing/squashing-inside-perspective-expected.txt
@@ -8,9 +8,9 @@
     },
     {
       "name": "LayoutNGBlockFlow (positioned) DIV",
+      "position": [8, 8],
       "contentsOpaque": true,
-      "drawsContent": false,
-      "transform": 1
+      "drawsContent": false
     },
     {
       "name": "LayoutNGBlockFlow (positioned) DIV",
@@ -26,20 +26,21 @@
       "transform": [
         [1, 0, 0, 0],
         [0, 1, 0, 0],
-        [0, 0, 1, 0],
-        [8, 8, 0, 1]
-      ]
+        [0, 0, 1, -0.001],
+        [0, 0, 0, 1]
+      ],
+      "origin": [8, 8]
     },
     {
       "id": 2,
       "parent": 1,
       "transform": [
         [1, 0, 0, 0],
         [0, 1, 0, 0],
-        [0, 0, 1, -0.001],
-        [0, 0, 0, 1]
+        [0, 0, 1, 0],
+        [8, 8, 0, 1]
       ],
-      "origin": [0, 0]
+      "flattenInheritedTransform": false
     },
     {
       "id": 3,

diff --git a/...arty/blink/web_tests/css3/blending/mix-blend-mode-composited-reason-children-expected.txt b/...arty/blink/web_tests/css3/blending/mix-blend-mode-composited-reason-children-expected.txt
@@ -6,17 +6,17 @@
     },
     {
       "name": "LayoutNGBlockFlow (positioned) DIV class='stacking-context'",
+      "position": [8, 8],
       "contentsOpaque": true,
       "drawsContent": false,
-      "backgroundColor": "#FF0000",
-      "transform": 1
+      "backgroundColor": "#FF0000"
     },
     {
       "name": "LayoutNGBlockFlow (positioned) DIV class='stacking-context'",
+      "position": [8, 8],
       "bounds": [160, 90],
       "contentsOpaque": true,
-      "backgroundColor": "#008000",
-      "transform": 1
+      "backgroundColor": "#008000"
     },
     {
       "name": "LayoutNGBlockFlow DIV class='blended'",
@@ -31,8 +31,8 @@
     },
     {
       "name": "LayoutNGBlockFlow (positioned) DIV class='stacking-context' (foreground) Layer",
-      "drawsContent": false,
-      "transform": 1
+      "position": [8, 8],
+      "drawsContent": false
     },
     {
       "name": "LayoutNGBlockFlow HTML (foreground) Layer",

diff --git a/...y/blink/web_tests/css3/blending/mix-blend-mode-isolation-2-stacking-contexts-expected.txt b/...y/blink/web_tests/css3/blending/mix-blend-mode-isolation-2-stacking-contexts-expected.txt
@@ -6,17 +6,17 @@
     },
     {
       "name": "LayoutNGBlockFlow (positioned) DIV class='stacking-context'",
+      "position": [8, 8],
       "contentsOpaque": true,
       "drawsContent": false,
-      "backgroundColor": "#FF0000",
-      "transform": 1
+      "backgroundColor": "#FF0000"
     },
     {
       "name": "LayoutNGBlockFlow (positioned) DIV class='stacking-context'",
+      "position": [8, 8],
       "bounds": [160, 90],
       "contentsOpaque": true,
-      "backgroundColor": "#008000",
-      "transform": 1
+      "backgroundColor": "#008000"
     },
     {
       "name": "LayoutImage IMG class='accelerated blended'",
@@ -25,8 +25,8 @@
     },
     {
       "name": "LayoutNGBlockFlow (positioned) DIV class='stacking-context' (foreground) Layer",
-      "drawsContent": false,
-      "transform": 1
+      "position": [8, 8],
+      "drawsContent": false
     },
     {
       "name": "LayoutNGBlockFlow (positioned) DIV class='accelerated stacking-context'",

diff --git a/third_party/blink/web_tests/css3/blending/mix-blend-mode-isolation-layer-expected.txt b/third_party/blink/web_tests/css3/blending/mix-blend-mode-isolation-layer-expected.txt
@@ -6,10 +6,10 @@
     },
     {
       "name": "LayoutNGBlockFlow (positioned) DIV class='stacking-context'",
+      "position": [8, 8],
       "bounds": [160, 90],
       "contentsOpaque": true,
-      "backgroundColor": "#008000",
-      "transform": 1
+      "backgroundColor": "#008000"
     },
     {
       "name": "LayoutImage IMG class='accelerated blended'",