Bug 100536 - Coordinated Graphics: Animation jerkiness when rAF is enabled
Summary: Coordinated Graphics: Animation jerkiness when rAF is enabled
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Noam Rosenthal
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-10-26 10:14 PDT by Noam Rosenthal
Modified: 2012-10-27 23:49 PDT (History)
4 users (show)

See Also:

Attachments
Patch (2.67 KB, patch)
2012-10-26 10:18 PDT, Noam Rosenthal 		no flags Details | Formatted Diff | Diff
Patch (10.17 KB, patch)
2012-10-26 15:02 PDT, Noam Rosenthal 		no flags Details | Formatted Diff | Diff
Patch (10.38 KB, patch)
2012-10-26 15:40 PDT, Noam Rosenthal 		no flags Details | Formatted Diff | Diff
Patch (12.77 KB, patch)
2012-10-26 16:08 PDT, Noam Rosenthal 		no flags Details | Formatted Diff | Diff
Patch (13.01 KB, patch)
2012-10-26 16:32 PDT, Noam Rosenthal 		no flags Details | Formatted Diff | Diff
Patch (13.45 KB, patch)
2012-10-26 16:36 PDT, Noam Rosenthal 		no flags Details | Formatted Diff | Diff
Patch (13.67 KB, patch)
2012-10-27 09:08 PDT, Noam Rosenthal 		no flags Details | Formatted Diff | Diff
Patch for landing (13.73 KB, patch)
2012-10-27 12:49 PDT, Noam Rosenthal 		no flags Details | Formatted Diff | Diff
Patch for landing (14.96 KB, patch)
2012-10-27 15:27 PDT, Noam Rosenthal 		no flags Details | Formatted Diff | Diff
Patch for landing (13.72 KB, patch)
2012-10-27 15:29 PDT, Noam Rosenthal 		no flags Details | Formatted Diff | Diff
Patch for landing (1.30 KB, patch)
2012-10-27 18:59 PDT, Noam Rosenthal 		no flags Details | Formatted Diff | Diff
Show Obsolete (10) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Noam Rosenthal 2012-10-26 10:14:45 PDT 
Coordinated Graphics: Animation jerkiness when rAF is enabled
Comment 1 Noam Rosenthal 2012-10-26 10:18:24 PDT 
Created attachment 170951 [details]
Patch
Comment 2 Noam Rosenthal 2012-10-26 14:51:55 PDT 
Comment on attachment 170951 [details]
Patch

I have another fix that's better :)
Comment 3 Noam Rosenthal 2012-10-26 15:02:37 PDT 
Created attachment 171024 [details]
Patch
Comment 4 Kenneth Rohde Christiansen 2012-10-26 15:32:42 PDT 
Comment on attachment 171024 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=171024&action=review

> Source/WebKit2/ChangeLog:17
> +        UI process has actually painted the previous frame. We do so by sending a
> +        RequestAnimationFrame message to the UI process, which resonds with AnimationFrameReady
> +        after the UI process paints.
> +
> +        * UIProcess/CoordinatedGraphics/LayerTreeCoordinatorProxy.cpp:

Tests???

> Source/WebKit2/UIProcess/CoordinatedGraphics/LayerTreeCoordinatorProxy.cpp:184
> +}
> +void LayerTreeCoordinatorProxy::animationFrameReady()

missing newline
Comment 5 Noam Rosenthal 2012-10-26 15:40:01 PDT 
Created attachment 171030 [details]
Patch
Comment 6 Kenneth Rohde Christiansen 2012-10-26 15:42:45 PDT 
Comment on attachment 171030 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=171030&action=review

> Source/WebKit2/ChangeLog:10
> +        frame returns right away instead of wait till the previous frame is actually rendered.

waiting?

> Source/WebKit2/ChangeLog:14
> +        RequestAnimationFrame message to the UI process, which resonds with AnimationFrameReady

responds?

> Source/WebKit2/ChangeLog:18
> +        This code path is tested by existing requestAnimationFrame tests. There is no
> +        infrastructure to test animation jerkiness beyond that.

why wasnt it caught before?
Comment 7 Noam Rosenthal 2012-10-26 16:08:15 PDT 
Created attachment 171038 [details]
Patch
Comment 8 Noam Rosenthal 2012-10-26 16:32:23 PDT 
Created attachment 171045 [details]
Patch
Comment 9 Noam Rosenthal 2012-10-26 16:36:51 PDT 
Created attachment 171047 [details]
Patch
Comment 10 WebKit Review Bot 2012-10-26 21:53:14 PDT 
Comment on attachment 171047 [details]
Patch

Attachment 171047 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/14614295

New failing tests:
fast/animation/request-animation-frame-too-rapid.html
Comment 11 Noam Rosenthal 2012-10-27 09:08:34 PDT 
Created attachment 171095 [details]
Patch
Comment 12 WebKit Review Bot 2012-10-27 10:57:58 PDT 
Comment on attachment 171095 [details]
Patch

Attachment 171095 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/14614521

New failing tests:
fast/animation/request-animation-frame-too-rapid.html
Comment 13 Noam Rosenthal 2012-10-27 12:49:25 PDT 
Created attachment 171102 [details]
Patch for landing
Comment 14 Noam Rosenthal 2012-10-27 12:50:36 PDT 
Patch should work fine with Chromium/Mac tests now - if it gives trouble to anyone please feel free to roll out.
Comment 15 Noam Rosenthal 2012-10-27 15:27:27 PDT 
Created attachment 171104 [details]
Patch for landing
Comment 16 Noam Rosenthal 2012-10-27 15:29:59 PDT 
Created attachment 171105 [details]
Patch for landing
Comment 17 WebKit Review Bot 2012-10-27 18:33:14 PDT 
Comment on attachment 171105 [details]
Patch for landing

Clearing flags on attachment: 171105

Committed r132742: <http://trac.webkit.org/changeset/132742>
Comment 18 WebKit Review Bot 2012-10-27 18:33:18 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 19 Noam Rosenthal 2012-10-27 18:58:57 PDT 
Reopening to attach new patch.
Comment 20 Noam Rosenthal 2012-10-27 18:59:00 PDT 
Created attachment 171112 [details]
Patch for landing
Comment 21 Csaba Osztrogonác 2012-10-27 23:47:17 PDT 
Comment on attachment 171112 [details]
Patch for landing

Clearing flags on attachment: 171112

Committed r132747: <http://trac.webkit.org/changeset/132747>
Comment 22 Csaba Osztrogonác 2012-10-27 23:47:24 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 23 Csaba Osztrogonác 2012-10-27 23:49:24 PDT 
It seems CQ is unreliable and extremely slow. :(

Using it for important buildfixes isn't a good idea.
Could you commit buildfixes directly next time please?