Bug 112114 - [EFL] Disable REQUEST_ANIMATION_FRAME_TIMER to render a new animation frame.
Summary: [EFL] Disable REQUEST_ANIMATION_FRAME_TIMER to render a new animation frame.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other Other
: P2 Normal
Assignee: JungJik Lee
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-03-12 01:22 PDT by JungJik Lee
Modified: 2013-03-20 03:03 PDT (History)
7 users (show)

See Also:


Attachments
manual test file (1.68 KB, patch)
2013-03-12 01:34 PDT, JungJik Lee
no flags Details | Formatted Diff | Diff
Patch (6.67 KB, patch)
2013-03-19 21:35 PDT, JungJik Lee
no flags Details | Formatted Diff | Diff
Patch (6.97 KB, patch)
2013-03-20 00:50 PDT, JungJik Lee
no flags Details | Formatted Diff | Diff
Patch (7.04 KB, patch)
2013-03-20 01:33 PDT, JungJik Lee
no flags Details | Formatted Diff | Diff
Patch (7.08 KB, patch)
2013-03-20 02:12 PDT, JungJik Lee
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description JungJik Lee 2013-03-12 01:22:18 PDT
We can not prepare the tiles (out of keepRect) which are coming from far away.
Because we now skips getting current transform of the moving object in computeTransformVisibileRect.
So while the tiles are moving from off-screen, the tile is not updated.
The tiles is shown after the animation is finished.
I filed a manual test file. we should see that the box go and come.
Comment 1 JungJik Lee 2013-03-12 01:34:30 PDT
Created attachment 192667 [details]
manual test file
Comment 2 JungJik Lee 2013-03-19 21:35:31 PDT
Created attachment 193981 [details]
Patch
Comment 3 Kenneth Rohde Christiansen 2013-03-19 23:39:35 PDT
Comment on attachment 193981 [details]
Patch

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

> Source/WTF/ChangeLog:13
> +
> +        EFL WK2 port uses WebChromeClient as same as QT port for scripted animation.
> +        While the layer has the active animation, CoordinatedLayerHost needs to schedule animations
> +        by servicing of scripted animations.
> +
> +        The testing is covered by ManualTests/animation/transition-on-and-offscreen-animation.html
> +

You don't explain at all why that solves it.

What, why, how are good ingredients of a changelog
Comment 4 JungJik Lee 2013-03-20 00:50:32 PDT
Created attachment 193994 [details]
Patch
Comment 5 Kenneth Rohde Christiansen 2013-03-20 00:54:12 PDT
Comment on attachment 193994 [details]
Patch

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

Better but still not there

> Source/WTF/ChangeLog:13
> +        This patch is to tie scripted animations with synchronizing the layer to update animations as same as QT port.

Still misses the how... like how does making methods being not implemented solve this issues. That is not obvious and must be detailed.

