Changeset 266901 in webkit

Timestamp:

Sep 10, 2020 5:32:46 PM (4 years ago)

Author:

Alan Bujtas

Message:

[Repaint] RenderLayerModelObject::styleWillChange may issue redundant repaint
​https://bugs.webkit.org/show_bug.cgi?id=216374
<rdar://problem/68657490>

Reviewed by Simon Fraser.

Source/WebCore:

Move the repaintIncludingDescendants() calls to repaintBeforeStyleChange() to avoid redundant repaints on the same renderer.

• rendering/RenderElement.cpp:

(WebCore::RenderElement::repaintBeforeStyleChange):

• rendering/RenderLayerModelObject.cpp:

(WebCore::RenderLayerModelObject::styleWillChange):

LayoutTests:

• css3/blending/repaint/blend-mode-isolate-stacking-context-expected.txt:
• platform/ios/css3/blending/repaint/blend-mode-isolate-stacking-context-expected.txt:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/css3/blending/repaint/blend-mode-isolate-stacking-context-expected.txt (modified) (1 diff)
• LayoutTests/platform/ios/css3/blending/repaint/blend-mode-isolate-stacking-context-expected.txt (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/RenderElement.cpp (modified) (3 diffs)
• Source/WebCore/rendering/RenderLayerModelObject.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2020-09-10  Zalan Bujtas  <zalan@apple.com>
+
+        [Repaint] RenderLayerModelObject::styleWillChange may issue redundant repaint
+        https://bugs.webkit.org/show_bug.cgi?id=216374
+        <rdar://problem/68657490>
+
+        Reviewed by Simon Fraser.
+
+        * css3/blending/repaint/blend-mode-isolate-stacking-context-expected.txt:
+        * platform/ios/css3/blending/repaint/blend-mode-isolate-stacking-context-expected.txt:
+
 2020-09-10  Karl Rackler  <rackler@apple.com>
 

trunk/LayoutTests/css3/blending/repaint/blend-mode-isolate-stacking-context-expected.txt

@@ -20 +20 @@
   (rect 48 526 60 60)
   (rect 48 408 60 60)
-  (rect 48 408 60 60)
   (rect 48 644 60 60)
   (rect 68 644 60 60)

trunk/LayoutTests/platform/ios/css3/blending/repaint/blend-mode-isolate-stacking-context-expected.txt

@@ -20 +20 @@
   (rect 48 536 60 60)
   (rect 48 416 60 60)
-  (rect 48 416 60 60)
   (rect 48 656 60 60)
   (rect 68 656 60 60)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2020-09-10  Zalan Bujtas  <zalan@apple.com>
+
+        [Repaint] RenderLayerModelObject::styleWillChange may issue redundant repaint
+        https://bugs.webkit.org/show_bug.cgi?id=216374
+        <rdar://problem/68657490>
+
+        Reviewed by Simon Fraser.
+
+        Move the repaintIncludingDescendants() calls to repaintBeforeStyleChange() to avoid redundant repaints on the same renderer.
+
+        * rendering/RenderElement.cpp:
+        (WebCore::RenderElement::repaintBeforeStyleChange):
+        * rendering/RenderLayerModelObject.cpp:
+        (WebCore::RenderLayerModelObject::styleWillChange):
+
 2020-09-10  Wenson Hsieh  <wenson_hsieh@apple.com>
 

trunk/Source/WebCore/rendering/RenderElement.cpp

@@ -387 +387 @@
         return false;
     }
