Changeset 238128 in webkit

Timestamp:

Nov 13, 2018 2:22:56 AM (5 years ago)

Author:

graouts@webkit.org

Message:

[Web Animations] Don't schedule animation frames or update style while an accelerated animation is running
​https://bugs.webkit.org/show_bug.cgi?id=191542
<rdar://problem/45356027>

Reviewed by Simon Fraser.

Source/WebCore:

Test: animations/no-style-recalc-during-accelerated-animation.html

In order to be more power-efficient, we stop scheduling calls to updateAnimationsAndSendEvents() when running only accelerated
animations. To do that, we prevent scheduling further animation resolution if we're in the process of updating animations, and
when we are done, call the new DocumentTimeline::scheduleNextTick() method that will check whether we have only accelerated
animations running, and in that case check which of those animations needs an update the soonest and starts a timer scheduled
for that time when we'll schedule animation resolution.

By default, animations compute the time until their natural completion but in the case of CSS Animations, we want to make sure
we also update animations in-flight to dispatch "animationiteration" events.

• animation/AnimationEffect.h: Make the simpleIterationProgress() public so it can be called by WebAnimation::timeToNextTick().
• animation/DocumentTimeline.cpp:

(WebCore::DocumentTimeline::DocumentTimeline): Create the m_tickScheduleTimer and set it up to call scheduleAnimationResolutionIfNeeded().
(WebCore::DocumentTimeline::suspendAnimations): If we don't already have a cached current time, cache the current time.
(WebCore::DocumentTimeline::resumeAnimations): Reset the cached current time to ensure we'll get a fresh one when updating animations next.
(WebCore::DocumentTimeline::liveCurrentTime const): Factor the code to compute the current time out of currentTime() so that we can
cache the current time in suspendAnimations() without also automatically clearing the current time.
(WebCore::DocumentTimeline::currentTime): Use liveCurrentTime() and cacheCurrentTime() since much of the code from this function has been
factored out into those. Additionally, we were failing to clear the current time if called inside an animation frame, which we now do correctly
by virtue of using cacheCurrentTime(). This fixes some flakiness.
(WebCore::DocumentTimeline::cacheCurrentTime): Factor the code to cache the current time out of currentTime().
(WebCore::DocumentTimeline::maybeClearCachedCurrentTime): No need to clear the current time if we get suspended.
(WebCore::DocumentTimeline::scheduleAnimationResolutionIfNeeded): Prevent scheduling an animation update if we're in the middle of one already,
scheduleNextTick() will be called after animations are updated to see if we should schedule an animation update instead.
(WebCore::DocumentTimeline::unscheduleAnimationResolution): Cancel the m_tickScheduleTimer if we need to unschedule animation resolution.
(WebCore::DocumentTimeline::animationResolutionTimerFired): Factor the call to applyPendingAcceleratedAnimations() out of updateAnimationsAndSendEvents()
and call scheduleNextTick().
(WebCore::DocumentTimeline::updateAnimationsAndSendEvents): Set the new m_isUpdatingAnimations member variable to true while this function is running.
(WebCore::DocumentTimeline::scheduleNextTick): Schedule an animation update immediately if we have any relevant animation that is not accelerated.
Otherwise, iterate through all animations to figure out the earliest moment at which we need to update animations.
(WebCore::DocumentTimeline::updateListOfElementsWithRunningAcceleratedAnimationsForElement): Use the new WebAnimation::isRunningAccelerated() function.

• animation/DocumentTimeline.h:
• animation/WebAnimation.cpp:

(WebCore::WebAnimation::isRunningAccelerated const): Since we end up checking if an animation is running with an accelerated effect, we introduce a new
function to get that information directly through the WebAnimation object without bothering about its effect.
(WebCore::WebAnimation::resolve): We should only call updateFinishedState() here since timingDidChange() would also notify the timeline about a potential
change in relevance, which is not necessary and which would schedule an animation frame even for animations that are accelerated.
(WebCore::WebAnimation::timeToNextTick const): Compute the time until our animation completion or, in the case of CSS animations, the next iteration.

• animation/WebAnimation.h:

LayoutTests:

