Title: [223362] releases/WebKitGTK/webkit-2.18/Source/WebCore
Revision
223362
Author
carlo...@webkit.org
Date
2017-10-16 03:07:17 -0700 (Mon, 16 Oct 2017)

Log Message

Merge r221916 - Remove RenderElement::isCSSAnimating boolean
https://bugs.webkit.org/show_bug.cgi?id=176779

Reviewed by Andreas Kling.

This optimization can be replaced with a simple style test that doesn't require keeping
two sources of truth in sync.

* page/animation/CSSAnimationController.cpp:
(WebCore::CSSAnimationControllerPrivate::ensureCompositeAnimation):
(WebCore::CSSAnimationControllerPrivate::clear):

    Can't test here as style might have become non-animating and we don't clear animation when that happens.
    This is only called on renderer destruction so it is not an important optimization.

(WebCore::CSSAnimationControllerPrivate::isRunningAnimationOnRenderer const):
(WebCore::CSSAnimationControllerPrivate::isRunningAcceleratedAnimationOnRenderer const):
(WebCore::CSSAnimationControllerPrivate::getAnimatedStyleForRenderer):
(WebCore::CSSAnimationControllerPrivate::computeExtentOfAnimation const):
(WebCore::CSSAnimationController::cancelAnimations):
(WebCore::CSSAnimationController::getAnimatedStyleForRenderer):
(WebCore::CSSAnimationController::computeExtentOfAnimation const):
(WebCore::CSSAnimationController::isRunningAnimationOnRenderer const):
(WebCore::CSSAnimationController::isRunningAcceleratedAnimationOnRenderer const):

    Test if the style has any animations. This is roughly equivalent of the old test.
    (it is actually somewhat better as the boolean was never cleared on style changes)

* rendering/RenderElement.cpp:
(WebCore::RenderElement::RenderElement):
* rendering/RenderElement.h:
(WebCore::RenderElement::isCSSAnimating const): Deleted.
(WebCore::RenderElement::setIsCSSAnimating): Deleted.
* rendering/style/RenderStyle.h:
(WebCore::RenderStyle::hasAnimationsOrTransitions const):

Modified Paths

Diff

Modified: releases/WebKitGTK/webkit-2.18/Source/WebCore/ChangeLog (223361 => 223362)


--- releases/WebKitGTK/webkit-2.18/Source/WebCore/ChangeLog	2017-10-16 09:56:12 UTC (rev 223361)
+++ releases/WebKitGTK/webkit-2.18/Source/WebCore/ChangeLog	2017-10-16 10:07:17 UTC (rev 223362)
@@ -1,3 +1,41 @@
+2017-09-12  Antti Koivisto  <an...@apple.com>
+
+        Remove RenderElement::isCSSAnimating boolean
+        https://bugs.webkit.org/show_bug.cgi?id=176779
+
+        Reviewed by Andreas Kling.
+
+        This optimization can be replaced with a simple style test that doesn't require keeping
+        two sources of truth in sync.
+
+        * page/animation/CSSAnimationController.cpp:
+        (WebCore::CSSAnimationControllerPrivate::ensureCompositeAnimation):
+        (WebCore::CSSAnimationControllerPrivate::clear):
+
+            Can't test here as style might have become non-animating and we don't clear animation when that happens.
+            This is only called on renderer destruction so it is not an important optimization.
+
+        (WebCore::CSSAnimationControllerPrivate::isRunningAnimationOnRenderer const):
+        (WebCore::CSSAnimationControllerPrivate::isRunningAcceleratedAnimationOnRenderer const):
+        (WebCore::CSSAnimationControllerPrivate::getAnimatedStyleForRenderer):
+        (WebCore::CSSAnimationControllerPrivate::computeExtentOfAnimation const):
+        (WebCore::CSSAnimationController::cancelAnimations):
+        (WebCore::CSSAnimationController::getAnimatedStyleForRenderer):
+        (WebCore::CSSAnimationController::computeExtentOfAnimation const):
+        (WebCore::CSSAnimationController::isRunningAnimationOnRenderer const):
+        (WebCore::CSSAnimationController::isRunningAcceleratedAnimationOnRenderer const):
+
+            Test if the style has any animations. This is roughly equivalent of the old test.
+            (it is actually somewhat better as the boolean was never cleared on style changes)
+
+        * rendering/RenderElement.cpp:
+        (WebCore::RenderElement::RenderElement):
+        * rendering/RenderElement.h:
+        (WebCore::RenderElement::isCSSAnimating const): Deleted.
+        (WebCore::RenderElement::setIsCSSAnimating): Deleted.
+        * rendering/style/RenderStyle.h:
+        (WebCore::RenderStyle::hasAnimationsOrTransitions const):
+
 2017-09-12  Manuel Rego Casasnovas  <r...@igalia.com>
 
         [css-grid] Use transferred size over content size for automatic minimum size

