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 685285 - No animations when dragging to create a new workspace
No animations when dragging to create a new workspace
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: general
3.6.x
Other Linux
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2012-10-02 13:49 UTC by Allan Day
Modified: 2013-03-17 18:36 UTC
See Also:
GNOME target: ---
GNOME version: 3.7/3.8


Attachments
WorkspaceThumbnails: animate the right workspace when creating by DND (3.58 KB, patch)
2012-12-16 22:56 UTC, Giovanni Campagna
none Details | Review
Workspace/WorkspaceThumbnail: don't destroy actors while accepting a drag (8.69 KB, patch)
2012-12-16 23:00 UTC, Giovanni Campagna
rejected Details | Review
Workspace/WorkspaceThumbnail: fix Clutter warnings from stacking code (3.27 KB, patch)
2013-01-19 01:30 UTC, Giovanni Campagna
accepted-commit_now Details | Review
DND: don't cancel a drag if the actor is destroyed inside acceptDrop (2.91 KB, patch)
2013-01-19 01:30 UTC, Giovanni Campagna
accepted-commit_now Details | Review
WorkspacesView: don't fail when doing window DND from the last workspace (1.63 KB, patch)
2013-01-19 01:30 UTC, Giovanni Campagna
needs-work Details | Review
WorkspaceThumbnails: animate the right workspace when creating by DND (3.15 KB, patch)
2013-01-19 01:34 UTC, Giovanni Campagna
accepted-commit_now Details | Review
WorkspacesView: don't fail when doing window DND from the last workspace (1.57 KB, patch)
2013-03-17 18:31 UTC, Giovanni Campagna
accepted-commit_now Details | Review

Description Allan Day 2012-10-02 13:49:47 UTC
Dragging a window or an application launcher between two workspaces creates a new workspace. To indicate this, we increase the padding between the two workspaces and insert a spacer graphic when the dragged object is hovered over the space between workspaces.

However, these transitions are not animated, nor is the insertion of the new workspace. The result is jerky and disharmonious, and it doesn't clearly convey the arrival of the new workspace.

Two things we can do:

 * For the hover state, smoothly increase the space between the workspaces and make the spacer appear to grow into the space.

 * Animate in the new workspace.
Comment 1 Jasper St. Pierre (not reading bugmail) 2012-10-02 18:27:02 UTC
Yeah, this was because I didn't have time to do this. It's a hard problem; the workspace thumbnails code is complex.
Comment 2 Giovanni Campagna 2012-12-16 22:56:37 UTC
Created attachment 231680 [details] [review]
WorkspaceThumbnails: animate the right workspace when creating by DND

To create a new workspace by dropping on the placeholder, we move all
the windows down one workspace and then wait for _checkWorkspaces() to
automatically create the empty one at the end.
This means that, from the implementation POV, the new workspace is not
the one created by DND but it's the last one, and this detail was exposed
in the UI because the animation was applied on that one.
Fix that by starting the animation manually from the DND code, and then
blocking the animation from happening when the new workspace is created
with a flag.
Comment 3 Giovanni Campagna 2012-12-16 23:00:15 UTC
Created attachment 231681 [details] [review]
Workspace/WorkspaceThumbnail: don't destroy actors while accepting a drag

The DND code treats a destroyed actor as a cancelled drag and emits the drag-end
signal. That causes clutter warnings from signal handlers that try to reparent
the destroyed actor.

I forgot, the previous patch is on top of this one, otherwise it probably won't
apply.
Comment 4 Jasper St. Pierre (not reading bugmail) 2013-01-15 19:15:25 UTC
Review of attachment 231681 [details] [review]:

So, what's happening is that the source actor is destroyed, which causes the clone to be destroyed as well? Wouldn't constructing a new WindowClone instead of using a ClutterClone for the DND actor work as well?

::: js/ui/workspace.js
@@ +1992,1 @@
             let index = this.metaWorkspace ? this.metaWorkspace.index() : global.screen.get_active_workspace_index();

At some point we gotta clean up the mutter codebase to be based around MetaWorkspace, not a hybrid of that and indexes.

@@ +2005,3 @@
+                                                     false, // don't create workspace
+                                                     time);
+            });

return false;

@@ +2008,3 @@
+
+            // Tell the actor we're accepting the drop, but we wish to keep it alive
+            Main.uiGroup.remove_actor(actor);

How does this work?
Comment 5 Giovanni Campagna 2013-01-19 01:29:08 UTC
Comment on attachment 231681 [details] [review]
Workspace/WorkspaceThumbnail: don't destroy actors while accepting a drag

