Merge lp:~larsu/notify-osd/spam-a-bit-less into lp:notify-osd

Proposed by Lars Karlitski on 2015-10-14
Status: Merged
Approved by: Sebastien Bacher on 2015-11-12
Approved revision: 494
Merged at revision: 497
Proposed branch: lp:~larsu/notify-osd/spam-a-bit-less
Merge into: lp:notify-osd
Diff against target: 314 lines (+71/-156)
3 files modified
src/bubble.c (+69/-150)
src/bubble.h (+1/-5)
src/stack.c (+1/-1)
To merge this branch: bzr merge lp:~larsu/notify-osd/spam-a-bit-less
Reviewer Review Type Date Requested Status
Sebastien Bacher 2015-10-14 Approve on 2015-11-12
Review via email: mp+274382@code.launchpad.net

Commit message

Refactor bubble_set_icon()

This function did the same as bubble_set_icon_for_path(), because paths are allowed in addition to icon names for both the app-icon parameter and the image-path hint.

Remove one of those functions and inline load_icon(), which gives us more control over when icon-not-found warnings are shown. Don't show one when using the fallback of prefixing the icon name with "notification-".

Remove TEMPORARY_ICON_PREFIX_WORKAROUND. It's been there since 2009 and
was never set to anything but 1.

Description of the change

Refactor bubble_set_icon() a bit to remove warnings about not found icons when using the "notification-" fallback.

To post a comment you must log in.
Sebastien Bacher (seb128) wrote :

looks fine and start of a new cycle is a good time to land such changes ;-)

review: Approve
Sebastien Bacher (seb128) wrote :

with that branch the sound icons displayed when doing mouseover rolling over indicator-sound are wrong ...

review: Needs Fixing
lp:~larsu/notify-osd/spam-a-bit-less updated on 2015-11-11
494. By Lars Karlitski on 2015-11-11

bubble_set_icon: try fallback icon name first

Fallback icon name is the one prefixed with "notification-". The
previous commit accidentally changed the order of lookups.

Remove TEMPORARY_ICON_PREFIX_WORKAROUND. It's been there since 2009 and
was never set to anything but 1.

Lars Karlitski (larsu) wrote :

Indeed. Fixed in r494.

Sebastien Bacher (seb128) wrote :

