GNOME Bugzilla – Bug 694046
Keyboard resizing is broken for gtk+ windows
Last modified: 2013-03-14 12:02:51 UTC
- Focus a Gtk3 window - Press Alt-F8, down, then up any number of times - Don't move the mouse! - Press Enter In theory, this is a keyboard resizing operation - and in fact it does that on any window that doesn't support the extended XSync protocol. For Gtk3 windows, though, the window actor appears to be stuck. The window is correctly resized, the outer_rect of the window changes (as you can see looking at the animation in the overview), but the window is never painted again. Trying a new grab op, mouse or keyboard, has no effect, and you need to close the window.
Created attachment 238757 [details] [review] keybindings: Make sure to set the window as unfrozen during keyboard resize This seems to be a simple oversight done when window freezing was first introduced. Without this, the window actor will be frozen without getting updates.
This technically isn't a new bug since frame sync, but meta_compositor_set_updates() was a FIXME before.
Sorry - I've been sitting on a partial diagnosis, but hadn't written anything down because the fix was messy. I'm pretty sure the problem here comes from the commit: 0503f6bb9a9298d10543371d4044c27792d137d8 Consistently use meta_grab_op_is_resizing() for _NET_WM_SYNC_REQUEST In different places we checked the grab op differently when determing whether we are using _NET_WM_SYNC_REQUEST. This was somewhat covered up previously by the fact that we only had a sync alarm when using _NET_WM_SYNC_REQUEST, but that is no longer the case, so consistently use meta_grab_op_is_resizing() everywhere. https://bugzilla.gnome.org/show_bug.cgi?id=685463 Which started turning on freezing for keyboard resizes - previously we didn't do that. But the problem is that some of the things that are there for mouse resizing aren't there for keyboard resizing - for mouse resizing we're continually keeping track of the position of the mouse, and when we get a sync back from the user, we use the last tracked position for a new resize. We don't do any sort of queuing like this for keyboard resizing. So, what happens is that you get the first resize, then it hits the: if (dx == 0 && dy == 0) return; in update_resize() and returns. I think this is actually *also* a problem for mouse resizes - we don't unfreeze if the user doesn't move the mouse again, so we lag behind unnecessarily, but usually the user is moving the mouse continually so it's hard to notice. Then there's a second problem that if there's a pending resize at the end of the grab op, then we'll never unfreeze. For mouse resizes we unfreeze at mouse release (which isn't quite right either - we can get one frame of unsynchronized display from that - theoretically we should still be waiting for the configure request update.) Your patch sort of brute forces things by unfreezing the window again right after we freeze it for a keyboard resize - this is going to break the frame sync working right for keyboard resize, and if we're going to do that, it's better just to revert out the patch (or do something to that effect a bit more consistently) and not freeze in the first place for keyboard resizes. Three possible approaches to a fix: A) Just revert out using frame sync for anything but mouse resizes. B) Make keyboard resizes work exactly like mouse resizes - hold off the next *resize* until we get confirmation from the client, etc. B') Ignore the fact that keyboard resizes aren't like mouse resizes, and and add a unfreeze in the no-change shortcut, and an unfreeze at the end of keyboard resizing and hope for the best. C) Do A) and, bypass the resize-throttling for keyboard resizes (and maximize, snap, etc.) but still use _NET_WM_SYNC_REQUEST to get visual synchronization. Have some sort of "freeze until the sync value reaches N" system in MetaWindow. The WM spec doesn't *require* throttling resizes when using _NET_WM_SYNC_REQUEST - you can send multiple _NET_WM_SYNC_REQUEST/Configure requests and expect the client to handle that properly and update the counter at the end. (Possibility of client bugs of course.) I think I like C) best, but I don't know yet how invasive it's going to be, and how much risk it will pose.
(In reply to comment #3) > Sorry - I've been sitting on a partial diagnosis, but hadn't written anything > down because the fix was messy. I'm pretty sure the problem here comes from the > commit: > > 0503f6bb9a9298d10543371d4044c27792d137d8 > > Consistently use meta_grab_op_is_resizing() for _NET_WM_SYNC_REQUEST Nope. I bisected it and it goes all the way back to https://git.gnome.org/browse/mutter/commit/?id=c9343e3ee3df0159fb3000bd38c61d9ff984fd8e
Created attachment 238811 [details] [review] Fix freezing of windows with keyboard resizing Here's a something that works fairly well. It's basically along the lines of approach C), except that I didn't fool around at all with what resizes use _NET_WM_SYNC_REQUEST - it's sort of tricky because a keyboard resize is *also* a mouse resize - you can start a resize with the keyboard then use the mouse. Bugs and future improvements: * If you have a unresponsive window, and the resize ends before we mark it unresponsive, you end up in a stuck state for that window. This is a regression in this patch, and needs to be fixed, since windows that draw a different size then they are are very confusing to the user. * If you have an unresponsive window, and keyboard resize it, the detection code doesn't run since it's tied up with the mouse event throttling code, so it ends up in a stuck state. The fix for both of these is likely to tie the unresponsive-window timer to the window, rather than to the grab and to separate it out from the resize-frequency throttling code that is used without the sync request. * Frame sync works well for keyboard resizing, along as you are pressing individual keys. Once you hold down a key, because we don't wait, and don't coellesce, the frame gets out of sync - we don't redraw the window until it signals that it is done, but before that we might have resized the frame again. Not something I want to try and fix 3.8. * There's no attempt to apply _NET_WM_SYNC_REQUEST to maximize, unmaximize, tiling, and other one-off resizes. ==== During resizing we froze window updates when configuring the window, and unfroze the window updates when processing the next resize. This wasn't absolutely reliable, because we might not have a next resize. Instead tie window freezing more directly to the current sync request value - a window is frozen until it catches up with the last value we sent it in _NET_WM_SYNC_REQUEST. Testing with unresponsive clients showed that there was a bug where window->disable_sync once set, would not actually disable sync, but it *would* disable noticing that the client was unresponsive for the next resize. Fix that by checking for ->disable_sync before sending _NET_WM_SYNC_REQUEST.
Created attachment 238813 [details] [review] Make handling of windows that don't respond to _NET_WM_SYNC_REQUEST reliable Previously, we were handling failure to respond to _NET_WM_SYNC_REQUEST in the code path for throttling motion events. But this meant that if a window didn't respond to _NET_WM_SYNC_REQUEST and there were no motion events - for a keyboard resize, or after the end of the grab operation - it would end up in a stuck state. Use a separate per-window timeout to reliably catch the failure to respond to _NET_WM_SYNC_REQUEST.
Review of attachment 238811 [details] [review]: OK.
Review of attachment 238813 [details] [review]: ::: src/core/window.c @@ +4692,3 @@ window->xwindow, False, 0, (XEvent*) &ev); + window->sync_request_timeout_id = g_timeout_add (1000, Seems arbitrary, but OK.
Pushed my patches. Added a comment for the 1-second timeout (which was the same as before - the code was just reorganized.) Attachment 238811 [details] pushed as 592374b - Fix freezing of windows with keyboard resizing Attachment 238813 [details] pushed as 97a4cc8 - Make handling of windows that don't respond to _NET_WM_SYNC_REQUEST reliable