GNOME Bugzilla – Bug 712778
Window stutters when holding ctrl+alt+shift+down
Last modified: 2015-03-27 13:36:41 UTC
When holding ctrl+alt+shift+down (which moves the focused window to the next workspace, by default) the window "stutters" because (I think) it tries to create a new empty workspace, move the window there and delete the old one, infinitely. I expect the window to be static, similar to what happens when you hold ctrl+alt+shift+up when you're already on the first workspace.
What version are you using? I am 3.10.2 and cannot recreate it.
I am using GNOME Shell 3.10.2.1
(In reply to Magdalen Berns (irc magpie) from comment #1) > What version are you using? I am 3.10.2 and cannot recreate it. I'm on 3.14.3 now and it's still happening. It's been more than a year.
Created attachment 299362 [details] [review] Add functions to block/unblock Dynamic Workspace Pause Dynamic Workspace Management by blocking it while workspaceSwitcherPopup is shown so as to eliminate infinite creation and distruction of workspaces while trying to move a window to last workspace. Unblock it when workspaceSwitcherPopup is destroyed.
Review of attachment 299362 [details] [review]: Works nicely for moving windows past the end, but unfortunately breaks in the opposite direction: in 3.16, we allow windows on the first workspace to be moved to a newly created workspace on top using <shift><super>PgUp ...
Created attachment 299699 [details] [review] Add functions to block/unblock Dynamic Workspace Pause Dynamic Workspace Management by blocking it while workspaceSwitcherPopup is shown so as to eliminate infinite creation and distruction of workspaces while trying to move a window to last workspace. Unblock it when workspaceSwitcherPopup is destroyed. Prepend a new workspace by creating a new workspace instead of only shifting the windows to next workspace. Added _isWorkspacePrepended flag to make sure only a single workspace could be prepended at a time.
(In reply to Florian Müllner from comment #5) > Review of attachment 299362 [details] [review] [review]: > > Works nicely for moving windows past the end, but unfortunately breaks in > the opposite direction: in 3.16, we allow windows on the first workspace to > be moved to a newly created workspace on top using <shift><super>PgUp ... I think this https://bugzilla.gnome.org/attachment.cgi?id=299699&action=diff should fix the issue which you mentioned. I have added a flag to make sure that only a single workspace could be prepended at a time and it is done by actually creating a new workspace instead of just shifting the windows down by one. Doing this would make sure switcherPopup is in sync and shows correct number of workspaces with focus on the right one.
Review of attachment 299699 [details] [review]: Not the prettiest, but I didn't come up with anything pretties either, so OK. However the commit message is misleading - "Add functions to block/unblock dynamic workspaces" misses about 80% of what the patch does. There are several bits in the patch: - Make sure to only prepend a single workspace while the popup is up - Don't rely on automatic workspace updates when inserting a workspace - Block dynamic workspace updates while showing the popup The last one might be a possible subject for the existing patch, or you can split the patch in three. In any case, the message body should be less of an description of what the patch does, and more an explanation of the reason behind the patch - you managed to not mention the stuttering at all :-)
Created attachment 300271 [details] [review] Block dynamic workspace updates while showing the popup Pause Dynamic Workspace Management by blocking it while workspaceSwitcherPopup is shown so as to eliminate infinite creation and distruction of workspaces, thus preventing stuttering while trying to move a window to last workspace. Added _isWorkspacePrepended flag to make sure only a single workspace could be prepended at a time thus preventing the possibility of prepending infinite workspaces while dynamic workspace management is on pause. Prepend a new workspace by creating a new workspace instead of only shifting the windows to next workspace so that the workspaceSwitcherPopup may appear in sync with what's happening behind the scene and display correct number of workspaces.
Created attachment 300272 [details] [review] Revert "Add functions to block/unblock Dynamic Workspace" This reverts commit 89eb1d88bd2ce089d425b993be69d64364bc6ec6.
Comment on attachment 299699 [details] [review] Add functions to block/unblock Dynamic Workspace >From 665e8f4874a7922a01f32b20234a8a84bcb309b6 Mon Sep 17 00:00:00 2001 >From: Shivam Mishra <shivam.m@live.com> >Date: Wed, 18 Mar 2015 16:19:24 +0530 >Subject: [PATCH] Add functions to block/unblock Dynamic Workspace > >Pause Dynamic Workspace Management by blocking it while >workspaceSwitcherPopup is shown so as to eliminate infinite creation >and distruction of workspaces while trying to move a window to last >workspace. >Unblock it when workspaceSwitcherPopup is destroyed. >Prepend a new workspace by creating a new workspace instead of only >shifting the windows to next workspace. >Added _isWorkspacePrepended flag to make sure only a single workspace >could be prepended at a time. > >https://bugzilla.gnome.org/show_bug.cgi?id=712778 >--- > js/ui/windowManager.js | 25 ++++++++++++++++++++++++- > 1 file changed, 24 insertions(+), 1 deletion(-) > >diff --git a/js/ui/windowManager.js b/js/ui/windowManager.js >index d04455b..5ab4bb6 100644 >--- a/js/ui/windowManager.js >+++ b/js/ui/windowManager.js >@@ -197,6 +197,8 @@ const WorkspaceTracker = new Lang.Class({ > this._workspaces = []; > this._checkWorkspacesId = 0; > >+ this._pauseWorkspaceCheck = false; >+ > let tracker = Shell.WindowTracker.get_default(); > tracker.connect('startup-sequence-changed', Lang.bind(this, this._queueCheckWorkspaces)); > >@@ -220,6 +222,14 @@ const WorkspaceTracker = new Lang.Class({ > return new Gio.Settings({ schema_id: 'org.gnome.mutter' }); > }, > >+ blockUpdates: function() { >+ this._pauseWorkspaceCheck = true; >+ }, >+ >+ unblockUpdates: function() { >+ this._pauseWorkspaceCheck = false; >+ }, >+ > _checkWorkspaces: function() { > let i; > let emptyWorkspaces = []; >@@ -229,6 +239,10 @@ const WorkspaceTracker = new Lang.Class({ > return false; > } > >+ // Update workspaces only if Dynamic Workspace Management has not been paused by some other function >+ if (this._pauseWorkspaceCheck) >+ return true; >+ > for (i = 0; i < this._workspaces.length; i++) { > let lastRemoved = this._workspaces[i]._lastRemovedWindow; > if ((lastRemoved && >@@ -627,6 +641,8 @@ const WindowManager = new Lang.Class({ > > this._allowedKeybindings = {}; > >+ this._isWorkspacePrepended = false; >+ > this._switchData = null; > this._shellwm.connect('kill-switch-workspace', Lang.bind(this, this._switchWorkspaceDone)); > this._shellwm.connect('kill-window-effects', Lang.bind(this, function (shellwm, actor) { >@@ -915,6 +931,8 @@ const WindowManager = new Lang.Class({ > if (!Meta.prefs_get_dynamic_workspaces()) > return; > >+ global.screen.append_new_workspace(false, global.get_current_time()); >+ > let windows = global.get_window_actors().map(function(winActor) { > return winActor.meta_window; > }); >@@ -1632,8 +1650,10 @@ const WindowManager = new Lang.Class({ > } else if (isNaN(target)) { > // Prepend a new workspace dynamically > if (screen.get_active_workspace_index() == 0 && >- action == 'move' && target == 'up') >+ action == 'move' && target == 'up' && this._isWorkspacePrepended == false) { > this.insertWorkspace(0); >+ this._isWorkspacePrepended = true; >+ } > > direction = Meta.MotionDirection[target.toUpperCase()]; > newWs = screen.get_active_workspace().get_neighbor(direction); >@@ -1658,9 +1678,12 @@ const WindowManager = new Lang.Class({ > > if (!Main.overview.visible) { > if (this._workspaceSwitcherPopup == null) { >+ this._workspaceTracker.blockUpdates(); > this._workspaceSwitcherPopup = new WorkspaceSwitcherPopup.WorkspaceSwitcherPopup(); > this._workspaceSwitcherPopup.connect('destroy', Lang.bind(this, function() { >+ this._workspaceTracker.unblockUpdates(); > this._workspaceSwitcherPopup = null; >+ this._isWorkspacePrepended = false; > })); > } > this._workspaceSwitcherPopup.display(direction, newWs.index()); >-- >2.1.0
Sorry about the last 2 comments (Comment 10, Comment 11). They got posted by mistake.
Review of attachment 300271 [details] [review]: There's a small glitch with using the isPrepended flag: - use shortcut to move window before first workspace - wait for popup to hide, then use shortcut again on the same window We end up prepending another workspace, leaving the second one empty until the popup is hidden again. But let's go with this for now ... Minor nit in the commit message: Should use present tense consistently ("Added _isWorkspacePrepended flag ...").
Created attachment 300440 [details] [review] windowManager: Block dynamic workspace updates while showing the popup Fix some style issues and typo in the commit message, don't use tab for indentation.
Attachment 300440 [details] pushed as aeb971c - windowManager: Block dynamic workspace updates while showing the popup