Are these methods already implemented for WebKit2 but just not called with the current path? So why does disabling something makes them become called?
Comment 6 JungJik Lee 2013-03-20 00:59:47 PDT
(In reply to comment #5)
> (From update of attachment 193994 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=193994&action=review
> 
> Better but still not there
> 
> > Source/WTF/ChangeLog:13
> > +        This patch is to tie scripted animations with synchronizing the layer to update animations as same as QT port.
> 
> Still misses the how... like how does making methods being not implemented solve this issues. That is not obvious and must be detailed.
> 
> Are these methods already implemented for WebKit2 but just not called with the current path? So why does disabling something makes them become called?

Thank you for review. Yes, these methods are already implemented by https://bugs.webkit.org/show_bug.cgi?id=88638. So for using r88638 we should disable the REQUEST_ANIMATION_FRAME_TIMER. I will write this to ChangeLog.
Comment 7 JungJik Lee 2013-03-20 01:33:48 PDT
Created attachment 193997 [details]
Patch
Comment 8 Kenneth Rohde Christiansen 2013-03-20 01:43:11 PDT
Comment on attachment 193997 [details]
Patch

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

> Source/WTF/ChangeLog:7
> +

Now please try to keep the line length similar!

> Source/WTF/ChangeLog:8
> +        The issue is that if the animation starts from out of keepRects,

outside of the area covered by keepRect, the web process does not...

> Source/WTF/ChangeLog:9
> +        WebProcess does not create tiles of the animation layer and the layer moves with having no tiles.

moves without having any tiles

> Source/WTF/ChangeLog:10
> +        To fix this issue, CoordinatedLayerHost should call scheduleLayerFlush to create new tiles 

In order to fix this issue ... must call

> Source/WTF/ChangeLog:11
> +        when the layer is coming inside keepRect.

when the layer enters the area covered by keepRect.

> Source/WTF/ChangeLog:13
> +        We can tie scripted animations with synchronizing the layer 

synchronization of

> Source/WTF/ChangeLog:14
> +        and that already has been implemented by r123786 in QT Port.

implemented in ... by the Qt port.
Comment 9 JungJik Lee 2013-03-20 02:12:44 PDT
Created attachment 194004 [details]
Patch
Comment 10 JungJik Lee 2013-03-20 02:13:43 PDT
(In reply to comment #8)
> (From update of attachment 193997 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=193997&action=review
> 
> > Source/WTF/ChangeLog:7
> > +
> 
> Now please try to keep the line length similar!
> 
> > Source/WTF/ChangeLog:8
> > +        The issue is that if the animation starts from out of keepRects,
> 
> outside of the area covered by keepRect, the web process does not...
> 
> > Source/WTF/ChangeLog:9
> > +        WebProcess does not create tiles of the animation layer and the layer moves with having no tiles.
> 
> moves without having any tiles
> 
> > Source/WTF/ChangeLog:10
> > +        To fix this issue, CoordinatedLayerHost should call scheduleLayerFlush to create new tiles 
> 
Thank you for comments.
> In order to fix this issue ... must call
> 
> > Source/WTF/ChangeLog:11
> > +        when the layer is coming inside keepRect.
> 
> when the layer enters the area covered by keepRect.
> 
> > Source/WTF/ChangeLog:13
> > +        We can tie scripted animations with synchronizing the layer 
> 
> synchronization of
> 
> > Source/WTF/ChangeLog:14
> > +        and that already has been implemented by r123786 in QT Port.
> 
> implemented in ... by the Qt port.
Comment 11 Kenneth Rohde Christiansen 2013-03-20 02:14:24 PDT
Comment on attachment 194004 [details]
Patch

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

> Source/WTF/ChangeLog:17
> +        The testing is covered by ManualTests/animation/transition-on-and-offscreen-animation.html
> +

any reason this cannot be a non-manual test? People never run manual tests :-(
Comment 12 JungJik Lee 2013-03-20 02:28:18 PDT
(In reply to comment #11)
> (From update of attachment 194004 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=194004&action=review
> 
> > Source/WTF/ChangeLog:17
> > +        The testing is covered by ManualTests/animation/transition-on-and-offscreen-animation.html
> > +
> 
> any reason this cannot be a non-manual test? People never run manual tests :-(

Thank you for review again. And I thought we cannot know whether the TiledBackingStore has tiles or not by animation-test-helper, can we?
Because the animation object moves correctly. tiles is just unseen.
Comment 13 Kenneth Rohde Christiansen 2013-03-20 02:40:07 PDT
Ask Dongsung Huang or Noam Rosenthal whether they have any ideas! dshuang and noamr on irc
Comment 14 JungJik Lee 2013-03-20 02:42:05 PDT
(In reply to comment #13)
> Ask Dongsung Huang or Noam Rosenthal whether they have any ideas! dshuang and noamr on irc

Thanks, I will ask :)
Comment 15 WebKit Review Bot 2013-03-20 03:03:47 PDT
Comment on attachment 194004 [details]
Patch

Clearing flags on attachment: 194004

Committed r146320: <http://trac.webkit.org/changeset/146320>
Comment 16 WebKit Review Bot 2013-03-20 03:03:53 PDT
All reviewed patches have been landed.  Closing bug.