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 744183 - layout: Compute strut side more precisely for non primary monitors
layout: Compute strut side more precisely for non primary monitors
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: window-management
unspecified
Other All
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2015-02-08 22:17 UTC by Sylvain Pasche
Modified: 2015-02-20 13:00 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
layout: Compute strut side more precisely for non primary monitors (4.75 KB, patch)
2015-02-08 22:18 UTC, Sylvain Pasche
none Details | Review
layout: Compute strut side more precisely for non primary monitors (4.84 KB, patch)
2015-02-10 21:12 UTC, Sylvain Pasche
reviewed Details | Review
layout: Compute strut side more precisely for non primary monitors (4.32 KB, patch)
2015-02-11 20:00 UTC, Sylvain Pasche
committed Details | Review

Description Sylvain Pasche 2015-02-08 22:17:59 UTC
I'm using the window-list extension on multiple monitors (with "show on all monitors").

If I try to maximize a window vertically on non primary monitors, the window goes under
the bottom panel.

I found out that it happens because gnome-shell creates a strut with META_SIDE_LEFT
instead of META_SIDE_BOTTOM for the bottom panel of my left non primary monitor.

The attached patch should improve how strut sides are computed on non primary monitors.

It seems to me that the "NetWM struts" comment might not be relevant any more?
Comment 1 Sylvain Pasche 2015-02-08 22:18:02 UTC
Created attachment 296402 [details] [review]
layout: Compute strut side more precisely for non primary monitors

Don't assume struts are on the primary monitor while computing
the strut side. Instead, find the first monitor that overlaps the
strut and compute the strut side using it.
Comment 2 Rui Matos 2015-02-10 17:28:07 UTC
Review of attachment 296402 [details] [review]:

Ok, this looks reasonable to me. Can you submit at patch with a fixed comment instead of the FIXME?

::: js/ui/layout.js
@@ +976,3 @@
+                // FIXME: is this still accurate? meta_workspace_set_builtin_struts()
+                // bypasses the NetWM struts limitation and should work fine in
+                // multi-monitor scenarios.

Yeah, I think should fix the the comment here, since the limitation only applies to the _NET_WORKAREA prop we export and no longer to mutter's internal concept of struts.
Comment 3 Sylvain Pasche 2015-02-10 21:12:58 UTC
Created attachment 296541 [details] [review]
layout: Compute strut side more precisely for non primary monitors

Hi Rui, thanks for the review. I updated the patch to remove the somewhat obsolete NetWM paragraph and updated the rest.
Comment 4 Florian Müllner 2015-02-10 21:19:42 UTC
Review of attachment 296541 [details] [review]:

::: js/ui/layout.js
@@ +990,3 @@
+
+                    if (x1 < m.x + m.width && x2 > m.x &&
+                        y1 < m.y + m.height && y2 > m.y) {

Should we make meta_screen_get_monitor_for_rect() public instead?
Comment 5 Sylvain Pasche 2015-02-10 21:54:14 UTC
Looks like meta_screen_get_monitor_index_for_rect() is public. And I see now that there's even a findMonitorForActor() in layout.js that could be handy. I'll try to update the patch.
Comment 6 Sylvain Pasche 2015-02-11 20:00:05 UTC
Created attachment 296635 [details] [review]
layout: Compute strut side more precisely for non primary monitors

Much simpler now :)
Comment 7 Florian Müllner 2015-02-11 21:32:27 UTC
Review of attachment 296635 [details] [review]:

Looks good to me, thanks!
Comment 8 Florian Müllner 2015-02-20 12:59:58 UTC
Attachment 296635 [details] pushed as c2f5813 - layout: Compute strut side more precisely for non primary monitors