Bug 93536 – [CoordinatedGraphics] Blink in the end of a CSS transform "translate"

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[WebKit Review Bot]

[Qt] Blink in the end of a CSS transform "translate"
Requested by rafaelbrandao on #webkit.

[Rafael Brandao]

I will get a small test case and attach soon. But the one I'm seeing is at the end of a CSS animation that uses: webkitTransform = "translate(320px, 0)". In the very end of the animation, the elements blink.

I suspect this has to do LayerTreeCoordinator finishing the animation and resetting the animation to the initial state (I can quickly see the squares going back to their initial position before they refresh). So it might be a problem when we try to synchronize things, flushing earlier than expected, and another time again when we fix the elements position to the final position.

I hope to not be confusing, I'm still trying to understand how this works exactly, so these are just thoughts I want to share before I forget. :-)

[Rafael Brandao]

Created attachment 157788 [details]
Attached small case that shows the blink at the end of animation.

Here's a public link for the html attached: http://dl.dropbox.com/u/93716615/animations.html
And yes, the animation seems to go back to the initial position right before updating it on its final position. Still investigating what's causing this.

[Rafael Brandao]

Ugh, I think I'm close now... still trying to understand. What I've found so far is that at the end of the animation, we call ~CoordinatedGraphicsLayer (as our animation has been removed from GraphicsLayerAnimations) and this one requests to detach its layer from CoordinatedGraphicsLayerClient. Then afterwards, LayerTreeCoordinator schedules a layer flush, but when it performs, we can see the blink (probably because we've removed that layer).

When I comment detachLayer from CoordinatedGraphicsLayer destructor, things start to work, but this is obviously the wrong approach. Sharing this status in case someone else have any thoughts.

[Noam Rosenthal]

(In reply to comment #3)
> Ugh, I think I'm close now... still trying to understand. What I've found so far is that at the end of the animation, we call ~CoordinatedGraphicsLayer (as our animation has been removed from GraphicsLayerAnimations) and this one requests to detach its layer from CoordinatedGraphicsLayerClient. Then afterwards, LayerTreeCoordinator schedules a layer flush, but when it performs, we can see the blink (probably because we've removed that layer).

Is the blink showing the layer at a wrong position, or not showing the layer at all while it should show it?

[Rafael Brandao]

(In reply to comment #4)
> (In reply to comment #3)
> > Ugh, I think I'm close now... still trying to understand. What I've found so far is that at the end of the animation, we call ~CoordinatedGraphicsLayer (as our animation has been removed from GraphicsLayerAnimations) and this one requests to detach its layer from CoordinatedGraphicsLayerClient. Then afterwards, LayerTreeCoordinator schedules a layer flush, but when it performs, we can see the blink (probably because we've removed that layer).
> 
> Is the blink showing the layer at a wrong position, or not showing the layer at all while it should show it?

I'm trying to figure out more precisely yet, but when I add usleep(80000) on LayerTreeCoordinator::performScheduledLayerFlush() I can see more clearly that what I see in a blink is the element on its initial position, then it quickly comes back to its final position. If you interrupt an animation in the middle, then I see the element back to its rightmost position, which is quickly returned to the final position. Any ideas?

[Rafael Brandao]

Just a quick update, the rabbit hole seems to be deeper. Commenting that line does not solve the problem completely, but it at least helps to make it blink less often. I'm still digging on this. :-)

[Rafael Brandao]

I believe I've finally got what was going on. I will try to explain.

When the animation ends, it triggers a Document::recalcStyle. Before that, we delete the animation and send the layer detachment event to the UIProcess. If we ask the recalcStyle to be slow enough, we will see that UIProcess will get this event, process the layer deletion before we have a chance to get the style recalculated at WebProcess. This is what leads us to see a "blink" in the end. Then the WebProcess recalc the style succesfully and do whatever it needs to update the UIProcess with the new layers / positions.

As Noam guessed, we have to deal differently with layer deletion, but I'm not sure of what we should do to deal with this. I'll think more about it and try to come with something later.

[Noam Rosenthal]

(In reply to comment #7)
> When the animation ends, it triggers a Document::recalcStyle. Before that, we delete the animation and send the layer detachment event to the UIProcess. If we ask the recalcStyle to be slow enough, we will see that UIProcess will get this event, process the layer deletion before we have a chance to get the style recalculated at WebProcess. This is what leads us to see a "blink" in the end. Then the WebProcess recalc the style succesfully and do whatever it needs to update the UIProcess with the new layers / positions.
> 

Ah!
Yes, I think what needs to be done is on LayerTreeCoordinator::detachLayer don't send the message to the UI process directly, but rather keep it in a list (layersToDelete), and flush the list, sending all the DeleteCompositingLayer messages during flushPendingLayerChanges. This will assure that layer deletion occurs in sync with the new layout.

[Rafael Brandao]

Created attachment 161448 [details]
Patch

[Noam Rosenthal]

Comment on attachment 161448 [details]
Patch

Come to think of it, this will be a noop since layer creation/deletion happens during relayout anyway.

[Alexey Proskuryakov]

This bug has [Qt] in title, which means that non-Qt folks have absolutely no need to look at it. But the patch changes files that look like they are cross-platform.

Or is COORDINATED_GRAPHICS a purely Qt thing that's not of interest to anyone else?

[Rafael Brandao]

(In reply to comment #11)
> This bug has [Qt] in title, which means that non-Qt folks have absolutely no need to look at it. But the patch changes files that look like they are cross-platform.
> 
> Or is COORDINATED_GRAPHICS a purely Qt thing that's not of interest to anyone else?

It seems that efl also uses it. But in this bug I'm starting to think that it's the mix of COORDINATED_GRAPHICS and ACCELERATED_COMPOSITING that causes the blink. Is there anything I should add to the title to make this more clear? I will remove [Qt] and replace by [WK2] for now.

Please feel free to CC folks you know that have worked with any of those subjects.

[Simon Fraser (smfr)]

Mac doesn't use CoordinatedGraphics. I have no idea what it means.

[Rafael Brandao]

Ok. I've changed to [CoordinatedGraphics] instead, I believe Qt is the only port which uses that and accelerated compositing which I believe is what leads to this bug.

[Noam Rosenthal]

Qt and EFL use (In reply to comment #13)
> Mac doesn't use CoordinatedGraphics. I have no idea what it means.
CoordinatedGraphics is used by Qt and EFL, but does not have Qt/EFL specific code.

[Rafael Brandao]

As the patch on bug #96919 is quite similar to this one, I believe we will no longer be able to see this bug happening when it lands.

[Balazs Kelemen]

The test case seems to work properly now.