Created attachment 254838 [details] [review]
overviewControls: Update workspaces geometry on monitor changes

We currently update workspaces geometry on allocation changes of
the overview group; however as the geometry is based on stage
coordinates, it can change without an allocation change as well
when the primary monitor's position changes but not its resolution.

(In reply to comment #1)
> Created an attachment (id=254838) [details] [review]
> overviewControls: Update workspaces geometry on monitor changes
> 
> We currently update workspaces geometry on allocation changes of
> the overview group; however as the geometry is based on stage
> coordinates, it can change without an allocation change as well
> when the primary monitor's position changes but not its resolution.

Mh? If the primary monitor moves, then the overview moves, which triggers a new allocation.

(In reply to comment #2)
> Mh? If the primary monitor moves, then the overview moves, which triggers a new
> allocation.

Yes, the overview does move. But none of the OverviewControl's actors move relative to their parent, so we are hitting a Clutter optimization[0].
I guess we can track Main.layoutManager.overviewGroup instead and hope no one changes the hierarchy again without updating this (or just use global.stage).

[0] https://git.gnome.org/browse/clutter/tree/clutter/clutter-actor.c#n2268

Created attachment 254849 [details] [review]
overviewControls: Update workspaces geometry on monitor changes

For what it's worth, here's one of the alternatives ...

Uhm, but that's because we're using notify::allocation instead of overriding the vfunc properly. I think we should use a custom actor that calls the appropriate update function.

So you are suggesting to reimplement either ClutterBoxLayout or ClutterBinLayout? Why that overkill?

(In reply to comment #6)
> So you are suggesting to reimplement either ClutterBoxLayout or
> ClutterBinLayout? Why that overkill?

You don't need that, you can call the layout manager from the allocate vfunc.

Created attachment 254851 [details] [review]
overviewControls: Use a custom layout to catch all geometry changes

We currently update workspaces geometry when we are notified about
allocation changes of the overview group; however as the geometry
is based on stage coordinates, we miss notifications when the
allocation relative to the parent is unchanged, which happens when
the primary monitor's position changes but not its resolution.
Use a custom layout manager to give us a signal that is emitted
reliably.


Still feels like overkill to me, but here you are ...

Review of attachment 254851 [details] [review]:

I think this way is more robust, and we don't need workarounds.

::: js/ui/overviewControls.js
@@ +517,3 @@
+    vfunc_allocate: function(container, box, flags) {
+        this.parent(container, box, flags);
+        this.emit('allocation-changed');

I was thinking more of calling the updateGeometry function directly, but yeah, this works too.

Attachment 254851 [details] pushed as df09109 - overviewControls: Use a custom layout to catch all geometry changes