GNOME Bugzilla – Bug 692901
Add clutter_stage_set_sync_delay()
Last modified: 2013-02-13 14:49:10 UTC
New experimental API is added to allow changing the way that redraws are timed for a stage to include a "sync delay" - a period after the vertical blanking period where Clutter simply waits for updates. In detail, the algorithm is that when the master clock is restarted after drawing a frame (in the case where there are timelines running) or started fresh in response to a queued redraw or relayout, the start is scheduled at the next sync point (sync_delay ms after the predicted vblank period) rather than done immediately.
Created attachment 234869 [details] [review] Add clutter_stage_set_sync_delay()
http://blog.fishsoup.net/2012/11/28/avoiding-jitter-in-composited-frame-display/ is probably useful background material in understanding the motivation.
Created attachment 235425 [details] [review] Add clutter_stage_set_sync_delay() Minor change for final Cogl API.
Review of attachment 235425 [details] [review]: the patch looks good to me, but I still need to test it (I've been traveling for the past two weeks and am about to go home); I also need to double check that Clutter builds on other backends, though it shouldn't be a problem. I just have a couple of questions on the documentation, though. ::: clutter/clutter-master-clock.c @@ +569,2 @@ /* We need to protect ourselves against stages being destroyed during * event handling the comment should be modified to say that list_ready_stages() returns a list of ref'd stage pointers, given that the explicit foreach(ref()) is not there any more. ::: clutter/clutter-stage.c @@ +4508,3 @@ + * @sync_delay: number of milliseconds after frame presentation to wait + * before painting the next frame. If less than zero, redraw is throttled + * to the refresh rate but not synchronized to it. is there a way to "undo" the sync delay and allow switching back to vblank synchronized drawing? the sync_delay argument documentation seems to imply that it's impossible, once set_sync_delay() is called, to go back to restore the default behaviour of ClutterStage. if not: it should be more explicit in the documentation. @@ +4522,3 @@ + * and 8 ms (up to one-half of a typical 60hz frame rate) is appropriate. + * using a larger value will reduce latency but risks skipping a frame if + * drawing the stage takes too long. should we cap the sync_delay so that it's impossible to set a nonsensical value? @@ +4523,3 @@ + * using a larger value will reduce latency but risks skipping a frame if + * drawing the stage takes too long. + */ missing "Since: 1.14" and "Stability: unstable" annotations for gtk-doc @@ +4540,3 @@ + * possible, ignoring any delay that clutter_stage_set_sync_delay() + * would normally cause. + */ as above, missing Since and Stability annotations ::: clutter/clutter-stage.h @@ +204,3 @@ +#ifdef CLUTTER_ENABLE_EXPERIMENTAL_API +void clutter_stage_set_sync_delay (ClutterStage *stage, missing CLUTTER_AVAILABLE_IN_1_14 annotation. @@ +206,3 @@ +void clutter_stage_set_sync_delay (ClutterStage *stage, + gint sync_delay); +void clutter_stage_skip_sync_delay (ClutterStage *stage); missing CLUTTER_AVAILABLE_IN_1_14 annotation.
(In reply to comment #4) > Review of attachment 235425 [details] [review]: > > the patch looks good to me, but I still need to test it (I've been traveling > for the past two weeks and am about to go home); I also need to double check > that Clutter builds on other backends, though it shouldn't be a problem. I just > have a couple of questions on the documentation, though. > > ::: clutter/clutter-master-clock.c > @@ +569,2 @@ > /* We need to protect ourselves against stages being destroyed during > * event handling > > the comment should be modified to say that list_ready_stages() returns a list > of ref'd stage pointers, given that the explicit foreach(ref()) is not there > any more. Fixed. > ::: clutter/clutter-stage.c > @@ +4508,3 @@ > + * @sync_delay: number of milliseconds after frame presentation to wait > + * before painting the next frame. If less than zero, redraw is throttled > + * to the refresh rate but not synchronized to it. > > is there a way to "undo" the sync delay and allow switching back to vblank > synchronized drawing? the sync_delay argument documentation seems to imply that > it's impossible, once set_sync_delay() is called, to go back to restore the > default behaviour of ClutterStage. > > if not: it should be more explicit in the documentation. Changed the text slightly to say: * @sync_delay: number of milliseconds after frame presentation to wait * before painting the next frame. If less than zero, restores the * default behavior where redraw is throttled to the refresh rate but * not synchronized to it. > @@ +4522,3 @@ > + * and 8 ms (up to one-half of a typical 60hz frame rate) is appropriate. > + * using a larger value will reduce latency but risks skipping a frame if > + * drawing the stage takes too long. > > should we cap the sync_delay so that it's impossible to set a nonsensical > value? I don't think so. The exact range of meaningful will depend on the frame rate - whether the display is 50hz or 120hz, and I don't think we want to have property rangges depend on something like that which isn't even fixed in the lifetime of the program. And ruling out values which are clearly bad like 1s doesn't really help the programmer any. > @@ +4523,3 @@ > + * using a larger value will reduce latency but risks skipping a frame if > + * drawing the stage takes too long. > + */ > > missing "Since: 1.14" and "Stability: unstable" annotations for gtk-doc Added. > @@ +4540,3 @@ > + * possible, ignoring any delay that clutter_stage_set_sync_delay() > + * would normally cause. > + */ > > as above, missing Since and Stability annotations and added. > ::: clutter/clutter-stage.h > @@ +204,3 @@ > > +#ifdef CLUTTER_ENABLE_EXPERIMENTAL_API > +void clutter_stage_set_sync_delay (ClutterStage > *stage, > > missing CLUTTER_AVAILABLE_IN_1_14 annotation. > > @@ +206,3 @@ > +void clutter_stage_set_sync_delay (ClutterStage > *stage, > + gint > sync_delay); > +void clutter_stage_skip_sync_delay (ClutterStage > *stage); > > missing CLUTTER_AVAILABLE_IN_1_14 annotation. Added. Will attach a new version.
Created attachment 235870 [details] [review] Add clutter_stage_set_sync_delay() New version with fixes.
Review of attachment 235870 [details] [review]: looks good. please, push to master and to the clutter-1.14 branch.
Pushed to master and clutter-1.14