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 681743 - Unlock screen spans multiple monitors
Unlock screen spans multiple monitors
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: lock-screen
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2012-08-13 09:55 UTC by Stef Walter
Modified: 2013-01-03 01:08 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Unlock screen spanning multiple monitors (943.84 KB, image/png)
2012-08-13 09:55 UTC, Stef Walter
  Details
LayoutManager: add a ClutterConstraint for tracking monitor sizes (6.00 KB, patch)
2012-08-14 14:46 UTC, Giovanni Campagna
reviewed Details | Review
ScreenShield: place the lock screen contents only on the primary monitor (5.06 KB, patch)
2012-08-14 14:46 UTC, Giovanni Campagna
committed Details | Review
AltTab: get rid of ShellGenericContainer (12.48 KB, patch)
2012-08-14 14:46 UTC, Giovanni Campagna
none Details | Review
Port modal dialogs to the new MonitorConstraint (3.15 KB, patch)
2012-08-16 19:39 UTC, Giovanni Campagna
committed Details | Review
LayoutManager: add a ClutterConstraint for tracking monitor sizes (3.50 KB, patch)
2012-08-16 19:40 UTC, Giovanni Campagna
committed Details | Review

Description Stef Walter 2012-08-13 09:55:53 UTC
Created attachment 220981 [details]
Unlock screen spanning multiple monitors

The new 'shield' unlock screen spans multiple monitors in an awkward way. I had my monitors together so the effect was somewhat mitigated. See attached screenshot.

This is gnome-shell from git master.
Comment 1 Giovanni Campagna 2012-08-14 14:45:56 UTC
Hijacking this bug for some cleanups in the layout code...
Comment 2 Giovanni Campagna 2012-08-14 14:46:15 UTC
Created attachment 221148 [details] [review]
LayoutManager: add a ClutterConstraint for tracking monitor sizes

Instead of connecting manually to LayoutManager, or using ShellGenericContainer,
make a ClutterConstraint subclass that can track automatically
a specific monitor index, or the primary monitor.
Comment 3 Giovanni Campagna 2012-08-14 14:46:24 UTC
Created attachment 221149 [details] [review]
ScreenShield: place the lock screen contents only on the primary monitor

Use the new monitor constraint to place the clock and notification
box on the primary monitor only. The background is still extended
to the whole screen.
Get rid of the LockDialogGroup hack, now that ClutterBinLayout
respects fixed position correctly.
Comment 4 Giovanni Campagna 2012-08-14 14:46:32 UTC
Created attachment 221150 [details] [review]
AltTab: get rid of ShellGenericContainer

Use the new MonitorConstraint instead of requesting the screen size
and then allocating based on the primary monitor, and while we're
there rewrite AltTabPopup to be a St.Widget.
Comment 5 Jasper St. Pierre (not reading bugmail) 2012-08-14 15:45:53 UTC
Review of attachment 221148 [details] [review]:

Separate patches for modalDialog/wanda (???) as well, of course.

Mostly good. I'm impressed!

