Hi, it seems that when multiple notifications are sent at the same time, they overlap each other. It'd be nice if they could be stacked in some way vertically so one can see multiple at the same time. Thanks,
Jerome is working on this...
Created attachment 2519 Patch for smart notification placement Here is the first fully functionnal version of my patch, it should also work correctly with multiple monitors. Comments and tests are welcome. Here is the latest screencast showing how it behaves: http://pocentek.net/~jeromeg/screencasts/notifications2.avi I'm not really happy with my two seconds sleep hack on monitor insertion/removal, but it works... Stats for fun: xfce4-notifyd/xfce-notify-daemon.c | 491 ++++++++++++++++++++++++++++++++++-- xfce4-notifyd/xfce-notify-window.c | 58 ++++- xfce4-notifyd/xfce-notify-window.h | 12 + 3 files changed, 535 insertions(+), 26 deletions(-)
Created attachment 2522 Update copyright stuff The code I took from galago's trac was written by someone whose nick is M.S and there is no email adress available, so I just added a link to the original patch, I think it's more usefull than a simple nick.
Created attachment 2557 New candidate patch! I finally found some time to work on this. I do not know exactly how I fixed it, but now it works without the ugly sleep :D I also removed the possibility to move notifications, as we had discussed. Time for review now ;)
I noticed a small problem with placement and the workarea edges. It looks like the distance between edge bubbles and the others is sometimes less than between normal bubbles. Here's a screenshot: http://lunar-linux.org/~jannis/screenshots/xfce/xfce4-notifyd-smart-placement-20091001-1.png
Hmm, I think that might be a bug in the workarea calculation. It must be trying to place notifications slightly overlapping the panels, and the WM moves it up or down to avoid that.
Created attachment 2568 And another candidate! Here is a new patch which fixes a few things compared to the previous one: - all glib critical warnings should be fixed now - I fixed the workarea function by porting it to gdk, this was pretty straightforward, simplifies the code and seems to work ;)
Ok, here goes... > diff --git a/xfce4-notifyd/xfce-notify-daemon.c b/xfce4-notifyd/xfce-notify-daemon.c > index 660aa0a..d437abe 100644 > --- a/xfce4-notifyd/xfce-notify-daemon.c > +++ b/xfce4-notifyd/xfce-notify-daemon.c > @@ -1,7 +1,11 @@ > /* > * xfce4-notifyd > * > - * Copyright (c) 2008 Brian Tarricone <bjt23@cornell.edu> > + * Copyright (c) 2008-2009 Brian Tarricone <bjt23@cornell.edu> > + * Copyright (c) 2009 Jérôme Guelfucci <jeromeg@xfce.org> > + * > + * The workarea per monitor code is taken from > + * http://trac.galago-project.org/attachment/ticket/5/10-nd-improve-multihead-support.patch > * > * This program is free software; you can redistribute it and/or modify > * it under the terms of the GNU General Public License as published by > @@ -36,6 +40,8 @@ > #include "xfce-notify-window.h" > #include "xfce-notify-marshal.h" > > +#define SPACE 16 > + I'd prefer if this was a style property, though I'm not sure what widget you'd put that on. Maybe configurable through the dialog? I dunno, maybe just leave it for now and we can fix it later. > struct _XfceNotifyDaemon > { > GObject parent; > @@ -49,6 +55,10 @@ struct _XfceNotifyDaemon > XfconfChannel *settings; > > GTree *active_notifications; > + GList ***reserved_rectangles; > + GArray *monitor_numbers; From what I can tell you're using this to associate the number of monitors with each GdkScreen. I'd rather you just stuff this in each GdkScreen using g_object_set_qdata(). GdkScreens never get created or destroyed while the app is running, so that's fine. You can update the values there as needed. You could actually do the same with reserved_rectangles. > + > + gint inspected_screen; ... and then you probably wouldn't need inspected_screen anymore either. > > guint32 last_notification_id; > }; > @@ -72,11 +82,27 @@ enum > URGENCY_CRITICAL, > }; > > +static void xfce_notify_daemon_screen_changed(GdkScreen *screen, > + gpointer userdata); > + > +static void xfce_notify_daemon_update_reserved_rectangles(gpointer key, > + gpointer value, > + gpointer data); > + > static void xfce_notify_daemon_finalize(GObject *obj); > > +static void xfce_gdk_rectangle_largest_box(GdkRectangle *src1, > + GdkRectangle *src2, > + GdkRectangle *dest); > + > +static gboolean xfce_notify_daemon_get_workarea(GdkScreen *screen, > + guint monitor, > + GdkRectangle *rect); > + > static gboolean notify_get_capabilities(XfceNotifyDaemon *xndaemon, > gchar ***OUT_capabilities, > GError *error); > + Please don't add extra whitespace. > static gboolean notify_notify(XfceNotifyDaemon *xndaemon, > const gchar *app_name, > guint replaces_id, > @@ -88,9 +114,11 @@ static gboolean notify_notify(XfceNotifyDaemon *xndaemon, > gint expire_timeout, > guint *OUT_id, > GError **error); > + Ditto. > static gboolean notify_close_notification(XfceNotifyDaemon *xndaemon, > guint id, > GError **error); > + Ditto. > static gboolean notify_get_server_information(XfceNotifyDaemon *xndaemon, > gchar **OUT_name, > gchar **OUT_vendor, > @@ -156,12 +184,58 @@ xfce_direct_compare(gconstpointer a, > return (gint)((gchar *)a - (gchar *)b); > } > > + > + > +static void > +xfce_notify_daemon_screen_changed(GdkScreen *screen, gpointer userdata) > +{ > + XfceNotifyDaemon *xndaemon = XFCE_NOTIFY_DAEMON(userdata); > + gint j; > + gint new_nmonitor = gdk_screen_get_n_monitors(screen); > + gint screen_number = gdk_screen_get_number(screen); > + gint old_nmonitor = g_array_index(xndaemon->monitor_numbers, gint, screen_number); > + > + /* Free the current reserved rectangles on screen */ > + for(j = 0; j < old_nmonitor; j++) > + g_list_free(xndaemon->reserved_rectangles[screen_number][j]); > + > + g_free(xndaemon->reserved_rectangles[screen_number]); > + > + /* Initialize a new reserved rectangles array for screen */ > + xndaemon->reserved_rectangles[screen_number] = g_new0(GList *, new_nmonitor); > + xndaemon->inspected_screen = screen_number; > + > + /* Traverse the active notifications tree to fill the new reserved rectangles array for screen */ > + g_tree_foreach(xndaemon->active_notifications, > + (GTraverseFunc)xfce_notify_daemon_update_reserved_rectangles, > + xndaemon); > +} > + > static void > xfce_notify_daemon_init(XfceNotifyDaemon *xndaemon) > { > + gint nscreen = gdk_display_get_n_screens(gdk_display_get_default()); > + gint i; > + > + DBG("Initialize the daemon"); > + > xndaemon->active_notifications = g_tree_new_full(xfce_direct_compare, > NULL, NULL, > (GDestroyNotify)gtk_widget_destroy); > + xndaemon->monitor_numbers = g_array_new(FALSE, FALSE, sizeof(gint)); > + xndaemon->reserved_rectangles = g_new(GList **, nscreen); > + > + for(i = 0; i < nscreen; ++i) { > + GdkScreen *screen = gdk_display_get_screen(gdk_display_get_default(), i); > + gint nmonitor = gdk_screen_get_n_monitors(screen); > + > + g_array_append_val(xndaemon->monitor_numbers, nmonitor); > + > + g_signal_connect(G_OBJECT(screen), "monitors-changed", > + G_CALLBACK(xfce_notify_daemon_screen_changed), xndaemon); > + > + xndaemon->reserved_rectangles[i] = g_new0(GList *, nmonitor); Use g_slice for same-sized array members. > + } > > xndaemon->last_notification_id = 1; > } > @@ -169,8 +243,28 @@ xfce_notify_daemon_init(XfceNotifyDaemon *xndaemon) > static void > xfce_notify_daemon_finalize(GObject *obj) > { > + gint nscreen = gdk_display_get_n_screens(gdk_display_get_default()); > + gint i, j; > XfceNotifyDaemon *xndaemon = XFCE_NOTIFY_DAEMON(obj); > - > + > + DBG("Finalize the daemon"); Shouldn't need this anymore... > + > + g_array_free(xndaemon->monitor_numbers, TRUE); > + > + for(i = 0; i < nscreen; ++i) { > + GdkScreen *screen = gdk_display_get_screen(gdk_display_get_default(), i); > + gint nmonitor = gdk_screen_get_n_monitors(screen); > + > + for(j = 0; j < nmonitor; j++) { > + if (xndaemon->reserved_rectangles[i][j]) > + g_list_free(xndaemon->reserved_rectangles[i][j]); > + } > + > + g_free(xndaemon->reserved_rectangles[i]); g_slice > + } > + > + g_free(xndaemon->reserved_rectangles); > + > g_tree_destroy(xndaemon->active_notifications); > > if(xndaemon->settings) > @@ -212,8 +306,15 @@ xfce_notify_daemon_window_closed(XfceNotifyWindow *window, > { > XfceNotifyDaemon *xndaemon = user_data; > gpointer id_p = g_object_get_data(G_OBJECT(window), "--notify-id"); > + GList *list; > + > + /* Remove the reserved rectangle from the list */ > + list = xndaemon->reserved_rectangles[xfce_notify_window_get_screen(window)][xfce_notify_window_get_monitor(window)]; > + list = g_list_remove(list, xfce_notify_window_get_geometry(window)); > + xndaemon->reserved_rectangles[xfce_notify_window_get_screen(window)][xfce_notify_window_get_monitor(window)] = list; > > g_tree_remove(xndaemon->active_notifications, id_p); > + Don't add extra whitespace. > #ifdef USE_OLD_NOTIFICATION_CLOSED_SIGNATURE > g_signal_emit(G_OBJECT(xndaemon), signals[SIG_NOTIFICATION_CLOSED], 0, > GPOINTER_TO_UINT(id_p)); > @@ -223,48 +324,362 @@ xfce_notify_daemon_window_closed(XfceNotifyWindow *window, > #endif > } > > +static void xfce_gdk_rectangle_largest_box(GdkRectangle *src1, GdkRectangle *src2, GdkRectangle *dest) Coding style. > +{ > + gint t_h = MAX(src2->y,src1->y); > + gint l_w = MAX(src2->x,src1->x); > + gint b_h = MAX(src2->height, src1->height) - > + MIN(src2->y + src2->height, src1->y + src1->height); If you must wrap long lines like this, line up the second line with the text after the = on the previous. > + gint r_w = MAX(src2->width, src1->width) - > + MIN(src2->x + src2->width, src1->x + src1->width); Ditto. > + gint m_h = MAX(t_h,b_h); > + gint m_w = MAX(l_w,r_w); I have no idea what all these variables are. Please give them more informative names. Also, spaces after the commas in the macro calls. > + > + /* height is largest */ This comment goes inside the if block. > + if(m_h >= m_w) { > + if(t_h >= b_h) { > + dest->x = src1->x; > + dest->y = src1->y; > + dest->width = src1->width; > + dest->height = t_h + MIN(0,b_h); > + } > + else { Coding style. > + dest->x = src1->x; > + dest->y = src2->y + src2->height; > + dest->width = src1->width; > + dest->height = b_h + MIN(0,t_h); Spaces. > + } > + } > + /* width is largest */ > + else { Coding style. Also put the comment inside the else block. > + if(l_w >= r_w) { > + dest->x = src1->x; > + dest->y = src1->y; > + dest->width = m_w + MIN(0,r_w); Spaces. > + dest->height = src1->height; > + } > + else { > + dest->x = src2->x + src2->width; > + dest->y = src1->y; > + dest->width = m_w + MIN(0,l_w); Spaces. > + dest->height = src1->height; > + } > + } > +} > + > +static void > +translate_origin(GdkRectangle *src1, gint xoffset, gint yoffset) Should be marked inline. > +{ > + src1->x += xoffset; > + src1->y += yoffset; > +} > + > +/* Returns the workarea (largest non-panel/dock occupied rectangle) for a given > + monitor. */ > +static gboolean > +xfce_notify_daemon_get_workarea(GdkScreen *screen, > + guint monitor_num, > + GdkRectangle *workarea) > +{ > + GList *windows_list, *l; > + gint monitor_xoff, monitor_yoff; > + > + /* Defaults */ > + gdk_screen_get_monitor_geometry(screen, monitor_num, workarea); > + > + monitor_xoff = workarea->x; > + monitor_yoff = workarea->y; > + > + if(workarea == NULL) Style: -> "!workarea". > + return FALSE; > + > + windows_list = gdk_screen_get_window_stack(screen); > + > + for(l = g_list_first(windows_list); l != NULL; l = g_list_next(l)) { > + GdkWindow *window = l->data; > + > + printf("Test if it's a dock"); -> DBG(), or delete. > + > + if(gdk_window_get_type_hint(window) == GDK_WINDOW_TYPE_HINT_DOCK) { > + GdkRectangle window_geom, intersection; > + > + printf ("It's a dock"); -> DBG(), or delete. > + > + gdk_window_get_frame_extents(window, &window_geom); > + > + if(gdk_rectangle_intersect(workarea, &window_geom, &intersection)){ > + translate_origin(workarea, -monitor_xoff, -monitor_yoff); > + translate_origin(&intersection, -monitor_xoff, -monitor_yoff); > + > + xfce_gdk_rectangle_largest_box(workarea, &intersection, workarea); > + > + translate_origin(workarea, monitor_xoff, monitor_yoff); > + translate_origin(&intersection, monitor_xoff, monitor_yoff); > + } > + } > + > + g_object_unref(window); > + } > + > + g_list_free(windows_list); > + > + return TRUE; > +} > + > static void > xfce_notify_daemon_window_size_allocate(GtkWidget *widget, > GtkAllocation *allocation, > gpointer user_data) > { > XfceNotifyDaemon *xndaemon = user_data; > + XfceNotifyWindow *window = XFCE_NOTIFY_WINDOW(widget); > GdkScreen *screen = NULL; > - gint x, y, monitor; > - GdkRectangle geom; > + gint x, y, monitor, max_width; > + GdkRectangle geom, initial, workarea; > + GdkRectangle *widget_geom = g_new0(GdkRectangle, 1); > + GList *list; > + gboolean found = FALSE; > + > + DBG("Size allocate called."); > + > + workarea.x = 0; > + workarea.y = 0; > + workarea.width = 0; > + workarea.height = 0; > + > + /* Notification has already been placed previously. Not sure if that can happen. */ > + if(xfce_notify_window_get_geometry(window)) { > + GList *old_list = > + xndaemon->reserved_rectangles[xfce_notify_window_get_screen(window)][xfce_notify_window_get_monitor(window)]; Line is way too long. Use temporaries for the array indices and don't wrap the line. > + > + old_list = g_list_remove(old_list, xfce_notify_window_get_geometry(window)); > + xndaemon->reserved_rectangles[xfce_notify_window_get_screen(window)][xfce_notify_window_get_monitor(window)] = old_list; Temporaries. > + > + g_free(xfce_notify_window_get_geometry(window)); > + } > > gdk_display_get_pointer(gdk_display_get_default(), &screen, &x, &y, NULL); > monitor = gdk_screen_get_monitor_at_point(screen, x, y); > gdk_screen_get_monitor_geometry(screen, monitor, &geom); > > + if(xfce_notify_daemon_get_workarea(screen, monitor, &workarea)) { > + DBG("Workarea: (%i,%i), width: %i, height:%i", > + workarea.x, workarea.y, workarea.width, workarea.height); > + geom.x = workarea.x; > + geom.y = workarea.y; > + geom.width = workarea.width; > + geom.height = workarea.height; > + } > + > + DBG("We are on the monitor %i, screen %i", monitor, gdk_screen_get_number(screen)); > + > gtk_window_set_screen(GTK_WINDOW(widget), screen); > > + /* Set initial geometry */ > + initial.width = allocation->width; > + initial.height = allocation->height; > + > switch(xndaemon->notify_location) { > case GTK_CORNER_TOP_LEFT: > - x = geom.x + 32; > - y = geom.y + 32; > + initial.x = geom.x + SPACE; > + initial.y = geom.y + SPACE; > break; > case GTK_CORNER_BOTTOM_LEFT: > - x = geom.x + 32; > - y = geom.height - allocation->height - 32; > + initial.x = geom.x + SPACE; > + initial.y = geom.y + geom.height - allocation->height - SPACE; > break; > case GTK_CORNER_TOP_RIGHT: > - x = geom.width - allocation->width - 32; > - y = geom.y + 32; > + initial.x = geom.x + geom.width - allocation->width - SPACE; > + initial.y = geom.y + SPACE; > break; > case GTK_CORNER_BOTTOM_RIGHT: > - x = geom.width - allocation->width - 32; > - y = geom.height - allocation->height - 32; > + initial.x = geom.x + geom.width - allocation->width - SPACE; > + initial.y = geom.y + geom.height - allocation->height - SPACE; > break; > default: > g_warning("Invalid notify location: %d", xndaemon->notify_location); > return; > } > > - gtk_window_move(GTK_WINDOW(widget), x, y); > + widget_geom->x = initial.x; > + widget_geom->y = initial.y; > + widget_geom->width = initial.width; > + widget_geom->height = initial.height; > + max_width = 0; > + > + /* Get the list of reserved places */ > + list = xndaemon->reserved_rectangles[gdk_screen_get_number(screen)][monitor]; > + > + /* If the list is empty, there are no displayed notifications */ > + if(!list) { Comment goes inside the if block. > + DBG("No notifications on this monitor"); > + xfce_notify_window_set_geometry(XFCE_NOTIFY_WINDOW(widget), widget_geom); > + xfce_notify_window_set_monitor(XFCE_NOTIFY_WINDOW(widget), monitor); > + xfce_notify_window_set_screen(XFCE_NOTIFY_WINDOW(widget), > + gdk_screen_get_number(screen)); > + > + list = g_list_prepend(list, widget_geom); > + xndaemon->reserved_rectangles[gdk_screen_get_number(screen)][monitor] = list; > + > + DBG("Notification position: x=%i y=%i", widget_geom->x, widget_geom->y); > + gtk_window_move(GTK_WINDOW(widget), widget_geom->x, widget_geom->y); > + return; > + } > + /* Else, we try to find the appropriate position on the monitor */ > + else { Coding style, and comment goes inside the else block. > + while(!found) { > + gboolean overlaps = FALSE; > + GList *l = NULL; > + gint notification_y, notification_height; > + > + DBG("Test if the candidate overlaps one of the existing notifications."); > + > + for(l = g_list_first(list); l; l = l->next) { > + GdkRectangle *rectangle = l->data; > + > + DBG("Overlaps with (x=%i, y=%i) ?", rectangle->x, rectangle->y); > + > + overlaps = overlaps || gdk_rectangle_intersect(rectangle, widget_geom, NULL); > + > + if(overlaps) { > + DBG("Yes"); > + > + if(rectangle->width > max_width) > + max_width = rectangle->width; > + > + notification_y = rectangle->y; > + notification_height = rectangle->height; > + > + break; > + } > + else Coding style. > + DBG("No"); > + } > + > + if(!overlaps) { > + DBG("We found a correct position."); > + found = TRUE; > + } > + else { Coding style. > + switch(xndaemon->notify_location) { > + case GTK_CORNER_TOP_LEFT: > + DBG("Try under the current candiadate position."); > + widget_geom->y = notification_y + notification_height + SPACE; > + > + if(widget_geom->y + widget_geom->height > geom.height + geom.y) { > + DBG("We reached the bottom of the monitor"); > + widget_geom->y = geom.y + SPACE; > + widget_geom->x = widget_geom->x + max_width + SPACE; > + max_width = 0; > + > + if(widget_geom->x + widget_geom->width > geom.width + geom.x) { > + DBG("There was no free space."); > + widget_geom->x = initial.x; > + widget_geom->y = initial.y; > + found = TRUE; > + } > + } > + break; > + case GTK_CORNER_BOTTOM_LEFT: > + DBG("Try above the current candidate position"); > + widget_geom->y = notification_y - widget_geom->height - SPACE; > + > + if(widget_geom->y < geom.y) { > + DBG("We reached the top of the monitor"); > + widget_geom->y = geom.y + geom.height - widget_geom->height - SPACE; > + widget_geom->x = widget_geom->x + max_width + SPACE; > + max_width = 0; > + > + if(widget_geom->x + widget_geom->width > geom.width + geom.x) { > + DBG("There was no free space."); > + widget_geom->x = initial.x; > + widget_geom->y = initial.y; > + found = TRUE; > + } > + } > + break; > + case GTK_CORNER_TOP_RIGHT: > + DBG("Try under the current candidate position."); > + widget_geom->y = notification_y + notification_height + SPACE; > + > + if(widget_geom->y + widget_geom->height > geom.height + geom.y) { > + DBG("We reached the bottom of the monitor"); > + widget_geom->y = geom.y + SPACE; > + widget_geom->x = widget_geom->x - max_width - SPACE; > + max_width = 0; > + > + if(widget_geom->x < geom.x) { > + DBG("There was no free space."); > + widget_geom->x = initial.x; > + widget_geom->y = initial.y; > + found = TRUE; > + } > + } > + break; > + case GTK_CORNER_BOTTOM_RIGHT: > + DBG("Try above the current candidate position"); > + widget_geom->y = notification_y - widget_geom->height - SPACE; > + > + if(widget_geom->y < geom.y) { > + DBG("We reached the top of the screen"); > + widget_geom->y = geom.y + geom.height - widget_geom->height - SPACE; > + widget_geom->x = widget_geom->x - max_width - SPACE; > + max_width = 0; > + > + if(widget_geom->x < geom.x) { > + DBG("There was no free space"); > + widget_geom->x = initial.x; > + widget_geom->y = initial.y; > + found = TRUE; > + } > + } > + break; > + > + default: > + g_warning("Invalid notify location: %d", xndaemon->notify_location); > + return; > + } > + } > + } > + } > + > + xfce_notify_window_set_geometry(XFCE_NOTIFY_WINDOW(widget), widget_geom); > + xfce_notify_window_set_monitor(XFCE_NOTIFY_WINDOW(widget), monitor); > + xfce_notify_window_set_screen(XFCE_NOTIFY_WINDOW(widget), > + gdk_screen_get_number(screen)); > + > + list = g_list_prepend(list, widget_geom); > + xndaemon->reserved_rectangles[gdk_screen_get_number(screen)][monitor] = list; > + > + DBG("Move the notification to: x=%i, y=%i", widget_geom->x, widget_geom->y); > + gtk_window_move(GTK_WINDOW(widget), widget_geom->x, widget_geom->y); > } > > > +static void > +xfce_notify_daemon_update_reserved_rectangles(gpointer key, > + gpointer value, > + gpointer data) > +{ > + XfceNotifyDaemon *xndaemon = XFCE_NOTIFY_DAEMON(data); > + XfceNotifyWindow *window = XFCE_NOTIFY_WINDOW(value); > + gint width, height; > + GtkAllocation allocation; > + > + if(xfce_notify_window_get_screen(window) != xndaemon->inspected_screen) > + return; > + > + /* Get the size of the notification */ > + gtk_window_get_size(GTK_WINDOW(window), &width, &height); > + > + allocation.x = 0; > + allocation.y = 0; > + allocation.width = width; > + allocation.height = height; > + > + xfce_notify_daemon_window_size_allocate(GTK_WIDGET(window), &allocation, xndaemon); > +} > + > > static gboolean > notify_get_capabilities(XfceNotifyDaemon *xndaemon, > @@ -396,9 +811,6 @@ notify_notify(XfceNotifyDaemon *xndaemon, > } > > gtk_widget_realize(GTK_WIDGET(window)); > - xfce_notify_daemon_window_size_allocate(GTK_WIDGET(window), > - >K_WIDGET(window)->allocation, > - xndaemon); > > return TRUE; > } > @@ -500,7 +912,7 @@ xfce_notify_daemon_set_theme(XfceNotifyDaemon *xndaemon, > const gchar *theme) > { > gchar *file, **files; > - > + > /* old-style ~/.themes ... */ > file = g_build_filename(xfce_get_homedir(), ".themes", theme, > "xfce-notify-4.0", "gtkrc", NULL); > @@ -515,7 +927,7 @@ xfce_notify_daemon_set_theme(XfceNotifyDaemon *xndaemon, > files = xfce_resource_lookup_all(XFCE_RESOURCE_DATA, file); > if(files[0]) > gtk_rc_parse(files[0]); > - > + > g_free(file); > g_strfreev(files); > } > @@ -559,7 +971,7 @@ xfce_notify_daemon_start(XfceNotifyDaemon *xndaemon, > { > int ret; > DBusError derror; > - > + > xndaemon->dbus_conn = dbus_g_bus_get(DBUS_BUS_SESSION, error); > if(G_UNLIKELY(!xndaemon->dbus_conn)) { > if(error && !*error) { > @@ -568,7 +980,7 @@ xfce_notify_daemon_start(XfceNotifyDaemon *xndaemon, > } > return FALSE; > } > - > + > dbus_error_init(&derror); > ret = dbus_bus_request_name(dbus_g_connection_get_connection(xndaemon->dbus_conn), > "org.freedesktop.Notifications", > @@ -583,7 +995,7 @@ xfce_notify_daemon_start(XfceNotifyDaemon *xndaemon, > g_set_error(error, DBUS_GERROR, DBUS_GERROR_FAILED, > _("Another notification xndaemon is already running")); > } > - > + > return FALSE; > } > > @@ -649,3 +1061,4 @@ xfce_notify_daemon_new_unique(GError **error) > > return xndaemon; > } > + > diff --git a/xfce4-notifyd/xfce-notify-window.c b/xfce4-notifyd/xfce-notify-window.c > index 3c43dfb..e5361ef 100644 > --- a/xfce4-notifyd/xfce-notify-window.c > +++ b/xfce4-notifyd/xfce-notify-window.c > @@ -1,7 +1,8 @@ > /* > * xfce4-notifyd > * > - * Copyright (c) 2008 Brian Tarricone <bjt23@cornell.edu> > + * Copyright (c) 2008-2009 Brian Tarricone <bjt23@cornell.edu> > + * Copyright (c) 2009 Jérôme Guelfucci <jeromeg@xfce.org> > * > * This program is free software; you can redistribute it and/or modify > * it under the terms of the GNU General Public License as published by > @@ -48,6 +49,10 @@ struct _XfceNotifyWindow > { > GtkWindow parent; > > + GdkRectangle *geometry; > + gint monitor; > + gint screen; > + > guint expire_timeout; > > gboolean mouse_hover; > @@ -62,18 +67,12 @@ struct _XfceNotifyWindow > GtkWidget *summary; > GtkWidget *body; > GtkWidget *button_box; > - > + > guint64 expire_start_timestamp; > guint expire_id; > guint fade_id; > guint op_change_steps; > gdouble op_change_delta; > - > - gboolean maybe_begin_drag; > - gboolean dragging; > - gint press_start_x; > - gint press_start_y; > - guint32 press_timestamp; > }; > > typedef struct > @@ -102,12 +101,8 @@ static gboolean xfce_notify_window_expose(GtkWidget *widget, > GdkEventExpose *evt); > static gboolean xfce_notify_window_enter_leave(GtkWidget *widget, > GdkEventCrossing *evt); > -static gboolean xfce_notify_window_button_press(GtkWidget *widget, > - GdkEventButton *evt); > static gboolean xfce_notify_window_button_release(GtkWidget *widget, > GdkEventButton *evt); > -static gboolean xfce_notify_window_motion_notify(GtkWidget *widget, > - GdkEventMotion *evt); > static gboolean xfce_notify_window_configure_event(GtkWidget *widget, > GdkEventConfigure *evt); > static void xfce_notify_window_style_set(GtkWidget *widget, > @@ -144,9 +139,7 @@ xfce_notify_window_class_init(XfceNotifyWindowClass *klass) > widget_class->expose_event = xfce_notify_window_expose; > widget_class->enter_notify_event = xfce_notify_window_enter_leave; > widget_class->leave_notify_event = xfce_notify_window_enter_leave; > - widget_class->button_press_event = xfce_notify_window_button_press; > widget_class->button_release_event = xfce_notify_window_button_release; > - widget_class->motion_notify_event = xfce_notify_window_motion_notify; > widget_class->configure_event = xfce_notify_window_configure_event; > widget_class->style_set = xfce_notify_window_style_set; > > @@ -204,7 +197,7 @@ xfce_notify_window_init(XfceNotifyWindow *window) > { > GdkScreen *screen; > GtkWidget *topvbox, *vbox, *hbox, *spacer; > - > + > GTK_WINDOW(window)->type = GTK_WINDOW_TOPLEVEL; > window->expire_timeout = DEFAULT_EXPIRE_TIMEOUT; > window->normal_opacity = DEFAULT_NORMAL_OPACITY; > @@ -224,7 +217,7 @@ xfce_notify_window_init(XfceNotifyWindow *window) > gtk_widget_add_events(GTK_WIDGET(window), > GDK_BUTTON_PRESS_MASK | GDK_BUTTON_RELEASE_MASK > | GDK_POINTER_MOTION_MASK); > - > + > screen = gtk_widget_get_screen(GTK_WIDGET(window)); > if(gdk_screen_is_composited(screen)) { > GdkColormap *cmap = gdk_screen_get_rgba_colormap(screen); > @@ -304,6 +297,9 @@ static void > xfce_notify_window_finalize(GObject *object) > { > G_OBJECT_CLASS(xfce_notify_window_parent_class)->finalize(object); > + > + if(XFCE_NOTIFY_WINDOW(object)->geometry) > + g_free(XFCE_NOTIFY_WINDOW(object)->geometry); > } > > static void > @@ -451,7 +447,7 @@ xfce_notify_window_ensure_bg_path(XfceNotifyWindow *window, > } > > window->bg_path = cairo_copy_path(cr); > - > + > flat_path = cairo_copy_path_flat(cr); > fill_rule = (cairo_get_fill_rule(cr) == CAIRO_FILL_RULE_WINDING > ? GDK_WINDING_RULE : GDK_EVEN_ODD_RULE); > @@ -578,11 +574,6 @@ xfce_notify_window_enter_leave(GtkWidget *widget, > XfceNotifyWindow *window = XFCE_NOTIFY_WINDOW(widget); > > if(evt->type == GDK_ENTER_NOTIFY) { > - /* after a drag, we don't get a button release, but we do get an > - * enter notify */ > - window->maybe_begin_drag = FALSE; > - window->dragging = FALSE; > - > if(window->expire_timeout) { > if(window->expire_id) { > g_source_remove(window->expire_id); > @@ -593,12 +584,11 @@ xfce_notify_window_enter_leave(GtkWidget *widget, > window->fade_id = 0; > } > } > + > gtk_window_set_opacity(GTK_WINDOW(widget), 1.0); > window->mouse_hover = TRUE; > gtk_widget_queue_draw(widget); > - } else if(evt->type == GDK_LEAVE_NOTIFY > - && evt->detail != GDK_NOTIFY_INFERIOR > - && !window->dragging) > + } else if(evt->type == GDK_LEAVE_NOTIFY) > { Coding style. > xfce_notify_window_start_expiration(window); > window->mouse_hover = FALSE; > @@ -609,26 +599,6 @@ xfce_notify_window_enter_leave(GtkWidget *widget, > } > > static gboolean > -xfce_notify_window_button_press(GtkWidget *widget, > - GdkEventButton *evt) > -{ > - XfceNotifyWindow *window = XFCE_NOTIFY_WINDOW(widget); > - > - if(GDK_BUTTON_PRESS == evt->type && 1 == evt->button) { > - window->maybe_begin_drag = TRUE; > - window->dragging = FALSE; > - window->press_start_x = evt->x_root; > - window->press_start_y = evt->y_root; > - window->press_timestamp = evt->time; > - } else { > - window->maybe_begin_drag = FALSE; > - window->dragging = FALSE; > - } > - > - return FALSE; > -} > - > -static gboolean > xfce_notify_window_button_release(GtkWidget *widget, > GdkEventButton *evt) > { > @@ -648,28 +618,6 @@ xfce_notify_window_button_release(GtkWidget *widget, > } > > static gboolean > -xfce_notify_window_motion_notify(GtkWidget *widget, > - GdkEventMotion *evt) > -{ > - XfceNotifyWindow *window = XFCE_NOTIFY_WINDOW(widget); > - > - if(window->maybe_begin_drag) { > - if(gtk_drag_check_threshold(widget, window->press_start_x, > - window->press_start_y, > - evt->x_root, evt->y_root)) > - { > - window->dragging = TRUE; > - gtk_window_begin_move_drag(GTK_WINDOW(widget), 1, > - window->press_start_x, > - window->press_start_y, > - window->press_timestamp); > - } > - } > - > - return FALSE; > -} > - > -static gboolean > xfce_notify_window_configure_event(GtkWidget *widget, > GdkEventConfigure *evt) > { > @@ -1077,17 +1025,56 @@ xfce_notify_window_set_body(XfceNotifyWindow *window, > } > > void > +xfce_notify_window_set_geometry(XfceNotifyWindow *window, > + GdkRectangle *rectangle) > +{ > + window->geometry = rectangle; > +} > + > +GdkRectangle > +*xfce_notify_window_get_geometry (XfceNotifyWindow *window) > +{ > + return window->geometry; > +} I feel like we talked about this already, but is there a particular reason you're using dynamically allocated memory for the GdkRectangles? It's fine to just pass them by value on the stack instead of returning pointers. Unless we decided this way was better for some reason. > + > +void > +xfce_notify_window_set_monitor(XfceNotifyWindow *window, > + gint monitor) > +{ > + window->monitor = monitor; > +} > + > +gint > +xfce_notify_window_get_monitor(XfceNotifyWindow *window) > +{ > + return window->monitor; > +} > + > +void > +xfce_notify_window_set_screen(XfceNotifyWindow *window, > + gint screen) > +{ > + window->screen = screen; > +} > + > +gint > +xfce_notify_window_get_screen(XfceNotifyWindow *window) > +{ > + return window->screen; > +} I'm not really a fan of these two. What's wrong with: GdkScreen *screen = gtk_widget_get_screen(window); gint num = gtk_screen_get_number(screen); > + > +void > xfce_notify_window_set_icon_name(XfceNotifyWindow *window, > const gchar *icon_name) > { > gboolean icon_set = FALSE; > - > + > g_return_if_fail(XFCE_IS_NOTIFY_WINDOW(window)); > > if(icon_name && *icon_name) { > gint w, h; > GdkPixbuf *pix; > - > + > gtk_icon_size_lookup(GTK_ICON_SIZE_DIALOG, &w, &h); > pix = xfce_themed_icon_load(icon_name, w); > if(pix) { > @@ -1097,7 +1084,7 @@ xfce_notify_window_set_icon_name(XfceNotifyWindow *window, > icon_set = TRUE; > } > } > - > + > if(!icon_set) { > gtk_image_set_from_pixbuf(GTK_IMAGE(window->icon), NULL); > gtk_widget_hide(window->icon); > diff --git a/xfce4-notifyd/xfce-notify-window.h b/xfce4-notifyd/xfce-notify-window.h > index 562906d..2521a9e 100644 > --- a/xfce4-notifyd/xfce-notify-window.h > +++ b/xfce4-notifyd/xfce-notify-window.h > @@ -1,7 +1,8 @@ > /* > * xfce4-notifyd > * > - * Copyright (c) 2008 Brian Tarricone <bjt23@cornell.edu> > + * Copyright (c) 2008-2009 Brian Tarricone <bjt23@cornell.edu> > + * Copyright (c) 2009 Jérôme Guelfucci <jeromeg@xfce.org> > * > * This program is free software; you can redistribute it and/or modify > * it under the terms of the GNU General Public License as published by > @@ -58,6 +59,18 @@ void xfce_notify_window_set_summary(XfceNotifyWindow *window, > void xfce_notify_window_set_body(XfceNotifyWindow *window, > const gchar *body); > > +void xfce_notify_window_set_geometry(XfceNotifyWindow *window, > + GdkRectangle *rectangle); > +GdkRectangle *xfce_notify_window_get_geometry(XfceNotifyWindow *window); > + > +void xfce_notify_window_set_monitor(XfceNotifyWindow *window, > + gint monitor); > +gint xfce_notify_window_get_monitor(XfceNotifyWindow *window); > + > +void xfce_notify_window_set_screen(XfceNotifyWindow *window, > + gint screen); > +gint xfce_notify_window_get_screen(XfceNotifyWindow *window); > + > void xfce_notify_window_set_icon_name(XfceNotifyWindow *window, > const gchar *icon_name); > void xfce_notify_window_set_icon_pixbuf(XfceNotifyWindow *window,
Also, what do you think about splitting this into two patches? The first one should remove the ability to drag and move the notifications around, and the second patch should add the positioning code. I'd like to keep the changes logically separate if possible.
Also, sorry for the spam but... if you don't mind, please commit the changes to your local git repo and use git format-patch to generate patches... that way when we're done I can just use git-am to import the changes with the correct author info on the commits.
Looking at this a bit more... you just can't re-calculate the workarea *every* time size-allocate occurs on a window. It's incredibly wasteful, and involves quite a few unnecessary round-trips to the server. This is something that definitely needs to be cached, and you should be watching PropertyNotify events on the root window.
Created attachment 2635 Remove possibility to move notifications Ok, thanks for the detailed review! The easy part first: attached is the patch to remove the possibility to move notifications.
Pushed move-removal patch with a small fix. Now we just need the big one ^_^
Created attachment 2647 Next iteration of the patch Here is the last version of my patch. I hope I did not forget one of the items you mentioned!
Ok, only a few minor things. > +inline static void > +translate_origin(GdkRectangle *src1, gint xoffset, gint yoffset) Should be "static inline void" -- gcc doesn't care about the order, but other compilers might (and I like this order better). > + /* Notification has already been placed previously. Not sure if that can happen. */ > + if(xfce_notify_window_get_geometry(window)) { With the new get_geometry code, this will *always* return true since it's just returning a pointer to a struct member that always exists. If you want to make sure the geometry has been set before, you need to get a pointer to the geom and make sure width and height aren't zero. I'm not sure if that's the way you want to fix it, or if you want to do something else. > +GdkRectangle > +*xfce_notify_window_get_geometry (XfceNotifyWindow *window) Coding style: should be "GdkRectangle *" on the first line. That's about it.
Actually I think I'll check the patch in as is and then make a followup commit to fix the little things. Not a big deal.
Ok, checked in an pushed. Yay!