Bug 112114 – [EFL] Disable REQUEST_ANIMATION_FRAME_TIMER to render a new animation frame.

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[JungJik Lee]

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.

[JungJik Lee]

Created attachment 192667 [details]
manual test file

[JungJik Lee]

Created attachment 193981 [details]
Patch

[Kenneth Rohde Christiansen]

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

[JungJik Lee]

Created attachment 193994 [details]
Patch

[Kenneth Rohde Christiansen]

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?

[JungJik Lee]

(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.

[JungJik Lee]

Created attachment 193997 [details]
Patch

[Kenneth Rohde Christiansen]

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.

[JungJik Lee]

Created attachment 194004 [details]
Patch

[JungJik Lee]

(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.

[Kenneth Rohde Christiansen]

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 :-(

[JungJik Lee]

(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.

[Kenneth Rohde Christiansen]

Ask Dongsung Huang or Noam Rosenthal whether they have any ideas! dshuang and noamr on irc

[JungJik Lee]

(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 :)

[WebKit Review Bot]

Comment on attachment 194004 [details]
Patch

Clearing flags on attachment: 194004

Committed r146320: <http://trac.webkit.org/changeset/146320>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.