Animation and other code is too aggressive about invalidating layer composition
Created attachment 358859 [details] Patch
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.
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
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
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
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
I guess we need some of these.
> 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.
rdar://problem/44366110
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
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
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
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
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
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
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.
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
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!
Created attachment 358933 [details] Patch
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.
Created attachment 358970 [details] Patch
Created attachment 358972 [details] Patch
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.
Created attachment 358973 [details] Testcase
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?
Yes, there are clients: things related to composited iframes, plugins etc.
Comment on attachment 358970 [details] Patch I moved this patch to bug 193435.
Comment on attachment 358972 [details] Patch I moved this patch to bug 193450.
Main patch landed in r239965, other patches landed via other bugs.