Add a test that checks that we make only minimal style updates and still dispatch events while an accelerated animation is running.

• animations/no-style-recalc-during-accelerated-animation-expected.txt: Added.
• animations/no-style-recalc-during-accelerated-animation.html: Added.
• fast/layers/no-clipping-overflow-hidden-added-after-transform-expected.html:
• fast/layers/no-clipping-overflow-hidden-added-after-transform.html: Change the colors to avoid a tiny ImageOnlyFailure.
• platform/win/TestExpectations: Mark some regressions tracked by webkit.org/b/191584.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/animations/no-style-recalc-during-accelerated-animation-expected.txt (added)
• LayoutTests/animations/no-style-recalc-during-accelerated-animation.html (added)
• LayoutTests/fast/layers/no-clipping-overflow-hidden-added-after-transform-expected.html (modified) (1 diff)
• LayoutTests/fast/layers/no-clipping-overflow-hidden-added-after-transform.html (modified) (3 diffs)
• LayoutTests/platform/win/TestExpectations (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/animation/AnimationEffect.h (modified) (2 diffs)
• Source/WebCore/animation/DocumentTimeline.cpp (modified) (15 diffs)
• Source/WebCore/animation/DocumentTimeline.h (modified) (4 diffs)
• Source/WebCore/animation/WebAnimation.cpp (modified) (3 diffs)
• Source/WebCore/animation/WebAnimation.h (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2018-11-12  Antoine Quint  <graouts@apple.com>
+
+        [Web Animations] Don't schedule animation frames or update style while an accelerated animation is running
+        https://bugs.webkit.org/show_bug.cgi?id=191542
+        <rdar://problem/45356027>
+
+        Reviewed by Simon Fraser.
+
+        Add a test that checks that we make only minimal style updates and still dispatch events while an accelerated animation is running.
+
+        * animations/no-style-recalc-during-accelerated-animation-expected.txt: Added.
+        * animations/no-style-recalc-during-accelerated-animation.html: Added.
+        * fast/layers/no-clipping-overflow-hidden-added-after-transform-expected.html:
+        * fast/layers/no-clipping-overflow-hidden-added-after-transform.html: Change the colors to avoid a tiny ImageOnlyFailure.
+        * platform/win/TestExpectations: Mark some regressions tracked by webkit.org/b/191584.
+
 2018-11-12  Darshan Kadu  <darsh7807@gmail.com>
 

trunk/LayoutTests/fast/layers/no-clipping-overflow-hidden-added-after-transform-expected.html

@@ -10 +10 @@
 #overflowHidden {
     overflow: hidden;
-    background: purple;
+    background: white;
 }
 
 #transformed {
     -webkit-transform: rotate(45deg) translate3d(0, 0, 0);
-    background: green;
+    background: black;
 }
 </style>

trunk/LayoutTests/fast/layers/no-clipping-overflow-hidden-added-after-transform.html

@@ -10 +10 @@
 #overflowHidden {
     overflow: hidden;
-    background: purple;
+    background: white;
 }
 
@@ -16 +16 @@
     -webkit-transform: rotate(0deg) translate3d(0, 0, 0);
     -webkit-transition: -webkit-transform linear 1ms;
-    background: green;
+    background: black;
 }
 
@@ -33 +33 @@
     function transitionFinished() {
         if (window.testRunner)
-            requestAnimationFrame(() => window.testRunner.notifyDone());
+            window.testRunner.notifyDone();
     }
 

trunk/LayoutTests/platform/win/TestExpectations

@@ -4252 +4252 @@
 webkit.org/b/191368 imported/blink/fast/text/international/complex-text-trailing-space.html [ Crash ]
 webkit.org/b/191368 imported/blink/fast/text/sub-pixel/complex-text-preferred-width.html [ Crash ]
