Bug 61446

void TextureMapperNode::syncCompositingStateSelf(GraphicsLayerTextureMapper* graphicsLayer, TextureMapper* textureMapper) { ...... if (!hasRunningOpacityAnimation()) m_opacity = m_state.opacity; if (!hasRunningTransformAnimation()) { m_transforms.base = m_state.transform; } } The transform matrix of an accelerated animation is initialized to zero by above code after the animation is paused. Does the transform matrix really have to be synced with GraphicsLayer even when the node has a paused animation? I wonder if it should be synced only when it doesn't have any animations. Any idea?

Probably doesn't need to be reverted if we're paused, I think you're right.

Created attachment 94897 [details] Patch

(In reply to comment #2) > Probably doesn't need to be reverted if we're paused, I think you're right. Thanks for the comment :) I uploaded a patch with new test.

The content looks good to me.

Comment on attachment 94897 [details] Patch Attachment 94897 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/8737123 New failing tests: animations/play-state-paused.html

Created attachment 94941 [details] Archive of layout-test-results from ec2-cr-linux-02 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: ec2-cr-linux-02 Port: Chromium Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick

Created attachment 94981 [details] Patch

Test case is slightly modified to appease the chromium bot.

No'am can you have a look at this?

My LGTM from above still stands, since the only change in this patch is in the test :)

(In reply to comment #11) > My LGTM from above still stands, since the only change in this patch is in the test :) Not really... Please look again :-)

(In reply to comment #12) > (In reply to comment #11) > > My LGTM from above still stands, since the only change in this patch is in the test :) > > Not really... Please look again :-) Looked again. Patch looks OK.

Comment on attachment 94981 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=94981&action=review > LayoutTests/ChangeLog:8 > + The transform matrix of an accelerated animation shouldn't be synced when the animation is paused. Looking at the png they seem pretty sync'ed.

(In reply to comment #14) > (From update of attachment 94981 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=94981&action=review > > > LayoutTests/ChangeLog:8 > > + The transform matrix of an accelerated animation shouldn't be synced when the animation is paused. > > Looking at the png they seem pretty sync'ed. Thanks for the review, Kenneth. The changeLog means that the transform matrix of an accelerated animation shouldn't be synced with 'the matrix of the GraphicsLayer' and is not about the png. Well, that log could be a bit confused. :(

cq-, because the patch has made commit-bots fail over and over again. (Oops..) Patch 94981 from bug 61446: 3 minutes ago (cr-jail-8) Patch does not pass tests [results] Patch 94981 from bug 61446: 7 minutes ago (cr-jail-7) Patch does not pass tests [results] Patch 94981 from bug 61446: 17 minutes ago (cr-jail-8) Patch does not pass tests [results] Patch 94981 from bug 61446: 21 minutes ago (cr-jail-7) Patch does not pass tests [results] Patch 94981 from bug 61446: 32 minutes ago (cr-jail-8) Built patch

It seems that I have to run the test in chromium-mac port and figure out why it fails.

animations/play-state-paused.html -> failed was failing (flaky?) while trying to commit this.

Created attachment 95610 [details] Patch

Testcase has been changed to use runAnimationTest() to avoid flakiness, and the WebCore part remains unchanged.

Comment on attachment 95610 [details] Patch Clearing flags on attachment: 95610 Committed r88247: <http://trac.webkit.org/changeset/88247>

All reviewed patches have been landed. Closing bug.