diff --git a/.valgrind.suppressions b/.valgrind.suppressions index 7201368..db7a88a 100644 --- a/.valgrind.suppressions +++ b/.valgrind.suppressions @@ -20,10 +20,30 @@ fun:g_error_new_valist fun:g_set_error obj:*/librsvg-2.so* + fun:rsvg_handle_close + obj:*/loaders/libpixbufloader-svg.so + fun:gdk_pixbuf_loader_close + fun:gdk_pixbuf_get_file_info + fun:get_pixbuf_from_file + ... +} + +# same as above, but as occurs in CI environment +{ + rsvg_error_handle_close2 + Memcheck:Leak + match-leak-kinds: definite + fun:malloc + fun:g_malloc + fun:g_slice_alloc + fun:g_error_new_valist + fun:g_set_error + obj:*/librsvg-2.so* obj:*/librsvg-2.so* obj:*/loaders/libpixbufloader-svg.so obj:*/libgdk_pixbuf-2.0.so* - fun:gdk_pixbuf_new_from_file + fun:gdk_pixbuf_loader_close + fun:gdk_pixbuf_get_file_info fun:get_pixbuf_from_file ... } @@ -45,7 +65,8 @@ fun:rsvg_handle_write obj:*/loaders/libpixbufloader-svg.so obj:*/libgdk_pixbuf-2.0.so* - fun:gdk_pixbuf_new_from_file + fun:gdk_pixbuf_loader_close + fun:gdk_pixbuf_get_file_info fun:get_pixbuf_from_file ... } diff --git a/config.h b/config.h index 7026d38..81f82ec 100644 --- a/config.h +++ b/config.h @@ -65,6 +65,7 @@ struct settings defaults = { .browser = "/usr/bin/firefox", +.min_icon_size = 0, .max_icon_size = 0, /* paths to default icons */ diff --git a/docs/dunst.pod b/docs/dunst.pod index f9d0ee5..4c25b2c 100644 --- a/docs/dunst.pod +++ b/docs/dunst.pod @@ -384,14 +384,28 @@ ACTIONS below for further details. Defines the position of the icon in the notification window. Setting it to off disables icons. +=item B (default: 0) + +Defines the minimum size in pixels for the icons. +If the icon is larger than or equal to the specified value it won't be affected. +If it's smaller then it will be scaled up so that the smaller axis is equivalent +to the specified size. + +Set to 0 to disable icon upscaling. (default) + +If B is set to off, this setting is ignored. + =item B (default: 0) Defines the maximum size in pixels for the icons. -If the icon is smaller than the specified value it won't be affected. +If the icon is smaller than or equal to the specified value it won't be affected. If it's larger then it will be scaled down so that the larger axis is equivalent to the specified size. -Set to 0 to disable icon scaling. (default) +Set to 0 to disable icon downscaling. (default) + +If both B and B are enabled, the latter +gets the last say. If B is set to off, this setting is ignored. diff --git a/dunstrc b/dunstrc index d8665fe..c9b149e 100644 --- a/dunstrc +++ b/dunstrc @@ -162,6 +162,11 @@ # Align icons left/right/off icon_position = off + # Scale small icons up to this size, set to 0 to disable. Helpful + # for e.g. small files or high-dpi screens. In case of conflict, + # max_icon_size takes precedence over this. + min_icon_size = 0 + # Scale larger icons down to this size, set to 0 to disable max_icon_size = 32 diff --git a/src/icon.c b/src/icon.c index 71a4731..7529944 100644 --- a/src/icon.c +++ b/src/icon.c @@ -110,25 +110,56 @@ cairo_surface_t *gdk_pixbuf_to_cairo_surface(GdkPixbuf *pixbuf) return icon_surface; } -GdkPixbuf *icon_pixbuf_scale(GdkPixbuf *pixbuf) +/** + * Scales the given image dimensions if necessary according to the settings. + * + * @param w a pointer to the image width, to be modified in-place + * @param h a pointer to the image height, to be modified in-place + * @return TRUE if the dimensions were updated, FALSE if they were left unchanged + */ +static bool icon_size_clamp(int *w, int *h) { + int _w = *w, _h = *h; + int landscape = _w > _h; + int orig_larger = landscape ? _w : _h; + double larger = orig_larger; + double smaller = landscape ? _h : _w; + if (settings.min_icon_size && smaller < settings.min_icon_size) { + larger = larger / smaller * settings.min_icon_size; + smaller = settings.min_icon_size; + } + if (settings.max_icon_size && larger > settings.max_icon_size) { + smaller = smaller / larger * settings.max_icon_size; + larger = settings.max_icon_size; + } + if ((int) larger != orig_larger) { + *w = (int) (landscape ? larger : smaller); + *h = (int) (landscape ? smaller : larger); + return TRUE; + } + return FALSE; +} + +/** + * 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. + */ +static 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; + if (icon_size_clamp(&w, &h)) { GdkPixbuf *scaled = gdk_pixbuf_scale_simple( pixbuf, - scaled_w, - scaled_h, + w, + h, GDK_INTERP_BILINEAR); g_object_unref(pixbuf); pixbuf = scaled; @@ -141,8 +172,19 @@ GdkPixbuf *get_pixbuf_from_file(const char *filename) { char *path = string_to_path(g_strdup(filename)); GError *error = NULL; + gint w, h; - GdkPixbuf *pixbuf = gdk_pixbuf_new_from_file(path, &error); + if (!gdk_pixbuf_get_file_info (path, &w, &h)) { + LOG_W("Failed to load image info for %s", filename); + g_free(path); + return NULL; + } + icon_size_clamp(&w, &h); + GdkPixbuf *pixbuf = gdk_pixbuf_new_from_file_at_scale(path, + w, + h, + TRUE, + &error); if (error) { LOG_W("%s", error->message); @@ -320,6 +362,8 @@ GdkPixbuf *icon_get_for_data(GVariant *data, char **id) g_free(data_chk); g_variant_unref(data_variant); + pixbuf = icon_pixbuf_scale(pixbuf); + return pixbuf; } diff --git a/src/icon.h b/src/icon.h index 37cbea0..2de0740 100644 --- a/src/icon.h +++ b/src/icon.h @@ -8,18 +8,7 @@ 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. +/** Retrieve an icon by its full filepath, scaled according to settings. * * @param filename A string representing a readable file path * @@ -28,7 +17,7 @@ GdkPixbuf *icon_pixbuf_scale(GdkPixbuf *pixbuf); */ GdkPixbuf *get_pixbuf_from_file(const char *filename); -/** Retrieve an icon by its name sent via the notification bus +/** Retrieve an icon by its name sent via the notification bus, scaled according to settings * * @param iconname A string describing a `file://` URL, an arbitary filename * or an icon name, which then gets searched for in the @@ -39,7 +28,7 @@ GdkPixbuf *get_pixbuf_from_file(const char *filename); */ GdkPixbuf *get_pixbuf_from_icon(const char *iconname); -/** Read an icon from disk and convert it to a GdkPixbuf. +/** Read an icon from disk and convert it to a GdkPixbuf, scaled according to settings * * 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. @@ -54,7 +43,7 @@ GdkPixbuf *get_pixbuf_from_icon(const char *iconname); */ GdkPixbuf *icon_get_for_name(const char *name, char **id); -/** Convert a GVariant like described in GdkPixbuf +/** Convert a GVariant like described in GdkPixbuf, scaled according to settings * * 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. diff --git a/src/notification.c b/src/notification.c index f1cc14d..6518b49 100644 --- a/src/notification.c +++ b/src/notification.c @@ -252,7 +252,6 @@ void notification_icon_replace_path(struct notification *n, const char *new_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) @@ -264,7 +263,6 @@ void notification_icon_replace_data(struct notification *n, GVariant *new_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 */ diff --git a/src/settings.c b/src/settings.c index fde3715..90077fb 100644 --- a/src/settings.c +++ b/src/settings.c @@ -423,6 +423,12 @@ void load_settings(char *cmdline_config_path) g_free(c); } + settings.min_icon_size = option_get_int( + "global", + "min_icon_size", "-min_icon_size", defaults.min_icon_size, + "Scale smaller icons up to this size, set to 0 to disable. If max_icon_size also specified, that has the final say." + ); + settings.max_icon_size = option_get_int( "global", "max_icon_size", "-max_icon_size", defaults.max_icon_size, diff --git a/src/settings.h b/src/settings.h index 6bcf6b8..6e541c2 100644 --- a/src/settings.h +++ b/src/settings.h @@ -75,6 +75,7 @@ struct settings { char *browser; char **browser_cmd; enum icon_position icon_position; + int min_icon_size; int max_icon_size; char *icon_path; enum follow_mode f_mode; diff --git a/test/dbus.c b/test/dbus.c index f212468..659731a 100644 --- a/test/dbus.c +++ b/test/dbus.c @@ -6,6 +6,7 @@ #include #include +#include "helpers.h" #include "queues.h" extern const char *base; @@ -252,33 +253,13 @@ bool dbus_notification_fire(struct dbus_notification *n, uint *id) void dbus_notification_set_raw_image(struct dbus_notification *n_dbus, const char *path) { - GdkPixbuf *pb = gdk_pixbuf_new_from_file(path, NULL); - - if (!pb) + GVariant *hint = notification_setup_raw_image(path); + if (!hint) return; - GVariant *hint_data = g_variant_new_from_data( - G_VARIANT_TYPE("ay"), - gdk_pixbuf_read_pixels(pb), - gdk_pixbuf_get_byte_length(pb), - TRUE, - (GDestroyNotify) g_object_unref, - g_object_ref(pb)); - - GVariant *hint = g_variant_new( - "(iiibii@ay)", - gdk_pixbuf_get_width(pb), - gdk_pixbuf_get_height(pb), - gdk_pixbuf_get_rowstride(pb), - gdk_pixbuf_get_has_alpha(pb), - gdk_pixbuf_get_bits_per_sample(pb), - gdk_pixbuf_get_n_channels(pb), - hint_data); - g_hash_table_insert(n_dbus->hints, g_strdup("image-data"), g_variant_ref_sink(hint)); - g_object_unref(pb); } /////// TESTS diff --git a/test/helpers.c b/test/helpers.c new file mode 100644 index 0000000..da3cde2 --- /dev/null +++ b/test/helpers.c @@ -0,0 +1,35 @@ +#include + +#include "helpers.h" + +GVariant *notification_setup_raw_image(const char *path) +{ + GdkPixbuf *pb = gdk_pixbuf_new_from_file(path, NULL); + + if (!pb) + return NULL; + + GVariant *hint_data = g_variant_new_from_data( + G_VARIANT_TYPE("ay"), + gdk_pixbuf_read_pixels(pb), + gdk_pixbuf_get_byte_length(pb), + TRUE, + (GDestroyNotify) g_object_unref, + g_object_ref(pb)); + + GVariant *hint = g_variant_new( + "(iiibii@ay)", + gdk_pixbuf_get_width(pb), + gdk_pixbuf_get_height(pb), + gdk_pixbuf_get_rowstride(pb), + gdk_pixbuf_get_has_alpha(pb), + gdk_pixbuf_get_bits_per_sample(pb), + gdk_pixbuf_get_n_channels(pb), + hint_data); + + g_object_unref(pb); + + return hint; +} + +/* vim: set tabstop=8 shiftwidth=8 expandtab textwidth=0: */ diff --git a/test/helpers.h b/test/helpers.h new file mode 100644 index 0000000..0b9867a --- /dev/null +++ b/test/helpers.h @@ -0,0 +1,9 @@ +#ifndef DUNST_TEST_HELPERS_H +#define DUNST_TEST_HELPERS_H + +#include + +GVariant *notification_setup_raw_image(const char *path); + +#endif +/* vim: set tabstop=8 shiftwidth=8 expandtab textwidth=0: */ diff --git a/test/icon.c b/test/icon.c index 5bd96aa..94a29ce 100644 --- a/test/icon.c +++ b/test/icon.c @@ -113,6 +113,61 @@ TEST test_get_pixbuf_from_icon_fileuri(void) PASS(); } +TEST test_icon_size_clamp_too_small(void) +{ + int w = 12, h = 24; + bool resized = icon_size_clamp(&w, &h); + ASSERT(resized); + ASSERT_EQ(w, 16); + ASSERT_EQ(h, 32); + + PASS(); +} + +TEST test_icon_size_clamp_not_necessary(void) +{ + int w = 20, h = 30; + bool resized = icon_size_clamp(&w, &h); + ASSERT(!resized); + ASSERT_EQ(w, 20); + ASSERT_EQ(h, 30); + + PASS(); +} + +TEST test_icon_size_clamp_too_big(void) +{ + int w = 75, h = 150; + bool resized = icon_size_clamp(&w, &h); + ASSERT(resized); + ASSERT_EQ(w, 50); + ASSERT_EQ(h, 100); + + PASS(); +} + +TEST test_icon_size_clamp_too_small_then_too_big(void) +{ + int w = 8, h = 80; + bool resized = icon_size_clamp(&w, &h); + ASSERT(resized); + ASSERT_EQ(w, 10); + ASSERT_EQ(h, 100); + + PASS(); +} + +TEST test_get_pixbuf_from_icon_both_is_scaled(void) +{ + GdkPixbuf *pixbuf = get_pixbuf_from_icon("onlypng"); + ASSERT(pixbuf); + ASSERT_EQ(gdk_pixbuf_get_width(pixbuf), 16); + ASSERT_EQ(gdk_pixbuf_get_height(pixbuf), 16); + g_clear_pointer(&pixbuf, g_object_unref); + + PASS(); +} + SUITE(suite_icon) { settings.icon_path = g_strconcat( @@ -129,6 +184,31 @@ SUITE(suite_icon) RUN_TEST(test_get_pixbuf_from_icon_onlypng); RUN_TEST(test_get_pixbuf_from_icon_filename); RUN_TEST(test_get_pixbuf_from_icon_fileuri); + RUN_TEST(test_icon_size_clamp_not_necessary); + + settings.min_icon_size = 16; + settings.max_icon_size = 100; + + RUN_TEST(test_get_pixbuf_from_icon_both_is_scaled); + RUN_TEST(test_icon_size_clamp_too_small); + RUN_TEST(test_icon_size_clamp_not_necessary); + RUN_TEST(test_icon_size_clamp_too_big); + RUN_TEST(test_icon_size_clamp_too_small_then_too_big); + + settings.min_icon_size = 16; + settings.max_icon_size = 0; + + RUN_TEST(test_icon_size_clamp_too_small); + RUN_TEST(test_icon_size_clamp_not_necessary); + + settings.min_icon_size = 0; + settings.max_icon_size = 100; + + RUN_TEST(test_icon_size_clamp_not_necessary); + RUN_TEST(test_icon_size_clamp_too_big); + + settings.min_icon_size = 0; + settings.max_icon_size = 0; g_clear_pointer(&settings.icon_path, g_free); } diff --git a/test/notification.c b/test/notification.c index 69040f6..ad73f34 100644 --- a/test/notification.c +++ b/test/notification.c @@ -1,5 +1,6 @@ #include "../src/notification.c" #include "greatest.h" +#include "helpers.h" #include "../src/option_parser.h" #include "../src/settings.h" @@ -124,6 +125,76 @@ TEST test_notification_referencing(void) PASS(); } + +static struct notification *notification_load_icon_with_scaling(int min_icon_size, int max_icon_size) +{ + struct notification *n = notification_create(); + + char *path = g_strconcat(base, "/data/icons/valid.svg", NULL); // 16x16 + + GVariant *rawIcon = notification_setup_raw_image(path); + + settings.min_icon_size = min_icon_size; + settings.max_icon_size = max_icon_size; + notification_icon_replace_data(n, rawIcon); + settings.min_icon_size = 0; + settings.max_icon_size = 0; + + g_variant_unref(rawIcon); + g_free(path); + + return n; +} + +TEST test_notification_icon_scaling_toosmall(void) +{ + struct notification *n = notification_load_icon_with_scaling(20, 100); + + ASSERT_EQ(gdk_pixbuf_get_width(n->icon), 20); + ASSERT_EQ(gdk_pixbuf_get_height(n->icon), 20); + + notification_unref(n); + + PASS(); +} + + +TEST test_notification_icon_scaling_toolarge(void) +{ + struct notification *n = notification_load_icon_with_scaling(5, 10); + + ASSERT_EQ(gdk_pixbuf_get_width(n->icon), 10); + ASSERT_EQ(gdk_pixbuf_get_height(n->icon), 10); + + notification_unref(n); + + PASS(); +} + +TEST test_notification_icon_scaling_notconfigured(void) +{ + struct notification *n = notification_load_icon_with_scaling(0, 0); + + ASSERT_EQ(gdk_pixbuf_get_width(n->icon), 16); + ASSERT_EQ(gdk_pixbuf_get_height(n->icon), 16); + + notification_unref(n); + + PASS(); +} + +TEST test_notification_icon_scaling_notneeded(void) +{ + struct notification *n = notification_load_icon_with_scaling(10, 20); + + ASSERT_EQ(gdk_pixbuf_get_width(n->icon), 16); + ASSERT_EQ(gdk_pixbuf_get_height(n->icon), 16); + + notification_unref(n); + + PASS(); +} + TEST test_notification_format_message(struct notification *n, const char *format, const char *exp) { n->format = format; @@ -167,6 +238,10 @@ SUITE(suite_notification) RUN_TEST(test_notification_is_duplicate); RUN_TEST(test_notification_replace_single_field); RUN_TEST(test_notification_referencing); + RUN_TEST(test_notification_icon_scaling_toosmall); + RUN_TEST(test_notification_icon_scaling_toolarge); + RUN_TEST(test_notification_icon_scaling_notconfigured); + RUN_TEST(test_notification_icon_scaling_notneeded); // TEST notification_format_message struct notification *a = notification_create();