Changeset 216591 in webkit

Timestamp:

May 10, 2017 9:50:56 AM (7 years ago)

Author:

Antti Koivisto

Message:

REGRESSION (r207372) Visibility property is not inherited when used in an animation
​https://bugs.webkit.org/show_bug.cgi?id=171883
<rdar://problem/32086550>

Reviewed by Simon Fraser.

Source/WebCore:

The problem here is that our animation code is tied to renderers. We don't have renderers during
the initial style resolution so animations are not applied yet. When constructing renderers we set
their style to the initial animated style but this step can't implement inheritance.

Normally this is invisible as the first animation frame will immediately inherit the style correctly.
However in this case the animation is discrete and the first frame is the same as the initial state.
With r207372 we optimize the descendant style change away.

This patch fixes the problem by tracking that the renderer has initial animated style and inheriting
it to descendants during next style resolution even if it doesn't change.

Test: animations/animation-initial-inheritance.html

• rendering/RenderElement.cpp:

(WebCore::RenderElement::RenderElement):

• rendering/RenderElement.h:

(WebCore::RenderElement::hasInitialAnimatedStyle):
(WebCore::RenderElement::setHasInitialAnimatedStyle):

• style/RenderTreeUpdater.cpp:

(WebCore::RenderTreeUpdater::createRenderer):

Set a bit on renderer indicating it has initial animated style.

• style/StyleTreeResolver.cpp:

(WebCore::Style::TreeResolver::createAnimatedElementUpdate):

Return at least 'Inherit' for style change when updating renderer with initial animated style.

LayoutTests:

• animations/animation-initial-inheritance-expected.html: Added.
• animations/animation-initial-inheritance.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/animations/animation-initial-inheritance-expected.html (added)
• LayoutTests/animations/animation-initial-inheritance.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/RenderElement.cpp (modified) (1 diff)
• Source/WebCore/rendering/RenderElement.h (modified) (2 diffs)
• Source/WebCore/style/RenderTreeUpdater.cpp (modified) (1 diff)
• Source/WebCore/style/StyleTreeResolver.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2017-05-10  Antti Koivisto  <antti@apple.com>
+
+        REGRESSION (r207372) Visibility property is not inherited when used in an animation
+        https://bugs.webkit.org/show_bug.cgi?id=171883
+        <rdar://problem/32086550>
+
+        Reviewed by Simon Fraser.
+
+        * animations/animation-initial-inheritance-expected.html: Added.
+        * animations/animation-initial-inheritance.html: Added.
+
 2017-05-10  Per Arne Vollan  <pvollan@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2017-05-10  Antti Koivisto  <antti@apple.com>
+
+        REGRESSION (r207372) Visibility property is not inherited when used in an animation
+        https://bugs.webkit.org/show_bug.cgi?id=171883
+        <rdar://problem/32086550>
+
+        Reviewed by Simon Fraser.
+
+        The problem here is that our animation code is tied to renderers. We don't have renderers during
+        the initial style resolution so animations are not applied yet. When constructing renderers we set
+        their style to the initial animated style but this step can't implement inheritance.
+
+        Normally this is invisible as the first animation frame will immediately inherit the style correctly.
+        However in this case the animation is discrete and the first frame is the same as the initial state.
+        With r207372 we optimize the descendant style change away.
+
+        This patch fixes the problem by tracking that the renderer has initial animated style and inheriting
+        it to descendants during next style resolution even if it doesn't change.
+
+        Test: animations/animation-initial-inheritance.html
+
+        * rendering/RenderElement.cpp:
+        (WebCore::RenderElement::RenderElement):
+        * rendering/RenderElement.h:
+        (WebCore::RenderElement::hasInitialAnimatedStyle):
+        (WebCore::RenderElement::setHasInitialAnimatedStyle):
+        * style/RenderTreeUpdater.cpp:
+        (WebCore::RenderTreeUpdater::createRenderer):
+
+            Set a bit on renderer indicating it has initial animated style.
+
+        * style/StyleTreeResolver.cpp:
+        (WebCore::Style::TreeResolver::createAnimatedElementUpdate):
+
+            Return at least 'Inherit' for style change when updating renderer with initial animated style.
+
 2017-05-10  Myles C. Maxfield  <mmaxfield@apple.com>
 

trunk/Source/WebCore/rendering/RenderElement.cpp

@@ -103 +103 @@
     , m_ancestorLineBoxDirty(false)
     , m_hasInitializedStyle(false)
+    , m_hasInitialAnimatedStyle(false)
     , m_renderInlineAlwaysCreatesLineBoxes(false)
     , m_renderBoxNeedsLazyRepaint(false)

trunk/Source/WebCore/rendering/RenderElement.h

@@ -134 +134 @@
     void setStyleInternal(RenderStyle&& style) { m_style = WTFMove(style); }
 
+    bool hasInitialAnimatedStyle() const { return m_hasInitialAnimatedStyle; }
+    void setHasInitialAnimatedStyle(bool b) { m_hasInitialAnimatedStyle = b; }
+
     // Repaint only if our old bounds and new bounds are different. The caller may pass in newBounds and newOutlineBox if they are known.
     bool repaintAfterLayoutIfNeeded(const RenderLayerModelObject* repaintContainer, const LayoutRect& oldBounds, const LayoutRect& oldOutlineBox, const LayoutRect* newBoundsPtr = nullptr, const LayoutRect* newOutlineBoxPtr = nullptr);
@@ -330 +333 @@
     unsigned m_ancestorLineBoxDirty : 1;
     unsigned m_hasInitializedStyle : 1;
+    unsigned m_hasInitialAnimatedStyle : 1;
 
     unsigned m_renderInlineAlwaysCreatesLineBoxes : 1;

trunk/Source/WebCore/style/RenderTreeUpdater.cpp

@@ -354 +354 @@
     std::unique_ptr<RenderStyle> animatedStyle;
     newRenderer->animation().updateAnimations(*newRenderer, initialStyle, animatedStyle);
-    if (animatedStyle)
+    if (animatedStyle) {
         newRenderer->setStyleInternal(WTFMove(*animatedStyle));
+        newRenderer->setHasInitialAnimatedStyle(true);
+    }
 
     newRenderer->initializeStyle();

trunk/Source/WebCore/style/StyleTreeResolver.cpp

@@ -259 +259 @@
     if (animatedStyle) {
         auto change = determineChange(renderer->style(), *animatedStyle);
+        if (renderer->hasInitialAnimatedStyle()) {
+            renderer->setHasInitialAnimatedStyle(false);
+            // When we initialize a newly created renderer with initial animated style we don't inherit it to descendants.
+            // The first animation frame needs to correct this.
+            // FIXME: We should compute animated style correctly during initial style resolution when we don't have renderers yet.
+            //        https://bugs.webkit.org/show_bug.cgi?id=171926
+            change = std::max(change, Inherit);
+        }
         // If animation forces render tree reconstruction pass the original style. The animation will be applied on renderer construction.
         // FIXME: We should always use the animated style here.