+
+webkit.org/b/191584 animations/animation-direction-normal.html [ Failure ]
+webkit.org/b/191584 animations/animation-direction-reverse.html [ Failure ]
+webkit.org/b/191584 animations/dynamic-stylesheet-loading.html [ Failure ]
+webkit.org/b/191584 animations/play-state-paused.html [ Failure ]
+webkit.org/b/191584 animations/transform-non-accelerated.html [ Failure ]
+webkit.org/b/191584 transitions/start-transform-transition.html [ Failure ]
+webkit.org/b/191584 http/wpt/css/css-animations/set-animation-play-state-to-paused-001.html [ ImageOnlyFailure ]
+webkit.org/b/191584 webanimations/accelerated-animation-with-delay.html [ ImageOnlyFailure ]
+webkit.org/b/191584 webanimations/accelerated-transition-by-removing-property.html [ ImageOnlyFailure ]
+webkit.org/b/191584 fast/animation/css-animation-resuming-when-visible-with-style-change.html [ Timeout ]

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2018-11-12  Antoine Quint  <graouts@apple.com>
+
+        [Web Animations] Don't schedule animation frames or update style while an accelerated animation is running
+        https://bugs.webkit.org/show_bug.cgi?id=191542
+        <rdar://problem/45356027>
+
+        Reviewed by Simon Fraser.
+
+        Test: animations/no-style-recalc-during-accelerated-animation.html
+
+        In order to be more power-efficient, we stop scheduling calls to updateAnimationsAndSendEvents() when running only accelerated
+        animations. To do that, we prevent scheduling further animation resolution if we're in the process of updating animations, and
+        when we are done, call the new DocumentTimeline::scheduleNextTick() method that will check whether we have only accelerated
+        animations running, and in that case check which of those animations needs an update the soonest and starts a timer scheduled
+        for that time when we'll schedule animation resolution.
+
+        By default, animations compute the time until their natural completion but in the case of CSS Animations, we want to make sure
+        we also update animations in-flight to dispatch "animationiteration" events.
+
+        * animation/AnimationEffect.h: Make the simpleIterationProgress() public so it can be called by WebAnimation::timeToNextTick().
+        * animation/DocumentTimeline.cpp:
+        (WebCore::DocumentTimeline::DocumentTimeline): Create the m_tickScheduleTimer and set it up to call scheduleAnimationResolutionIfNeeded().
+        (WebCore::DocumentTimeline::suspendAnimations): If we don't already have a cached current time, cache the current time.
+        (WebCore::DocumentTimeline::resumeAnimations): Reset the cached current time to ensure we'll get a fresh one when updating animations next.
+        (WebCore::DocumentTimeline::liveCurrentTime const): Factor the code to compute the current time out of currentTime() so that we can
+        cache the current time in suspendAnimations() without also automatically clearing the current time.
+        (WebCore::DocumentTimeline::currentTime): Use liveCurrentTime() and cacheCurrentTime() since much of the code from this function has been
+        factored out into those. Additionally, we were failing to clear the current time if called inside an animation frame, which we now do correctly
+        by virtue of using cacheCurrentTime(). This fixes some flakiness.
+        (WebCore::DocumentTimeline::cacheCurrentTime): Factor the code to cache the current time out of currentTime(). 
+        (WebCore::DocumentTimeline::maybeClearCachedCurrentTime): No need to clear the current time if we get suspended.
+        (WebCore::DocumentTimeline::scheduleAnimationResolutionIfNeeded): Prevent scheduling an animation update if we're in the middle of one already,
+        scheduleNextTick() will be called after animations are updated to see if we should schedule an animation update instead.
+        (WebCore::DocumentTimeline::unscheduleAnimationResolution): Cancel the m_tickScheduleTimer if we need to unschedule animation resolution.
+        (WebCore::DocumentTimeline::animationResolutionTimerFired): Factor the call to applyPendingAcceleratedAnimations() out of updateAnimationsAndSendEvents()
+        and call scheduleNextTick().
+        (WebCore::DocumentTimeline::updateAnimationsAndSendEvents): Set the new m_isUpdatingAnimations member variable to true while this function is running.
+        (WebCore::DocumentTimeline::scheduleNextTick): Schedule an animation update immediately if we have any relevant animation that is not accelerated.
+        Otherwise, iterate through all animations to figure out the earliest moment at which we need to update animations.
+        (WebCore::DocumentTimeline::updateListOfElementsWithRunningAcceleratedAnimationsForElement): Use the new WebAnimation::isRunningAccelerated() function.
+        * animation/DocumentTimeline.h:
+        * animation/WebAnimation.cpp:
+        (WebCore::WebAnimation::isRunningAccelerated const): Since we end up checking if an animation is running with an accelerated effect, we introduce a new
+        function to get that information directly through the WebAnimation object without bothering about its effect.
+        (WebCore::WebAnimation::resolve): We should only call updateFinishedState() here since timingDidChange() would also notify the timeline about a potential
+        change in relevance, which is not necessary and which would schedule an animation frame even for animations that are accelerated.
+        (WebCore::WebAnimation::timeToNextTick const): Compute the time until our animation completion or, in the case of CSS animations, the next iteration.
+        * animation/WebAnimation.h:
+
 2018-11-13  Miguel Gomez  <magomez@igalia.com>
 

