Changeset 230703 in webkit


Ignore:
Timestamp:
Apr 17, 2018 12:33:57 AM (6 years ago)
Author:
graouts@webkit.org
Message:

Layout Test animations/needs-layout.html is a flaky Image Failure.
https://bugs.webkit.org/show_bug.cgi?id=172397

Reviewed by Dean Jackson.

Source/WebCore:

Animations that animate a transform and uses a relative value for either the x or y components
require a layout before starting, which CSSAnimationController would perform in the call to
CSSAnimationControllerPrivate::animationTimerFired() made immediately after a CSS animation was
created.

We now perform a similar task where upon setting new blending keyframes we compute a flag indicating
if the keyframe effect is animating a transform with relative x or y components. Then, when we perform
the first invalidation task, which runs in the next run loop after a change to the timing model has
been made, such as a call to play() on a CSSAnimation made in the TreeResolver::createAnimatedElementUpdate()
where the CSSAnimation was created, we call forceLayout() on this element's FrameView. We also ensure
we commit animations on the compositor immediately after that too, instead of waiting until the next
DisplayRefreshMonitor callback.

  • animation/DocumentTimeline.cpp:

(WebCore::DocumentTimeline::performInvalidationTask):
(WebCore::DocumentTimeline::updateAnimations):

  • animation/KeyframeEffectReadOnly.cpp:

(WebCore::KeyframeEffectReadOnly::forceLayoutIfNeeded):
(WebCore::KeyframeEffectReadOnly::setBlendingKeyframes):
(WebCore::KeyframeEffectReadOnly::computedNeedsForcedLayout):
(WebCore::KeyframeEffectReadOnly::applyPendingAcceleratedActions):

  • animation/KeyframeEffectReadOnly.h:

LayoutTests:

No longer mark this test as flaky.

  • platform/ios-wk2/TestExpectations:
  • platform/mac-wk1/TestExpectations:
  • platform/mac-wk2/TestExpectations:
