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 622552 - doesn't support multiple screens or monitor configuration changes
doesn't support multiple screens or monitor configuration changes
Status: RESOLVED FIXED
Product: notification-daemon
Classification: Other
Component: general
git master
Other Linux
: Normal normal
: ---
Assigned To: William Jon McCann
Depends on:
Blocks:
 
 
Reported: 2010-06-24 01:18 UTC by William Jon McCann
Modified: 2010-07-01 12:27 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Make the daemon support multiple screens and monitor changes (19.15 KB, patch)
2010-06-24 03:18 UTC, William Jon McCann
needs-work Details | Review
Also respond to monitor size changes (4.95 KB, patch)
2010-06-24 18:44 UTC, William Jon McCann
none Details | Review

Description William Jon McCann 2010-06-24 01:18:36 UTC
The daemon doesn't support multiple screens or monitor configuration changes.
Comment 1 William Jon McCann 2010-06-24 03:18:46 UTC
Created attachment 164471 [details] [review]
Make the daemon support multiple screens and monitor changes
Comment 2 Bastien Nocera 2010-06-24 03:40:42 UTC
Review of attachment 164471 [details] [review]:

A couple of nitpicks really.

::: src/daemon/daemon.c
@@ +53,2 @@
 #define IMAGE_SIZE 48
+#define IDLE_SECONDS 120

Not here.

@@ +327,3 @@
+        daemon->priv->n_screens = gdk_display_get_n_screens (display);
+
+        daemon->priv->screens = g_renew (NotifyScreen *,

Given the assertion above:
daemon->priv->screens = g_new0 (NotifyScreen *, daemon->priv->n_screens);

@@ +340,3 @@
+        }
+}
+

Extraneous line feed.

@@ +1539,3 @@
+                monitor_num = gdk_screen_get_monitor_at_point (screen, x, y);
+
+                if (screen_num >= priv->n_screens) {

The screen_num will never change.

@@ +1546,3 @@
+                if (monitor_num >= priv->screens[screen_num]->n_stacks) {
+                        /* screw it - dump it on the last one we'll get
+                         a monitors-changed signal soon enough*/

This is a band-aid, and we should be failing hard instead.

@@ +1714,3 @@
         gtk_main ();
+
+        g_object_unref (daemon);

Unrelated leak fix.

::: src/daemon/engines.c
@@ +177,3 @@
                 char           *enginename;
 
+                client = gconf_client_get_default ();

Unrelated change.

::: src/daemon/stack.c
@@ +42,3 @@
 };
 
+GList *

return a const? That would make it clear you're not supposed to change this.
Comment 3 William Jon McCann 2010-06-24 04:06:22 UTC
Pushed with some modifications.  Thanks for the review.
Comment 4 Matthias Clasen 2010-06-24 14:25:43 UTC
I've tested this, and while it handles monitors coming and going perfectly now, it doesn't handle resolution changes so perfectly.

If I change the monitor resolution, the stack is only repositioned when the next notification arrives. Existing bubbles hang in the middle of the screen or half-way off, depending on which way I change the resolution
Comment 5 William Jon McCann 2010-06-24 15:05:01 UTC
And it leaks.
Comment 6 William Jon McCann 2010-06-24 18:44:42 UTC
Created attachment 164542 [details] [review]
Also respond to monitor size changes
Comment 7 William Jon McCann 2010-06-24 19:50:48 UTC
Would appreciate some help testing this latest patch.  Unfortunately my intel driver hangs the system when I change resolutions.
Comment 8 William Jon McCann 2010-06-26 23:21:03 UTC
Pushed

commit 7889698ec9041c0979869e6d9230acc29dee159d
Author: William Jon McCann <jmccann@redhat.com>
Date:   Thu Jun 24 14:35:17 2010 -0400

    Also respond to monitor size changes
    
    By watching for changes to _NET_WORKAREA.  Responding to changes
    in screen or monitor size isn't enough since we compute the position
    relative to the work area.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=622552
Comment 9 Martin Szulecki 2010-07-01 12:27:20 UTC
Just a hint at some work I did 3 years ago regarding this which might help here:
http://trac.galago-project.org/ticket/5

It just did not get in due to someone being lazy.

It correctly handled multiple monitor setups and especially correctly computes the workarea since _NET_WORKAREA does not take dock changes into account sufficiently.