Animations work under SPv2. |
||||||||||||||||||||
Issue descriptionCreating bug to track details of getting some of the basic test cases under virtual/threaded/animations that fail today working on SPv2. Focusing on: virtual/threaded/animations/composited-animation-style-update.html which can be tested locally via: % ./out/Default/content_shell --expose-internals-for-testing --enable-slimming-paint-v2 --enable-threaded-compositing --disable-composited-antialiasing third_party/WebKit/LayoutTests/virtual/threaded/animations/composited-animation-style-update.html Comparing with and without SPv2 flag, we never see the composited animation come into being. CompositorPendingAnimations::update() considers the animation, which returns false for hasActiveAnimationsOnCompositor() with SPv2, and true without. hasActiveAnimationsOnCompositor() is driven by the associated KeyframeEffectReadOnly, which has an internal list of animation ids for that keyframe effect. Those ids are populated when started via KeyframeEffectReadOnly::maybeStartAnimationOnCompositor(), which passes the mutable list of animation ids on to CompositorAnimations::startAnimationOnCompositor(). Via logging we determine that CompositorAnimations::canStartAnimationOnCompositor() returns false in SPv2, due to the target layout object failing this check: element.layoutObject()->compositingState() == PaintsIntoOwnBacking This failure is easily explained as we no longer run the legacy compositing logic which is surely where that state is set in SPv1. Need to look into details of SPv1 logic that produces PaintsIntoOwnBacking state. Not sure whether it's safe to just blindly presume that an animation otherwise deemed appropriate for compositing will end up as such with new PaintArtifactCompositor logic. It looks like the animation is falling into an alternate code path in CompositorPendingAnimations::update(), since it did not end up in the startedSynchronizedOnCompositor case, where it's tacked onto waitingForStartTime. While we could explore the separate issue of why it isn't eventually started on that path (am guessing due to our discussion earlier today leading to http://crbug.com/674258 ), I'm sure we want this synchronized start path to still function for performance reasons if possible.
,
Dec 15 2016
Proposal from email: for SPv2, rather than checking compositing state on the object's paint layer, check whether the element has a transform or opacity node on its ObjectPaintProperties that has a direct compositing reason. Ref: http://crrev.com/2571043002.
,
Dec 20 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2bf83fd5b59fe18bc2a25e05f29a965a4887dc6b commit 2bf83fd5b59fe18bc2a25e05f29a965a4887dc6b Author: wkorman <wkorman@chromium.org> Date: Tue Dec 20 01:37:12 2016 Support compositing for active animations in SPv2. Revise CompositorAnimations::canStartAnimationOnCompositor() to consider direct compositing reasons rather than layer compositing state. BUG= 674317 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Review-Url: https://codereview.chromium.org/2578343002 Cr-Commit-Position: refs/heads/master@{#439657} [modify] https://crrev.com/2bf83fd5b59fe18bc2a25e05f29a965a4887dc6b/third_party/WebKit/LayoutTests/virtual/threaded/animations/composited-animation-style-update.html [modify] https://crrev.com/2bf83fd5b59fe18bc2a25e05f29a965a4887dc6b/third_party/WebKit/Source/core/animation/CompositorAnimations.cpp [modify] https://crrev.com/2bf83fd5b59fe18bc2a25e05f29a965a4887dc6b/third_party/WebKit/Source/core/animation/CompositorAnimationsTest.cpp [modify] https://crrev.com/2bf83fd5b59fe18bc2a25e05f29a965a4887dc6b/third_party/WebKit/Source/core/animation/CompositorPendingAnimations.cpp [modify] https://crrev.com/2bf83fd5b59fe18bc2a25e05f29a965a4887dc6b/third_party/WebKit/Source/core/layout/LayoutObject.h [modify] https://crrev.com/2bf83fd5b59fe18bc2a25e05f29a965a4887dc6b/third_party/WebKit/Source/core/layout/LayoutObjectTest.cpp [modify] https://crrev.com/2bf83fd5b59fe18bc2a25e05f29a965a4887dc6b/third_party/WebKit/Source/core/paint/PaintLayer.cpp [modify] https://crrev.com/2bf83fd5b59fe18bc2a25e05f29a965a4887dc6b/third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp [modify] https://crrev.com/2bf83fd5b59fe18bc2a25e05f29a965a4887dc6b/third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilderTest.cpp [modify] https://crrev.com/2bf83fd5b59fe18bc2a25e05f29a965a4887dc6b/third_party/WebKit/Source/platform/graphics/CompositingReasons.cpp [modify] https://crrev.com/2bf83fd5b59fe18bc2a25e05f29a965a4887dc6b/third_party/WebKit/Source/web/tests/WebPluginContainerTest.cpp
,
Dec 21 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/03fe8f6efab3e8e3cc7833c8a7534d2c633bb308 commit 03fe8f6efab3e8e3cc7833c8a7534d2c633bb308 Author: wkorman <wkorman@chromium.org> Date: Wed Dec 21 21:17:54 2016 Constrain transform animation direct compositing reason to transforms. Only affects SPv2 code path. Follow-up to http://crrev.com/2578343002 which was overly broad in considering animation state. BUG= 674317 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Review-Url: https://codereview.chromium.org/2593833002 Cr-Commit-Position: refs/heads/master@{#440215} [modify] https://crrev.com/03fe8f6efab3e8e3cc7833c8a7534d2c633bb308/third_party/WebKit/Source/core/layout/compositing/CompositingReasonFinder.cpp [modify] https://crrev.com/03fe8f6efab3e8e3cc7833c8a7534d2c633bb308/third_party/WebKit/Source/core/layout/compositing/CompositingReasonFinder.h [modify] https://crrev.com/03fe8f6efab3e8e3cc7833c8a7534d2c633bb308/third_party/WebKit/Source/core/layout/compositing/CompositingReasonFinderTest.cpp [modify] https://crrev.com/03fe8f6efab3e8e3cc7833c8a7534d2c633bb308/third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp [modify] https://crrev.com/03fe8f6efab3e8e3cc7833c8a7534d2c633bb308/third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilderTest.cpp [modify] https://crrev.com/03fe8f6efab3e8e3cc7833c8a7534d2c633bb308/third_party/WebKit/Source/core/paint/PaintPropertyTreeUpdateTests.cpp
,
Dec 22 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fbbcd30726ccc750fb8434b0992e7a006b106473 commit fbbcd30726ccc750fb8434b0992e7a006b106473 Author: wkorman <wkorman@chromium.org> Date: Thu Dec 22 01:20:11 2016 Create compositor animations for effect animations in SPv2. Further work needed to complete starting animation cc-side inclusive of routing start time back to Blink. Addition of direct compositing reason to EffectPaintProperyNode in this change is lifted from http://crrev.com/2581843002. BUG= 674317 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Review-Url: https://codereview.chromium.org/2596623002 Cr-Commit-Position: refs/heads/master@{#440295} [modify] https://crrev.com/fbbcd30726ccc750fb8434b0992e7a006b106473/third_party/WebKit/Source/core/animation/CompositorAnimations.cpp [modify] https://crrev.com/fbbcd30726ccc750fb8434b0992e7a006b106473/third_party/WebKit/Source/core/animation/CompositorAnimationsTest.cpp [modify] https://crrev.com/fbbcd30726ccc750fb8434b0992e7a006b106473/third_party/WebKit/Source/core/layout/LayoutObject.h [modify] https://crrev.com/fbbcd30726ccc750fb8434b0992e7a006b106473/third_party/WebKit/Source/core/layout/compositing/CompositingReasonFinder.cpp [modify] https://crrev.com/fbbcd30726ccc750fb8434b0992e7a006b106473/third_party/WebKit/Source/core/layout/compositing/CompositingReasonFinder.h [modify] https://crrev.com/fbbcd30726ccc750fb8434b0992e7a006b106473/third_party/WebKit/Source/core/layout/compositing/CompositingReasonFinderTest.cpp [modify] https://crrev.com/fbbcd30726ccc750fb8434b0992e7a006b106473/third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp [modify] https://crrev.com/fbbcd30726ccc750fb8434b0992e7a006b106473/third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilderTest.cpp [modify] https://crrev.com/fbbcd30726ccc750fb8434b0992e7a006b106473/third_party/WebKit/Source/core/paint/PaintPropertyTreeUpdateTests.cpp [modify] https://crrev.com/fbbcd30726ccc750fb8434b0992e7a006b106473/third_party/WebKit/Source/platform/graphics/paint/EffectPaintPropertyNode.h
,
Dec 22 2016
Note re: next work item: CompositorAnimations::attachCompositedLayers() never ends up calling CompositorAnimationPlayer::attachElement() due to early-out requiring existence of LayoutObject -> PaintLayer -> CompositedLayerMapping which is not SPv2 compatible.
,
Dec 28 2016
Rough patch set plan to allow for smaller incremental changes: 1. generate and store compositor element ids on transform/effect paint property nodes when there is an animation present for them 2. update PaintArtifactCompositor to generate maps from compositor element id to cc property tree nodes and set them in cc without cc yet making use of them 3. update cc PropertyTreeBuilder to generate those same maps for SPv1 but not yet make use of them 4. update cc to actually use the maps from (2) and (3), basically, the rest of http://crbug.com/674258#c1 5. order TBD, but somewhere we need to skip the CLM check noted in c#6 above when SPv2 is enabled. We reserve the right to change the patch set plan as new information comes to light.
,
Jan 3 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8a4ccdb5a2bef0e128a64cea83fa9de3daf785a8 commit 8a4ccdb5a2bef0e128a64cea83fa9de3daf785a8 Author: wkorman <wkorman@chromium.org> Date: Tue Jan 03 23:55:33 2017 Store compositor element id in paint properties for animated objects. BUG= 674258 , 674317 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Review-Url: https://codereview.chromium.org/2608543002 Cr-Commit-Position: refs/heads/master@{#441255} [modify] https://crrev.com/8a4ccdb5a2bef0e128a64cea83fa9de3daf785a8/third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp [modify] https://crrev.com/8a4ccdb5a2bef0e128a64cea83fa9de3daf785a8/third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilderTest.cpp [modify] https://crrev.com/8a4ccdb5a2bef0e128a64cea83fa9de3daf785a8/third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilderTest.h [modify] https://crrev.com/8a4ccdb5a2bef0e128a64cea83fa9de3daf785a8/third_party/WebKit/Source/core/paint/PaintPropertyTreeUpdateTests.cpp [add] https://crrev.com/8a4ccdb5a2bef0e128a64cea83fa9de3daf785a8/third_party/WebKit/Source/core/paint/test_data/opacity-animation.html [add] https://crrev.com/8a4ccdb5a2bef0e128a64cea83fa9de3daf785a8/third_party/WebKit/Source/core/paint/test_data/transform-animation.html [modify] https://crrev.com/8a4ccdb5a2bef0e128a64cea83fa9de3daf785a8/third_party/WebKit/Source/platform/graphics/paint/EffectPaintPropertyNode.cpp [modify] https://crrev.com/8a4ccdb5a2bef0e128a64cea83fa9de3daf785a8/third_party/WebKit/Source/platform/graphics/paint/EffectPaintPropertyNode.h [modify] https://crrev.com/8a4ccdb5a2bef0e128a64cea83fa9de3daf785a8/third_party/WebKit/Source/platform/graphics/paint/TransformPaintPropertyNode.cpp [modify] https://crrev.com/8a4ccdb5a2bef0e128a64cea83fa9de3daf785a8/third_party/WebKit/Source/platform/graphics/paint/TransformPaintPropertyNode.h
,
Jan 19 2017
Brief update on http://crbug.com/674317#c7 have done #1, 2, and 4, and am now working on #3 and 5, with whatever discoveries ensue. Have been submitting changes against http://crbug.com/674258 mostly.
,
Jan 19 2017
A few changes I am planning to prepare per discussion today: 1a. Add compositor element id field to ScrollPaintPropertyNode, similar to what we did in http://crrev.com/2608543002. These nodes are created/updated from a few different places, so I am presuming there will be an apparent owning layout object (from the ScrollableArea) from each callsite. 1b. ScrollPaintPropertyNode has an associated TransformPaintPropertyNode for the scroll offset. This may also need the compositor element id set on it. Maybe won't do this until we determine it is in fact needed unless someone has feedback. I think this also has multiple update callsites. 2. Call SetElementId on cc::Layer when we create it in PaintArtifactCompositor, pulling the element id from any of the involved scroll/effect/transform paint property nodes. For loyso@ awareness, a comment from ajuma@: "...ideally we'd remove [the fact that animation logic around registering elements depends on Layers] eventually so that property tree nodes can stand on their own (without assuming an owning layer)".
,
Jan 21 2017
For anyone following along, work in flight, referencing comment and items above: c#10: 1a = http://crrev.com/2638333007 - Add compositor element id for scroll nodes in SPv2 c#10: 1b = postponing for now until we see need c#10: 2 = http://crrev.com/2637383006 - Set layer element id when building layers in PaintArtifactCompositor c#7 : 2 = http://crrev.com/2642183003 - Populate cc element id maps for transform/effect nodes c#7 : 5 = http://crrev.com/2643283003 - For SPv2, attach element to compositor animation player without a CLM Note that in c#9 I said I had done #2 but I actually meant #3.
,
Jan 21 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/cfdf2d92414d581e8fdc5fdc04d825c668113fcc commit cfdf2d92414d581e8fdc5fdc04d825c668113fcc Author: wkorman <wkorman@chromium.org> Date: Sat Jan 21 01:26:05 2017 Add compositor element id for scroll nodes in SPv2. - Add compositor element id field to ScrollPaintPropertyNode. - Generate compositor element id for ScrollPaintPropertyNodes when created/updated. - Copy element id into cc::ScrollNode when generating cc property trees. BUG= 674317 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Review-Url: https://codereview.chromium.org/2638333007 Cr-Commit-Position: refs/heads/master@{#445240} [modify] https://crrev.com/cfdf2d92414d581e8fdc5fdc04d825c668113fcc/third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp [modify] https://crrev.com/cfdf2d92414d581e8fdc5fdc04d825c668113fcc/third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilderTest.cpp [modify] https://crrev.com/cfdf2d92414d581e8fdc5fdc04d825c668113fcc/third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositorTest.cpp [modify] https://crrev.com/cfdf2d92414d581e8fdc5fdc04d825c668113fcc/third_party/WebKit/Source/platform/graphics/compositing/PropertyTreeManager.cpp [modify] https://crrev.com/cfdf2d92414d581e8fdc5fdc04d825c668113fcc/third_party/WebKit/Source/platform/graphics/paint/ScrollPaintPropertyNode.cpp [modify] https://crrev.com/cfdf2d92414d581e8fdc5fdc04d825c668113fcc/third_party/WebKit/Source/platform/graphics/paint/ScrollPaintPropertyNode.h
,
Jan 21 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4243922107810fd1644fafa33cd8329ed1616515 commit 4243922107810fd1644fafa33cd8329ed1616515 Author: wkorman <wkorman@chromium.org> Date: Sat Jan 21 03:15:34 2017 Set layer element id when building layers in PaintArtifactCompositor. BUG= 674317 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Review-Url: https://codereview.chromium.org/2637383006 Cr-Commit-Position: refs/heads/master@{#445260} [modify] https://crrev.com/4243922107810fd1644fafa33cd8329ed1616515/third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositor.cpp [modify] https://crrev.com/4243922107810fd1644fafa33cd8329ed1616515/third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositorTest.cpp [modify] https://crrev.com/4243922107810fd1644fafa33cd8329ed1616515/third_party/WebKit/Source/platform/graphics/paint/PropertyTreeState.cpp [modify] https://crrev.com/4243922107810fd1644fafa33cd8329ed1616515/third_party/WebKit/Source/platform/graphics/paint/PropertyTreeState.h [modify] https://crrev.com/4243922107810fd1644fafa33cd8329ed1616515/third_party/WebKit/Source/platform/graphics/paint/PropertyTreeStateTest.cpp
,
Jan 23 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/653f787c53cd2f9104e85dc61238d9ef25e9d347 commit 653f787c53cd2f9104e85dc61238d9ef25e9d347 Author: wkorman <wkorman@chromium.org> Date: Mon Jan 23 21:41:33 2017 For SPv2, attach element to compositor animation player without a CLM. BUG= 674317 Review-Url: https://codereview.chromium.org/2643283003 Cr-Commit-Position: refs/heads/master@{#445494} [modify] https://crrev.com/653f787c53cd2f9104e85dc61238d9ef25e9d347/third_party/WebKit/Source/core/animation/CompositorAnimations.cpp
,
Jan 24 2017
Brief update: a simple composited text translate animation starts and runs in SPv2 now, but we hit a DCHECK failure around differing compositor element ids in the scroll vs transform nodes when the animation finishes and text de-composites, and visual behavior following is incorrect. Investigating.
,
Jan 24 2017
,
Jan 30 2017
SPv2 currently passes 50% (21 / 42) of the tests under virtual/threaded. Of the failures, 16 are timeouts and 5 are text failures. 5 look obviously scroll related (we know there is scrolling work still to be done, see for example http://crbug.com/683774 ). Some of the failures look like there's an issue with cancelling animations. Test failures below: Regressions: Unexpected text-only failures (5) virtual/threaded/animations/composited-filter-webkit-filter.html [ Failure ] virtual/threaded/animations/compositor-independent-transform-cancel.html [ Failure ] virtual/threaded/fast/compositorworker/basic-plumbing-main-to-worker.html [ Failure ] virtual/threaded/fast/compositorworker/basic-plumbing-worker-to-main.html [ Failure ] virtual/threaded/fast/scroll-behavior/smooth-scroll/fixed-background-in-iframe.html [ Failure ] Regressions: Unexpected timeouts (16) virtual/threaded/animations/KeyframeEffectReadOnly-composited-animation.html [ Timeout ] virtual/threaded/animations/composited-animations-rotate-zero-degrees.html [ Timeout ] virtual/threaded/animations/composited-animations-simple.html [ Timeout ] virtual/threaded/animations/composited-animations-timing-function.html [ Timeout ] virtual/threaded/animations/composited-animations-translate-rotate-scale.html [ Timeout ] virtual/threaded/animations/element-animate-positive-delay.html [ Timeout ] virtual/threaded/animations/img-element-transform.html [ Timeout ] virtual/threaded/animations/inline-block-transform.html [ Timeout ] virtual/threaded/animations/transitions-retarget.html [ Timeout ] virtual/threaded/fast/compositorworker/compositor-attribute-change-worker.html [ Timeout ] virtual/threaded/fast/compositorworker/compositor-proxy-disconnect-worker.html [ Timeout ] virtual/threaded/fast/compositorworker/visual-update.html [ Timeout ] virtual/threaded/fast/scroll-behavior/first-scroll-runs-on-compositor.html [ Timeout ] virtual/threaded/fast/scroll-behavior/smooth-scroll/main-thread-scrolling-reason-added.html [ Timeout ] virtual/threaded/fast/scroll-behavior/smooth-scroll/main-thread-scrolling-reason-correctness.html [ Timeout ] virtual/threaded/fast/scroll-behavior/smooth-scroll/mousewheel-scroll-interrupted.html [ Timeout ]
,
Jan 30 2017
,
Jan 31 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/42bd2b1a523f7c73cdade1e528703e3a1e9b768f commit 42bd2b1a523f7c73cdade1e528703e3a1e9b768f Author: wkorman <wkorman@chromium.org> Date: Tue Jan 31 03:57:01 2017 Enable half of virtual/threaded tests for SPv2. BUG= 674317 , 686897 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Review-Url: https://codereview.chromium.org/2667743002 Cr-Commit-Position: refs/heads/master@{#447182} [modify] https://crrev.com/42bd2b1a523f7c73cdade1e528703e3a1e9b768f/third_party/WebKit/LayoutTests/FlagExpectations/enable-slimming-paint-v2
,
Feb 10 2017
Brief update, currently investigating virtual/threaded/animations/composited-animations-simple.html as a test timeout failure likely representative of a larger batch. Instrumenting composited-animation-test.js shows that this.waitForCompositor().then(...) is hanging indefinitely. The adventure continues!
,
Feb 10 2017
The call to internals.pauseAnimations(0) in composited-animation-test.js causes hang in SPv2. For some reason pausing animations doesn't produce hang in SPv1. Need to look further. Commenting out pauseAnimations() call leads to a subsequent failure for that test in SPv2 with what looks like a missing transform node for the various transform animation test cases such as scale(). Opacity animation appears to be fine, however.
,
Feb 13 2017
In SPv1: 0. Internals::pauseAnimations is called by test page script and ends up calling AnimationPlayer::PauseAnimation 1. Thus animation starts out ticking in PAUSED run state with AnimationPlayer.needs_to_start_animations = true 2. AnimationPlayer::StartAnimations considers the animation, does not add it to animations_waiting_for_target, and sets AnimationPlayer.needs_to_start_animations = false 3. animation transitions to WAITING_FOR_TARGET_AVAILABILITY in some manner that also sets AnimationPlayer.needs_to_start_animations = true again 4. AnimationPlayer::StartAnimations executes and sets the animation to STARTING In SPv2 (0 - 2) happen, but we never see (3/4) and so we tick forever with AnimationPlayer.needs_to_start_animations = false and never call AnimationPlayer::StartAnimations again (and if we did, presumably animation state is still not WAITING_FOR_TARGET_AVAILABILITY so we'd still not actually start it).
,
Feb 13 2017
More clarity: - there are two animations: the one intended to vary with the test case, which is paused (animation_id=1, player_id=4); and a second one associated with the 'error' span, created by waitForCompositor(), and which is not paused (animation_id=2, player_id=5). - the ready-promise that's hanging is blocked on the latter. - for that one, in both SPv1 and SPv2 we construct the animation player and call AnimationPlayer::AnimationAdded and AnimationPlayer::UpdateTickingState. However has_element_in_any_list is always false in SPv2, thus we never add the player to the ticking players via AnimationHost::AddToTicking. Since it's not in the vector of ticking players we never start its animation. - this difference is explained by: in SPv1, between construction of that animation player and the call to AnimationPlayer::AnimationAdded on it, we see a call to ElementAnimations::InitAffectedElementTypes which executes set_has_element_in_active_list=1. So, need to figure out (something along the lines of) why we never seem to attach the composited layer for element id=5 in SPv2. Stack trace excerpt for SPv1: #2 0x7f73c455a8d0 cc::ElementAnimations::InitAffectedElementTypes() #3 0x7f73c4532f00 cc::AnimationHost::RegisterPlayerForElement() #4 0x7f73c45448c7 cc::AnimationPlayer::RegisterPlayer() #5 0x7f73c4544a95 cc::AnimationPlayer::AttachElement() #6 0x7f73cd021757 blink::CompositorAnimationPlayer::attachElement() #7 0x7f73c9a81d84 blink::CompositorAnimations::attachCompositedLayers() #8 0x7f73c9ab9181 blink::KeyframeEffectReadOnly::attachCompositedLayers() #9 0x7f73c99eeb01 blink::Animation::attachCompositedLayers() #10 0x7f73c99eb68d blink::Animation::createCompositorPlayer() #11 0x7f73c99eb2c3 blink::Animation::preCommit() #12 0x7f73c9a8d273 blink::CompositorPendingAnimations::update() #13 0x7f73c9a95661 blink::DocumentAnimations::updateAnimations() #14 0x7f73ca8ff4d3 blink::PaintLayerCompositor::updateIfNeededRecursiveInternal()
,
Feb 14 2017
- N.B. in SPv2 the element id for error span is 4, rather than 5. - it appears that we're never adding a layer with that element id in PaintArtifactCompositor, thus we fail animation_host_->mutator_host_client()->IsElementInList(...) check in ElementAnimations::InitAffectedElementTypes. I haven't figured out why we're running sans-layer yet.
,
Feb 14 2017
Checkpointing a simplified test case that fails in SPv2 and passes in SPv1, representative of a batch of pause-animation tests as explored above. Run locally via: % ./out/Default/content_shell --run-layout-test --expose-internals-for-testing --enable-threaded-compositing [--enable-slimming-paint-v2] composited-animation-ready-hang.html
,
Feb 15 2017
Filed http://crbug.com/692310 to track work around making sure ready promises eventually resolve for animations in SPv2. What we see in this case is that the 'error' span, which is animated to effect (I believe) a sort of "final single tick flush" of the composited animation subsystem, paints no content = has no paint chunk = has no layer = thus no layer exists with an element id, and the composited animation is never started. That bug, even if addressed (proposal noted in the bug is to not bother making animations that paint no content composited, and then the animation should follow normal non-composited animation subsystem readiness and promise resolution), would not fix what we see here since we still want, for testing purposes, a way to flush the composited animation subsystem. An interim fix could be to add some content such as a colored background to the error span. A more proper fix would be to add an internal testing method that does whatever motivated the sort-of-fake animate() call on the error span.
,
Feb 15 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5cfa87ff259fb01681ec6f509f1bad1d0702fa75 commit 5cfa87ff259fb01681ec6f509f1bad1d0702fa75 Author: wkorman <wkorman@chromium.org> Date: Wed Feb 15 20:20:51 2017 Enable most of virtual/threaded/transitions for SPv2. BUG= 674317 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Review-Url: https://codereview.chromium.org/2691933006 Cr-Commit-Position: refs/heads/master@{#450787} [modify] https://crrev.com/5cfa87ff259fb01681ec6f509f1bad1d0702fa75/third_party/WebKit/LayoutTests/FlagExpectations/enable-slimming-paint-v2
,
Feb 16 2017
Re #27 and the use of animation ready promises in tests: If we had a test helper like internals.waitForCompositorFrame() that would be absolutely ideal and would bypass the need to create dummy elements with paint content.
,
Mar 15 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/391cd0aa0d814f9f35bcd5a29df7ba91200e5d4f commit 391cd0aa0d814f9f35bcd5a29df7ba91200e5d4f Author: wkorman <wkorman@chromium.org> Date: Wed Mar 15 09:25:33 2017 Enable more tests now passing under SPv2. - stop skipping animations/ and transitions/ as most of those directories are now passing. - five scroll-behavior tests that failed with --enable-threaded-compositing previously are now passing. - perhaps unrelated to animation/scrolling specific work, but two compositing/ tests are now passing. BUG= 666986 , 674317 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Review-Url: https://codereview.chromium.org/2743373004 Cr-Commit-Position: refs/heads/master@{#457041} [modify] https://crrev.com/391cd0aa0d814f9f35bcd5a29df7ba91200e5d4f/third_party/WebKit/LayoutTests/FlagExpectations/enable-slimming-paint-v2
,
Mar 16 2017
,
Mar 16 2017
,
Mar 16 2017
,
Mar 16 2017
,
Mar 16 2017
,
Mar 21 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4b27b9c50ff50212f502eccdba7179650c1cd9db commit 4b27b9c50ff50212f502eccdba7179650c1cd9db Author: wkorman <wkorman@chromium.org> Date: Tue Mar 21 02:14:18 2017 Assign more granular bugs to remaining SPv2 animation test failures. BUG= 674317 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Review-Url: https://codereview.chromium.org/2758203003 Cr-Commit-Position: refs/heads/master@{#458280} [modify] https://crrev.com/4b27b9c50ff50212f502eccdba7179650c1cd9db/third_party/WebKit/LayoutTests/FlagExpectations/enable-slimming-paint-v2
,
Mar 28 2017
,
May 11 2017
Issue 666986 has been merged into this issue.
,
May 11 2017
,
May 11 2017
,
May 30 2017
,
Dec 7 2017
,
Dec 7 2017
,
Dec 7
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue. Sorry for the inconvenience if the bug really should have been left as Available. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Dec 7
,
Dec 7
I don't think there will be animation differences between SPV2 (now called CompositeAfterPaint) and BlinkGenPropertyTrees. I think this can be merged into https://crbug.com/836897.
,
Dec 7
Will examine the failure entries in the FlagExpectations/enable-blink-features=CompositeAfterPaint, and merge into bug 836897 if there is no CAP-specific failures.
,
Dec 7
The following two failures are CAP-specific: crbug.com/702350 virtual/threaded/transitions/opacity-transform-transitions-inside-iframe.html [ Timeout ] crbug.com/702353 virtual/threaded/transitions/transition-end-event-destroy-iframe.html [ Timeout ] Given bug 702350 has been fixed, I think both of the failures are because of bug 702353 .
,
Dec 11
All blocking bugs have been fixed. |
||||||||||||||||||||
►
Sign in to add a comment |
||||||||||||||||||||
Comment 1 by wkorman@chromium.org
, Dec 14 2016