Ok, this patch is totally wrong. I have a better series, which addresses the real problems.
Comment 6 Giovanni Campagna 2013-01-19 01:30:11 UTC
Created attachment 233818 [details] [review]
Workspace/WorkspaceThumbnail: fix Clutter warnings from stacking code

Windows can be restacked at any time, including when the stackAbove
property of the window clone is during a drag, and thus parented to
the uiGroup. To do stacking properly, we need to skip it for the duration
of the drag, and sync it again at the end (which is already done by
mutter because of the workspace change)
Comment 7 Giovanni Campagna 2013-01-19 01:30:22 UTC
Created attachment 233819 [details] [review]
DND: don't cancel a drag if the actor is destroyed inside acceptDrop

This happens in the case of Workspace/WorkspaceThumbnail: they call
meta_window_change_workspace_by_index(), which fires window-removed
on the old workspace, thus destroying the window clone.
Comment 8 Giovanni Campagna 2013-01-19 01:30:36 UTC
Created attachment 233820 [details] [review]
WorkspacesView: don't fail when doing window DND from the last workspace

DND of windows has a lot of side effects, including the possibility of
current workspace disappering from under our feet. We need to account
for that when trying to activate it.
Comment 9 Giovanni Campagna 2013-01-19 01:34:02 UTC
Created attachment 233821 [details] [review]
WorkspaceThumbnails: animate the right workspace when creating by DND

To create a new workspace by dropping on the placeholder, we move all
the windows down one workspace and then wait for _checkWorkspaces() to
automatically create the empty one at the end.
This means that, from the implementation POV, the new workspace is not
the one created by DND but it's the last one, and this detail was exposed
in the UI because the animation was applied on that one.
Fix that by starting the animation manually from the DND code, and then
blocking the animation from happening when the new workspace is created
with a flag.

Rebased (and maybe not even requiring all the stacking changes if one
just wants to test the animation)
Comment 10 Jasper St. Pierre (not reading bugmail) 2013-02-16 17:38:01 UTC
Review of attachment 233818 [details] [review]:

"including when the stackAbove property of the window clone is during a drag"

I think you're missing a word here.
Comment 11 Giovanni Campagna 2013-03-16 22:42:29 UTC
Hard code freeze ping!

(I'll commit the acn one before the release if you don't review the others)
Comment 12 Jasper St. Pierre (not reading bugmail) 2013-03-17 17:42:08 UTC
Review of attachment 233821 [details] [review]:

OK.

::: js/ui/workspaceThumbnail.js
@@ +981,3 @@
         this._indicator.raise_top();
+
+        // Clear the splice index, we got the message

"the message"? What message?
Comment 13 Jasper St. Pierre (not reading bugmail) 2013-03-17 17:43:28 UTC
Review of attachment 233820 [details] [review]:

::: js/ui/workspacesView.js
@@ +417,3 @@
+
+            if (this._workspaces[current]) {
+                metaWorkspace = this._workspaces[current].metaWorkspace;

if (!this._workspaces[current]) {
    // The current workspace was destroyed.
    // ...
    current = this._workspaces.length - 1;
}
Comment 14 Jasper St. Pierre (not reading bugmail) 2013-03-17 18:19:49 UTC
Review of attachment 233819 [details] [review]:

OK.
Comment 15 Giovanni Campagna 2013-03-17 18:31:22 UTC
Created attachment 239069 [details] [review]
WorkspacesView: don't fail when doing window DND from the last workspace

DND of windows has a lot of side effects, including the possibility of
current workspace disappering from under our feet. We need to account
for that when trying to activate it.
Comment 16 Jasper St. Pierre (not reading bugmail) 2013-03-17 18:32:54 UTC
Review of attachment 239069 [details] [review]:

::: js/ui/workspacesView.js
@@ +401,3 @@
+            }
+
+            metaWorkspace = this._workspaces[current].metaWorkspace;

Uh, you can move the "let" back here now.
Comment 17 Giovanni Campagna 2013-03-17 18:36:52 UTC
Attachment 233818 [details] pushed as ceae032 - Workspace/WorkspaceThumbnail: fix Clutter warnings from stacking code
Attachment 233819 [details] pushed as 57f2757 - DND: don't cancel a drag if the actor is destroyed inside acceptDrop
Attachment 233821 [details] pushed as ee0040e - WorkspaceThumbnails: animate the right workspace when creating by DND
Attachment 239069 [details] pushed as 9e60a55 - WorkspacesView: don't fail when doing window DND from the last workspace