215229 2020-08-06 11:17:08 -0700 Step animations invalidate style on every rendering update whether or not they need to 2020-08-27 22:26:14 -0700 1 1 1 Unclassified WebKit Animations Safari Technology Preview Unspecified Unspecified RESOLVED FIXED https://bugs.webkit.org/show_bug.cgi?id=214712 https://bugs.webkit.org/show_bug.cgi?id=215241 InRadar P2 Normal --- 1 simon.fraser webkit-unassigned dino graouts graouts koivisto simon.fraser webkit-bug-importer oldest_to_newest 1678309 0 simon.fraser 2020-08-06 11:17:08 -0700 DocumentTimelinesController::updateAnimationsAndSendEvents() is called on every rendering update. It calls DeclarativeAnimation::tick(), which calls down to KeyframeEffect::invalidate() for every animation that is not suspended, unconditionally. That means that every rendering update (which may be triggered by something other than animations) invalidates style, thus causing a style update, which is a power hit. * frame #0: 0x000000069a53549c WebCore`WebCore::KeyframeEffect::invalidate(this=0x00000006bbcc6ae0) at KeyframeEffect.cpp:1241:23 frame #1: 0x000000069a53608d WebCore`WebCore::KeyframeEffect::animationDidTick(this=0x00000006bbcc6ae0) at KeyframeEffect.cpp:1547:5 frame #2: 0x000000069a53ec3d WebCore`WebCore::WebAnimation::tick(this=0x00000006bbbd3c50) at WebAnimation.cpp:1238:19 frame #3: 0x000000069a500c65 WebCore`WebCore::DeclarativeAnimation::tick(this=0x00000006bbbd3c50) at DeclarativeAnimation.cpp:68:19 frame #4: 0x000000069a52b768 WebCore`WebCore::DocumentTimelinesController::updateAnimationsAndSendEvents(this=0x00000006bbc16550, timestamp=(m_value = 24.493000000000002)) at DocumentTimelinesController.cpp:117:24 frame #5: 0x000000069b8ef2cc WebCore`WebCore::Page::updateRendering(this=0x00000006bbc72a58, document={ origin = file://, url = file:///Volumes/Data/Development/system/webkit/testcontent/scrolling/scrolling-tree/fixed-node-order.html, inMainFrame = 1, backForwardCacheState = NotInBackForwardCache })::$_23::operator()(WebCore::Document&) const at Page.cpp:1479:34 frame #6: 0x000000069b8ef233 WebCore`WTF::Detail::CallableWrapper<WebCore::Page::updateRendering()::$_23, void, WebCore::Document&>::call(this=0x00000006bbc72a50, in={ origin = file://, url = file:///Volumes/Data/Development/system/webkit/testcontent/scrolling/scrolling-tree/fixed-node-order.html, inMainFrame = 1, backForwardCacheState = NotInBackForwardCache }) at Function.h:52:39 frame #7: 0x000000069b8b826a WebCore`WTF::Function<void (WebCore::Document&)>::operator(this=0x00007ffee2261028, in={ origin = file://, url = file:///Volumes/Data/Development/system/webkit/testcontent/scrolling/scrolling-tree/fixed-node-order.html, inMainFrame = 1, backForwardCacheState = NotInBackForwardCache })(WebCore::Document&) const at Function.h:83:35 frame #8: 0x000000069b8aa9dc WebCore`WebCore::Page::forEachDocument(this=0x00000006bd9fa000, functor=0x00007ffee2261028)> const&) const at Page.cpp:3109:9 frame #9: 0x000000069b8b115d WebCore`WebCore::Page::updateRendering(this=0x00000006bd9fa000) at Page.cpp:1474:5 frame #10: 0x000000010f36a056 WebKit`WebKit::WebPage::updateRendering(this=0x00007f8839009008) at WebPage.cpp:3864:13 frame #11: 0x000000010edbb4e0 WebKit`WebKit::TiledCoreAnimationDrawingArea::updateRendering(this=0x00000006bbced880, flushType=Normal) at TiledCoreAnimationDrawingArea.mm:459:19 frame #12: 0x000000010edbfded WebKit`WebKit::TiledCoreAnimationDrawingArea::updateRenderingRunLoopCallback(this=0x00000006bbced880) at TiledCoreAnimationDrawingArea.mm:911:5 frame #13: 0x000000010edd0168 WebKit`WebKit::TiledCoreAnimationDrawingArea::TiledCoreAnimationDrawingArea(this=0x00000006bbcfafb8)::$_0::operator()() const at TiledCoreAnimationDrawingArea.mm:88:15 1678311 1 webkit-bug-importer 2020-08-06 11:17:36 -0700 <rdar://problem/66636153> 1678357 2 simon.fraser 2020-08-06 12:51:26 -0700 This is tested by the (failing) animations/steps-transform-rendering-updates.html 1683351 3 407395 koivisto 2020-08-27 06:11:38 -0700 Created attachment 407395 wip 1683359 4 koivisto 2020-08-27 07:15:10 -0700 This bug is specific to step timing functions. Rendering updates will tick animations only if DocumentTimeline::m_animationResolutionScheduled bit is set which correctly doesn't happen during accelerated animations. Step timing functions with transforms try and fail to start accelerated which causes them to repeatedly schedule unnecessary rendering updates. 1683362 5 407397 koivisto 2020-08-27 07:29:22 -0700 Created attachment 407397 patch 1683379 6 ews-feeder 2020-08-27 08:42:12 -0700 Committed r266232: <https://trac.webkit.org/changeset/266232> All reviewed patches have been landed. Closing bug and clearing flags on attachment 407397. 1683408 7 407397 simon.fraser 2020-08-27 09:46:52 -0700 Comment on attachment 407397 patch View in context: https://bugs.webkit.org/attachment.cgi?id=407397&action=review > Source/WebCore/animation/KeyframeEffect.cpp:1498 > + if (m_acceleratedPropertiesState == AcceleratedProperties::None || m_someKeyframesUseStepsTimingFunction || is<StepsTimingFunction>(timingFunction())) It's GraphicsLayerCA that decides that it can't accelerate animations with steps() timing functions, so adding this code here is spreading that logic around into two places. Not great. 1683447 8 koivisto 2020-08-27 12:16:17 -0700 GraphicsLayerCA is a totally wrong place for that sort of logic. Duplication can be removed after the legacy animation code is removed. 1683458 9 simon.fraser 2020-08-27 12:36:58 -0700 But the idea is that some native platforms might be able to support steps() natively. You're hard-coding Core Animation limitations into cross-platform code. 1683474 10 koivisto 2020-08-27 13:30:25 -0700 We should have a better way to communicate that sort of information to the animation engine (when we actually have a reason to abstract it). 1683494 11 graouts 2020-08-27 15:10:32 -0700 We can probably have this logic sit somewhere in between GraphicsLayerCA and KeyframeEffect that would allow for some platform-specific logic. 1683588 12 koivisto 2020-08-27 22:26:14 -0700 Specific accelerated properties are already hardcoded in animation code. This seems like a much bigger deal if we want to be (unnecessarily?) generic. 407395 2020-08-27 06:11:38 -0700 2020-08-27 07:29:22 -0700 wip animation-step-invalidation.patch text/plain 2577 koivisto ZGlmZiAtLWdpdCBhL0xheW91dFRlc3RzL0NoYW5nZUxvZyBiL0xheW91dFRlc3RzL0NoYW5nZUxv ZwppbmRleCA2ZmQ3ZDVkNGE5MzcuLmM4OTYyZjY4ZmFlMCAxMDA2NDQKLS0tIGEvTGF5b3V0VGVz dHMvQ2hhbmdlTG9nCisrKyBiL0xheW91dFRlc3RzL0NoYW5nZUxvZwpAQCAtMSwzICsxLDEzIEBA CisyMDIwLTA4LTI3ICBBbnR0aSBLb2l2aXN0byAgPGFudHRpQGFwcGxlLmNvbT4KKworICAgICAg ICBBbmltYXRpb25zIGludmFsaWRhdGUgc3R5bGUgb24gZXZlcnkgcmVuZGVyaW5nIHVwZGF0ZSB3 aGV0aGVyIG9yIG5vdCB0aGV5IG5lZWQgdG8KKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5v cmcvc2hvd19idWcuY2dpP2lkPTIxNTIyOQorICAgICAgICA8cmRhcjovL3Byb2JsZW0vNjY2MzYx NTM+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgKiBh bmltYXRpb25zL3N0ZXBzLXRyYW5zZm9ybS1yZW5kZXJpbmctdXBkYXRlcy1leHBlY3RlZC50eHQ6 CisKIDIwMjAtMDgtMTcgIFl1c3VrZSBTdXp1a2kgIDx5c3V6dWtpQGFwcGxlLmNvbT4KIAogICAg ICAgICBKU0RPTUNvbnN0cnVjdG9yTm90Q29uc3RydWN0YWJsZSBzaG91bGQgYmUgYSBjb25zdHJ1 Y3RvcgpkaWZmIC0tZ2l0IGEvTGF5b3V0VGVzdHMvYW5pbWF0aW9ucy9zdGVwcy10cmFuc2Zvcm0t cmVuZGVyaW5nLXVwZGF0ZXMtZXhwZWN0ZWQudHh0IGIvTGF5b3V0VGVzdHMvYW5pbWF0aW9ucy9z dGVwcy10cmFuc2Zvcm0tcmVuZGVyaW5nLXVwZGF0ZXMtZXhwZWN0ZWQudHh0CmluZGV4IGRiOWJm ZDZlYmYyMS4uODViMmEzOGNhMjczIDEwMDY0NAotLS0gYS9MYXlvdXRUZXN0cy9hbmltYXRpb25z L3N0ZXBzLXRyYW5zZm9ybS1yZW5kZXJpbmctdXBkYXRlcy1leHBlY3RlZC50eHQKKysrIGIvTGF5 b3V0VGVzdHMvYW5pbWF0aW9ucy9zdGVwcy10cmFuc2Zvcm0tcmVuZGVyaW5nLXVwZGF0ZXMtZXhw ZWN0ZWQudHh0CkBAIC0xLDUgKzEsNSBAQAogUEFTUyBjb3VudCBpcyAwCi1GQUlMIGNvdW50IDwg NiBzaG91bGQgYmUgdHJ1ZS4gV2FzIGZhbHNlLgorUEFTUyBjb3VudCA8IDYgaXMgdHJ1ZQogUEFT UyBzdWNjZXNzZnVsbHlQYXJzZWQgaXMgdHJ1ZQogCiBURVNUIENPTVBMRVRFCmRpZmYgLS1naXQg YS9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5k ZXggOWM1NGM1MDg0NDNkLi4yYWQ3ZjM4OWRhYTAgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3Jl L0NoYW5nZUxvZworKysgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxNiBA QAorMjAyMC0wOC0yNyAgQW50dGkgS29pdmlzdG8gIDxhbnR0aUBhcHBsZS5jb20+CisKKyAgICAg ICAgQW5pbWF0aW9ucyBpbnZhbGlkYXRlIHN0eWxlIG9uIGV2ZXJ5IHJlbmRlcmluZyB1cGRhdGUg d2hldGhlciBvciBub3QgdGhleSBuZWVkIHRvCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQu b3JnL3Nob3dfYnVnLmNnaT9pZD0yMTUyMjkKKyAgICAgICAgPHJkYXI6Ly9wcm9ibGVtLzY2NjM2 MTUzPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgICog YW5pbWF0aW9uL0tleWZyYW1lRWZmZWN0LmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OktleWZyYW1l RWZmZWN0Ojp1cGRhdGVBY2NlbGVyYXRlZEFjdGlvbnMpOgorCisgICAgICAgIFN0ZXAgdGltaW5n IGZ1bmN0aW9ucyBhcmUgbmV2ZXIgYWNjZWxlcmF0ZWQgYnV0IHRyeWluZyB0byBzdGFydCB0aGVt IGFjY2VsZXJhdGVkIGxlYWRzIHRvIGFuIGludmFsaWRhdGlvbiBsb29wLgorCiAyMDIwLTA4LTE3 ICBZdXN1a2UgU3V6dWtpICA8eXN1enVraUBhcHBsZS5jb20+CiAKICAgICAgICAgSlNET01Db25z dHJ1Y3Rvck5vdENvbnN0cnVjdGFibGUgc2hvdWxkIGJlIGEgY29uc3RydWN0b3IKZGlmZiAtLWdp dCBhL1NvdXJjZS9XZWJDb3JlL2FuaW1hdGlvbi9LZXlmcmFtZUVmZmVjdC5jcHAgYi9Tb3VyY2Uv V2ViQ29yZS9hbmltYXRpb24vS2V5ZnJhbWVFZmZlY3QuY3BwCmluZGV4IDAzZTRhYmY0ODBiYi4u NmQwYTMzZjI3YTdkIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9hbmltYXRpb24vS2V5ZnJh bWVFZmZlY3QuY3BwCisrKyBiL1NvdXJjZS9XZWJDb3JlL2FuaW1hdGlvbi9LZXlmcmFtZUVmZmVj dC5jcHAKQEAgLTE0OTUsNyArMTQ5NSw3IEBAIFRpbWluZ0Z1bmN0aW9uKiBLZXlmcmFtZUVmZmVj dDo6dGltaW5nRnVuY3Rpb25Gb3JLZXlmcmFtZUF0SW5kZXgoc2l6ZV90IGluZGV4KSBjCiAKIHZv aWQgS2V5ZnJhbWVFZmZlY3Q6OnVwZGF0ZUFjY2VsZXJhdGVkQWN0aW9ucygpCiB7Ci0gICAgaWYg KG1fYWNjZWxlcmF0ZWRQcm9wZXJ0aWVzU3RhdGUgPT0gQWNjZWxlcmF0ZWRQcm9wZXJ0aWVzOjpO b25lKQorICAgIGlmIChtX2FjY2VsZXJhdGVkUHJvcGVydGllc1N0YXRlID09IEFjY2VsZXJhdGVk UHJvcGVydGllczo6Tm9uZSB8fCBtX3NvbWVLZXlmcmFtZXNVc2VTdGVwc1RpbWluZ0Z1bmN0aW9u KQogICAgICAgICByZXR1cm47CiAKICAgICBhdXRvIGNvbXB1dGVkVGltaW5nID0gZ2V0Q29tcHV0 ZWRUaW1pbmcoKTsK 407397 2020-08-27 07:29:22 -0700 2020-08-27 08:42:12 -0700 patch animation-step-invalidation-2.patch text/plain 2846 koivisto ZGlmZiAtLWdpdCBhL0xheW91dFRlc3RzL0NoYW5nZUxvZyBiL0xheW91dFRlc3RzL0NoYW5nZUxv ZwppbmRleCA2ZmQ3ZDVkNGE5MzcuLmUwMjgzMjg1ZDY1MyAxMDA2NDQKLS0tIGEvTGF5b3V0VGVz dHMvQ2hhbmdlTG9nCisrKyBiL0xheW91dFRlc3RzL0NoYW5nZUxvZwpAQCAtMSwzICsxLDEzIEBA CisyMDIwLTA4LTI3ICBBbnR0aSBLb2l2aXN0byAgPGFudHRpQGFwcGxlLmNvbT4KKworICAgICAg ICBTdGVwIGFuaW1hdGlvbnMgaW52YWxpZGF0ZSBzdHlsZSBvbiBldmVyeSByZW5kZXJpbmcgdXBk YXRlIHdoZXRoZXIgb3Igbm90IHRoZXkgbmVlZCB0bworICAgICAgICBodHRwczovL2J1Z3Mud2Vi a2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MjE1MjI5CisgICAgICAgIDxyZGFyOi8vcHJvYmxlbS82 NjYzNjE1Mz4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAg ICAqIGFuaW1hdGlvbnMvc3RlcHMtdHJhbnNmb3JtLXJlbmRlcmluZy11cGRhdGVzLWV4cGVjdGVk LnR4dDoKKwogMjAyMC0wOC0xNyAgWXVzdWtlIFN1enVraSAgPHlzdXp1a2lAYXBwbGUuY29tPgog CiAgICAgICAgIEpTRE9NQ29uc3RydWN0b3JOb3RDb25zdHJ1Y3RhYmxlIHNob3VsZCBiZSBhIGNv bnN0cnVjdG9yCmRpZmYgLS1naXQgYS9MYXlvdXRUZXN0cy9hbmltYXRpb25zL3N0ZXBzLXRyYW5z Zm9ybS1yZW5kZXJpbmctdXBkYXRlcy1leHBlY3RlZC50eHQgYi9MYXlvdXRUZXN0cy9hbmltYXRp b25zL3N0ZXBzLXRyYW5zZm9ybS1yZW5kZXJpbmctdXBkYXRlcy1leHBlY3RlZC50eHQKaW5kZXgg ZGI5YmZkNmViZjIxLi44NWIyYTM4Y2EyNzMgMTAwNjQ0Ci0tLSBhL0xheW91dFRlc3RzL2FuaW1h dGlvbnMvc3RlcHMtdHJhbnNmb3JtLXJlbmRlcmluZy11cGRhdGVzLWV4cGVjdGVkLnR4dAorKysg Yi9MYXlvdXRUZXN0cy9hbmltYXRpb25zL3N0ZXBzLXRyYW5zZm9ybS1yZW5kZXJpbmctdXBkYXRl cy1leHBlY3RlZC50eHQKQEAgLTEsNSArMSw1IEBACiBQQVNTIGNvdW50IGlzIDAKLUZBSUwgY291 bnQgPCA2IHNob3VsZCBiZSB0cnVlLiBXYXMgZmFsc2UuCitQQVNTIGNvdW50IDwgNiBpcyB0cnVl CiBQQVNTIHN1Y2Nlc3NmdWxseVBhcnNlZCBpcyB0cnVlCiAKIFRFU1QgQ09NUExFVEUKZGlmZiAt LWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxv ZwppbmRleCA5YzU0YzUwODQ0M2QuLjJjMGE4NDFkNWExMSAxMDA2NDQKLS0tIGEvU291cmNlL1dl YkNvcmUvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsx LDE5IEBACisyMDIwLTA4LTI3ICBBbnR0aSBLb2l2aXN0byAgPGFudHRpQGFwcGxlLmNvbT4KKwor ICAgICAgICBTdGVwIGFuaW1hdGlvbnMgaW52YWxpZGF0ZSBzdHlsZSBvbiBldmVyeSByZW5kZXJp bmcgdXBkYXRlIHdoZXRoZXIgb3Igbm90IHRoZXkgbmVlZCB0bworICAgICAgICBodHRwczovL2J1 Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MjE1MjI5CisgICAgICAgIDxyZGFyOi8vcHJv YmxlbS82NjYzNjE1Mz4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKwor ICAgICAgICBTdGVwIHRpbWluZyBmdW5jdGlvbnMgd2l0aCB0cmFuc2Zvcm1zIHRyeSBhbmQgZmFp bCB0byBzdGFydCBhY2NlbGVyYXRlZCB3aGljaCBjYXVzZXMgdGhlbSB0byByZXBlYXRlZGx5IHNj aGVkdWxlIHVubmVjZXNzYXJ5IHJlbmRlcmluZyB1cGRhdGVzLgorCisgICAgICAgICogYW5pbWF0 aW9uL0tleWZyYW1lRWZmZWN0LmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OktleWZyYW1lRWZmZWN0 Ojp1cGRhdGVBY2NlbGVyYXRlZEFjdGlvbnMpOgorCisgICAgICAgIFN0ZXAgdGltaW5nIGZ1bmN0 aW9ucyBhcmUgbmV2ZXIgYWNjZWxlcmF0ZWQgc28gZG9uJ3QgYm90aGVyIHRyeWluZyB0byBzdGFy dCB0aGVtIGluIGFjY2VsZXJhdGVkIHN0YXRlLgorICAgICAgICBUaGlzIGlzIHNpbWlsYXIgdG8g dHJlYXRtZWFudCBvZiB1bmFjY2VsZXJhdGVkIHByb3BlcnRpZXMuCisKIDIwMjAtMDgtMTcgIFl1 c3VrZSBTdXp1a2kgIDx5c3V6dWtpQGFwcGxlLmNvbT4KIAogICAgICAgICBKU0RPTUNvbnN0cnVj dG9yTm90Q29uc3RydWN0YWJsZSBzaG91bGQgYmUgYSBjb25zdHJ1Y3RvcgpkaWZmIC0tZ2l0IGEv U291cmNlL1dlYkNvcmUvYW5pbWF0aW9uL0tleWZyYW1lRWZmZWN0LmNwcCBiL1NvdXJjZS9XZWJD b3JlL2FuaW1hdGlvbi9LZXlmcmFtZUVmZmVjdC5jcHAKaW5kZXggMDNlNGFiZjQ4MGJiLi5lMjZi NTA2OGM0YjMgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL2FuaW1hdGlvbi9LZXlmcmFtZUVm ZmVjdC5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvYW5pbWF0aW9uL0tleWZyYW1lRWZmZWN0LmNw cApAQCAtMTQ5NSw3ICsxNDk1LDcgQEAgVGltaW5nRnVuY3Rpb24qIEtleWZyYW1lRWZmZWN0Ojp0 aW1pbmdGdW5jdGlvbkZvcktleWZyYW1lQXRJbmRleChzaXplX3QgaW5kZXgpIGMKIAogdm9pZCBL ZXlmcmFtZUVmZmVjdDo6dXBkYXRlQWNjZWxlcmF0ZWRBY3Rpb25zKCkKIHsKLSAgICBpZiAobV9h Y2NlbGVyYXRlZFByb3BlcnRpZXNTdGF0ZSA9PSBBY2NlbGVyYXRlZFByb3BlcnRpZXM6Ok5vbmUp CisgICAgaWYgKG1fYWNjZWxlcmF0ZWRQcm9wZXJ0aWVzU3RhdGUgPT0gQWNjZWxlcmF0ZWRQcm9w ZXJ0aWVzOjpOb25lIHx8IG1fc29tZUtleWZyYW1lc1VzZVN0ZXBzVGltaW5nRnVuY3Rpb24gfHwg aXM8U3RlcHNUaW1pbmdGdW5jdGlvbj4odGltaW5nRnVuY3Rpb24oKSkpCiAgICAgICAgIHJldHVy bjsKIAogICAgIGF1dG8gY29tcHV0ZWRUaW1pbmcgPSBnZXRDb21wdXRlZFRpbWluZygpOwo=