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 712778 - Window stutters when holding ctrl+alt+shift+down
Window stutters when holding ctrl+alt+shift+down
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: general
3.10.x
Other Linux
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2013-11-21 00:23 UTC by sam113101
Modified: 2015-03-27 13:36 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Add functions to block/unblock Dynamic Workspace (2.46 KB, patch)
2015-03-13 21:17 UTC, Shivam Mishra
none Details | Review
Add functions to block/unblock Dynamic Workspace (4.12 KB, patch)
2015-03-18 11:08 UTC, Shivam Mishra
reviewed Details | Review
Block dynamic workspace updates while showing the popup (4.34 KB, patch)
2015-03-25 11:25 UTC, Shivam Mishra
none Details | Review
Revert "Add functions to block/unblock Dynamic Workspace" (3.74 KB, patch)
2015-03-25 12:01 UTC, Shivam Mishra
none Details | Review
windowManager: Block dynamic workspace updates while showing the popup (4.34 KB, patch)
2015-03-27 13:15 UTC, Florian Müllner
committed Details | Review

Description sam113101 2013-11-21 00:23:24 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.
Comment 1 Magdalen Berns (irc magpie) 2013-11-21 03:37:25 UTC
What version are you using? I am 3.10.2 and cannot recreate it.
Comment 2 sam113101 2013-11-26 02:47:40 UTC
I am using GNOME Shell 3.10.2.1
Comment 3 sam113101 2015-02-27 19:18:44 UTC
(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.
Comment 4 Shivam Mishra 2015-03-13 21:17:51 UTC
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.
Comment 5 Florian Müllner 2015-03-17 20:07:36 UTC
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 ...
Comment 6 Shivam Mishra 2015-03-18 11:08:36 UTC
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.
Comment 7 Shivam Mishra 2015-03-18 11:22:29 UTC
(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.
Comment 8 Florian Müllner 2015-03-23 22:18:36 UTC
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 :-)
Comment 9 Shivam Mishra 2015-03-25 11:25:51 UTC
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.
Comment 10 Shivam Mishra 2015-03-25 12:01:32 UTC
Created attachment 300272 [details] [review]
Revert "Add functions to block/unblock Dynamic Workspace"

This reverts commit 89eb1d88bd2ce089d425b993be69d64364bc6ec6.
Comment 11 Shivam Mishra 2015-03-25 12:20:52 UTC
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
Comment 12 Shivam Mishra 2015-03-25 12:35:09 UTC
Sorry about the last 2 comments (Comment 10, Comment 11). They got posted by mistake.
Comment 13 Florian Müllner 2015-03-25 22:05:36 UTC
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 ...").
Comment 14 Florian Müllner 2015-03-27 13:15:22 UTC
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.
Comment 15 Florian Müllner 2015-03-27 13:36:36 UTC
Attachment 300440 [details] pushed as aeb971c - windowManager: Block dynamic workspace updates while showing the popup