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.

Author: bebehei

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;

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)
 {
+        ASSERT_OR_RET(name, NULL);
+        ASSERT_OR_RET(id, NULL);
+
+        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(raw_image->data,
+        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;
 }

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 *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 *get_pixbuf_from_raw_image(const struct raw_image *raw_image);
+GdkPixbuf *icon_get_for_data(GVariant *data, char **id);
 
 #endif
 /* vim: set tabstop=8 shiftwidth=8 expandtab textwidth=0: */

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,41 @@ 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,);
+
+        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);
+}
+
+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);
+}
+
 /* see notification.h */
 void notification_replace_single_field(char **haystack,
                                        char **needle,
@@ -332,8 +346,10 @@ 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)
+                n->icon = icon_get_for_name(n->iconname, &n->icon_id);
+        if (!n->icon && !n->iconname)
+                notification_icon_replace_path(n, settings.icons[n->urgency]);
 
         /* Color hints */
         struct notification_colors defcolors;
@@ -365,25 +381,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);