-    auto shouldRepaintBeforeStyleChange = [&] {
+    enum class RequiredRepaint { None, RendererOnly, RendererAndDescendantsRenderersWithLayers };
+    auto shouldRepaintBeforeStyleChange = [&]() -> RequiredRepaint {
         if (!parent()) {
             // Can't resolve absolute coordinates.
-            return false;
+            return RequiredRepaint::None;
+        }
+        if (is<RenderLayerModelObject>(this) && hasLayer()) {
+            if (diff == StyleDifference::RepaintLayer)
+                return RequiredRepaint::RendererAndDescendantsRenderersWithLayers;
+            if (diff == StyleDifference::Layout || diff == StyleDifference::SimplifiedLayout) {
+                // Certain style changes require layer repaint, since the layer could end up being destroyed.
+                auto layerMayGetDestroyed = oldStyle.position() != newStyle.position()
+                    || oldStyle.usedZIndex() != newStyle.usedZIndex()
+                    || oldStyle.hasAutoUsedZIndex() != newStyle.hasAutoUsedZIndex()
+                    || oldStyle.clip() != newStyle.clip()
+                    || oldStyle.hasClip() != newStyle.hasClip()
+                    || oldStyle.opacity() != newStyle.opacity()
+                    || oldStyle.transform() != newStyle.transform()
+                    || oldStyle.filter() != newStyle.filter();
+                if (layerMayGetDestroyed)
+                    return RequiredRepaint::RendererAndDescendantsRenderersWithLayers;
+            }
         }
         if (shouldRepaintForStyleDifference(diff))
-            return true;
+            return RequiredRepaint::RendererOnly;
         if (newStyle.outlineSize() < oldStyle.outlineSize())
-            return true;
+            return RequiredRepaint::RendererOnly;
         if (is<RenderLayerModelObject>(*this)) {
             // If we don't have a layer yet, but we are going to get one because of transform or opacity, then we need to repaint the old position of the object.
@@ -401 +419 @@
             bool willHaveLayer = newStyle.hasTransform() || newStyle.opacity() < 1 || newStyle.hasFilter() || newStyle.hasBackdropFilter();
             if (!hasLayer && willHaveLayer)
-                return true;
+                return RequiredRepaint::RendererOnly;
         }
         if (is<RenderBox>(*this)) {
             if (diff == StyleDifference::Layout && oldStyle.position() != newStyle.position() && oldStyle.position() == PositionType::Static)
-                return true;
+                return RequiredRepaint::RendererOnly;
         }
         if (diff > StyleDifference::RepaintLayer && oldStyle.visibility() != newStyle.visibility()) {
@@ -411 +429 @@
                 auto rendererWillBeHidden = newStyle.visibility() != Visibility::Visible;
                 if (rendererWillBeHidden && enclosingLayer->hasVisibleContent() && (this == &enclosingLayer->renderer() || enclosingLayer->renderer().style().visibility() != Visibility::Visible))
-                    return true;
+                    return RequiredRepaint::RendererOnly;
             }
         }
-        return false;
+        return RequiredRepaint::None;
     }();
-    if (shouldRepaintBeforeStyleChange) {
+    if (shouldRepaintBeforeStyleChange == RequiredRepaint::RendererAndDescendantsRenderersWithLayers) {
+        ASSERT(hasLayer());
+        downcast<RenderLayerModelObject>(*this).layer()->repaintIncludingDescendants();
+        return true;
+    }
+    if (shouldRepaintBeforeStyleChange == RequiredRepaint::RendererOnly) {
         repaint();
         return true;

trunk/Source/WebCore/rendering/RenderLayerModelObject.cpp

@@ -114 +114 @@
         s_layerWasSelfPainting = layer()->isSelfPaintingLayer();
 
-    // If our z-index changes value or our visibility changes,
-    // we need to dirty our stacking context's z-order list.
-    const RenderStyle* oldStyle = hasInitializedStyle() ? &style() : nullptr;
-    if (oldStyle) {
-        if (parent()) {
-            // Do a repaint with the old style first, e.g., for example if we go from
-            // having an outline to not having an outline.
-            if (diff == StyleDifference::RepaintLayer) {
-                layer()->repaintIncludingDescendants();
-                if (!(oldStyle->clip() == newStyle.clip()))
-                    layer()->clearClipRectsIncludingDescendants();
-            }
-        }
-
-        if (diff == StyleDifference::Layout || diff == StyleDifference::SimplifiedLayout) {
-            // When a layout hint happens, we do a repaint of the layer, since the layer could end up being destroyed.
-            if (hasLayer()) {
-                if (oldStyle->position() != newStyle.position()
-                    || oldStyle->usedZIndex() != newStyle.usedZIndex()
-                    || oldStyle->hasAutoUsedZIndex() != newStyle.hasAutoUsedZIndex()
-                    || !(oldStyle->clip() == newStyle.clip())
-                    || oldStyle->hasClip() != newStyle.hasClip()
-                    || oldStyle->opacity() != newStyle.opacity()
-                    || oldStyle->transform() != newStyle.transform()
-                    || oldStyle->filter() != newStyle.filter()
-                    )
-                layer()->repaintIncludingDescendants();
-            }
-        }
-    }
-
+    auto* oldStyle = hasInitializedStyle() ? &style() : nullptr;
+    if (diff == StyleDifference::RepaintLayer && parent() && oldStyle && oldStyle->clip() != newStyle.clip())
+        layer()->clearClipRectsIncludingDescendants();
     RenderElement::styleWillChange(diff, newStyle);
 }