From 6f8b53c4e8e0edc45758316dc3ccf006f931f86b Mon Sep 17 00:00:00 2001 From: Benedikt Heine Date: Thu, 3 Jan 2019 20:17:18 +0100 Subject: [PATCH] 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;