Bug 100536

Summary: Coordinated Graphics: Animation jerkiness when rAF is enabled
Product: WebKit Reporter: Noam Rosenthal <noam>
Component: New BugsAssignee: Noam Rosenthal <noam>
Status: RESOLVED FIXED    
Severity: Normal CC: dglazkov, jturcotte, ossy, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch for landing
none
Patch for landing
none
Patch for landing
none
Patch for landing none

Noam Rosenthal
Reported 2012-10-26 10:14:45 PDT
Coordinated Graphics: Animation jerkiness when rAF is enabled
Attachments
Patch (2.67 KB, patch)
2012-10-26 10:18 PDT, Noam Rosenthal 		no flags
DetailsFormatted DiffDiff
Patch (10.17 KB, patch)
2012-10-26 15:02 PDT, Noam Rosenthal 		no flags
DetailsFormatted DiffDiff
Patch (10.38 KB, patch)
2012-10-26 15:40 PDT, Noam Rosenthal 		no flags
DetailsFormatted DiffDiff
Patch (12.77 KB, patch)
2012-10-26 16:08 PDT, Noam Rosenthal 		no flags
DetailsFormatted DiffDiff
Patch (13.01 KB, patch)
2012-10-26 16:32 PDT, Noam Rosenthal 		no flags
DetailsFormatted DiffDiff
Patch (13.45 KB, patch)
2012-10-26 16:36 PDT, Noam Rosenthal 		no flags
DetailsFormatted DiffDiff
Patch (13.67 KB, patch)
2012-10-27 09:08 PDT, Noam Rosenthal 		no flags
DetailsFormatted DiffDiff
Patch for landing (13.73 KB, patch)
2012-10-27 12:49 PDT, Noam Rosenthal 		no flags
DetailsFormatted DiffDiff
Patch for landing (14.96 KB, patch)
2012-10-27 15:27 PDT, Noam Rosenthal 		no flags
DetailsFormatted DiffDiff
Patch for landing (13.72 KB, patch)
2012-10-27 15:29 PDT, Noam Rosenthal 		no flags
DetailsFormatted DiffDiff
Patch for landing (1.30 KB, patch)
2012-10-27 18:59 PDT, Noam Rosenthal 		no flags
DetailsFormatted DiffDiff
Show Obsolete (10) View All Add attachment proposed patch, testcase, etc.
Noam Rosenthal
Comment 1 2012-10-26 10:18:24 PDT
Created attachment 170951 [details] Patch
Noam Rosenthal
Comment 2 2012-10-26 14:51:55 PDT
Comment on attachment 170951 [details] Patch I have another fix that's better :)
Noam Rosenthal
Comment 3 2012-10-26 15:02:37 PDT
Created attachment 171024 [details] Patch
Kenneth Rohde Christiansen
Comment 4 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
Noam Rosenthal
Comment 5 2012-10-26 15:40:01 PDT
Created attachment 171030 [details] Patch
Kenneth Rohde Christiansen
Comment 6 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?
Noam Rosenthal
Comment 7 2012-10-26 16:08:15 PDT
Created attachment 171038 [details] Patch
Noam Rosenthal
Comment 8 2012-10-26 16:32:23 PDT
Created attachment 171045 [details] Patch
Noam Rosenthal
Comment 9 2012-10-26 16:36:51 PDT
Created attachment 171047 [details] Patch
WebKit Review Bot
Comment 10 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
Noam Rosenthal
Comment 11 2012-10-27 09:08:34 PDT
Created attachment 171095 [details] Patch
WebKit Review Bot
Comment 12 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
Noam Rosenthal
Comment 13 2012-10-27 12:49:25 PDT
Created attachment 171102 [details] Patch for landing
Noam Rosenthal
Comment 14 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.
Noam Rosenthal
Comment 15 2012-10-27 15:27:27 PDT
Created attachment 171104 [details] Patch for landing
Noam Rosenthal
Comment 16 2012-10-27 15:29:59 PDT
Created attachment 171105 [details] Patch for landing
WebKit Review Bot
Comment 17 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>
WebKit Review Bot
Comment 18 2012-10-27 18:33:18 PDT
All reviewed patches have been landed. Closing bug.
Noam Rosenthal
Comment 19 2012-10-27 18:58:57 PDT
Reopening to attach new patch.
Noam Rosenthal
Comment 20 2012-10-27 18:59:00 PDT
Created attachment 171112 [details] Patch for landing
Csaba Osztrogonác
Comment 21 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>
Csaba Osztrogonác
Comment 22 2012-10-27 23:47:24 PDT
All reviewed patches have been landed. Closing bug.
Csaba Osztrogonác
Comment 23 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?
Note You need to log in before you can comment on or make changes to this bug.