After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 561745 - Use a Tweener "Frame Ticker" with a ClutterTimeline backend
Use a Tweener "Frame Ticker" with a ClutterTimeline backend
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2008-11-21 00:22 UTC by Owen Taylor
Modified: 2009-01-20 18:34 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Use a Tweener "Frame Ticker" with a ClutterTimeline backend (3.62 KB, patch)
2008-11-21 00:22 UTC, Owen Taylor
committed Details | Review
add getTime() to the frame ticker object (6.29 KB, patch)
2009-01-19 21:29 UTC, Havoc Pennington
committed Details | Review

Description Owen Taylor 2008-11-21 00:22:53 UTC
Call Tweener.setFrameTicker() with a custom object that bridges to
ClutterTimeline to get new frame notifications. Combined with a
hack to dynamically adjust the frame ticker's frame rate when
Clutter drops frames, this means that our animations play in the
intended time even if rendering is too slow to maintain a full
60HZ frame rate.
Comment 1 Owen Taylor 2008-11-21 00:22:58 UTC
Created attachment 123143 [details] [review]
Use a Tweener "Frame Ticker" with a ClutterTimeline backend
Comment 2 Owen Taylor 2008-11-21 00:26:47 UTC
Havoc - can you take a look at this and see if it looks reasonable and 
if the the hacks are really necessary? Is there some easier way to get
Tweener to deal with dropped frames?

Comment 3 Havoc Pennington 2008-11-21 01:53:58 UTC
It looks to me like adding an API to drop frames, such as an arg to prepare-frame that is the current frame number or delta, might be a clean solution. Then increment the current time inside tweener using the delta.

The "frame ticker" API is just something I made up, not in the original tweener. I didn't implement dropping frames yet in litl's app, so no thought has been given to that.

The algorithm I am using in litl so far is:

* when ticker is enabled (there are >0 animations), install timeout at interval frame_length
* inside the timeout,
** start a GTimer frame_timer
** emit prepare-frame which should set all tween properties and thus queue draw
** unqueue draw and force immediate repaint
** wait for vsync and swap buffers
** get elapsed time from the GTimer frame_timer
** reinstall timeout for MAX(0, frame_length - elapsed)
* when ticker is disabled, remove the timeout

So if this gets behind, it just runs "as fast as possible" (installs the timeout at 0 interval).

According to a comment in the code, it was essential to force the immediate repaint instead of relying on repaint idle, because the timeout and X event queue can starve the idle otherwise if the animation gets behind.

I have a "FIXME figure out how to drop frames" in the code that has never really been an issue... I think in practice for short half-second transition animations, frame dropping is maybe not very important, because if things are slow enough to drop many frames you're doomed anyway, and also in practice it can look better to just play all the frames too slowly rather than do a fast animation where objects move too much in each frame.

Assuming the animation is keeping up (generally we can draw each frame in less than frame_length), I think it was pretty important to implement the timeout reinstallation for frame_length-elapsed, because just a plain glib timeout resulted in a visibly uneven frame rate. Checking just now, it looks like ClutterTimeoutPool also does this.
Comment 4 Owen Taylor 2008-11-23 04:15:58 UTC
I've now committed a version of my patch (with a bug fix to actually
set this._frame), but I don't think it's really the "last word"; I think
it would make sense for Tweener to use (new Date()).getTime() to do timing
in the normal case.

Or does it make sense to have getTime() be a method on the FrameTicker?

I'll leave this bug open for the moment to think about the appropriate
patch to submit against gjs's Tweener.
Comment 5 Havoc Pennington 2009-01-19 21:29:45 UTC
Created attachment 126794 [details] [review]
add getTime() to the frame ticker object

I think this patch is probably enough to avoid the hack to change FRAME_RATE, and basically allow frame tickers to do things however they want?

I haven't tried setting up our stuff to take advantage of this yet. Let me know if this patch would work for gnome-shell
Comment 6 Havoc Pennington 2009-01-19 21:42:50 UTC
Hmm, the FRAME_RATE can be removed from the ticker API now too, probably
Comment 7 Owen Taylor 2009-01-20 16:06:24 UTC
Looks like it would work fine for us, and would get rid of the really gross part of our frame ticker (fudging the frame rate to deal with skipped frames.)
Comment 8 Havoc Pennington 2009-01-20 18:12:52 UTC
Patch committed, not closing bug in case there's gnome-shell work remaining
Comment 9 Owen Taylor 2009-01-20 18:34:15 UTC
Committed a patch that implements getTime() and makes thing work again.

I'm going to close this since there is now a reasonable tweener API though 
certainly more work remains to be done - eventually I'd like to have vblank syncing in gnome-shell as well