Bug 93536 - [CoordinatedGraphics] Blink in the end of a CSS transform "translate"
Summary: [CoordinatedGraphics] Blink in the end of a CSS transform "translate"
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Rafael Brandao
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-08-08 15:22 PDT by WebKit Review Bot
Modified: 2012-10-15 00:20 PDT (History)
9 users (show)

See Also:


Attachments
Attached small case that shows the blink at the end of animation. (312 bytes, text/html)
2012-08-10 12:25 PDT, Rafael Brandao
no flags Details
Patch (3.81 KB, patch)
2012-08-30 05:25 PDT, Rafael Brandao
noam: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description WebKit Review Bot 2012-08-08 15:22:07 PDT
[Qt] Blink in the end of a CSS transform "translate"
Requested by rafaelbrandao on #webkit.
Comment 1 Rafael Brandao 2012-08-08 15:27:48 PDT
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. :-)
Comment 2 Rafael Brandao 2012-08-10 12:25:47 PDT
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.
Comment 3 Rafael Brandao 2012-08-22 15:27:26 PDT
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.
Comment 4 Noam Rosenthal 2012-08-22 15:31:06 PDT
(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?
Comment 5 Rafael Brandao 2012-08-22 15:46:17 PDT
(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?
Comment 6 Rafael Brandao 2012-08-24 06:32:03 PDT
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. :-)
Comment 7 Rafael Brandao 2012-08-29 15:32:24 PDT
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.
Comment 8 Noam Rosenthal 2012-08-29 16:08:33 PDT
(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.
Comment 9 Rafael Brandao 2012-08-30 05:25:31 PDT
Created attachment 161448 [details]
Patch
Comment 10 Noam Rosenthal 2012-08-30 07:34:24 PDT
Comment on attachment 161448 [details]
Patch

Come to think of it, this will be a noop since layer creation/deletion happens during relayout anyway.
Comment 11 Alexey Proskuryakov 2012-08-30 10:00:27 PDT
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?
Comment 12 Rafael Brandao 2012-08-31 05:43:20 PDT
(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.
Comment 13 Simon Fraser (smfr) 2012-08-31 09:12:22 PDT
Mac doesn't use CoordinatedGraphics. I have no idea what it means.
Comment 14 Rafael Brandao 2012-08-31 09:17:17 PDT
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.
Comment 15 Noam Rosenthal 2012-08-31 10:44:57 PDT
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.
Comment 16 Rafael Brandao 2012-09-18 12:03:12 PDT
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.
Comment 17 Balazs Kelemen 2012-10-15 00:20:13 PDT
The test case seems to work properly now.