trunk/Source/WebCore/animation/AnimationEffect.h

@@ -91 +91 @@
     std::optional<Seconds> activeTime() const;
     Seconds endTime() const;
+    std::optional<double> simpleIterationProgress() const;
     std::optional<double> iterationProgress() const;
     std::optional<double> currentIteration() const;
@@ -105 +106 @@
 
     std::optional<double> overallProgress() const;
-    std::optional<double> simpleIterationProgress() const;
     AnimationEffect::ComputedDirection currentDirection() const;
     std::optional<double> directedProgress() const;

trunk/Source/WebCore/animation/DocumentTimeline.cpp

@@ -64 +64 @@
     , m_document(&document)
     , m_originTime(originTime)
+    , m_tickScheduleTimer(*this, &DocumentTimeline::scheduleAnimationResolutionIfNeeded)
 #if !USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR)
     , m_animationResolutionTimer(*this, &DocumentTimeline::animationResolutionTimerFired)
@@ -209 +210 @@
         return;
 
+    if (!m_cachedCurrentTime)
+        m_cachedCurrentTime = liveCurrentTime();
+
     for (const auto& animation : m_animations)
         animation->setSuspended(true);
@@ -223 +227 @@
     if (!animationsAreSuspended())
         return;
+
+    m_cachedCurrentTime = std::nullopt;
 
     m_isSuspended = false;
@@ -245 +251 @@
     }
     return count;
+}
+
+Seconds DocumentTimeline::liveCurrentTime() const
+{
+#if USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR)
+    return m_document->animationScheduler().lastTimestamp();
+#else
+    return Seconds(m_document->domWindow()->nowTimestamp());
+#endif
 }
 
@@ -260 +275 @@
     }
 
+    auto currentTime = liveCurrentTime();
+
 #if USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR)
     // If we're in the middle of firing a frame, either due to a requestAnimationFrame callback
@@ -265 +282 @@
     // the timestamp for requestAnimationFrame() callbacks.
     if (m_document->animationScheduler().isFiring())
-        m_cachedCurrentTime = m_document->animationScheduler().lastTimestamp();
+        cacheCurrentTime(currentTime);
 #endif
 
@@ -274 +291 @@
         // like setTimeout() or handling events will get a time that's only updating at around 60fps, or less if
         // we're throttled.
-        auto lastAnimationSchedulerTimestamp = m_document->animationScheduler().lastTimestamp();
+        auto lastAnimationSchedulerTimestamp = currentTime;
         auto delta = Seconds(m_document->domWindow()->nowTimestamp()) - lastAnimationSchedulerTimestamp;
         int frames = std::floor(delta.seconds() / animationInterval().seconds());
-        m_cachedCurrentTime = lastAnimationSchedulerTimestamp + Seconds(frames * animationInterval().seconds());
+        cacheCurrentTime(lastAnimationSchedulerTimestamp + Seconds(frames * animationInterval().seconds()));
 #else
-        m_cachedCurrentTime = Seconds(m_document->domWindow()->nowTimestamp());
+        cacheCurrentTime(currentTime);
 #endif
