Bug 193343 – Animation and other code is too aggressive about invalidating layer composition

• Status: RESOLVED
• Resolution: FIXED
• Component: Animations

[Simon Fraser (smfr)]

Animation and other code is too aggressive about invalidating layer composition

[Simon Fraser (smfr)]

Created attachment 358859 [details]
Patch

[EWS Watchlist]

Attachment 358859 [details] did not pass style-queue:


ERROR: Source/WebCore/ChangeLog:8:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 1 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[EWS Watchlist]

Comment on attachment 358859 [details]
Patch

Attachment 358859 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/10705314

New failing tests:
legacy-animation-engine/compositing/visible-rect/animated-from-none.html
legacy-animation-engine/compositing/animation/animation-compositing.html
legacy-animation-engine/compositing/visible-rect/animated.html
legacy-animation-engine/compositing/layer-creation/translate-animation-overlap.html
legacy-animation-engine/compositing/contents-scale/animating.html
legacy-animation-engine/compositing/backing/backing-store-attachment-empty-keyframe.html
legacy-animation-engine/compositing/layer-creation/scale-rotation-animation-overlap.html
legacy-animation-engine/compositing/layer-creation/overlap-animation.html
css3/filters/reference-filter-update-on-attribute-change.html
css3/filters/effect-reference-delete.html
legacy-animation-engine/compositing/layer-creation/overlap-animation-container.html
legacy-animation-engine/compositing/layer-creation/overlap-animation-clipping.html

[EWS Watchlist]

Created attachment 358865 [details]
Archive of layout-test-results from ews102 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-sierra  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 358859 [details]
Patch

Attachment 358859 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/10705321

New failing tests:
legacy-animation-engine/compositing/visible-rect/animated-from-none.html
legacy-animation-engine/compositing/animation/animation-compositing.html
legacy-animation-engine/compositing/visible-rect/animated.html
legacy-animation-engine/compositing/layer-creation/translate-animation-overlap.html
legacy-animation-engine/compositing/contents-scale/animating.html
legacy-animation-engine/compositing/backing/backing-store-attachment-empty-keyframe.html
legacy-animation-engine/compositing/layer-creation/scale-rotation-animation-overlap.html
legacy-animation-engine/compositing/layer-creation/overlap-animation.html
css3/filters/reference-filter-update-on-attribute-change.html
css3/filters/effect-reference-delete.html
legacy-animation-engine/compositing/layer-creation/overlap-animation-container.html
legacy-animation-engine/compositing/layer-creation/overlap-animation-clipping.html

[EWS Watchlist]

Created attachment 358867 [details]
Archive of layout-test-results from ews104 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6

[Simon Fraser (smfr)]

I guess we need some of these.

[Simon Fraser (smfr)]

> New failing tests:
> legacy-animation-engine/compositing/visible-rect/animated-from-none.html
> legacy-animation-engine/compositing/animation/animation-compositing.html
> legacy-animation-engine/compositing/visible-rect/animated.html
> legacy-animation-engine/compositing/layer-creation/translate-animation-
> overlap.html
> legacy-animation-engine/compositing/contents-scale/animating.html
> legacy-animation-engine/compositing/backing/backing-store-attachment-empty-
> keyframe.html
> legacy-animation-engine/compositing/layer-creation/scale-rotation-animation-
> overlap.html
> legacy-animation-engine/compositing/layer-creation/overlap-animation.html
> css3/filters/reference-filter-update-on-attribute-change.html
> css3/filters/effect-reference-delete.html
> legacy-animation-engine/compositing/layer-creation/overlap-animation-
> container.html
> legacy-animation-engine/compositing/layer-creation/overlap-animation-
> clipping.html

Most of these tests seem to dump layers in response to an animationStart event, and that timing changes with this patch. The tests look OK when run manually; I think we'll just create the layers later.

[Simon Fraser (smfr)]

rdar://problem/44366110

[EWS Watchlist]

Comment on attachment 358859 [details]
Patch

Attachment 358859 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/10705320

New failing tests:
legacy-animation-engine/compositing/animation/animation-compositing.html
legacy-animation-engine/compositing/visible-rect/animated.html
legacy-animation-engine/compositing/layer-creation/translate-animation-overlap.html
legacy-animation-engine/compositing/contents-scale/animating.html
legacy-animation-engine/compositing/backing/backing-store-attachment-empty-keyframe.html
legacy-animation-engine/compositing/layer-creation/scale-rotation-animation-overlap.html
legacy-animation-engine/compositing/layer-creation/overlap-animation.html
legacy-animation-engine/compositing/layer-creation/overlap-animation-container.html
css3/filters/effect-reference-delete.html
css3/filters/reference-filter-update-on-attribute-change.html
legacy-animation-engine/compositing/layer-creation/overlap-animation-clipping.html

[EWS Watchlist]

Created attachment 358869 [details]
Archive of layout-test-results from ews123 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews123  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6

