From b4e5ab72e62d7471fb7155f75caf88bc9127d814 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Mon, 29 Apr 2024 20:36:38 +0200 Subject: [PATCH 1/7] windowManager: Honor animation blocking when activating a workspace When the workspace switch with move window is started we the workspace animation is started twice: first because the workspace is activated as part of insertWorkspace(), and then because of the explicit change workspace in actionMoveWindow(). However, the code in insertWorkspace() was supposed not to trigger any animation, but during the revert of commit ff9bb539 (happened in commit 3d39b32a), reverting 1cac7b22 was forgotten, leading to this issue. This implies that the animation isn't performed properly (e.g. the window moves to the new workspace after the animation is completed, and it's not moved with the animation). As per this, add back the check on blockAnimations in shouldAnimate. --- js/ui/windowManager.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/js/ui/windowManager.js b/js/ui/windowManager.js index dd67b8ea3..c4dc880ed 100644 --- a/js/ui/windowManager.js +++ b/js/ui/windowManager.js @@ -1129,6 +1129,9 @@ export class WindowManager { } _shouldAnimate() { + if (this._blockAnimations) + return false; + const overviewOpen = Main.overview.visible && !Main.overview.closing; return !(overviewOpen || this._workspaceAnimation.gestureActive); } From bb3bb326859d012fbca7f4bf54e1e461dde0bce8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Mon, 29 Apr 2024 17:34:37 +0200 Subject: [PATCH 2/7] windowManager: Do not cancel workspace animation on kill-switch-workspace The kill-switch-workspace event is emitted by the shell plugin (via mutter plugin manager) just before a workspace switch is requested however this implies that we reset all the animation state information even if the switch is happening while another workspace switch is happening, and this helps to contribute breaking the workspace animations because once a new workspace switch is requested, we end up starting over the animation from the initial point. We now still complete the animation, but we rely on the animation completion for that to happen now. --- js/ui/windowManager.js | 1 - 1 file changed, 1 deletion(-) diff --git a/js/ui/windowManager.js b/js/ui/windowManager.js index c4dc880ed..349693ca5 100644 --- a/js/ui/windowManager.js +++ b/js/ui/windowManager.js @@ -956,7 +956,6 @@ export class WindowManager { new WorkspaceAnimation.WorkspaceAnimationController(); this._shellwm.connect('kill-switch-workspace', () => { - this._workspaceAnimation.cancelSwitchAnimation(); this._switchWorkspaceDone(); }); } From 5e6a3df432a5592a04e815fee2564570c0f0e9f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Mon, 29 Apr 2024 18:07:59 +0200 Subject: [PATCH 3/7] workspaceAnimation: Move monitor group workspace group creation to function Currently once a monitor group is created it can't be changed at runtime so it's not possible to add or remove workspace groups to it. Move this functionality out to the constructor and to a function so that it's possible to adjust the workspace indices at runtime --- js/ui/workspaceAnimation.js | 73 +++++++++++++++++++++++++------------ 1 file changed, 50 insertions(+), 23 deletions(-) diff --git a/js/ui/workspaceAnimation.js b/js/ui/workspaceAnimation.js index b278113e2..7f392206a 100644 --- a/js/ui/workspaceAnimation.js +++ b/js/ui/workspaceAnimation.js @@ -50,6 +50,10 @@ class WorkspaceGroup extends Clutter.Actor { this._syncStacking.bind(this), this); } + get monitor() { + return this._monitor; + } + get workspace() { return this._workspace; } @@ -174,26 +178,64 @@ export const MonitorGroup = GObject.registerClass({ this.add_child(stickyGroup); this._workspaceGroups = []; + this._workspaceIndices = []; + this._movingWindow = movingWindow; const workspaceManager = global.workspace_manager; - const vertical = workspaceManager.layout_rows === -1; const activeWorkspace = workspaceManager.get_active_workspace(); + this.setWorkspaceIndices(workspaceIndices); + + this.progress = this.getWorkspaceProgress(activeWorkspace); + + if (monitor.index === Main.layoutManager.primaryIndex) { + this._workspacesAdjustment = Main.createWorkspacesAdjustment(this); + this.bind_property_full('progress', + this._workspacesAdjustment, 'value', + GObject.BindingFlags.SYNC_CREATE, + (_bind, source) => { + const indices = [ + this._workspaceIndices[Math.floor(source)], + this._workspaceIndices[Math.ceil(source)], + ]; + return [true, Util.lerp(...indices, source % 1.0)]; + }, + null); + + this.connect('destroy', () => { + delete this._workspacesAdjustment; + }); + } + } + + setWorkspaceIndices(workspaceIndices) { + if (workspaceIndices.length === this._workspaceIndices.length && + workspaceIndices.every((idx, i) => idx === this._workspaceIndices[i])) + return; + + const {workspaceManager} = global; + const vertical = workspaceManager.layout_rows === -1; + let x = 0; let y = 0; + this._workspaceGroups = []; + this._container.destroy_all_children(); + for (const i of workspaceIndices) { const ws = workspaceManager.get_workspace_by_index(i); - const fullscreen = ws.list_windows().some(w => w.get_monitor() === monitor.index && w.is_fullscreen()); + const fullscreen = ws.list_windows().some(w => + w.get_monitor() === this._monitor.index && w.is_fullscreen()); - if (i > 0 && vertical && !fullscreen && monitor.index === Main.layoutManager.primaryIndex) { + if (i > 0 && vertical && !fullscreen && + this._monitor.index === Main.layoutManager.primaryIndex) { // We have to shift windows up or down by the height of the panel to prevent having a // visible gap between the windows while switching workspaces. Since fullscreen windows // hide the panel, they don't need to be shifted up or down. y -= Main.panel.height; } - const group = new WorkspaceGroup(ws, monitor, movingWindow); + const group = new WorkspaceGroup(ws, this._monitor, this._movingWindow); this._workspaceGroups.push(group); this._container.add_child(group); @@ -207,26 +249,11 @@ export const MonitorGroup = GObject.registerClass({ x += this.baseDistance; } - this.progress = this.getWorkspaceProgress(activeWorkspace); + this._workspaceIndices = workspaceIndices; + } - if (monitor.index === Main.layoutManager.primaryIndex) { - this._workspacesAdjustment = Main.createWorkspacesAdjustment(this); - this.bind_property_full('progress', - this._workspacesAdjustment, 'value', - GObject.BindingFlags.SYNC_CREATE, - (bind, source) => { - const indices = [ - workspaceIndices[Math.floor(source)], - workspaceIndices[Math.ceil(source)], - ]; - return [true, Util.lerp(...indices, source % 1.0)]; - }, - null); - - this.connect('destroy', () => { - delete this._workspacesAdjustment; - }); - } + get workspaceIndices() { + return this._workspaceIndices; } get baseDistance() { From d331d6682497262a95e59e70f19a8ba893b7a026 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Mon, 29 Apr 2024 18:42:03 +0200 Subject: [PATCH 4/7] workspaceAnimation: Add support for adding workspace groups dynamically We may need to include more workspaces we animate to if an user requires a workspace switch while we're already animating between a fixed number of workspaces. To do this, we just recreate the monitors group container, trying to recycle the groups we already have and adjusting the container position so that the animation can continue were we've left it. --- js/ui/workspaceAnimation.js | 50 ++++++++++++++++++++++++++++++++----- 1 file changed, 44 insertions(+), 6 deletions(-) diff --git a/js/ui/workspaceAnimation.js b/js/ui/workspaceAnimation.js index 7f392206a..aea265abe 100644 --- a/js/ui/workspaceAnimation.js +++ b/js/ui/workspaceAnimation.js @@ -58,6 +58,10 @@ class WorkspaceGroup extends Clutter.Actor { return this._workspace; } + get movingWindow() { + return this._movingWindow; + } + _shouldShowWindow(window) { if (!window.showing_on_its_workspace() || this._isDesktopWindow(window)) return false; @@ -209,18 +213,16 @@ export const MonitorGroup = GObject.registerClass({ } setWorkspaceIndices(workspaceIndices) { - if (workspaceIndices.length === this._workspaceIndices.length && - workspaceIndices.every((idx, i) => idx === this._workspaceIndices[i])) - return; - const {workspaceManager} = global; const vertical = workspaceManager.layout_rows === -1; let x = 0; let y = 0; + let oldFirstGroup; + const oldGroups = this._workspaceGroups; this._workspaceGroups = []; - this._container.destroy_all_children(); + this._container.remove_all_children(); for (const i of workspaceIndices) { const ws = workspaceManager.get_workspace_by_index(i); @@ -235,7 +237,18 @@ export const MonitorGroup = GObject.registerClass({ y -= Main.panel.height; } - const group = new WorkspaceGroup(ws, this._monitor, this._movingWindow); + let group; + let groupIndex = oldGroups.findIndex(g => + g.workspace === ws && g.monitor === this._monitor && + g.movingWindow === this._movingWindow); + if (groupIndex === -1) { + group = new WorkspaceGroup(ws, this._monitor, this._movingWindow); + } else { + [group] = oldGroups.splice(groupIndex, 1); + + if (!oldFirstGroup && group.x === 0 && group.y === 0) + oldFirstGroup = group; + } this._workspaceGroups.push(group); this._container.add_child(group); @@ -249,13 +262,38 @@ export const MonitorGroup = GObject.registerClass({ x += this.baseDistance; } + if (oldFirstGroup) { + if (vertical) + this._container.y -= oldFirstGroup.y; + else + this._container.x -= oldFirstGroup.x; + } + + oldGroups.forEach(g => g.destroy()); this._workspaceIndices = workspaceIndices; } + addWorkspaceIndex(wsIndex) { + const indices = new Set([...this._workspaceIndices, wsIndex]); + this.setWorkspaceIndices([...indices].sort((a, b) => a - b)); + } + get workspaceIndices() { return this._workspaceIndices; } + set movingWindow(movingWindow) { + if (this._movingWindow === movingWindow) + return; + + this._movingWindow = movingWindow; + this.setWorkspaceIndices(this._workspaceIndices); + } + + get movingWindow() { + return this._movingWindow; + } + get baseDistance() { const spacing = WORKSPACE_SPACING * St.ThemeContext.get_for_stage(global.stage).scale_factor; From 781724b2c51008e14427f72298bb97b08a40aadd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Mon, 29 Apr 2024 21:32:56 +0200 Subject: [PATCH 5/7] workspaceAnimation: Don't change monitorGroup animation progress when animating If a new workspace animation is started when anther was still ongoing, we were resetting the animation progress to the initial state, causing a visual imperfection: the animation is started again from the beginning making the windows to be shown twice. So, in case a new animation has been requested while we're already animating, just keep the animation progress to the value we are at, but ensure that the monitor group contains all the workspaces items we're animating from and to, so that they can be dynamically added if we're switching to a workspace that wasn't previously part of the animation (as it happens when hitting the key-bindings again while we're switching workspace). As per this, also reduce the animation time so that it's proportional to the time needed to reach the requested progress level. --- js/ui/workspaceAnimation.js | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/js/ui/workspaceAnimation.js b/js/ui/workspaceAnimation.js index aea265abe..0665f3fba 100644 --- a/js/ui/workspaceAnimation.js +++ b/js/ui/workspaceAnimation.js @@ -480,13 +480,25 @@ export class WorkspaceAnimationController { workspaceIndices.reverse(); this._prepareWorkspaceSwitch(workspaceIndices); + const wasInProgress = this._switchData.inProgress; this._switchData.inProgress = true; const fromWs = global.workspace_manager.get_workspace_by_index(from); const toWs = global.workspace_manager.get_workspace_by_index(to); for (const monitorGroup of this._switchData.monitors) { - monitorGroup.progress = monitorGroup.getWorkspaceProgress(fromWs); + if (wasInProgress) { + monitorGroup.movingWindow = this._movingWindow; + + if (!monitorGroup.workspaceIndices.includes(from)) + monitorGroup.addWorkspaceIndex(from); + + if (!monitorGroup.workspaceIndices.includes(to)) + monitorGroup.addWorkspaceIndex(to); + } else { + monitorGroup.progress = monitorGroup.getWorkspaceProgress(fromWs); + } + const progress = monitorGroup.getWorkspaceProgress(toWs); const params = { @@ -494,6 +506,14 @@ export class WorkspaceAnimationController { mode: Clutter.AnimationMode.EASE_OUT_CUBIC, }; + if (wasInProgress) { + const {progress: monitorGroupProgress} = monitorGroup; + if (progress > monitorGroupProgress) + params.duration *= progress - monitorGroupProgress; + else + params.duration *= monitorGroupProgress; + } + if (monitorGroup.index === Main.layoutManager.primaryIndex) { params.onComplete = () => { this._finishWorkspaceSwitch(this._switchData); From 41fd8ed688c2eab9b498754d5db298f4d960c178 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Mon, 29 Apr 2024 21:45:01 +0200 Subject: [PATCH 6/7] workspaceAnimation: Do not try to access invalid window records In some cases (such as for override redirect windows) we don't create window records, but we may still try to access them when doing stack synchronization. Avoid the error. --- js/ui/workspaceAnimation.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/js/ui/workspaceAnimation.js b/js/ui/workspaceAnimation.js index 0665f3fba..2cf647991 100644 --- a/js/ui/workspaceAnimation.js +++ b/js/ui/workspaceAnimation.js @@ -92,6 +92,8 @@ class WorkspaceGroup extends Clutter.Actor { for (const windowActor of windowActors) { const record = this._windowRecords.find(r => r.windowActor === windowActor); + if (!record) + continue; this.set_child_above_sibling(record.clone, lastRecord ? lastRecord.clone : bottomActor); From 3293820d67fbd668263cae248a83289b5b6e6003 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Tue, 30 Apr 2024 21:41:54 +0200 Subject: [PATCH 7/7] workspaceAnimation: Compute the base distance only when needed This value is used during animation to get the progress, but it can be easily cached since it does not depend on any variable value. So let's just compute when required and avoid further JS->C calls while animating. --- js/ui/workspaceAnimation.js | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/js/ui/workspaceAnimation.js b/js/ui/workspaceAnimation.js index 2cf647991..ca826d994 100644 --- a/js/ui/workspaceAnimation.js +++ b/js/ui/workspaceAnimation.js @@ -190,6 +190,12 @@ export const MonitorGroup = GObject.registerClass({ const workspaceManager = global.workspace_manager; const activeWorkspace = workspaceManager.get_active_workspace(); + this._updateBaseDistance(); + St.ThemeContext.get_for_stage(global.stage).connectObject( + 'notify::scale-factor', () => this._updateBaseDistance(), this); + Main.layoutManager.connectObject('monitors-changed', + () => this._updateBaseDistance(), this); + this.setWorkspaceIndices(workspaceIndices); this.progress = this.getWorkspaceProgress(activeWorkspace); @@ -214,6 +220,13 @@ export const MonitorGroup = GObject.registerClass({ } } + _updateBaseDistance() { + this._baseDistance = global.workspace_manager.layout_rows === -1 + ? this._monitor.height : this._monitor.width; + this._baseDistance += WORKSPACE_SPACING * + St.ThemeContext.get_for_stage(global.stage).scaleFactor; + } + setWorkspaceIndices(workspaceIndices) { const {workspaceManager} = global; const vertical = workspaceManager.layout_rows === -1; @@ -297,12 +310,7 @@ export const MonitorGroup = GObject.registerClass({ } get baseDistance() { - const spacing = WORKSPACE_SPACING * St.ThemeContext.get_for_stage(global.stage).scale_factor; - - if (global.workspace_manager.layout_rows === -1) - return this._monitor.height + spacing; - else - return this._monitor.width + spacing; + return this._baseDistance; } get progress() {