[Web Animations] Ensure renderers with accelerated animations have layers
authorgraouts@webkit.org <graouts@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 26 Sep 2018 14:34:10 +0000 (14:34 +0000)
committergraouts@webkit.org <graouts@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 26 Sep 2018 14:34:10 +0000 (14:34 +0000)
https://bugs.webkit.org/show_bug.cgi?id=189990
<rdar://problem/44791222>

Reviewed by Zalan Bujtas.

We have done some work already in webkit.org/b/189784 to prevent never-ending calls to DocumentTimeline::updateAnimations(). This was due to
the change made for webkit.org/b/186930 where we queued calls to updateAnimations() in KeyframeEffectReadOnly::applyPendingAcceleratedActions()
while we were waiting for a renderer with a layer backing for a given animation target. Instead of doing this, we now ensure renderers always
have a layer when they have an accelerated animation applied.

No new tests, this is already covered by webanimations/accelerated-animation-with-delay.html and webanimations/opacity-animation-yields-compositing-span.html
which respectively check that we can apply an accelerated animation to a non-positioned block and an inline element.

* animation/DocumentTimeline.cpp:
(WebCore::DocumentTimeline::runningAnimationsForElementAreAllAccelerated const): This method should have been marked const all along and it is
now required so it can be called through RenderBox::requiresLayer() and RenderInline::requiresLayer().
(WebCore::DocumentTimeline::runningAnimationsForElementAreAllAccelerated): Deleted.
* animation/DocumentTimeline.h:
* animation/KeyframeEffectReadOnly.cpp:
(WebCore::KeyframeEffectReadOnly::applyPendingAcceleratedActions): Stop enqueuing the accelerated actions in case we're lacking a composited renderer
since this situation should no longer arise.
* rendering/RenderBox.h: Make requiresLayer() return true if this renderer's element is the target of accelerated animations.
* rendering/RenderBoxModelObject.cpp:
(WebCore::RenderBoxModelObject::hasRunningAcceleratedAnimations const): Query the document timeline, if it exists, to check that this renderer's element
has accelerated animations applied.
* rendering/RenderBoxModelObject.h:
* rendering/RenderInline.h: Make requiresLayer() return true if this renderer's element is the target of accelerated animations.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@236501 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/animation/DocumentTimeline.cpp
Source/WebCore/animation/DocumentTimeline.h
Source/WebCore/animation/KeyframeEffectReadOnly.cpp
Source/WebCore/rendering/RenderBox.h
Source/WebCore/rendering/RenderBoxModelObject.cpp
Source/WebCore/rendering/RenderBoxModelObject.h
Source/WebCore/rendering/RenderInline.h

index d45e947..5d0e95c 100644 (file)
@@ -1,3 +1,34 @@
+2018-09-26  Antoine Quint  <graouts@apple.com>
+
+        [Web Animations] Ensure renderers with accelerated animations have layers
+        https://bugs.webkit.org/show_bug.cgi?id=189990
+        <rdar://problem/44791222>
+
+        Reviewed by Zalan Bujtas.
+
+        We have done some work already in webkit.org/b/189784 to prevent never-ending calls to DocumentTimeline::updateAnimations(). This was due to
+        the change made for webkit.org/b/186930 where we queued calls to updateAnimations() in KeyframeEffectReadOnly::applyPendingAcceleratedActions()
+        while we were waiting for a renderer with a layer backing for a given animation target. Instead of doing this, we now ensure renderers always
+        have a layer when they have an accelerated animation applied.
+
+        No new tests, this is already covered by webanimations/accelerated-animation-with-delay.html and webanimations/opacity-animation-yields-compositing-span.html
+        which respectively check that we can apply an accelerated animation to a non-positioned block and an inline element.
+
+        * animation/DocumentTimeline.cpp:
+        (WebCore::DocumentTimeline::runningAnimationsForElementAreAllAccelerated const): This method should have been marked const all along and it is
+        now required so it can be called through RenderBox::requiresLayer() and RenderInline::requiresLayer().
+        (WebCore::DocumentTimeline::runningAnimationsForElementAreAllAccelerated): Deleted.
+        * animation/DocumentTimeline.h:
+        * animation/KeyframeEffectReadOnly.cpp:
+        (WebCore::KeyframeEffectReadOnly::applyPendingAcceleratedActions): Stop enqueuing the accelerated actions in case we're lacking a composited renderer
+        since this situation should no longer arise.
+        * rendering/RenderBox.h: Make requiresLayer() return true if this renderer's element is the target of accelerated animations.
+        * rendering/RenderBoxModelObject.cpp:
+        (WebCore::RenderBoxModelObject::hasRunningAcceleratedAnimations const): Query the document timeline, if it exists, to check that this renderer's element
+        has accelerated animations applied.
+        * rendering/RenderBoxModelObject.h:
+        * rendering/RenderInline.h: Make requiresLayer() return true if this renderer's element is the target of accelerated animations.
+
 2018-09-25  Eric Carlson  <eric.carlson@apple.com>
 
         [MediaStream] Add Mac window capture source
