GNOME Bugzilla – Bug 634771
MetaStackTracker: Avoid queueing resync for obvious no-ops
Last modified: 2010-11-18 15:09:02 UTC
Since we can't distinguish a ConfigureEvent that indicates a raise from a ConfigureEvent that indicates a move, we get lots of STACK_OP_RAISE_ABOVE events for windows that are already in the right place in the stacking order. Avoid queueing a restack in that case.
Created attachment 174402 [details] [review] MetaStackTracker: Avoid queueing resync for obvious no-ops
Created attachment 174403 [details] [review] Optimize out restacks that don't change actor order This is the suspenders to the last patch's belt. It triggers a fair bit if I add a debugging statement, but I couldn't find a place it was obviously saving work other than the stack resyncs when a window is being dragged that the last patch fixes. I think it's worthwhile. (You could argue that we should be also/instead checking for identical lists before calling into the compositor, but I think once at the beginning of the process and once at the end is enough.) === For various reasons (mostly the stack tracker correctly predicting the stacking order before getting events, but also because of the processing that the compositor does to get the actor stacking order) the compositor can be told to sync the stack when it has nothing to do. Detect this at the last moment before actually telling Clutter to restack to avoid triggering unnecessary redraws.
Comment on attachment 174402 [details] [review] MetaStackTracker: Avoid queueing resync for obvious no-ops Should probably update the comment at the top of the file. "When we receive a new event ... we drop the predicted stacking order" is no longer always true. > g_queue_pop_head (tracker->queued_requests); > meta_stack_op_free (queued_op); >+ need_sync = TRUE; Why need_sync here? We've already established that server_stack didn't change.
Comment on attachment 174403 [details] [review] Optimize out restacks that don't change actor order basically looks good >+ sync_actor_stacking (info, info->windows); you could just pass just info and let sync_actor_stacking() extract info->windows from it >- /* NB: The first entry in the list is stacked the lowest */ >+ /* NB: The first entry in the lists are stacked the lowest */ s/entry/entries/ >+ /* old == NULL: someone reparented a window out of the window group, >+ * ordered undefined, always restack */ I think s/ordered/order/ ?
(In reply to comment #3) > (From update of attachment 174402 [details] [review]) > Should probably update the comment at the top of the file. "When we receive a > new event ... we drop the predicted stacking order" is no longer always true. > > > g_queue_pop_head (tracker->queued_requests); > > meta_stack_op_free (queued_op); > >+ need_sync = TRUE; > > Why need_sync here? We've already established that server_stack didn't change. The stacking order we expose to clients - the "predicted stack" = consists of the server stack with the stack operations in 'queued requests' applied to it. When we delete things from queued requests that potentially changes what will be in the predicted stack. It's potentially possible to notice the case when we get an incoming notification that exactly matches the queued requests we are removing. An optimization not taken.
(In reply to comment #3) > (From update of attachment 174402 [details] [review]) > Should probably update the comment at the top of the file. "When we receive a > new event ... we drop the predicted stacking order" is no longer always true. Added a 'if necessary'
Attachment 174402 [details] pushed as f372fa2 - MetaStackTracker: Avoid queueing resync for obvious no-ops Attachment 174403 [details] pushed as bac668d - Optimize out restacks that don't change actor order
(In reply to comment #4) > (From update of attachment 174403 [details] [review]) > basically looks good > > >+ sync_actor_stacking (info, info->windows); > > you could just pass just info and let sync_actor_stacking() extract > info->windows from it Yeah, would be cleaner, done. > >- /* NB: The first entry in the list is stacked the lowest */ > >+ /* NB: The first entry in the lists are stacked the lowest */ > > s/entry/entries/ > > >+ /* old == NULL: someone reparented a window out of the window group, > >+ * ordered undefined, always restack */ > > I think s/ordered/order/ ? Fixed.