Modified: releases/WebKitGTK/webkit-2.18/Source/WebCore/page/animation/CSSAnimationController.cpp (223361 => 223362)


--- releases/WebKitGTK/webkit-2.18/Source/WebCore/page/animation/CSSAnimationController.cpp	2017-10-16 09:56:12 UTC (rev 223361)
+++ releases/WebKitGTK/webkit-2.18/Source/WebCore/page/animation/CSSAnimationController.cpp	2017-10-16 10:07:17 UTC (rev 223362)
@@ -85,11 +85,9 @@
 
 CompositeAnimation& CSSAnimationControllerPrivate::ensureCompositeAnimation(RenderElement& renderer)
 {
-    auto result = m_compositeAnimations.add(&renderer, nullptr);
-    if (result.isNewEntry) {
-        result.iterator->value = CompositeAnimation::create(*this);
-        renderer.setIsCSSAnimating(true);
-    }
+    auto result = m_compositeAnimations.ensure(&renderer, [&] {
+        return CompositeAnimation::create(*this);
+    });
 
     if (animationsAreSuspendedForDocument(&renderer.document()))
         result.iterator->value->suspendAnimations();
@@ -99,11 +97,12 @@
 
 bool CSSAnimationControllerPrivate::clear(RenderElement& renderer)
 {
+    auto it = m_compositeAnimations.find(&renderer);
+    if (it == m_compositeAnimations.end())
+        return false;
+
     LOG(Animations, "CSSAnimationControllerPrivate %p clear: %p", this, &renderer);
 
-    ASSERT(renderer.isCSSAnimating());
-    ASSERT(m_compositeAnimations.contains(&renderer));
-
     Element* element = renderer.element();
 
     m_eventsToDispatch.removeAllMatching([element] (const EventToDispatch& info) {
@@ -116,11 +115,14 @@
     
     // Return false if we didn't do anything OR we are suspended (so we don't try to
     // do a invalidateStyleForSubtree() when suspended).
-    RefPtr<CompositeAnimation> animation = m_compositeAnimations.take(&renderer);
-    ASSERT(animation);
-    renderer.setIsCSSAnimating(false);
-    animation->clearRenderer();
-    return animation->isSuspended();
+    // FIXME: The code below does the opposite of what the comment above says regarding suspended state.
+    auto& animation = *it->value;
+    bool result = animation.isSuspended();
+    animation.clearRenderer();
+
+    m_compositeAnimations.remove(it);
+
+    return result;
 }
 
 std::optional<Seconds> CSSAnimationControllerPrivate::updateAnimations(SetChanged callSetChanged/* = DoNotCallSetChanged*/)
@@ -287,18 +289,18 @@
 
 bool CSSAnimationControllerPrivate::isRunningAnimationOnRenderer(RenderElement& renderer, CSSPropertyID property, AnimationBase::RunningState runningState) const
 {
-    ASSERT(renderer.isCSSAnimating());
-    ASSERT(m_compositeAnimations.contains(&renderer));
-    const CompositeAnimation& animation = *m_compositeAnimations.get(&renderer);
-    return animation.isAnimatingProperty(property, false, runningState);
+    auto* animation = m_compositeAnimations.get(&renderer);
+    if (!animation)
+        return false;
+    return animation->isAnimatingProperty(property, false, runningState);
 }
 
 bool CSSAnimationControllerPrivate::isRunningAcceleratedAnimationOnRenderer(RenderElement& renderer, CSSPropertyID property, AnimationBase::RunningState runningState) const
 {
-    ASSERT(renderer.isCSSAnimating());
-    ASSERT(m_compositeAnimations.contains(&renderer));
-    const CompositeAnimation& animation = *m_compositeAnimations.get(&renderer);
-    return animation.isAnimatingProperty(property, true, runningState);
+    auto* animation = m_compositeAnimations.get(&renderer);
+    if (!animation)
+        return false;
+    return animation->isAnimatingProperty(property, true, runningState);
 }
 
 void CSSAnimationControllerPrivate::updateThrottlingState()
@@ -467,12 +469,13 @@
 
 std::unique_ptr<RenderStyle> CSSAnimationControllerPrivate::getAnimatedStyleForRenderer(RenderElement& renderer)
 {
+    auto* animation = m_compositeAnimations.get(&renderer);
+    if (!animation)
+        return RenderStyle::clonePtr(renderer.style());
+
     AnimationPrivateUpdateBlock animationUpdateBlock(*this);
 
-    ASSERT(renderer.isCSSAnimating());
-    ASSERT(m_compositeAnimations.contains(&renderer));
-    const CompositeAnimation& rendererAnimations = *m_compositeAnimations.get(&renderer);
-    std::unique_ptr<RenderStyle> animatingStyle = rendererAnimations.getAnimatedStyle();
+    std::unique_ptr<RenderStyle> animatingStyle = animation->getAnimatedStyle();
     if (!animatingStyle)
         animatingStyle = RenderStyle::clonePtr(renderer.style());
     
@@ -481,14 +484,14 @@
 
 bool CSSAnimationControllerPrivate::computeExtentOfAnimation(RenderElement& renderer, LayoutRect& bounds) const
 {
-    ASSERT(renderer.isCSSAnimating());
-    ASSERT(m_compositeAnimations.contains(&renderer));
+    auto* animation = m_compositeAnimations.get(&renderer);
+    if (!animation)
+        return true;
 
-    const CompositeAnimation& rendererAnimations = *m_compositeAnimations.get(&renderer);
-    if (!rendererAnimations.isAnimatingProperty(CSSPropertyTransform, false, AnimationBase::Running | AnimationBase::Paused))
+    if (!animation->isAnimatingProperty(CSSPropertyTransform, false, AnimationBase::Running | AnimationBase::Paused))
         return true;
 
-    return rendererAnimations.computeExtentOfTransformAnimation(bounds);
+    return animation->computeExtentOfTransformAnimation(bounds);
 }
 
 unsigned CSSAnimationControllerPrivate::numberOfActiveAnimations(Document* document) const
@@ -630,9 +633,6 @@
 
 void CSSAnimationController::cancelAnimations(RenderElement& renderer)
 {
-    if (!renderer.isCSSAnimating())
-        return;
-
     if (!m_data->clear(renderer))
         return;
 
@@ -680,14 +680,15 @@
 
 std::unique_ptr<RenderStyle> CSSAnimationController::getAnimatedStyleForRenderer(RenderElement& renderer)
 {
-    if (!renderer.isCSSAnimating())
+    if (!renderer.style().hasAnimationsOrTransitions())
         return RenderStyle::clonePtr(renderer.style());
+
     return m_data->getAnimatedStyleForRenderer(renderer);
 }
 
 bool CSSAnimationController::computeExtentOfAnimation(RenderElement& renderer, LayoutRect& bounds) const
 {
-    if (!renderer.isCSSAnimating())
+    if (!renderer.style().hasAnimationsOrTransitions())
         return true;
 
     return m_data->computeExtentOfAnimation(renderer, bounds);
@@ -721,12 +722,16 @@
 
 bool CSSAnimationController::isRunningAnimationOnRenderer(RenderElement& renderer, CSSPropertyID property, AnimationBase::RunningState runningState) const
 {
-    return renderer.isCSSAnimating() && m_data->isRunningAnimationOnRenderer(renderer, property, runningState);
+    if (!renderer.style().hasAnimationsOrTransitions())
+        return false;
+    return m_data->isRunningAnimationOnRenderer(renderer, property, runningState);
 }
 
 bool CSSAnimationController::isRunningAcceleratedAnimationOnRenderer(RenderElement& renderer, CSSPropertyID property, AnimationBase::RunningState runningState) const
 {
-    return renderer.isCSSAnimating() && m_data->isRunningAcceleratedAnimationOnRenderer(renderer, property, runningState);
+    if (!renderer.style().hasAnimationsOrTransitions())
+        return false;
+    return m_data->isRunningAcceleratedAnimationOnRenderer(renderer, property, runningState);
 }
 
 bool CSSAnimationController::isSuspended() const

Modified: releases/WebKitGTK/webkit-2.18/Source/WebCore/rendering/RenderElement.cpp (223361 => 223362)


--- releases/WebKitGTK/webkit-2.18/Source/WebCore/rendering/RenderElement.cpp	2017-10-16 09:56:12 UTC (rev 223361)
+++ releases/WebKitGTK/webkit-2.18/Source/WebCore/rendering/RenderElement.cpp	2017-10-16 10:07:17 UTC (rev 223362)
@@ -107,7 +107,6 @@
     , m_renderBoxNeedsLazyRepaint(false)
     , m_hasPausedImageAnimations(false)
     , m_hasCounterNodeMap(false)
-    , m_isCSSAnimating(false)
     , m_hasContinuation(false)
     , m_hasValidCachedFirstLineStyle(false)
     , m_renderBlockHasMarginBeforeQuirk(false)

Modified: releases/WebKitGTK/webkit-2.18/Source/WebCore/rendering/RenderElement.h (223361 => 223362)


--- releases/WebKitGTK/webkit-2.18/Source/WebCore/rendering/RenderElement.h	2017-10-16 09:56:12 UTC (rev 223361)
+++ releases/WebKitGTK/webkit-2.18/Source/WebCore/rendering/RenderElement.h	2017-10-16 10:07:17 UTC (rev 223362)
@@ -203,9 +203,6 @@
     bool hasCounterNodeMap() const { return m_hasCounterNodeMap; }
     void setHasCounterNodeMap(bool f) { m_hasCounterNodeMap = f; }
 
-    bool isCSSAnimating() const { return m_isCSSAnimating; }
-    void setIsCSSAnimating(bool b) { m_isCSSAnimating = b; }
-    
     const RenderElement* enclosingRendererWithTextDecoration(TextDecoration, bool firstLine) const;
     void drawLineForBoxSide(GraphicsContext&, const FloatRect&, BoxSide, Color, EBorderStyle, float adjacentWidth1, float adjacentWidth2, bool antialias = false) const;
 
@@ -338,7 +335,6 @@
     unsigned m_renderBoxNeedsLazyRepaint : 1;
     unsigned m_hasPausedImageAnimations : 1;
     unsigned m_hasCounterNodeMap : 1;
-    unsigned m_isCSSAnimating : 1;
     unsigned m_hasContinuation : 1;
     mutable unsigned m_hasValidCachedFirstLineStyle : 1;
 

Modified: releases/WebKitGTK/webkit-2.18/Source/WebCore/rendering/style/RenderStyle.h (223361 => 223362)


--- releases/WebKitGTK/webkit-2.18/Source/WebCore/rendering/style/RenderStyle.h	2017-10-16 09:56:12 UTC (rev 223361)
+++ releases/WebKitGTK/webkit-2.18/Source/WebCore/rendering/style/RenderStyle.h	2017-10-16 10:07:17 UTC (rev 223362)
@@ -662,7 +662,7 @@
     AnimationList* animations() { return m_rareNonInheritedData->animations.get(); }
     AnimationList* transitions() { return m_rareNonInheritedData->transitions.get(); }
     
-    bool hasAnimationsOrTransitions() const { return m_rareNonInheritedData->hasAnimationsOrTransitions(); }
+    bool hasAnimationsOrTransitions() const { return hasAnimations() || hasTransitions(); }
 
     AnimationList& ensureAnimations();
     AnimationList& ensureTransitions();

Modified: releases/WebKitGTK/webkit-2.18/Source/WebCore/rendering/style/StyleRareNonInheritedData.h (223361 => 223362)


--- releases/WebKitGTK/webkit-2.18/Source/WebCore/rendering/style/StyleRareNonInheritedData.h	2017-10-16 09:56:12 UTC (rev 223361)
+++ releases/WebKitGTK/webkit-2.18/Source/WebCore/rendering/style/StyleRareNonInheritedData.h	2017-10-16 10:07:17 UTC (rev 223362)
@@ -92,8 +92,6 @@
 
     bool hasOpacity() const { return opacity < 1; }
 
-    bool hasAnimationsOrTransitions() const { return animations || transitions; }
-
     float opacity;
 
     float aspectRatioDenominator;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to