From 4043e1a18e65189599ef3dbad290fd2d52ca2137 Mon Sep 17 00:00:00 2001 From: Benedikt Heine Date: Fri, 28 Dec 2018 17:56:06 +0100 Subject: [PATCH 1/6] Refactor pixbuf scaling into seperate method --- src/icon.c | 47 ++++++++++++++++++++++++++++------------------- src/icon.h | 11 +++++++++++ 2 files changed, 39 insertions(+), 19 deletions(-) diff --git a/src/icon.c b/src/icon.c index 8cdc194..7bf30aa 100644 --- a/src/icon.c +++ b/src/icon.c @@ -110,6 +110,33 @@ cairo_surface_t *gdk_pixbuf_to_cairo_surface(GdkPixbuf *pixbuf) return icon_surface; } +GdkPixbuf *icon_pixbuf_scale(GdkPixbuf *pixbuf) +{ + ASSERT_OR_RET(pixbuf, NULL); + + int w = gdk_pixbuf_get_width(pixbuf); + int h = gdk_pixbuf_get_height(pixbuf); + int larger = w > h ? w : h; + if (settings.max_icon_size && larger > settings.max_icon_size) { + int scaled_w = settings.max_icon_size; + int scaled_h = settings.max_icon_size; + if (w >= h) + scaled_h = (settings.max_icon_size * h) / w; + else + scaled_w = (settings.max_icon_size * w) / h; + + GdkPixbuf *scaled = gdk_pixbuf_scale_simple( + pixbuf, + scaled_w, + scaled_h, + GDK_INTERP_BILINEAR); + g_object_unref(pixbuf); + pixbuf = scaled; + } + + return pixbuf; +} + GdkPixbuf *get_pixbuf_from_file(const char *filename) { char *path = string_to_path(g_strdup(filename)); @@ -208,25 +235,7 @@ cairo_surface_t *icon_get_for_notification(const struct notification *n) ASSERT_OR_RET(pixbuf, NULL); - int w = gdk_pixbuf_get_width(pixbuf); - int h = gdk_pixbuf_get_height(pixbuf); - int larger = w > h ? w : h; - if (settings.max_icon_size && larger > settings.max_icon_size) { - int scaled_w = settings.max_icon_size; - int scaled_h = settings.max_icon_size; - if (w >= h) - scaled_h = (settings.max_icon_size * h) / w; - else - scaled_w = (settings.max_icon_size * w) / h; - - GdkPixbuf *scaled = gdk_pixbuf_scale_simple( - pixbuf, - scaled_w, - scaled_h, - GDK_INTERP_BILINEAR); - g_object_unref(pixbuf); - pixbuf = scaled; - } + pixbuf = icon_pixbuf_scale(pixbuf); cairo_surface_t *ret = gdk_pixbuf_to_cairo_surface(pixbuf); g_object_unref(pixbuf); diff --git a/src/icon.h b/src/icon.h index a93b8ae..7b4e3e5 100644 --- a/src/icon.h +++ b/src/icon.h @@ -8,6 +8,17 @@ cairo_surface_t *gdk_pixbuf_to_cairo_surface(GdkPixbuf *pixbuf); +/** + * Scales the given GdkPixbuf if necessary according to the settings. + * + * @param pixbuf (nullable) The pixbuf, which may be too big. + * Takes ownership of the reference. + * @return the scaled version of the pixbuf. If scaling wasn't + * necessary, it returns the same pixbuf. Transfers full + * ownership of the reference. + */ +GdkPixbuf *icon_pixbuf_scale(GdkPixbuf *pixbuf); + /** Retrieve an icon by its full filepath. * * @param filename A string representing a readable file path From 088907488cc329206d04e390bb48f0df49c2e2f6 Mon Sep 17 00:00:00 2001 From: Benedikt Heine Date: Fri, 28 Dec 2018 18:33:34 +0100 Subject: [PATCH 2/6] Rename icon field to iconname --- src/dbus.c | 6 +++--- src/icon.c | 4 ++-- src/notification.c | 20 ++++++++++---------- src/notification.h | 2 +- src/rules.c | 6 +++--- test/dbus.c | 2 +- test/notification.c | 10 +++++----- 7 files changed, 25 insertions(+), 25 deletions(-) diff --git a/src/dbus.c b/src/dbus.c index 679a489..0ff6a2b 100644 --- a/src/dbus.c +++ b/src/dbus.c @@ -183,7 +183,7 @@ static struct notification *dbus_message_to_notification(const gchar *sender, GV g_variant_iter_next(&i, "s", &n->appname); g_variant_iter_next(&i, "u", &n->id); - g_variant_iter_next(&i, "s", &n->icon); + g_variant_iter_next(&i, "s", &n->iconname); g_variant_iter_next(&i, "s", &n->summary); g_variant_iter_next(&i, "s", &n->body); g_variant_iter_next(&i, "^a&s", &actions); @@ -230,8 +230,8 @@ static struct notification *dbus_message_to_notification(const gchar *sender, GV } if ((dict_value = g_variant_lookup_value(hints, "image-path", G_VARIANT_TYPE_STRING))) { - g_free(n->icon); - n->icon = g_variant_dup_string(dict_value, NULL); + g_free(n->iconname); + n->iconname = g_variant_dup_string(dict_value, NULL); g_variant_unref(dict_value); } diff --git a/src/icon.c b/src/icon.c index 7bf30aa..4fbb69b 100644 --- a/src/icon.c +++ b/src/icon.c @@ -228,8 +228,8 @@ cairo_surface_t *icon_get_for_notification(const struct notification *n) if (n->raw_icon) pixbuf = get_pixbuf_from_raw_image(n->raw_icon); - else if (n->icon) - pixbuf = get_pixbuf_from_icon(n->icon); + else if (n->iconname) + pixbuf = get_pixbuf_from_icon(n->iconname); else return NULL; diff --git a/src/notification.c b/src/notification.c index 968dd7b..5b224bd 100644 --- a/src/notification.c +++ b/src/notification.c @@ -52,7 +52,7 @@ void notification_print(const struct notification *n) printf("\tappname: '%s'\n", n->appname); printf("\tsummary: '%s'\n", n->summary); printf("\tbody: '%s'\n", n->body); - printf("\ticon: '%s'\n", n->icon); + printf("\ticon: '%s'\n", n->iconname); printf("\traw_icon set: %s\n", (n->raw_icon ? "true" : "false")); printf("\tcategory: %s\n", n->category); printf("\ttimeout: %ld\n", n->timeout/1000); @@ -103,7 +103,7 @@ void notification_run_script(struct notification *n) const char *appname = n->appname ? n->appname : ""; const char *summary = n->summary ? n->summary : ""; const char *body = n->body ? n->body : ""; - const char *icon = n->icon ? n->icon : ""; + const char *icon = n->iconname ? n->iconname : ""; const char *urgency = notification_urgency_to_string(n->urgency); @@ -183,7 +183,7 @@ int notification_is_duplicate(const struct notification *a, const struct notific return STR_EQ(a->appname, b->appname) && STR_EQ(a->summary, b->summary) && STR_EQ(a->body, b->body) - && (settings.icon_position != ICON_OFF ? STR_EQ(a->icon, b->icon) : 1) + && (settings.icon_position != ICON_OFF ? STR_EQ(a->iconname, b->iconname) : 1) && a->urgency == b->urgency; } @@ -227,7 +227,7 @@ void notification_unref(struct notification *n) g_free(n->appname); g_free(n->summary); g_free(n->body); - g_free(n->icon); + g_free(n->iconname); g_free(n->msg); g_free(n->dbus_client); g_free(n->category); @@ -328,10 +328,10 @@ void notification_init(struct notification *n) n->timeout = settings.timeouts[n->urgency]; /* Icon handling */ - if (STR_EMPTY(n->icon)) - g_clear_pointer(&n->icon, g_free); - if (!n->raw_icon && !n->icon) - n->icon = g_strdup(settings.icons[n->urgency]); + if (STR_EMPTY(n->iconname)) + g_clear_pointer(&n->iconname, g_free); + if (!n->raw_icon && !n->iconname) + n->iconname = g_strdup(settings.icons[n->urgency]); /* Color hints */ struct notification_colors defcolors; @@ -404,7 +404,7 @@ static void notification_format_message(struct notification *n) n->markup); break; case 'I': - icon_tmp = g_strdup(n->icon); + icon_tmp = g_strdup(n->iconname); notification_replace_single_field( &n->msg, &substr, @@ -416,7 +416,7 @@ static void notification_format_message(struct notification *n) notification_replace_single_field( &n->msg, &substr, - n->icon ? n->icon : "", + n->iconname ? n->iconname : "", MARKUP_NO); break; case 'p': diff --git a/src/notification.h b/src/notification.h index f8f23e8..fc69216 100644 --- a/src/notification.h +++ b/src/notification.h @@ -56,7 +56,7 @@ struct notification { char *category; enum urgency urgency; - char *icon; /**< plain icon information (may be a path or just a name) */ + char *iconname; /**< plain icon information (may be a path or just a name) */ struct raw_image *raw_icon; /**< passed icon data of notification, takes precedence over icon */ gint64 start; /**< begin of current display */ diff --git a/src/rules.c b/src/rules.c index d23c580..b1f1329 100644 --- a/src/rules.c +++ b/src/rules.c @@ -27,8 +27,8 @@ void rule_apply(struct rule *r, struct notification *n) if (r->markup != MARKUP_NULL) n->markup = r->markup; if (r->new_icon) { - g_free(n->icon); - n->icon = g_strdup(r->new_icon); + g_free(n->iconname); + n->iconname = g_strdup(r->new_icon); g_clear_pointer(&n->raw_icon, rawimage_free); } if (r->fg) { @@ -102,7 +102,7 @@ bool rule_matches_notification(struct rule *r, struct notification *n) return ( (!r->appname || (n->appname && !fnmatch(r->appname, n->appname, 0))) && (!r->summary || (n->summary && !fnmatch(r->summary, n->summary, 0))) && (!r->body || (n->body && !fnmatch(r->body, n->body, 0))) - && (!r->icon || (n->icon && !fnmatch(r->icon, n->icon, 0))) + && (!r->icon || (n->iconname && !fnmatch(r->icon, n->iconname,0))) && (!r->category || (n->category && !fnmatch(r->category, n->category, 0))) && (!r->stack_tag || (n->stack_tag && !fnmatch(r->stack_tag, n->stack_tag, 0))) && (r->match_transient == -1 || (r->match_transient == n->transient)) diff --git a/test/dbus.c b/test/dbus.c index 7102067..a98a9eb 100644 --- a/test/dbus.c +++ b/test/dbus.c @@ -511,7 +511,7 @@ TEST test_hint_icons(void) n = queues_debug_find_notification_by_id(id); - ASSERT_STR_EQ(iconname, n->icon); + ASSERT_STR_EQ(iconname, n->iconname); dbus_notification_free(n_dbus); diff --git a/test/notification.c b/test/notification.c index 1bf44e0..0e0e6e1 100644 --- a/test/notification.c +++ b/test/notification.c @@ -25,14 +25,14 @@ TEST test_notification_is_duplicate(void) a->appname = g_strdup("Test"); a->summary = g_strdup("Summary"); a->body = g_strdup("Body"); - a->icon = g_strdup("Icon"); + a->iconname = g_strdup("Icon"); a->urgency = URG_NORM; struct notification *b = notification_create(); b->appname = g_strdup("Test"); b->summary = g_strdup("Summary"); b->body = g_strdup("Body"); - b->icon = g_strdup("Icon"); + b->iconname = g_strdup("Icon"); b->urgency = URG_NORM; CHECK_CALL(test_notification_is_duplicate_field(&(b->appname), a, b)); @@ -52,13 +52,13 @@ TEST test_notification_is_duplicate(void) b->raw_icon = NULL; settings.icon_position = ICON_LEFT; - CHECK_CALL(test_notification_is_duplicate_field(&(b->icon), a, b)); + CHECK_CALL(test_notification_is_duplicate_field(&(b->iconname), a, b)); b->raw_icon = (struct raw_image*)0xff; ASSERT_FALSE(notification_is_duplicate(a, b)); b->raw_icon = NULL; settings.icon_position = ICON_RIGHT; - CHECK_CALL(test_notification_is_duplicate_field(&(b->icon), a, b)); + CHECK_CALL(test_notification_is_duplicate_field(&(b->iconname), a, b)); b->raw_icon = (struct raw_image*)0xff; ASSERT_FALSE(notification_is_duplicate(a, b)); b->raw_icon = NULL; @@ -173,7 +173,7 @@ SUITE(suite_notification) a->appname = g_strdup("MyApp"); a->summary = g_strdup("I've got a summary!"); a->body = g_strdup("Look at my shiny "); - a->icon = g_strdup("/this/is/my/icoknpath.png"); + a->iconname = g_strdup("/this/is/my/icoknpath.png"); a->progress = 95; const char *strings[] = { From cd09d5a88e59b30356094af816d0adb72051ba08 Mon Sep 17 00:00:00 2001 From: Benedikt Heine Date: Fri, 28 Dec 2018 18:45:16 +0100 Subject: [PATCH 3/6] Cache GdkPixbuf in notification structure --- src/icon.c | 17 +---------------- src/notification.c | 17 +++++++++++++++++ src/notification.h | 2 ++ test/dbus.c | 4 ++++ test/queues.c | 4 ++++ 5 files changed, 28 insertions(+), 16 deletions(-) diff --git a/src/icon.c b/src/icon.c index 4fbb69b..f7acadd 100644 --- a/src/icon.c +++ b/src/icon.c @@ -224,22 +224,7 @@ GdkPixbuf *get_pixbuf_from_raw_image(const struct raw_image *raw_image) cairo_surface_t *icon_get_for_notification(const struct notification *n) { - GdkPixbuf *pixbuf; - - if (n->raw_icon) - pixbuf = get_pixbuf_from_raw_image(n->raw_icon); - else if (n->iconname) - pixbuf = get_pixbuf_from_icon(n->iconname); - else - return NULL; - - ASSERT_OR_RET(pixbuf, NULL); - - pixbuf = icon_pixbuf_scale(pixbuf); - - cairo_surface_t *ret = gdk_pixbuf_to_cairo_surface(pixbuf); - g_object_unref(pixbuf); - return ret; + return gdk_pixbuf_to_cairo_surface(n->icon); } /* vim: set tabstop=8 shiftwidth=8 expandtab textwidth=0: */ diff --git a/src/notification.c b/src/notification.c index 5b224bd..55b6d22 100644 --- a/src/notification.c +++ b/src/notification.c @@ -15,6 +15,7 @@ #include "dbus.h" #include "dunst.h" +#include "icon.h" #include "log.h" #include "markup.h" #include "menu.h" @@ -23,6 +24,7 @@ #include "settings.h" #include "utils.h" +static void notification_update_icon(struct notification *n); static void notification_extract_urls(struct notification *n); static void notification_format_message(struct notification *n); @@ -363,10 +365,25 @@ void notification_init(struct notification *n) rule_apply_all(n); /* UPDATE derived fields */ + notification_update_icon(n); notification_extract_urls(n); notification_format_message(n); } +static void notification_update_icon(struct notification *n) +{ + g_return_if_fail(n); + + g_clear_object(&n->icon); + + if (n->raw_icon) + n->icon = get_pixbuf_from_raw_image(n->raw_icon); + else if (n->iconname) + n->icon = get_pixbuf_from_icon(n->iconname); + + n->icon = icon_pixbuf_scale(n->icon); +} + static void notification_format_message(struct notification *n) { g_clear_pointer(&n->msg, g_free); diff --git a/src/notification.h b/src/notification.h index fc69216..7db5f8c 100644 --- a/src/notification.h +++ b/src/notification.h @@ -2,6 +2,7 @@ #ifndef DUNST_NOTIFICATION_H #define DUNST_NOTIFICATION_H +#include #include #include @@ -56,6 +57,7 @@ struct notification { char *category; enum urgency urgency; + GdkPixbuf *icon; char *iconname; /**< plain icon information (may be a path or just a name) */ struct raw_image *raw_icon; /**< passed icon data of notification, takes precedence over icon */ diff --git a/test/dbus.c b/test/dbus.c index a98a9eb..88db471 100644 --- a/test/dbus.c +++ b/test/dbus.c @@ -796,6 +796,8 @@ gpointer run_threaded_tests(gpointer data) SUITE(suite_dbus) { + settings.icon_path = ""; + GTestDBus *dbus_bus; g_test_dbus_unset(); queues_init(); @@ -813,6 +815,8 @@ SUITE(suite_dbus) g_object_unref(dbus_bus); g_thread_unref(thread_tests); g_main_loop_unref(loop); + + settings.icon_path = NULL; } /* vim: set tabstop=8 shiftwidth=8 expandtab textwidth=0: */ diff --git a/test/queues.c b/test/queues.c index 1ba8216..67e7651 100644 --- a/test/queues.c +++ b/test/queues.c @@ -698,6 +698,8 @@ TEST test_queues_timeout_before_paused(void) SUITE(suite_queues) { + settings.icon_path = ""; + RUN_TEST(test_datachange_beginning_empty); RUN_TEST(test_datachange_endless); RUN_TEST(test_datachange_endless_agethreshold); @@ -722,6 +724,8 @@ SUITE(suite_queues) RUN_TEST(test_queues_update_seeping); RUN_TEST(test_queues_update_xmore); RUN_TEST(test_queues_timeout_before_paused); + + settings.icon_path = NULL; } /* vim: set tabstop=8 shiftwidth=8 expandtab textwidth=0: */ From f9f5804b08b5091fbee68ad106deabffd1c8de70 Mon Sep 17 00:00:00 2001 From: Benedikt Heine Date: Fri, 28 Dec 2018 18:48:13 +0100 Subject: [PATCH 4/6] Remove notification_icon_get wrapper --- src/draw.c | 4 ++-- src/icon.c | 5 ----- src/icon.h | 8 -------- 3 files changed, 2 insertions(+), 15 deletions(-) diff --git a/src/draw.c b/src/draw.c index bae6a30..e1032c0 100644 --- a/src/draw.c +++ b/src/draw.c @@ -270,8 +270,8 @@ static struct colored_layout *layout_init_shared(cairo_t *c, const struct notifi pango_layout_set_ellipsize(cl->l, ellipsize); } - if (settings.icon_position != ICON_OFF) { - cl->icon = icon_get_for_notification(n); + if (settings.icon_position != ICON_OFF && n->icon) { + cl->icon = gdk_pixbuf_to_cairo_surface(n->icon); } else { cl->icon = NULL; } diff --git a/src/icon.c b/src/icon.c index f7acadd..a0b9f5d 100644 --- a/src/icon.c +++ b/src/icon.c @@ -222,9 +222,4 @@ GdkPixbuf *get_pixbuf_from_raw_image(const struct raw_image *raw_image) return pixbuf; } -cairo_surface_t *icon_get_for_notification(const struct notification *n) -{ - return gdk_pixbuf_to_cairo_surface(n->icon); -} - /* vim: set tabstop=8 shiftwidth=8 expandtab textwidth=0: */ diff --git a/src/icon.h b/src/icon.h index 7b4e3e5..841611d 100644 --- a/src/icon.h +++ b/src/icon.h @@ -41,13 +41,5 @@ GdkPixbuf *get_pixbuf_from_icon(const char *iconname); */ GdkPixbuf *get_pixbuf_from_raw_image(const struct raw_image *raw_image); -/** - * Get a cairo surface with the appropriate icon for the notification, scaled - * according to the current settings - * - * @return a cairo_surface_t pointer or NULL if no icon could be retrieved. - */ -cairo_surface_t *icon_get_for_notification(const struct notification *n); - #endif /* vim: set tabstop=8 shiftwidth=8 expandtab textwidth=0: */ From 8a46b88da9e4116460ae66d8760b2bc751e3faef Mon Sep 17 00:00:00 2001 From: Benedikt Heine Date: Thu, 3 Jan 2019 19:03:16 +0100 Subject: [PATCH 5/6] Use g_strcmp0 for string comparisons g_strcmp0 handles NULL values correctly. This allows to omit NULL pointer checks, which would be otherwise crucial. --- src/utils.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils.h b/src/utils.h index 8a6f652..65f4cd8 100644 --- a/src/utils.h +++ b/src/utils.h @@ -10,7 +10,7 @@ //! Test if a string is non-NULL and not empty #define STR_FULL(s) !(STR_EMPTY(s)) //! Test if string a and b contain the same chars -#define STR_EQ(a, b) (strcmp(a, b) == 0) +#define STR_EQ(a, b) (g_strcmp0(a, b) == 0) //! Test if string a and b are same up to n chars #define STRN_EQ(a, b, n) (strncmp(a, b, n) == 0) //! Test if string a and b are the same case-insensitively From 6f8b53c4e8e0edc45758316dc3ccf006f931f86b Mon Sep 17 00:00:00 2001 From: Benedikt Heine Date: Thu, 3 Jan 2019 20:17:18 +0100 Subject: [PATCH 6/6] Compare raw icons by their checksums Currently, we just skipped the notification comparison, if the notification had a raw icon attached. This is a bit counterintuitive. Calculating a checksum of the raw icon's data is the solution. For that we cache the pixel buffer and introduce a field, which saves the current icon's id. The icon_id may be a path or a hash. So you can compare two notifications by their icon_id field regardless of their icon type by their icon_id field. --- src/dbus.c | 40 +-------------- src/icon.c | 121 ++++++++++++++++++++++++++++++++++++++++---- src/icon.h | 29 ++++++++++- src/notification.c | 79 ++++++++++++++++------------- src/notification.h | 46 +++++++++-------- src/rules.c | 7 +-- test/dbus.c | 3 +- test/notification.c | 22 ++++---- 8 files changed, 222 insertions(+), 125 deletions(-) diff --git a/src/dbus.c b/src/dbus.c index 0ff6a2b..b08bcc7 100644 --- a/src/dbus.c +++ b/src/dbus.c @@ -86,8 +86,6 @@ struct dbus_method { GVariant *parameters, \ GDBusMethodInvocation *invocation) -static struct raw_image *get_raw_image_from_data_hint(GVariant *icon_data); - int cmp_methods(const void *vkey, const void *velem) { const char *key = (const char*)vkey; @@ -241,7 +239,7 @@ static struct notification *dbus_message_to_notification(const gchar *sender, GV if (!dict_value) dict_value = g_variant_lookup_value(hints, "icon_data", G_VARIANT_TYPE("(iiibiiay)")); if (dict_value) { - n->raw_icon = get_raw_image_from_data_hint(dict_value); + notification_icon_replace_data(n, dict_value); g_variant_unref(dict_value); } @@ -577,42 +575,6 @@ static void dbus_cb_name_lost(GDBusConnection *connection, exit(1); } -static struct raw_image *get_raw_image_from_data_hint(GVariant *icon_data) -{ - struct raw_image *image = g_malloc(sizeof(struct raw_image)); - GVariant *data_variant; - gsize expected_len; - - g_variant_get(icon_data, - "(iiibii@ay)", - &image->width, - &image->height, - &image->rowstride, - &image->has_alpha, - &image->bits_per_sample, - &image->n_channels, - &data_variant); - - expected_len = (image->height - 1) * image->rowstride + image->width - * ((image->n_channels * image->bits_per_sample + 7) / 8); - - if (expected_len != g_variant_get_size (data_variant)) { - LOG_W("Expected image data to be of length %" G_GSIZE_FORMAT - " but got a length of %" G_GSIZE_FORMAT, - expected_len, - g_variant_get_size(data_variant)); - g_free(image); - g_variant_unref(data_variant); - return NULL; - } - - image->data = (guchar *) g_memdup(g_variant_get_data(data_variant), - g_variant_get_size(data_variant)); - g_variant_unref(data_variant); - - return image; -} - int dbus_init(void) { guint owner_id; diff --git a/src/icon.c b/src/icon.c index a0b9f5d..71a4731 100644 --- a/src/icon.c +++ b/src/icon.c @@ -205,19 +205,120 @@ GdkPixbuf *get_pixbuf_from_icon(const char *iconname) return pixbuf; } -GdkPixbuf *get_pixbuf_from_raw_image(const struct raw_image *raw_image) +GdkPixbuf *icon_get_for_name(const char *name, char **id) { - GdkPixbuf *pixbuf = NULL; + ASSERT_OR_RET(name, NULL); + ASSERT_OR_RET(id, NULL); - pixbuf = gdk_pixbuf_new_from_data(raw_image->data, + GdkPixbuf *pb = get_pixbuf_from_icon(name); + if (pb) + *id = g_strdup(name); + return pb; +} + +GdkPixbuf *icon_get_for_data(GVariant *data, char **id) +{ + ASSERT_OR_RET(data, NULL); + ASSERT_OR_RET(id, NULL); + + if (!STR_EQ("(iiibiiay)", g_variant_get_type_string(data))) { + LOG_W("Invalid data for pixbuf given."); + return NULL; + } + + /* The raw image is a big array of char data. + * + * The image is serialised rowwise pixel by pixel. The rows are aligned + * by a spacer full of garbage. The overall data length of data + garbage + * is called the rowstride. + * + * Mind the missing spacer at the last row. + * + * len: |<--------------rowstride---------------->| + * len: |<-width*pixelstride->| + * row 1: | data for row 1 | spacer of garbage | + * row 2: | data for row 2 | spacer of garbage | + * | . | spacer of garbage | + * | . | spacer of garbage | + * | . | spacer of garbage | + * row n-1: | data for row n-1 | spacer of garbage | + * row n: | data for row n | + */ + + GdkPixbuf *pixbuf = NULL; + GVariant *data_variant = NULL; + unsigned char *data_pb; + + gsize len_expected; + gsize len_actual; + gsize pixelstride; + + int width; + int height; + int rowstride; + int has_alpha; + int bits_per_sample; + int n_channels; + + g_variant_get(data, + "(iiibii@ay)", + &width, + &height, + &rowstride, + &has_alpha, + &bits_per_sample, + &n_channels, + &data_variant); + + // note: (A+7)/8 rounds up A to the next byte boundary + pixelstride = (n_channels * bits_per_sample + 7)/8; + len_expected = (height - 1) * rowstride + width * pixelstride; + len_actual = g_variant_get_size(data_variant); + + if (len_actual != len_expected) { + LOG_W("Expected image data to be of length %" G_GSIZE_FORMAT + " but got a length of %" G_GSIZE_FORMAT, + len_expected, + len_actual); + g_variant_unref(data_variant); + return NULL; + } + + data_pb = (guchar *) g_memdup(g_variant_get_data(data_variant), len_actual); + + pixbuf = gdk_pixbuf_new_from_data(data_pb, GDK_COLORSPACE_RGB, - raw_image->has_alpha, - raw_image->bits_per_sample, - raw_image->width, - raw_image->height, - raw_image->rowstride, - NULL, - NULL); + has_alpha, + bits_per_sample, + width, + height, + rowstride, + (GdkPixbufDestroyNotify) g_free, + data_pb); + if (!pixbuf) { + /* Dear user, I'm sorry, I'd like to give you a more specific + * error message. But sadly, I can't */ + LOG_W("Cannot serialise raw icon data into pixbuf."); + return NULL; + } + + /* To calculate a checksum of the current image, we have to remove + * all excess spacers, so that our checksummed memory only contains + * real data. */ + size_t data_chk_len = pixelstride * width * height; + unsigned char *data_chk = g_malloc(data_chk_len); + size_t rowstride_short = pixelstride * width; + + for (int i = 0; i < height; i++) { + memcpy(data_chk + (i*rowstride_short), + data_pb + (i*rowstride), + rowstride_short); + } + + *id = g_compute_checksum_for_data(G_CHECKSUM_MD5, data_chk, data_chk_len); + + g_free(data_chk); + g_variant_unref(data_variant); return pixbuf; } diff --git a/src/icon.h b/src/icon.h index 841611d..5e03bb3 100644 --- a/src/icon.h +++ b/src/icon.h @@ -37,9 +37,34 @@ GdkPixbuf *get_pixbuf_from_file(const char *filename); */ GdkPixbuf *get_pixbuf_from_icon(const char *iconname); -/** Convert a struct raw_image to a `GdkPixbuf` +/** Read an icon from disk and convert it to a GdkPixbuf. + * + * The returned id will be a unique identifier. To check if two given + * GdkPixbufs are equal, it's sufficient to just compare the id strings. + * + * @param name A string describing and icon. May be a full path, a file path or + * just a simple name. If it's a name without a slash, the icon will + * get searched in the folders of the icon_path setting. + * @param id (necessary) A unique identifier of the returned pixbuf. Only filled, + * if the return value is non-NULL. + * @return a pixbuf representing name's image. + * If an invalid path given, it will return NULL. */ -GdkPixbuf *get_pixbuf_from_raw_image(const struct raw_image *raw_image); +GdkPixbuf *icon_get_for_name(const char *name, char **id); + +/** Convert a GVariant like described in GdkPixbuf + * + * The returned id will be a unique identifier. To check if two given + * GdkPixbufs are equal, it's sufficient to just compare the id strings. + * + * @param data A GVariant in the format "(iiibii@ay)" filled with values + * like described in the notification spec. + * @param id (necessary) A unique identifier of the returned pixbuf. + * Only filled, if the return value is non-NULL. + * @return a pixbuf representing name's image. + * If an invalid GVariant is passed, it will return NULL. + */ +GdkPixbuf *icon_get_for_data(GVariant *data, char **id); #endif /* vim: set tabstop=8 shiftwidth=8 expandtab textwidth=0: */ diff --git a/src/notification.c b/src/notification.c index 55b6d22..f6e94f9 100644 --- a/src/notification.c +++ b/src/notification.c @@ -24,7 +24,6 @@ #include "settings.h" #include "utils.h" -static void notification_update_icon(struct notification *n); static void notification_extract_urls(struct notification *n); static void notification_format_message(struct notification *n); @@ -55,7 +54,8 @@ void notification_print(const struct notification *n) printf("\tsummary: '%s'\n", n->summary); printf("\tbody: '%s'\n", n->body); printf("\ticon: '%s'\n", n->iconname); - printf("\traw_icon set: %s\n", (n->raw_icon ? "true" : "false")); + printf("\traw_icon set: %s\n", (n->icon_id && !STR_EQ(n->iconname, n->icon_id)) ? "true" : "false"); + printf("\ticon_id: '%s'\n", n->icon_id); printf("\tcategory: %s\n", n->category); printf("\ttimeout: %ld\n", n->timeout/1000); printf("\turgency: %s\n", notification_urgency_to_string(n->urgency)); @@ -175,29 +175,15 @@ int notification_cmp_data(const void *va, const void *vb, void *data) return notification_cmp(a, b); } -int notification_is_duplicate(const struct notification *a, const struct notification *b) +bool notification_is_duplicate(const struct notification *a, const struct notification *b) { - //Comparing raw icons is not supported, assume they are not identical - if (settings.icon_position != ICON_OFF - && (a->raw_icon || b->raw_icon)) - return false; - return STR_EQ(a->appname, b->appname) && STR_EQ(a->summary, b->summary) && STR_EQ(a->body, b->body) - && (settings.icon_position != ICON_OFF ? STR_EQ(a->iconname, b->iconname) : 1) + && (settings.icon_position != ICON_OFF ? STR_EQ(a->icon_id, b->icon_id) : 1) && a->urgency == b->urgency; } -/* see notification.h */ -void rawimage_free(struct raw_image *i) -{ - ASSERT_OR_RET(i,); - - g_free(i->data); - g_free(i); -} - static void notification_private_free(NotificationPrivate *p) { g_free(p); @@ -241,13 +227,44 @@ void notification_unref(struct notification *n) g_free(n->stack_tag); g_hash_table_unref(n->actions); - rawimage_free(n->raw_icon); + + if (n->icon) + g_object_unref(n->icon); + g_free(n->icon_id); notification_private_free(n->priv); g_free(n); } +void notification_icon_replace_path(struct notification *n, const char *new_icon) +{ + ASSERT_OR_RET(n,); + ASSERT_OR_RET(new_icon,); + ASSERT_OR_RET(n->iconname != new_icon,); + + g_free(n->iconname); + n->iconname = g_strdup(new_icon); + + g_clear_object(&n->icon); + g_clear_pointer(&n->icon_id, g_free); + + n->icon = icon_get_for_name(new_icon, &n->icon_id); + n->icon = icon_pixbuf_scale(n->icon); +} + +void notification_icon_replace_data(struct notification *n, GVariant *new_icon) +{ + ASSERT_OR_RET(n,); + ASSERT_OR_RET(new_icon,); + + g_clear_object(&n->icon); + g_clear_pointer(&n->icon_id, g_free); + + n->icon = icon_get_for_data(new_icon, &n->icon_id); + n->icon = icon_pixbuf_scale(n->icon); +} + /* see notification.h */ void notification_replace_single_field(char **haystack, char **needle, @@ -332,8 +349,13 @@ void notification_init(struct notification *n) /* Icon handling */ if (STR_EMPTY(n->iconname)) g_clear_pointer(&n->iconname, g_free); - if (!n->raw_icon && !n->iconname) - n->iconname = g_strdup(settings.icons[n->urgency]); + if (!n->icon && n->iconname) { + char *icon = g_strdup(n->iconname); + notification_icon_replace_path(n, icon); + g_free(icon); + } + if (!n->icon && !n->iconname) + notification_icon_replace_path(n, settings.icons[n->urgency]); /* Color hints */ struct notification_colors defcolors; @@ -365,25 +387,10 @@ void notification_init(struct notification *n) rule_apply_all(n); /* UPDATE derived fields */ - notification_update_icon(n); notification_extract_urls(n); notification_format_message(n); } -static void notification_update_icon(struct notification *n) -{ - g_return_if_fail(n); - - g_clear_object(&n->icon); - - if (n->raw_icon) - n->icon = get_pixbuf_from_raw_image(n->raw_icon); - else if (n->iconname) - n->icon = get_pixbuf_from_icon(n->iconname); - - n->icon = icon_pixbuf_scale(n->icon); -} - static void notification_format_message(struct notification *n) { g_clear_pointer(&n->msg, g_free); diff --git a/src/notification.h b/src/notification.h index 7db5f8c..f02f3b0 100644 --- a/src/notification.h +++ b/src/notification.h @@ -27,16 +27,6 @@ enum urgency { URG_MAX = 2, /**< Maximum value, useful for boundary checking */ }; -struct raw_image { - int width; - int height; - int rowstride; - int has_alpha; - int bits_per_sample; - int n_channels; - unsigned char *data; -}; - typedef struct _notification_private NotificationPrivate; struct notification_colors { @@ -57,9 +47,11 @@ struct notification { char *category; enum urgency urgency; - GdkPixbuf *icon; - char *iconname; /**< plain icon information (may be a path or just a name) */ - struct raw_image *raw_icon; /**< passed icon data of notification, takes precedence over icon */ + GdkPixbuf *icon; /**< The raw cached icon data used to draw */ + char *icon_id; /**< plain icon information, which acts as the pixbuf's id, which is saved in .icon + May be a hash for a raw icon or a name/path for a regular app icon. */ + char *iconname; /**< plain icon information (may be a path or just a name) + Use this to compare the icon name with rules.*/ gint64 start; /**< begin of current display */ gint64 timestamp; /**< arrival time */ @@ -123,13 +115,6 @@ void notification_ref(struct notification *n); */ void notification_init(struct notification *n); -/** - * Free a #raw_image - * - * @param i (nullable): pointer to #raw_image - */ -void rawimage_free(struct raw_image *i); - /** * Decrease the reference counter of the notification. * @@ -148,7 +133,26 @@ int notification_cmp(const struct notification *a, const struct notification *b) */ int notification_cmp_data(const void *va, const void *vb, void *data); -int notification_is_duplicate(const struct notification *a, const struct notification *b); +bool notification_is_duplicate(const struct notification *a, const struct notification *b); + +/**Replace the current notification's icon with the icon specified by path. + * + * Removes the reference for the previous icon automatically and will also free the + * iconname field. So passing n->iconname as new_icon is invalid. + * + * @param n the notification to replace the icon + * @param new_icon The path of the new icon. May be an absolute path or an icon name. + */ +void notification_icon_replace_path(struct notification *n, const char *new_icon); + +/**Replace the current notification's icon with the raw icon given in the GVariant. + * + * Removes the reference for the previous icon automatically. + * + * @param n the notification to replace the icon + * @param new_icon The icon's data. Has to be in the format of the notification spec. + */ +void notification_icon_replace_data(struct notification *n, GVariant *new_icon); /** * Run the script associated with the diff --git a/src/rules.c b/src/rules.c index b1f1329..9fd9ce6 100644 --- a/src/rules.c +++ b/src/rules.c @@ -26,11 +26,8 @@ void rule_apply(struct rule *r, struct notification *n) n->transient = r->set_transient; if (r->markup != MARKUP_NULL) n->markup = r->markup; - if (r->new_icon) { - g_free(n->iconname); - n->iconname = g_strdup(r->new_icon); - g_clear_pointer(&n->raw_icon, rawimage_free); - } + if (r->new_icon) + notification_icon_replace_path(n, r->new_icon); if (r->fg) { g_free(n->colors.fg); n->colors.fg = g_strdup(r->fg); diff --git a/test/dbus.c b/test/dbus.c index 88db471..066a0d6 100644 --- a/test/dbus.c +++ b/test/dbus.c @@ -618,7 +618,8 @@ TEST test_hint_raw_image(void) ASSERT_EQ(queues_length_waiting(), len+1); n = queues_debug_find_notification_by_id(id); - ASSERT(n->raw_icon); + ASSERT(n->icon); + ASSERT(!STR_EQ(n->icon_id, n_dbus->app_icon)); dbus_notification_free(n_dbus); g_free(path); diff --git a/test/notification.c b/test/notification.c index 0e0e6e1..69040f6 100644 --- a/test/notification.c +++ b/test/notification.c @@ -26,6 +26,7 @@ TEST test_notification_is_duplicate(void) a->summary = g_strdup("Summary"); a->body = g_strdup("Body"); a->iconname = g_strdup("Icon"); + a->icon_id = g_strdup("Icon"); a->urgency = URG_NORM; struct notification *b = notification_create(); @@ -33,8 +34,11 @@ TEST test_notification_is_duplicate(void) b->summary = g_strdup("Summary"); b->body = g_strdup("Body"); b->iconname = g_strdup("Icon"); + b->icon_id = g_strdup("Icon"); b->urgency = URG_NORM; + ASSERT(notification_is_duplicate(a, b)); + CHECK_CALL(test_notification_is_duplicate_field(&(b->appname), a, b)); CHECK_CALL(test_notification_is_duplicate_field(&(b->summary), a, b)); CHECK_CALL(test_notification_is_duplicate_field(&(b->body), a, b)); @@ -47,21 +51,17 @@ TEST test_notification_is_duplicate(void) settings.icon_position = ICON_OFF; ASSERT(notification_is_duplicate(a, b)); //Setting pointer to a random value since we are checking for null - b->raw_icon = (struct raw_image*)0xff; - ASSERT(notification_is_duplicate(a, b)); - b->raw_icon = NULL; + char *icon_id = b->icon_id; + b->icon_id = "false"; + ASSERTm("Icons have to get ignored for duplicate check when icons are off", + notification_is_duplicate(a, b)); + b->icon_id = icon_id; settings.icon_position = ICON_LEFT; - CHECK_CALL(test_notification_is_duplicate_field(&(b->iconname), a, b)); - b->raw_icon = (struct raw_image*)0xff; - ASSERT_FALSE(notification_is_duplicate(a, b)); - b->raw_icon = NULL; + CHECK_CALL(test_notification_is_duplicate_field(&(b->icon_id), a, b)); settings.icon_position = ICON_RIGHT; - CHECK_CALL(test_notification_is_duplicate_field(&(b->iconname), a, b)); - b->raw_icon = (struct raw_image*)0xff; - ASSERT_FALSE(notification_is_duplicate(a, b)); - b->raw_icon = NULL; + CHECK_CALL(test_notification_is_duplicate_field(&(b->icon_id), a, b)); settings.icon_position = icon_setting_tmp;