Change the caching of icons

src/draw.c

@@ -47,7 +47,7 @@ void load_icon_themes(void)
                 char *theme = settings.icon_theme[i];
                 int theme_index = load_icon_theme(theme);
                 if (theme_index >= 0) {
-                        LOG_I("Adding theme %s", theme);
+                        LOG_I("Adding icon theme %s", theme);
                         add_default_theme(theme_index);
                         loaded_theme = true;
                 }

src/icon.c

@@ -183,11 +183,36 @@ static GdkPixbuf *icon_pixbuf_scale_to_size(GdkPixbuf *pixbuf, double dpi_scale,
         return pixbuf;
 }
 
-GdkPixbuf *get_pixbuf_from_file(const char *filename, int min_size, int max_size, double scale)
+static char *get_id_from_data(const uint8_t *data_pb, size_t width, size_t height, size_t pixelstride, size_t rowstride)
+{
+        /* 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);
+        }
+
+        char *id = g_compute_checksum_for_data(G_CHECKSUM_MD5, data_chk, data_chk_len);
+        g_free(data_chk);
+
+        return id;
+}
+
+GdkPixbuf *get_pixbuf_from_file(const char *filename, char **id, int min_size, int max_size, double scale)
 {
         GError *error = NULL;
         gint w, h;
 
+        ASSERT_OR_RET(filename, NULL);
+        ASSERT_OR_RET(id, NULL);
+
         if (!gdk_pixbuf_get_file_info (filename, &w, &h)) {
                 LOG_W("Failed to load image info for %s", STR_NN(filename));
                 return NULL;
@@ -206,6 +231,13 @@ GdkPixbuf *get_pixbuf_from_file(const char *filename, int min_size, int max_size
                 g_error_free(error);
         }
 
+        const uint8_t *data = gdk_pixbuf_get_pixels(pixbuf);
+        size_t rowstride = gdk_pixbuf_get_rowstride(pixbuf);
+        size_t n_channels = gdk_pixbuf_get_n_channels(pixbuf);
+        size_t bits_per_sample = gdk_pixbuf_get_bits_per_sample(pixbuf);
+        size_t pixelstride = (n_channels * bits_per_sample + 7)/8;
+
+        *id = get_id_from_data(data, w, h, pixelstride, rowstride);
         return pixbuf;
 }
 
@@ -221,8 +253,7 @@ char *get_path_from_icon_name(const char *iconname, int size)
                         return NULL;
                 }
                 return uri_path;
-        } else if (iconname[0] == '/' || iconname[0] == '~') {
-                // NOTE: Paths starting with ~ should have already been handled at this point
+        } else if (is_like_path(iconname)) {
                 return g_strdup(iconname);
         } else if (settings.enable_recursive_icon_lookup) {
                 char *path = find_icon_path(iconname, size);
@@ -366,22 +397,9 @@ GdkPixbuf *icon_get_for_data(GVariant *data, char **id, double dpi_scale, int mi
                 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);
+        *id = get_id_from_data(data_pb, width, height, pixelstride, rowstride);
 
-        g_free(data_chk);
         g_variant_unref(data_variant);
 
         pixbuf = icon_pixbuf_scale_to_size(pixbuf, dpi_scale, min_size, max_size);

src/icon.h

@@ -11,14 +11,16 @@ cairo_surface_t *gdk_pixbuf_to_cairo_surface(GdkPixbuf *pixbuf);
 /** Retrieve an icon by its full filepath, scaled according to settings.
  *
  * @param filename A string representing a readable file path
+ * @param id   (necessary) A unique identifier of the returned pixbuf.
+ *             Only filled, if the return value is non-NULL.
  * @param min_size An iteger representing the desired minimum unscaled icon size.
  * @param max_size An iteger representing the desired maximum unscaled icon size.
  * @param scale An integer representing the output dpi scaling.
  *
  * @return an instance of `GdkPixbuf`
  * @retval NULL: file does not exist, not readable, etc..
  */
-GdkPixbuf *get_pixbuf_from_file(const char *filename, int min_size, int max_size, double scale);
+GdkPixbuf *get_pixbuf_from_file(const char *filename, char **id, int min_size, int max_size, double scale);
 
 
 /**

src/notification.c

@@ -11,6 +11,7 @@
 #include <stdlib.h>
 #include <string.h>
 #include <sys/wait.h>
+#include <sys/stat.h>
 #include <unistd.h>
 
 #include "dbus.h"
@@ -57,10 +58,11 @@ void notification_print(const struct notification *n)
         printf("\tsummary: '%s'\n", STR_NN(n->summary));
         printf("\tbody: '%s'\n", STR_NN(n->body));
         printf("\ticon: '%s'\n", STR_NN(n->iconname));
-        printf("\traw_icon set: %s\n", (n->icon_id && !STR_EQ(n->iconname, n->icon_id)) ? "true" : "false");
+        printf("\traw_icon: %s\n", (n->icon_id && STR_EMPTY(n->iconname)) ? "true" : "false");
         printf("\ticon_id: '%s'\n", STR_NN(n->icon_id));
+        printf("\ticon_time: %"G_GINT64_FORMAT"\n", n->icon_time);
         printf("\tdesktop_entry: '%s'\n", n->desktop_entry ? n->desktop_entry : "");
-        printf("\tcategory: %s\n", STR_NN(n->category));
+        printf("\tcategory: '%s'\n", STR_NN(n->category));
         printf("\ttimeout: %"G_GINT64_FORMAT"\n", n->timeout/1000);
         printf("\tstart: %"G_GINT64_FORMAT"\n", n->start);
         printf("\ttimestamp: %"G_GINT64_FORMAT"\n", n->timestamp);
@@ -77,17 +79,19 @@ void notification_print(const struct notification *n)
         g_free(grad);
 
         printf("\tfullscreen: %s\n", enum_to_string_fullscreen(n->fullscreen));
-        printf("\tformat: %s\n", STR_NN(n->format));
+        printf("\tformat: '%s'\n", STR_NN(n->format));
         printf("\tprogress: %d\n", n->progress);
-        printf("\tstack_tag: %s\n", (n->stack_tag ? n->stack_tag : ""));
+        printf("\tstack_tag: '%s'\n", (n->stack_tag ? n->stack_tag : ""));
         printf("\tid: %d\n", n->id);
         if (n->urls) {
                 char *urls = string_replace_all("\n", "\t\t\n", g_strdup(n->urls));
                 printf("\turls:\n");
                 printf("\t{\n");
-                printf("\t\t%s\n", STR_NN(urls));
+                printf("\t\t'%s'\n", STR_NN(urls));
                 printf("\t}\n");
                 g_free(urls);
+        } else {
+                printf("\turls: {}\n");
         }
         if (g_hash_table_size(n->actions) == 0) {
                 printf("\tactions: {}\n");
@@ -243,8 +247,10 @@ bool notification_is_duplicate(const struct notification *a, const struct notifi
         return STR_EQ(a->appname, b->appname)
             && STR_EQ(a->summary, b->summary)
             && STR_EQ(a->body, b->body)
-            && (a->icon_position != ICON_OFF ? STR_EQ(a->icon_id, b->icon_id) : 1)
-            && a->urgency == b->urgency;
+            && a->urgency == b->urgency
+            && (a->icon_position == ICON_OFF || b->icon_position == ICON_OFF
+                            || (a->icon_id && b->icon_id ? STR_EQ(a->icon_id, b->icon_id)
+                            : (a->iconname && b->iconname ? STR_EQ(a->iconname, b->iconname) : 1)));
 }
 
 bool notification_is_locked(struct notification *n) {
@@ -337,14 +343,14 @@ void notification_unref(struct notification *n)
 
 void notification_transfer_icon(struct notification *from, struct notification *to)
 {
-        if (from->iconname && to->iconname
-                        && strcmp(from->iconname, to->iconname) == 0){
-                // Icons are the same. Transfer icon surface
-                to->icon = from->icon;
-
-                // prevent the surface being freed by the old notification
-                from->icon = NULL;
-        }
+        // Transfer icon surface
+        to->icon = from->icon;
+        to->icon_id = from->icon_id;
+        to->icon_time = from->icon_time;
+
+        // Prevent the surface being freed by the old notification
+        from->icon = NULL;
+        from->icon_id = NULL;
 }
 
 void notification_icon_replace_path(struct notification *n, const char *new_icon)
@@ -368,11 +374,12 @@ void notification_icon_replace_path(struct notification *n, const char *new_icon
         g_free(n->icon_path);
         n->icon_path = get_path_from_icon_name(new_icon, n->min_icon_size);
         if (n->icon_path) {
-                GdkPixbuf *pixbuf = get_pixbuf_from_file(n->icon_path,
+                GdkPixbuf *pixbuf = get_pixbuf_from_file(n->icon_path, &n->icon_id,
                                 n->min_icon_size, n->max_icon_size,
                                 draw_get_scale());
                 if (pixbuf) {
                         n->icon = gdk_pixbuf_to_cairo_surface(pixbuf);
+                        n->icon_time = time_now();
                         g_object_unref(pixbuf);
                 } else {
                         LOG_W("Failed to load icon from path: '%s'", n->icon_path);
@@ -392,8 +399,10 @@ void notification_icon_replace_data(struct notification *n, GVariant *new_icon)
         GdkPixbuf *icon = icon_get_for_data(new_icon, &n->icon_id,
                         draw_get_scale(), n->min_icon_size, n->max_icon_size);
         n->icon = gdk_pixbuf_to_cairo_surface(icon);
-        if (icon)
+        if (icon) {
+                n->icon_time = time_now();
                 g_object_unref(icon);
+        }
 }
 
 void notification_replace_format(struct notification *n, const char *format)

src/notification.h

@@ -65,6 +65,7 @@ struct notification {
         cairo_surface_t *icon;         /**< The raw cached icon data used to draw */
         char *icon_id;           /**< Plain icon information, which acts as the icon's id.
                                       May be a hash for a raw icon or a name/path for a regular app icon. */
+        gint64 icon_time;        /**< Time of reception of the icon (or opening of the file in case of a path) */
         char *iconname;          /**< plain icon information (may be a path or just a name) as recieved from dbus.
                                    Use this to compare the icon name with rules. May also be modified by rules.*/
         char *icon_path;         /**< Full path to the notification's icon. */

src/queues.c

@@ -193,8 +193,8 @@ int queues_notification_insert(struct notification *n)
         if (!inserted && STR_FULL(n->stack_tag) && queues_stack_by_tag(n))
                 inserted = true;
 
-        if (!inserted && settings.stack_duplicates && queues_stack_duplicate(n)){
-                if(settings.sort == SORT_TYPE_UPDATE){
+        if (!inserted && settings.stack_duplicates && queues_stack_duplicate(n)) {
+                if (settings.sort == SORT_TYPE_UPDATE) {
                         g_queue_sort(displayed, notification_cmp_data, NULL);
                 }
                 inserted = true;
@@ -203,6 +203,7 @@ int queues_notification_insert(struct notification *n)
         if (!inserted)
                 g_queue_insert_sorted(waiting, n, notification_cmp_data, NULL);
 
+        // The icon is loaded lazily. this is skipped if the icon was transferred
         if (!n->icon) {
                 notification_icon_replace_path(n, n->iconname);
         }
@@ -221,14 +222,35 @@ int queues_notification_insert(struct notification *n)
  */
 static bool queues_stack_duplicate(struct notification *new)
 {
+        gint64 modtime = -1;
+
         GQueue *allqueues[] = { displayed, waiting };
         for (size_t i = 0; i < sizeof(allqueues)/sizeof(GQueue*); i++) {
                 for (GList *iter = g_queue_peek_head_link(allqueues[i]); iter;
                      iter = iter->next) {
                         struct notification *old = iter->data;
                         if (notification_is_duplicate(old, new)) {
+
+                                // Additional check to see if the icon was modified
+                                // But only if the icon is from a file
+                                //
+                                if (old->icon && !new->icon_id && is_like_path(old->iconname)) {
+                                        if (modtime < 0)
+                                                modtime = modification_time(old->iconname);
+
+                                        // File was touched, check if the hash is the same
+                                        if (modtime > old->icon_time) {
+                                                notification_icon_replace_path(new, new->iconname);
+
+                                                if (!STR_EQ(new->icon_id, old->icon_id))
+                                                        continue;
+                                        } else {
+                                                notification_transfer_icon(old, new);
+                                        }
+                                }
+
                                 /* If the progress differs, probably notify-send was used to update the notification
-                                 * So only count it as a duplicate, if the progress was not the same.
+                                 * So only count it as a duplicate, if the progress was the same.
                                  * */
                                 if (old->progress == new->progress) {
                                         old->dup_count++;
@@ -243,8 +265,6 @@ static bool queues_stack_duplicate(struct notification *new)
                                 if (allqueues[i] == displayed)
                                         new->start = time_monotonic_now();
 
-                                notification_transfer_icon(old, new);
-
                                 notification_unref(old);
                                 return true;
                         }
@@ -272,15 +292,20 @@ static bool queues_stack_by_tag(struct notification *new)
                                 iter->data = new;
                                 new->dup_count = old->dup_count;
 
+                                if (old->icon && !new->icon_id && is_like_path(old->iconname)) {
+                                        gint64 modtime = modification_time(old->iconname);
+                                        if (modtime <= old->icon_time) {
+                                                notification_transfer_icon(old, new);
+                                        }
+                                }
+
                                 signal_notification_closed(old, 1);
 
                                 if (allqueues[i] == displayed) {
                                         new->start = time_monotonic_now();
                                         notification_run_script(new);
                                 }
 
-                                notification_transfer_icon(old, new);
-
                                 notification_unref(old);
                                 return true;
                         }

src/utils.c

@@ -336,6 +336,23 @@ gint64 time_monotonic_now(void)
         return S2US(tv_now.tv_sec) + tv_now.tv_nsec / 1000;
 }
 
+gint64 time_now(void)
+{
+        struct timespec tv_now;
+
+        clock_gettime(CLOCK_REALTIME, &tv_now);
+        return S2US(tv_now.tv_sec) + tv_now.tv_nsec / 1000;
+}
+
+gint64 modification_time(const char *path)
+{
+        struct stat statbuf;
+        if (stat(path, &statbuf) != 0)
+                return -1;
+
+        return S2US(statbuf.st_mtim.tv_sec) + statbuf.st_mtim.tv_nsec / 1000;
+}
+
 /* see utils.h */
 const char *user_get_home(void)
 {
@@ -489,4 +506,11 @@ bool string_is_int(const char *str) {
         return true;
 }
 
+bool is_like_path(const char *string)
+{
+        return string[0] == '/' || string[0] == '~'
+                || (string[0] == '.' && string[1] == '.' && string[2] == '/')
+                || (string[0] == '.' && string[1] == '/');
+}
+
 /* vim: set ft=c tabstop=8 shiftwidth=8 expandtab textwidth=0: */

src/utils.h

@@ -158,6 +158,10 @@ gint64 string_to_time(const char *string);
  */
 gint64 time_monotonic_now(void);
 
+gint64 time_now(void);
+
+gint64 modification_time(const char *path);
+
 /**
  * Retrieve the HOME directory of the user running dunst
  *
@@ -259,5 +263,10 @@ void add_paths_from_env(GPtrArray *arr, char *env_name, char *subdir, char *alte
 
 bool string_is_int(const char *str);
 
+// Actually checks only the first characters
+// Is true when the string starts with one of:
+// ./ ../ ~ /
+bool is_like_path(const char *string);
+
 #endif
 /* vim: set ft=c tabstop=8 shiftwidth=8 expandtab textwidth=0: */

src/x11/x.c

@@ -766,7 +766,7 @@ void x_win_show(window winptr)
  */
 void x_win_hide(window winptr)
 {
-        LOG_I("X11: Hiding window");
+        LOG_I("Hiding X11 window");
         struct window_x11 *win = (struct window_x11*)winptr;
         ASSERT_OR_RET(win->visible,);