-        // We want to be sure to keep this time cached until we've both finished running JS and finished updating
-        // animations, so we schedule the invalidation task and register a whenIdle callback on the VM, which will
-        // fire syncronously if no JS is running.
-        m_waitingOnVMIdle = true;
-        if (!m_currentTimeClearingTaskQueue.hasPendingTasks())
-            m_currentTimeClearingTaskQueue.enqueueTask(std::bind(&DocumentTimeline::maybeClearCachedCurrentTime, this));
-        m_document->vm().whenIdle([this, protectedThis = makeRefPtr(this)]() {
-            m_waitingOnVMIdle = false;
-            maybeClearCachedCurrentTime();
-        });
     }
     return m_cachedCurrentTime.value() - m_originTime;
+}
+
+void DocumentTimeline::cacheCurrentTime(Seconds newCurrentTime)
+{
+    m_cachedCurrentTime = newCurrentTime;
+    // We want to be sure to keep this time cached until we've both finished running JS and finished updating
+    // animations, so we schedule the invalidation task and register a whenIdle callback on the VM, which will
+    // fire syncronously if no JS is running.
+    m_waitingOnVMIdle = true;
+    if (!m_currentTimeClearingTaskQueue.hasPendingTasks())
+        m_currentTimeClearingTaskQueue.enqueueTask(std::bind(&DocumentTimeline::maybeClearCachedCurrentTime, this));
+    m_document->vm().whenIdle([this, protectedThis = makeRefPtr(this)]() {
+        m_waitingOnVMIdle = false;
+        maybeClearCachedCurrentTime();
+    });
 }
 
@@ -301 +323 @@
     // we're guaranteed to have a consistent current time reported for all work happening in a given
     // JS frame or throughout updating animations in WebCore.
-    if (!m_waitingOnVMIdle && !m_currentTimeClearingTaskQueue.hasPendingTasks())
+    if (!m_isSuspended && !m_waitingOnVMIdle && !m_currentTimeClearingTaskQueue.hasPendingTasks())
         m_cachedCurrentTime = std::nullopt;
 }
@@ -307 +329 @@
 void DocumentTimeline::scheduleAnimationResolutionIfNeeded()
 {
-    if (!m_isSuspended && !m_animations.isEmpty())
+    if (!m_isUpdatingAnimations && !m_isSuspended && !m_animations.isEmpty())
         scheduleAnimationResolution();
 }
@@ -338 +360 @@
 void DocumentTimeline::unscheduleAnimationResolution()
 {
+    m_tickScheduleTimer.stop();
 #if USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR)
     m_document->animationScheduler().unscheduleWebAnimationsResolution();
@@ -354 +377 @@
 {
     updateAnimationsAndSendEvents();
+    applyPendingAcceleratedAnimations();
+    scheduleNextTick();
 }
 
@@ -359 +384 @@
 {
     m_numberOfAnimationTimelineInvalidationsForTesting++;
+
+    m_isUpdatingAnimations = true;
 
     // https://drafts.csswg.org/web-animations/#update-animations-and-send-events
@@ -423 +450 @@
         transitionDidComplete(completedTransition);
 
-    applyPendingAcceleratedAnimations();
+    m_isUpdatingAnimations = false;
 }
 
@@ -437 +464 @@
         }
     }
+}
+
+void DocumentTimeline::scheduleNextTick()
+{
+    // There is no tick to schedule if we don't have any relevant animations.
+    if (m_animations.isEmpty())
+        return;
+
+    for (const auto& animation : m_animations) {
+        if (!animation->isRunningAccelerated()) {
+            scheduleAnimationResolutionIfNeeded();
+            return;
+        }
+    }
+
+    Seconds scheduleDelay = Seconds::infinity();
+
+    for (const auto& animation : m_animations) {
+        auto animationTimeToNextRequiredTick = animation->timeToNextTick();
+        if (animationTimeToNextRequiredTick < animationInterval()) {
+            scheduleAnimationResolutionIfNeeded();
+            return;
+        }
+        scheduleDelay = std::min(scheduleDelay, animationTimeToNextRequiredTick);
+    }
+
+    if (scheduleDelay < Seconds::infinity())
+        m_tickScheduleTimer.startOneShot(scheduleDelay);
 }
 
