[ios-simulator] LayoutTest compositing/animation/animation-backing.html is a flaky failure
@@ -1 +1 @@
-Saw layer flushes during animation: PASS
+No layer flushes during animation: FAIL
It looks like this regressed with "Enable optimized layer flushes on iOS" https://trac.webkit.org/changeset/215469/webkit
I'll take a look.
The flush needs to travers tiled layers under animating layers, because we have to recompute the coverage rect as the animation runs.
After your changes, how do we ensure the recomputation of coverage rects on all layers with scrolling etc, which is used to drop backing store outside the viewport, and compute the right tiling for tiled layers?
(In reply to Simon Fraser (smfr) from comment #3)
> The flush needs to travers tiled layers under animating layers, because we
> have to recompute the coverage rect as the animation runs.
> After your changes, how do we ensure the recomputation of coverage rects on
> all layers with scrolling etc, which is used to drop backing store outside
> the viewport, and compute the right tiling for tiled layers?
We traverse all descendants if coverage rect changes. For scrolling we take care to invalidate the scrolling layers so coverage gets recomputed. I assume we are missing or misoptimizing some invalidation with animations.
We can probably just invalidate all layers with accelerated animations in progress.
Created attachment 307750 [details]
Comment on attachment 307750 [details]
I don't think you need all this. We already have CommitState.ancestorHasTransformAnimation, why can't you key off of that?
Possibly. What I need is answer to "Does this layer have any descendants with accelerated transform animations?" Do we have this somewhere already?
Cleary I could just traverse the entire layer tree but that is much slower than what the patch does (just traverses the active animations).
Should be easy to add "descendant has accelerated transform animation" as a post-traversal bit set. I would prefer to keep this code in GraphicsLayerCA.
(In reply to Simon Fraser (smfr) from comment #10)
> Should be easy to add "descendant has accelerated transform animation" as a
> post-traversal bit set. I would prefer to keep this code in GraphicsLayerCA.
Yeah, should be doable.
Created attachment 307968 [details]
GraphicsLayerCA only (almost) patch
Comment on attachment 307968 [details]
View in context: https://bugs.webkit.org/attachment.cgi?id=307968&action=review
> + bool hasAcceleratedTransformAnimation = renderer().animation().isRunningAcceleratedAnimationOnRenderer(renderer(), CSSPropertyTransform, AnimationBase::Running | AnimationBase::Paused);
> + m_graphicsLayer->setHasAcceleratedTransformAnimation(hasAcceleratedTransformAnimation);
> + if (!hasAcceleratedTransformAnimation)
I don't think you need to set this from outside of GraphicsLayer(CA). GraphicsLayerCA::isRunningTransformAnimation() knows this, and GraphicsLayerCA::updateAnimations() can set or clear the state.
Created attachment 308117 [details]
Comment on attachment 308117 [details]
Clearing flags on attachment: 308117
Committed r215750: <http://trac.webkit.org/changeset/215750>
All reviewed patches have been landed. Closing bug.