Location:
trunk
Files:
8 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r230691 r230703  
     12018-04-16  Antoine Quint  <graouts@apple.com>
     2
     3        Layout Test animations/needs-layout.html is a flaky Image Failure.
     4        https://bugs.webkit.org/show_bug.cgi?id=172397
     5
     6        Reviewed by Dean Jackson.
     7
     8        No longer mark this test as flaky.
     9
     10        * platform/ios-wk2/TestExpectations:
     11        * platform/mac-wk1/TestExpectations:
     12        * platform/mac-wk2/TestExpectations:
     13
    1142018-04-16  Keith Rollin  <krollin@apple.com>
    215
  • trunk/LayoutTests/platform/ios-wk2/TestExpectations

    r230686 r230703  
    13201320webkit.org/b/177501 webrtc/video-mute.html [ Pass Timeout ]
    13211321
    1322 webkit.org/b/172397 [ Debug ] animations/needs-layout.html [ Pass ImageOnlyFailure ]
    13231322webkit.org/b/172397 [ Debug ] legacy-animation-engine/animations/needs-layout.html [ Pass ImageOnlyFailure ]
    13241323
  • trunk/LayoutTests/platform/mac-wk1/TestExpectations

    r230525 r230703  
    493493[ HighSierra+ ] fast/text/system-font-fallback-emoji.html [ Crash ]
    494494
    495 webkit.org/b/172397 [ Debug ] animations/needs-layout.html [ Pass ImageOnlyFailure ]
    496495webkit.org/b/172397 [ Debug ] legacy-animation-engine/animations/needs-layout.html [ Pass ImageOnlyFailure ]
    497496
  • trunk/LayoutTests/platform/mac-wk2/TestExpectations

    r230651 r230703  
    731731webkit.org/b/167757 workers/bomb.html [ Pass Timeout ]
    732732
    733 webkit.org/b/172397 [ Debug ] animations/needs-layout.html [ Pass ImageOnlyFailure ]
    734733webkit.org/b/172397 [ Debug ] legacy-animation-engine/animations/needs-layout.html [ Pass ImageOnlyFailure ]
    735734
  • trunk/Source/WebCore/ChangeLog

    r230702 r230703  
     12018-04-16  Antoine Quint  <graouts@apple.com>
     2
     3        Layout Test animations/needs-layout.html is a flaky Image Failure.
     4        https://bugs.webkit.org/show_bug.cgi?id=172397
     5
     6        Reviewed by Dean Jackson.
     7
     8        Animations that animate a transform and uses a relative value for either the x or y components
     9        require a layout before starting, which CSSAnimationController would perform in the call to
     10        CSSAnimationControllerPrivate::animationTimerFired() made immediately after a CSS animation was
     11        created.
     12
     13        We now perform a similar task where upon setting new blending keyframes we compute a flag indicating
     14        if the keyframe effect is animating a transform with relative x or y components. Then, when we perform
     15        the first invalidation task, which runs in the next run loop after a change to the timing model has
     16        been made, such as a call to play() on a CSSAnimation made in the TreeResolver::createAnimatedElementUpdate()
     17        where the CSSAnimation was created, we call forceLayout() on this element's FrameView. We also ensure
     18        we commit animations on the compositor immediately after that too, instead of waiting until the next
     19        DisplayRefreshMonitor callback.
     20
     21        * animation/DocumentTimeline.cpp:
     22        (WebCore::DocumentTimeline::performInvalidationTask):
     23        (WebCore::DocumentTimeline::updateAnimations):
     24        * animation/KeyframeEffectReadOnly.cpp:
     25        (WebCore::KeyframeEffectReadOnly::forceLayoutIfNeeded):
     26        (WebCore::KeyframeEffectReadOnly::setBlendingKeyframes):
     27        (WebCore::KeyframeEffectReadOnly::computedNeedsForcedLayout):
     28        (WebCore::KeyframeEffectReadOnly::applyPendingAcceleratedActions):
     29        * animation/KeyframeEffectReadOnly.h:
     30
    1312018-04-16  Pablo Saavedra  <psaavedra@igalia.com>
    232
  • trunk/Source/WebCore/animation/DocumentTimeline.cpp

    r230582 r230703  
    177177    }
    178178
     179    applyPendingAcceleratedAnimations();
     180
    179181    updateAnimationSchedule();
    180182    m_cachedCurrentTime = std::nullopt;
     
    244246        m_document->updateStyleIfNeeded();
    245247    }
    246 
    247     applyPendingAcceleratedAnimations();
    248248
    249249    // Time has advanced, the timing model requires invalidation now.
     
    333333void DocumentTimeline::applyPendingAcceleratedAnimations()
    334334{
    335     for (auto& animation : m_acceleratedAnimationsPendingRunningStateChange)
     335    bool hasForcedLayout = false;
     336    for (auto& animation : m_acceleratedAnimationsPendingRunningStateChange) {
     337        if (!hasForcedLayout) {
     338            auto* effect = animation->effect();
     339            if (is<KeyframeEffectReadOnly>(effect))
     340                hasForcedLayout |= downcast<KeyframeEffectReadOnly>(effect)->forceLayoutIfNeeded();
     341        }
    336342        animation->applyPendingAcceleratedActions();
     343    }
     344
    337345    m_acceleratedAnimationsPendingRunningStateChange.clear();
    338346}
  • trunk/Source/WebCore/animation/KeyframeEffectReadOnly.cpp

    r230595 r230703  
    4747#include "StyleResolver.h"
    4848#include "TimingFunction.h"
     49#include "TranslateTransformOperation.h"
    4950#include "WillChangeData.h"
    5051#include <wtf/UUID.h>
     
    694695}
    695696
     697bool KeyframeEffectReadOnly::forceLayoutIfNeeded()
     698{
     699    if (!m_needsForcedLayout || !m_target)
     700        return false;
     701
     702    auto* renderer = m_target->renderer();
     703    if (!renderer || !renderer->parent())
     704        return false;
     705
     706    auto* frameView = m_target->document().view();
     707    if (!frameView)
     708        return false;
     709
     710    frameView->forceLayout();
     711    return true;
     712}
     713
    696714void KeyframeEffectReadOnly::setBlendingKeyframes(KeyframeList& blendingKeyframes)
    697715{
    698716    m_blendingKeyframes = WTFMove(blendingKeyframes);
    699717
     718    computedNeedsForcedLayout();
    700719    computeStackingContextImpact();
    701720    computeShouldRunAccelerated();
     
    895914    // value than the new end style for this property.
    896915    return !CSSPropertyAnimation::propertiesEqual(property, m_blendingKeyframes[1].style(), &newStyle);
     916}
     917
     918void KeyframeEffectReadOnly::computedNeedsForcedLayout()
     919{
     920    m_needsForcedLayout = false;
     921    if (is<CSSTransition>(animation()) || !m_blendingKeyframes.containsProperty(CSSPropertyTransform))
     922        return;
     923
     924    size_t numberOfKeyframes = m_blendingKeyframes.size();
     925    for (size_t i = 0; i < numberOfKeyframes; i++) {
     926        auto* keyframeStyle = m_blendingKeyframes[i].style();
     927        if (!keyframeStyle) {
     928            ASSERT_NOT_REACHED();
     929            continue;
     930        }
     931        if (keyframeStyle->hasTransform()) {
     932            auto& transformOperations = keyframeStyle->transform();
     933            for (auto operation : transformOperations.operations()) {
     934                if (operation->isTranslateTransformOperationType()) {
     935                    auto translation = downcast<TranslateTransformOperation>(operation.get());
     936                    if (translation->x().isPercent() || translation->y().isPercent()) {
     937                        m_needsForcedLayout = true;
     938                        return;
     939                    }
     940                }
     941            }
     942        }
     943    }
    897944}
    898945
     
    11811228void KeyframeEffectReadOnly::applyPendingAcceleratedActions()
    11821229{
     1230    // Once an accelerated animation has been committed, we no longer want to force a layout.
     1231    // This should have been performed by a call to forceLayoutIfNeeded() prior to applying
     1232    // pending accelerated actions.
     1233    m_needsForcedLayout = false;
     1234
    11831235    auto* renderer = this->renderer();
    11841236    if (!renderer || !renderer->isComposited())
  • trunk/Source/WebCore/animation/KeyframeEffectReadOnly.h

    r230595 r230703  
    121121    bool computeTransformedExtentViaTransformList(const FloatRect&, const RenderStyle&, LayoutRect&) const;
    122122    bool computeTransformedExtentViaMatrix(const FloatRect&, const RenderStyle&, LayoutRect&) const;
     123    bool forceLayoutIfNeeded();
    123124
    124125protected:
     
    137138    TimingFunction* timingFunctionForKeyframeAtIndex(size_t);
    138139    Ref<const Animation> backingAnimationForCompositedRenderer() const;
     140    void computedNeedsForcedLayout();
    139141    void computeStackingContextImpact();
    140142    void updateBlendingKeyframes();
     
    150152
    151153    bool m_shouldRunAccelerated { false };
     154    bool m_needsForcedLayout { false };
    152155    bool m_triggersStackingContext { false };
    153156    bool m_started { false };
Note: See TracChangeset for help on using the changeset viewer.