@@ -541 +596 @@
     bool runningAnimationsForElementAreAllAccelerated = !animations.isEmpty();
     for (const auto& animation : animations) {
-        if (is<KeyframeEffect>(animation->effect()) && !downcast<KeyframeEffect>(animation->effect())->isRunningAccelerated()) {
+        if (!animation->isRunningAccelerated()) {
             runningAnimationsForElementAreAllAccelerated = false;
             break;

trunk/Source/WebCore/animation/DocumentTimeline.h

@@ -86 +86 @@
     DocumentTimeline(Document&, Seconds);
 
+    Seconds liveCurrentTime() const;
+    void cacheCurrentTime(Seconds);
     void scheduleAnimationResolutionIfNeeded();
     void scheduleInvalidationTaskIfNeeded();
@@ -97 +99 @@
     void updateListOfElementsWithRunningAcceleratedAnimationsForElement(Element&);
     void transitionDidComplete(RefPtr<CSSTransition>);
+    void scheduleNextTick();
 
     RefPtr<Document> m_document;
@@ -102 +105 @@
     bool m_isSuspended { false };
     bool m_waitingOnVMIdle { false };
+    bool m_isUpdatingAnimations { false };
     std::optional<Seconds> m_cachedCurrentTime;
     GenericTaskQueue<Timer> m_currentTimeClearingTaskQueue;
@@ -108 +112 @@
     unsigned m_numberOfAnimationTimelineInvalidationsForTesting { 0 };
     HashSet<Element*> m_elementsWithRunningAcceleratedAnimations;
+    Timer m_tickScheduleTimer;
 
 #if !USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR)

trunk/Source/WebCore/animation/WebAnimation.cpp

@@ -1095 +1095 @@
 }
 
+bool WebAnimation::isRunningAccelerated() const
+{
+    return is<KeyframeEffect>(m_effect) && downcast<KeyframeEffect>(*m_effect).isRunningAccelerated();
+}
+
 bool WebAnimation::needsTick() const
 {
@@ -1115 +1120 @@
 void WebAnimation::resolve(RenderStyle& targetStyle)
 {
-    timingDidChange(DidSeek::No, SynchronouslyNotify::Yes);
+    updateFinishedState(DidSeek::No, SynchronouslyNotify::Yes);
     if (m_effect)
         m_effect->apply(targetStyle);
@@ -1195 +1200 @@
 }
 
+Seconds WebAnimation::timeToNextTick() const
+{
+    ASSERT(isRunningAccelerated());
+
+    if (pending())
+        return 0_s;
+
+    // If we're not running, there's no telling when we'll end.
+    if (playState() != PlayState::Running)
+        return Seconds::infinity();
+
+    // CSS Animations dispatch events for each iteration, so compute the time until
+    // the end of this iteration. Any other animation only cares about remaning total time.
+    if (isCSSAnimation()) {
+        // If we're actively running, we need the time until the next iteration.
+        if (auto iterationProgress = effect()->simpleIterationProgress())
+            return effect()->iterationDuration() * (1 - iterationProgress.value());
+
+        // Otherwise we're probably in the before phase waiting to reach our start time.
+        if (auto animationCurrentTime = currentTime()) {
+            // If our current time is negative, we need to be scheduled to be resolved at the inverse
+            // of our current time, unless we fill backwards, in which case we want to invalidate as
+            // soon as possible.
+            auto localTime = animationCurrentTime.value();
+            if (localTime < 0_s)
+                return -localTime;
+            if (localTime < effect()->delay())
+                return effect()->delay() - localTime;
+        }
+    } else if (auto animationCurrentTime = currentTime())
+        return effect()->endTime() - animationCurrentTime.value();
+
+    ASSERT_NOT_REACHED();
+    return Seconds::infinity();
+}
+
 } // namespace WebCore

trunk/Source/WebCore/animation/WebAnimation.h

@@ -105 +105 @@
     virtual bool needsTick() const;
     virtual void tick();
+    Seconds timeToNextTick() const;
     virtual void resolve(RenderStyle&);
     void effectTargetDidChange(Element* previousTarget, Element* newTarget);
@@ -110 +111 @@
     void applyPendingAcceleratedActions();
 
+    bool isRunningAccelerated() const;
     bool isRelevant() const { return m_isRelevant; }
     void effectTimingDidChange();