::: js/ui/layout.js
@@ +22,3 @@
+    Name: 'MonitorConstraint',
+    Extends: Clutter.Constraint,
+    Properties: {'primary': GObject.ParamSpec.boolean('primary', 

Any specific reason you chose GObject properties?

@@ +62,3 @@
+    vfunc_set_actor: function(actor) {
+        if (actor) {
+    },

Define this._monitorChangedId in _init please.

::: js/ui/modalDialog.js
@@ +201,2 @@
     _fadeOpen: function() {
+        this._monitorConstraint.index = global.screen.get_current_monitor();

Not so sure I like primaryMonitor vs. index. Seems clunky.
Comment 6 Jasper St. Pierre (not reading bugmail) 2012-08-14 15:46:37 UTC
Review of attachment 221149 [details] [review]:

Minor nit.

::: js/ui/layout.js
@@ +108,3 @@
                                                  visible: false,
                                                  clip_to_allocation: true,
+                                                 layout_manager: new Clutter.BinLayout,

()
Comment 7 Giovanni Campagna 2012-08-16 14:16:20 UTC
(In reply to comment #5)
> Review of attachment 221148 [details] [review]:
> ::: js/ui/layout.js
> @@ +22,3 @@
> +    Name: 'MonitorConstraint',
> +    Extends: Clutter.Constraint,
> +    Properties: {'primary': GObject.ParamSpec.boolean('primary', 
> 
> Any specific reason you chose GObject properties?

With those I save filtering params in _init.

> @@ +62,3 @@
> +    vfunc_set_actor: function(actor) {
> +        if (actor) {
> +    },
> 
> Define this._monitorChangedId in _init please.

Ok

> ::: js/ui/modalDialog.js
> @@ +201,2 @@
>      _fadeOpen: function() {
> +        this._monitorConstraint.index = global.screen.get_current_monitor();
> 
> Not so sure I like primaryMonitor vs. index. Seems clunky.

It probably is, but some stuff wants to be on the same screen after XRandR changes (modal dialog), some other wants to be always on the primary screen (lock screen, part of the overview?). I'm open to changes, if you have a better idea.
Comment 8 Jasper St. Pierre (not reading bugmail) 2012-08-16 16:05:04 UTC
(In reply to comment #7)
> > Any specific reason you chose GObject properties?
> 
> With those I save filtering params in _init.

Heh. Is it that much of a save, though? Params.parse isn't that hard.

I just noticed, but you should probably guard against an invalid monitor index, e.g. I wonder what would happen if you put it on the secondary monitor, and then unplug it.

> It probably is, but some stuff wants to be on the same screen after XRandR
> changes (modal dialog), some other wants to be always on the primary screen
> (lock screen, part of the overview?). I'm open to changes, if you have a better
> idea.

Yeah. I don't have one. This is fine.
Comment 9 Giovanni Campagna 2012-08-16 19:35:54 UTC
(In reply to comment #8)
> (In reply to comment #7)
> > > Any specific reason you chose GObject properties?
> > 
> > With those I save filtering params in _init.
> 
> Heh. Is it that much of a save, though? Params.parse isn't that hard.

Param.parse doesn't do what I want, that is, removing non GObject properties to the object passed to .parent().
Comment 10 Giovanni Campagna 2012-08-16 19:39:46 UTC
Created attachment 221457 [details] [review]
Port modal dialogs to the new MonitorConstraint

This commit makes ModalDialog use the new MonitorConstraint instead
of custom code to force itself on the right monitor.
At the same it ports wanda, which has something similar to a modal
dialog, but is not using the ModalDialog module.
Comment 11 Giovanni Campagna 2012-08-16 19:40:09 UTC
Created attachment 221459 [details] [review]
LayoutManager: add a ClutterConstraint for tracking monitor sizes

Instead of connecting manually to LayoutManager, or using ShellGenericContainer,
make a ClutterConstraint subclass that can track automatically
a specific monitor index, or the primary monitor.
Comment 12 Jasper St. Pierre (not reading bugmail) 2012-08-16 19:42:54 UTC
Review of attachment 221459 [details] [review]:

OK, sure.
Comment 13 Jasper St. Pierre (not reading bugmail) 2012-08-16 19:43:04 UTC
Review of attachment 221457 [details] [review]:

Sure.
Comment 14 Jasper St. Pierre (not reading bugmail) 2012-08-16 19:43:12 UTC
Review of attachment 221457 [details] [review]:

.
Comment 15 Giovanni Campagna 2012-08-16 19:48:08 UTC
Attachment 221149 [details] pushed as 2b30afa - ScreenShield: place the lock screen contents only on the primary monitor
Attachment 221457 [details] pushed as 1a65374 - Port modal dialogs to the new MonitorConstraint
Attachment 221459 [details] pushed as 3e94f6b - LayoutManager: add a ClutterConstraint for tracking monitor sizes

The original bug is now fixed (in theory). Leaving open for the remaining patch (which is a low priority cleanup)
Comment 16 Allan Day 2012-08-23 12:50:56 UTC
Updating component.
Comment 17 Giovanni Campagna 2013-01-03 01:08:50 UTC
(In reply to comment #15)
> Attachment 221149 [details] pushed as 2b30afa - ScreenShield: place the lock screen
> contents only on the primary monitor
> Attachment 221457 [details] pushed as 1a65374 - Port modal dialogs to the new
> MonitorConstraint
> Attachment 221459 [details] pushed as 3e94f6b - LayoutManager: add a ClutterConstraint
> for tracking monitor sizes
> 
> The original bug is now fixed (in theory). Leaving open for the remaining patch
> (which is a low priority cleanup)

Nah, let's close this one. I'll file a new report if and when I decide to really deprecate ShellGenericContainer (which might be never)