index 19ad805..7564258 100644 (file)
@@ -446,7 +446,7 @@ bool DocumentTimeline::resolveAnimationsForElement(Element& element, RenderStyle
     return !hasNonAcceleratedAnimations && hasPendingAcceleratedAnimations;
 }
 
-bool DocumentTimeline::runningAnimationsForElementAreAllAccelerated(Element& element)
+bool DocumentTimeline::runningAnimationsForElementAreAllAccelerated(Element& element) const
 {
     // FIXME: This will let animations run using hardware compositing even if later in the active
     // span of the current animations a new animation should require hardware compositing to be
index e555b15..714eca2 100644 (file)
@@ -59,7 +59,7 @@ public:
     bool isRunningAcceleratedAnimationOnRenderer(RenderElement&, CSSPropertyID) const;
     void animationAcceleratedRunningStateDidChange(WebAnimation&);
     void applyPendingAcceleratedAnimations();
-    bool runningAnimationsForElementAreAllAccelerated(Element&);
+    bool runningAnimationsForElementAreAllAccelerated(Element&) const;
     bool resolveAnimationsForElement(Element&, RenderStyle&);
     void detachFromDocument();
 
index fa387ef..97372b7 100644 (file)
@@ -1277,11 +1277,8 @@ void KeyframeEffectReadOnly::applyPendingAcceleratedActions()
         return;
 
     auto* renderer = this->renderer();
-    if (!renderer || !renderer->isComposited()) {
-        if (m_lastRecordedAcceleratedAction != AcceleratedAction::Stop || m_pendingAcceleratedActions.last() != AcceleratedAction::Stop)
-            animation()->acceleratedStateDidChange();
+    if (!renderer || !renderer->isComposited())
         return;
-    }
 
     auto pendingAcceleratedActions = m_pendingAcceleratedActions;
     m_pendingAcceleratedActions.clear();
index 6098bac..7965495 100644 (file)
@@ -53,7 +53,7 @@ public:
     {
         return isDocumentElementRenderer() || isPositioned() || createsGroup() || hasClipPath() || hasOverflowClip()
             || hasTransformRelatedProperty() || hasHiddenBackface() || hasReflection() || style().specifiesColumns()
-            || !style().hasAutoZIndex();
+            || !style().hasAutoZIndex() || hasRunningAcceleratedAnimations();
     }
 
     bool backgroundIsKnownToBeOpaqueInRect(const LayoutRect& localRect) const final;
index d0ffc33..e442cf0 100644 (file)
@@ -29,6 +29,8 @@
 #include "BitmapImage.h"
 #include "BorderEdge.h"
 #include "CachedImage.h"
+#include "Document.h"
+#include "DocumentTimeline.h"
 #include "FloatRoundedRect.h"
 #include "Frame.h"
 #include "FrameView.h"
@@ -2688,4 +2690,13 @@ void RenderBoxModelObject::mapAbsoluteToLocalPoint(MapCoordinatesFlags mode, Tra
         transformState.move(containerOffset.width(), containerOffset.height(), preserve3D ? TransformState::AccumulateTransform : TransformState::FlattenTransform);
 }
 
+bool RenderBoxModelObject::hasRunningAcceleratedAnimations() const
+{
+    if (auto* node = element()) {
+        if (auto* timeline = node->document().existingTimeline())
+            return timeline->runningAnimationsForElementAreAllAccelerated(*node);
+    }
+    return false;
+}
+
 } // namespace WebCore
index ec3b1cb..6eeb8a4 100644 (file)
@@ -270,6 +270,8 @@ protected:
 
     DecodingMode decodingModeForImageDraw(const Image&, const PaintInfo&) const;
 
+    bool hasRunningAcceleratedAnimations() const;
+
 public:
     // For RenderBlocks and RenderInlines with m_style->styleType() == PseudoId::FirstLetter, this tracks their remaining text fragments
     RenderTextFragment* firstLetterRemainingText() const;
index 6e6bf87..a63ef0e 100644 (file)
@@ -120,7 +120,7 @@ private:
 
     bool nodeAtPoint(const HitTestRequest&, HitTestResult&, const HitTestLocation& locationInContainer, const LayoutPoint& accumulatedOffset, HitTestAction) final;
 
-    bool requiresLayer() const override { return isInFlowPositioned() || createsGroup() || hasClipPath() || willChangeCreatesStackingContext(); }
+    bool requiresLayer() const override { return isInFlowPositioned() || createsGroup() || hasClipPath() || willChangeCreatesStackingContext() || hasRunningAcceleratedAnimations(); }
 
     LayoutUnit offsetLeft() const final;
     LayoutUnit offsetTop() const final;