thanks

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/bubble.c'
2--- src/bubble.c 2015-10-12 13:56:53 +0000
3+++ src/bubble.c 2015-11-11 17:01:30 +0000
4@@ -94,7 +94,7 @@
5
6 // used to prevent unneeded updates of the tile-cache, for append-,
7 // update or replace-cases, needs to move into class Notification
8- GString* old_icon_filename;
9+ gchar * old_icon_filename;
10 };
11
12 G_DEFINE_TYPE_WITH_PRIVATE (Bubble, bubble, G_TYPE_OBJECT);
13@@ -124,12 +124,6 @@
14 gdouble value;
15 };
16
17-#define TEMPORARY_ICON_PREFIX_WORKAROUND 1
18-#ifdef TEMPORARY_ICON_PREFIX_WORKAROUND
19-#warning "--== Using the icon-name-substitution! This is a temp. workaround not going to be maintained for long! ==--"
20-#define NOTIFY_OSD_ICON_PREFIX "notification"
21-#endif
22-
23 // FIXME: this is in class Defaults already, but not yet hooked up so for the
24 // moment we use the macros here, these values reflect the visual-guideline
25 // for jaunty notifications
26@@ -1832,71 +1826,6 @@
27 }
28
29 static
30-GdkPixbuf*
31-load_icon (Bubble* self,
32- const gchar* filename,
33- gint icon_size)
34-{
35- GdkPixbuf* buffer = NULL;
36- GdkPixbuf* pixbuf = NULL;
37- GtkIconTheme* theme = NULL;
38- GError* error = NULL;
39- gint scale;
40-
41- /* sanity check */
42- g_return_val_if_fail (filename, NULL);
43-
44- /* Images referenced must always be local files. */
45- if (!strncmp (filename, "file://", 7))
46- filename += 7;
47-
48- scale = gtk_widget_get_scale_factor (self->priv->widget);
49-
50- if (filename[0] == '/')
51- {
52- /* load image into pixbuf */
53- pixbuf = gdk_pixbuf_new_from_file_at_scale (filename,
54- scale * icon_size,
55- scale * icon_size,
56- TRUE,
57- NULL);
58- } else {
59- /* TODO: rewrite, check for SVG support, raise apport
60- ** notification for low-res icons */
61- theme = gtk_icon_theme_get_default ();
62- buffer = gtk_icon_theme_load_icon_for_scale (theme,
63- filename,
64- icon_size,
65- scale,
66- GTK_ICON_LOOKUP_FORCE_SVG |
67- GTK_ICON_LOOKUP_GENERIC_FALLBACK |
68- GTK_ICON_LOOKUP_FORCE_SIZE,
69- &error);
70- if (error)
71- {
72- g_print ("loading icon '%s' caused error: '%s'",
73- filename,
74- error->message);
75- g_error_free (error);
76- error = NULL;
77- pixbuf = NULL;
78- }
79- else
80- {
81- /* copy and unref buffer so on an icon-theme change old
82- ** icons are not kept in memory due to dangling
83- ** references, this also makes sure we do not need to
84- ** connect to GtkWidget::style-set signal for the
85- ** GdkPixbuf we get from gtk_icon_theme_load_icon() */
86- pixbuf = gdk_pixbuf_copy (buffer);
87- g_object_unref (buffer);
88- }
89- }
90-
91- return pixbuf;
92-}
93-
94-static
95 gboolean
96 pointer_update (Bubble* bubble)
97 {
98@@ -2109,11 +2038,7 @@
99 priv->tile_indicator = NULL;
100 }
101
102- if (priv->old_icon_filename)
103- {
104- g_string_free ((gpointer) priv->old_icon_filename, TRUE);
105- priv->old_icon_filename = NULL;
106- }
107+ g_clear_pointer (&priv->old_icon_filename, g_free);
108
109 // chain up to the parent class
110 G_OBJECT_CLASS (bubble_parent_class)->dispose (gobject);
111@@ -2321,7 +2246,6 @@
112 this->priv->tile_body = NULL;
113 this->priv->tile_indicator = NULL;
114 this->priv->prevent_fade = FALSE;
115- this->priv->old_icon_filename = g_string_new ("");
116
117 update_input_shape (window);
118
119@@ -2432,89 +2356,84 @@
120 }
121
122 void
123-bubble_set_icon_from_path (Bubble* self,
124- const gchar* filepath)
125-{
126- Defaults* d;
127- BubblePrivate* priv;
128-
129- if (!self || !IS_BUBBLE (self) || !g_strcmp0 (filepath, ""))
130- return;
131-
132- priv = self->priv;
133-
134- // check if an app tries to set the same file as icon again, this check
135- // avoids superfluous regeneration of the tile/blur-cache for the icon,
136- // thus it improves performance in update- and append-cases
137- if (!g_strcmp0 (priv->old_icon_filename->str, filepath))
138- return;
139-
140- // store the new icon-basename
141- g_string_assign (priv->old_icon_filename, filepath);
142-
143- if (priv->icon_pixbuf)
144- {
145- g_object_unref (priv->icon_pixbuf);
146- priv->icon_pixbuf = NULL;
147- }
148-
149- d = self->defaults;
150- priv->icon_pixbuf = load_icon (self,
151- filepath,
152- EM2PIXELS (defaults_get_icon_size (d), d));
153-
154- _refresh_icon (self);
155-}
156-
157-void
158 bubble_set_icon (Bubble* self,
159- const gchar* filename)
160+ const gchar* name)
161 {
162- Defaults* d;
163- BubblePrivate* priv;
164-#ifdef TEMPORARY_ICON_PREFIX_WORKAROUND
165- gchar* notify_osd_iconname;
166-#endif
167+ BubblePrivate *priv;
168+ gint scale;
169+ gint icon_size;
170
171- if (!self || !IS_BUBBLE (self) || !g_strcmp0 (filename, ""))
172- return;
173+ g_return_if_fail (self != NULL);
174+ g_return_if_fail (name != NULL);
175
176 priv = self->priv;
177-
178- //basename = g_path_get_basename (filename);
179+ scale = gtk_widget_get_scale_factor (priv->widget);
180+ icon_size = EM2PIXELS (defaults_get_icon_size (self->defaults), self->defaults);
181
182 // check if an app tries to set the same file as icon again, this check
183 // avoids superfluous regeneration of the tile/blur-cache for the icon,
184 // thus it improves performance in update- and append-cases
185- if (!g_strcmp0 (priv->old_icon_filename->str, filename))
186+ if (!g_strcmp0 (priv->old_icon_filename, name))
187 return;
188
189+ g_clear_object (&priv->icon_pixbuf);
190+ g_clear_pointer (&priv->old_icon_filename, g_free);
191+
192+ if (g_str_has_prefix (name, "file://"))
193+ {
194+ gchar *filename;
195+ GError *error = NULL;
196+
197+ filename = g_filename_from_uri (name, NULL, &error);
198+ if (filename == NULL)
199+ {
200+ g_printerr ("%s is not a valid file uri: %s", name, error->message);
201+ g_error_free (error);
202+ return;
203+ }
204+
205+ priv->icon_pixbuf = gdk_pixbuf_new_from_file_at_scale (filename, scale * icon_size, scale * icon_size, TRUE, NULL);
206+
207+ g_free (filename);
208+ }
209+ else
210+ {
211+ GError *error = NULL;
212+ GdkPixbuf *buffer;
213+ GtkIconTheme *theme;
214+ GtkIconLookupFlags flags;
215+ gchar *fallback_name;
216+
217+ theme = gtk_icon_theme_get_default ();
218+ flags = GTK_ICON_LOOKUP_FORCE_SVG | GTK_ICON_LOOKUP_GENERIC_FALLBACK | GTK_ICON_LOOKUP_FORCE_SIZE;
219+
220+ fallback_name = g_strconcat ("notification-", name, NULL);
221+ buffer = gtk_icon_theme_load_icon_for_scale (theme, fallback_name, icon_size, scale, flags, &error);
222+ g_free (fallback_name);
223+
224+ if (buffer == NULL && g_error_matches (error, GTK_ICON_THEME_ERROR, GTK_ICON_THEME_NOT_FOUND)) {
225+ g_clear_error (&error);
226+ buffer = gtk_icon_theme_load_icon_for_scale (theme, name, icon_size, scale, flags, &error);
227+ }
228+
229+ if (buffer == NULL)
230+ {
231+ g_print ("Unable to load icon '%s': %s", name, error->message);
232+ g_error_free (error);
233+ return;
234+ }
235+
236+ /* copy and unref buffer so on an icon-theme change old
237+ ** icons are not kept in memory due to dangling
238+ ** references, this also makes sure we do not need to
239+ ** connect to GtkWidget::style-set signal for the
240+ ** GdkPixbuf we get from gtk_icon_theme_load_icon() */
241+ priv->icon_pixbuf = gdk_pixbuf_copy (buffer);
242+ g_object_unref (buffer);
243+ }
244+
245 // store the new icon-basename
246- g_string_assign (priv->old_icon_filename, filename);
247-
248- if (priv->icon_pixbuf)
249- {
250- g_object_unref (priv->icon_pixbuf);
251- priv->icon_pixbuf = NULL;
252- }
253-
254- d = self->defaults;
255-
256-#ifdef TEMPORARY_ICON_PREFIX_WORKAROUND
257- notify_osd_iconname = g_strdup_printf (NOTIFY_OSD_ICON_PREFIX "-%s",
258- filename);
259- priv->icon_pixbuf = load_icon (self,
260- notify_osd_iconname,
261- EM2PIXELS (defaults_get_icon_size (d),
262- d));
263- g_free (notify_osd_iconname);
264-#endif
265-
266- // fallback to non-notify-osd name
267- if (!priv->icon_pixbuf)
268- priv->icon_pixbuf = load_icon (self,
269- filename,
270- EM2PIXELS (defaults_get_icon_size (d), d));
271+ priv->old_icon_filename = g_strdup (name);
272
273 _refresh_icon (self);
274 }
275@@ -2591,7 +2510,7 @@
276 priv = self->priv;
277
278 // "reset" the stored the icon-filename, fixes LP: #451086
279- g_string_assign (priv->old_icon_filename, "\0");
280+ g_clear_pointer (&priv->old_icon_filename, g_free);
281
282 if (priv->icon_pixbuf)
283 {
284
285=== modified file 'src/bubble.h'
286--- src/bubble.h 2010-10-21 16:52:42 +0000
287+++ src/bubble.h 2015-11-11 17:01:30 +0000
288@@ -109,12 +109,8 @@
289 bubble_get_message_body (Bubble* self);
290
291 void
292-bubble_set_icon_from_path (Bubble* self,
293- const gchar* filepath);
294-
295-void
296 bubble_set_icon (Bubble* self,
297- const gchar* filename);
298+ const gchar* name);
299
300 void
301 bubble_set_icon_from_pixbuf (Bubble* self,
302
303=== modified file 'src/stack.c'
304--- src/stack.c 2013-11-07 15:37:20 +0000
305+++ src/stack.c 2015-11-11 17:01:30 +0000
306@@ -730,7 +730,7 @@
307 {
308 g_debug("Using image_path hint\n");
309 if ((data && G_VALUE_HOLDS_STRING (data)))
310- bubble_set_icon_from_path (bubble, g_value_get_string(data));
311+ bubble_set_icon (bubble, g_value_get_string(data));
312 else
313 g_warning ("image_path hint is not a string\n");
314 }

Subscribers

People subscribed via source and target branches