From 4926a9b8f958617d67d603622b1382c17fe4037c Mon Sep 17 00:00:00 2001

From: =?UTF-8?q?Jonas=20Dre=C3=9Fler?= <verdre@v0yd.nl>

Date: Wed, 20 May 2020 12:05:04 +0200

Subject: [PATCH 1/2] workspacesView: Avoid setting invalid geometries on views

The fullGeometry and the actualGeometry of the WorkspacesDisplay are set

from the allocation of the overviews ControlsManager and the

WorkspacesDisplay, that means they're only valid after those actors got

their allocations during Clutters allocation cycle.

Since WorkspacesDisplay._updateWorkspacesViews() is already called while

showing/mapping the WorkspacesDisplay, that allocation cycle didn't

happen yet and we end up either setting the geometries of the views to

null (in case of the fullGeometry) or to something wrong (a 0-sized

allocation in case of the actualGeometry).

So avoid setting invalid geometries on the views by initializing both

the fullGeometry and the actualGeometry to null, and then only updating

the geometries of the views after they're set to a correct value.

Note that this means we won't correctly animate the overview the first

time we open it since the animation depends on the geometries being set,

but is being started from show(), which means no allocations have

happened yet. In practice this introduces no regression though since

before this change we simply used incorrect geometries (see the 0-sized

allocation mentioned above) on the initial opening and the animation

didn't work either.

https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/1119

---

 js/ui/workspacesView.js | 28 +++++++++++++++++-----------

 1 file changed, 17 insertions(+), 11 deletions(-)

diff --git a/js/ui/workspacesView.js b/js/ui/workspacesView.js

index e302296a6..02baddc6e 100644

--- a/js/ui/workspacesView.js

+++ b/js/ui/workspacesView.js

@@ -521,6 +521,7 @@ var WorkspacesDisplay = class {

         this._scrollEventId = 0;

         this._keyPressEventId = 0;

+        this._actualGeometry = null;

         this._fullGeometry = null;

     }

@@ -675,8 +676,10 @@ var WorkspacesDisplay = class {

         this._workspacesViews.forEach(v => v.actor.show());

-        this._updateWorkspacesFullGeometry();

-        this._updateWorkspacesActualGeometry();

+        if (this._fullGeometry)

+            this._syncWorkspacesFullGeometry();

+        if (this._actualGeometry)

+            this._syncWorkspacesActualGeometry();

     }

     _scrollValueChanged() {

@@ -739,10 +742,10 @@ var WorkspacesDisplay = class {

     // the sliding controls were never slid in at all.

     setWorkspacesFullGeometry(geom) {

         this._fullGeometry = geom;

-        this._updateWorkspacesFullGeometry();

+        this._syncWorkspacesFullGeometry();

     }

-    _updateWorkspacesFullGeometry() {

+    _syncWorkspacesFullGeometry() {

         if (!this._workspacesViews.length)

             return;

@@ -754,18 +757,21 @@ var WorkspacesDisplay = class {

     }

     _updateWorkspacesActualGeometry() {

+        const [x, y] = this.actor.get_transformed_position();

+        const width = this.actor.allocation.get_width();

+        const height = this.actor.allocation.get_height();

+

+        this._actualGeometry = { x, y, width, height };

+        this._syncWorkspacesActualGeometry();

+    }

+

+    _syncWorkspacesActualGeometry() {

         if (!this._workspacesViews.length)

             return;

-        let [x, y] = this.actor.get_transformed_position();

-        let allocation = this.actor.allocation;

-        let width = allocation.x2 - allocation.x1;

-        let height = allocation.y2 - allocation.y1;

-        let primaryGeometry = { x: x, y: y, width: width, height: height };

-

         let monitors = Main.layoutManager.monitors;

         for (let i = 0; i < monitors.length; i++) {

-            let geometry = (i == this._primaryIndex) ? primaryGeometry : monitors[i];

+            let geometry = i === this._primaryIndex ? this._actualGeometry : monitors[i];

             this._workspacesViews[i].setActualGeometry(geometry);

         }

     }

-- 

2.26.2

From 4671eebccf4e6afce8c0a869d63095b39aa7e163 Mon Sep 17 00:00:00 2001

From: =?UTF-8?q?Jonas=20Dre=C3=9Fler?= <verdre@v0yd.nl>

Date: Wed, 20 May 2020 13:39:11 +0200

Subject: [PATCH 2/2] workspacesView: Only animate on show() when geometries

 are already set

Animating the window clones of the overview requires the fullGeometry

and the actualGeometry to be set, which they won't be when showing the

overview for the first time. So don't even try to animate the window

clones in that case because the geometries will still be null and

accessing them in workspace.js will throw errors.

The workspace views will still get the correct layout as soon as the

allocations happen because syncing the geometries will trigger updating

the window positions. Since animations are disabled for position changes

when syncing the geometry though, we won't get an animation and the

clones will jump into place. That's not a regression though since before

this change we also didn't animate in that case because the geometries

used were simply wrong (the actualGeometry was 0-sized as explained in

the last commit).

If we wanted to fix the initial animation of the overview, we'd have to

always enable animations of the window clones when syncing geometries,

but that would break the animation of the workspace when hovering the

workspaceThumbnail slider, because right now those animations are "glued

together" using the actualGeometry, so they would get out of sync.

The reason there are no errors happening in workspace.js with the

existing code is that due to a bug in Clutter the fullGeometry of

WorkspacesDisplay gets set very early while mapping the WorkspacesViews

(because the overviews ControlsManager gets an allocation during the

resource scale calculation of a ClutterClone, see

https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/1181), so it

won't be set to null anymore when calling

WorkspacesView.animateToOverview().

https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/1119

---

 js/ui/workspacesView.js | 17 ++++++++++-------

 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/js/ui/workspacesView.js b/js/ui/workspacesView.js

index 02baddc6e..3e9d77655 100644

--- a/js/ui/workspacesView.js

+++ b/js/ui/workspacesView.js

@@ -589,13 +589,16 @@ var WorkspacesDisplay = class {

     show(fadeOnPrimary) {

         this._updateWorkspacesViews();

-        for (let i = 0; i < this._workspacesViews.length; i++) {

-            let animationType;

-            if (fadeOnPrimary && i == this._primaryIndex)

-                animationType = AnimationType.FADE;

-            else

-                animationType = AnimationType.ZOOM;

-            this._workspacesViews[i].animateToOverview(animationType);

+

+        if (this._actualGeometry && this._fullGeometry) {

+            for (let i = 0; i < this._workspacesViews.length; i++) {

+                let animationType;

+                if (fadeOnPrimary && i == this._primaryIndex)

+                    animationType = AnimationType.FADE;

+                else

+                    animationType = AnimationType.ZOOM;

+                this._workspacesViews[i].animateToOverview(animationType);

+            }

         }

         this._restackedNotifyId =

-- 

2.26.2