[EWS Watchlist]

Comment on attachment 358859 [details]
Patch

Attachment 358859 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/10705357

New failing tests:
legacy-animation-engine/compositing/visible-rect/animated-from-none.html
legacy-animation-engine/compositing/visible-rect/animated.html
legacy-animation-engine/compositing/layer-creation/translate-animation-overlap.html
legacy-animation-engine/compositing/contents-scale/animating.html
legacy-animation-engine/compositing/backing/backing-store-attachment-empty-keyframe.html
legacy-animation-engine/compositing/layer-creation/scale-rotation-animation-overlap.html
legacy-animation-engine/compositing/layer-creation/overlap-animation.html
css3/filters/reference-filter-update-on-attribute-change.html
legacy-animation-engine/compositing/layer-creation/overlap-animation-container.html
legacy-animation-engine/compositing/layer-creation/overlap-animation-clipping.html

[EWS Watchlist]

Created attachment 358870 [details]
Archive of layout-test-results from ews113 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews113  Port: mac-sierra  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 358859 [details]
Patch

Attachment 358859 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/10706094

New failing tests:
css3/filters/effect-reference-delete.html
css3/filters/reference-filter-update-on-attribute-change.html

[EWS Watchlist]

Created attachment 358874 [details]
Archive of layout-test-results from ews205 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews205  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

[Simon Fraser (smfr)]

This patch breaks accelerated animations, because we don't do a compositing update after the animation starts (AnimationState New -> StartWaitTimer), so end up running the animation in software.

[Simon Fraser (smfr)]

We need to have already created the composing layer before:

  * frame #0: 0x0000000108bf98d4 WebCore`WebCore::KeyframeAnimation::startAnimation(this=0x0000000122dc95b0, timeOffset=0) at KeyframeAnimation.cpp:288
    frame #1: 0x0000000108baa805 WebCore`WebCore::AnimationBase::updateStateMachine(this=0x0000000122dc95b0, input=StyleAvailable, param=-1) at AnimationBase.cpp:276
    frame #2: 0x0000000108bb3378 WebCore`WebCore::AnimationBase::styleAvailable(this=0x0000000122dc95b0) at AnimationBase.h:207
    frame #3: 0x0000000108bb1c6d WebCore`WebCore::CSSAnimationControllerPrivate::styleAvailable(this=0x0000000122d996e0) at CSSAnimationController.cpp:529
    frame #4: 0x0000000108bb1b7c WebCore`WebCore::CSSAnimationControllerPrivate::endAnimationUpdate(this=0x0000000122d996e0) at CSSAnimationController.cpp:458
    frame #5: 0x0000000108bc26f8 WebCore`WebCore::AnimationPrivateUpdateBlock::~AnimationPrivateUpdateBlock(this=0x00007ffeefbfcbf0) at CSSAnimationController.cpp:65
    frame #6: 0x0000000108bafb35 WebCore`WebCore::AnimationPrivateUpdateBlock::~AnimationPrivateUpdateBlock(this=0x00007ffeefbfcbf0) at CSSAnimationController.cpp:64
    frame #7: 0x0000000108badebd WebCore`WebCore::CSSAnimationControllerPrivate::animationTimerFired(this=0x0000000122d996e0) at CSSAnimationController.cpp:293

[Simon Fraser (smfr)]

This is a delicate dance about doing compositing updates at the right time. We have to do a compositing update when the animation is in StartWaitStyleAvailable, not before!

[Simon Fraser (smfr)]

Created attachment 358933 [details]
Patch

[EWS Watchlist]

Attachment 358933 [details] did not pass style-queue:


ERROR: Source/WebCore/ChangeLog:8:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 1 in 12 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Simon Fraser (smfr)]

Created attachment 358970 [details]
Patch

[Simon Fraser (smfr)]

Created attachment 358972 [details]
Patch

[Simon Fraser (smfr)]

This patch sequence tidies up the legacy animation code path so that only animation changes that start/stop accelerated animations trigger recompositeLayer.

The Web Animations code path is fixed by the first patch.

[Simon Fraser (smfr)]

Created attachment 358973 [details]
Testcase

[Antti Koivisto]

Comment on attachment 358933 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=358933&action=review

> Source/WebCore/animation/KeyframeEffect.cpp:63
> -        element->invalidateStyleAndLayerComposition();
> +        element->invalidateStyle();

Are there any clients left for invalidateStyleAndLayerComposition after this? Can we remove the whole concept?

[Simon Fraser (smfr)]

Yes, there are clients: things related to composited iframes, plugins etc.

[Simon Fraser (smfr)]

Comment on attachment 358970 [details]
Patch

I moved this patch to bug 193435.

[Simon Fraser (smfr)]

Comment on attachment 358972 [details]
Patch

I moved this patch to bug 193450.

[Simon Fraser (smfr)]

Main patch landed in r239965, other patches landed via other bugs.