Merge lp:~tintou/slingshot/no-pixbuf into lp:~elementary-pantheon/slingshot/trunk

Proposed by Corentin Noël on 2015-12-22
Status: Merged
Approved by: Cody Garver on 2015-12-23
Approved revision: 617
Merged at revision: 617
Proposed branch: lp:~tintou/slingshot/no-pixbuf
Merge into: lp:~elementary-pantheon/slingshot/trunk
Diff against target: 400 lines (+65/-200)
3 files modified
src/Backend/App.vala (+24/-168)
src/Widgets/AppEntry.vala (+37/-30)
src/Widgets/SearchItem.vala (+4/-2)
To merge this branch: bzr merge lp:~tintou/slingshot/no-pixbuf
Reviewer Review Type Date Requested Status
Daniel Fore ux 2015-12-22 Approve on 2015-12-22
elementary Pantheon team code 2015-12-22 Pending
Review via email: mp+281234@code.launchpad.net

Commit message

Use GIcon instead of Pixbuf loaded at startup

Description of the change

Use GIcon instead of Pixbuf loaded at startup

To post a comment you must log in.
Daniel Fore (danrabbit) wrote :

I can confirm that this branch works as expected and that icons now render properly for HiDPI

review: Approve (ux)
Rico Tzschichholz (ricotz) wrote :

should "gtk_icon_theme_lookup_by_gicon_for_scale" be used?

Corentin Noël (tintou) wrote :

I depends of the behavior we want, do we want to show the fallback icon if there is no icon of the size we want or do we want to show the original icon but scaled to the size we request ?

Rico Tzschichholz (ricotz) wrote :

Nevermind, just saying the *_for_scale are meant to be used for hidpi support, but you are not using its result anyway.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/Backend/App.vala'
2--- src/Backend/App.vala 2014-10-18 01:57:49 +0000
3+++ src/Backend/App.vala 2015-12-22 17:09:38 +0000
4@@ -35,9 +35,8 @@
5 public string description { get; private set; default = ""; }
6 public string desktop_id { get; construct set; }
7 public string exec { get; private set; }
8- public string icon_name { get; private set; default = ""; }
9 public string[] keywords { get; private set;}
10- public Gdk.Pixbuf? icon { get; private set; default = null; }
11+ public Icon icon { get; private set; default = new ThemedIcon ("application-default-icon"); }
12 public double popularity { get; set; }
13 public double relevancy { get; set; }
14 public string desktop_path { get; private set; }
15@@ -49,16 +48,9 @@
16 public Synapse.Match? target { get; private set; default = null; }
17 public Gee.ArrayList<string> actions { get; private set; default = null; }
18 public Gee.HashMap<string, string> actions_map { get; private set; default = null; }
19-
20- public signal void icon_changed ();
21+
22 public signal void launched (App app);
23
24- // seconds to wait before retrying icon check
25- private const int RECHECK_TIMEOUT = 2;
26- private bool check_icon_again = true;
27-
28- private LoadableIcon loadable_icon = null;
29-
30 // for FDO Desktop Actions
31 // see http://standards.freedesktop.org/desktop-entry-spec/desktop-entry-spec-latest.html#extra-actions
32 private const string DESKTOP_ACTION_KEY = "Actions";
33@@ -75,9 +67,9 @@
34 app_type = AppType.APP;
35
36 unowned GLib.DesktopAppInfo info = entry.get_app_info ();
37- name = info.get_display_name ().dup ();
38- description = info.get_description ().dup () ?? name;
39- exec = info.get_commandline ().dup ();
40+ name = info.get_display_name ();
41+ description = info.get_description () ?? name;
42+ exec = info.get_commandline ();
43 desktop_id = entry.get_desktop_file_id ();
44 desktop_path = entry.get_desktop_file_path ();
45 #if HAVE_UNITY
46@@ -85,19 +77,14 @@
47 #endif
48 categories = info.get_categories ();
49 generic_name = info.get_generic_name ();
50-
51- if (info.get_icon () is ThemedIcon) {
52- icon_name = (info.get_icon () as ThemedIcon).get_names ()[0].dup ();
53- } else if (info.get_icon () is LoadableIcon) {
54- loadable_icon = info.get_icon () as LoadableIcon;
55- icon = get_loadable_icon ();
56- } else {
57- icon_name = "application-default-icon";
58+ var desktop_icon = info.get_icon ();
59+ if (desktop_icon != null) {
60+ icon = desktop_icon;
61 }
62- if (icon == null) {
63- update_icon ();
64
65- Slingshot.icon_theme.changed.connect (update_icon);
66+ weak Gtk.IconTheme theme = Gtk.IconTheme.get_default ();
67+ if (theme.lookup_by_gicon (icon, 64, Gtk.IconLookupFlags.GENERIC_FALLBACK|Gtk.IconLookupFlags.USE_BUILTIN) == null) {
68+ icon = new ThemedIcon ("application-default-icon");
69 }
70 }
71
72@@ -108,159 +95,28 @@
73 description = _("Run this command…");
74 exec = command;
75 desktop_id = command;
76- icon_name = "system-run";
77-
78- update_icon ();
79-
80+ icon = new ThemedIcon ("system-run");
81 }
82
83 public App.from_synapse_match (Synapse.Match match, Synapse.Match? target = null) {
84-
85 app_type = AppType.SYNAPSE;
86
87 name = match.title;
88 description = match.description;
89- icon_name = match.icon_name;
90+ if (match.match_type == Synapse.MatchType.CONTACT && match.has_thumbnail) {
91+ var file = File.new_for_path (match.thumbnail_path);
92+ icon = new FileIcon (file);
93+ } else if (match.icon_name != null) {
94+ icon = new ThemedIcon (match.icon_name);
95+ }
96+
97+ weak Gtk.IconTheme theme = Gtk.IconTheme.get_default ();
98+ if (theme.lookup_by_gicon (icon, 64, Gtk.IconLookupFlags.GENERIC_FALLBACK|Gtk.IconLookupFlags.USE_BUILTIN) == null) {
99+ icon = new ThemedIcon ("application-default-icon");
100+ }
101
102 this.match = match;
103 this.target = target;
104-
105- update_icon ();
106-
107- }
108-
109- ~App () {
110- Slingshot.icon_theme.changed.disconnect (update_icon);
111- }
112-
113- public void update_icon () {
114- icon = load_icon (Slingshot.settings.icon_size);
115-
116- icon_changed ();
117- }
118-
119- private delegate void IconLoadFallback ();
120-
121- private class IconLoadFallbackMethod {
122- public unowned IconLoadFallback load_icon;
123-
124- public IconLoadFallbackMethod (IconLoadFallback fallback) {
125- load_icon = fallback;
126- }
127- }
128-
129- public Gdk.Pixbuf? load_icon (int size) {
130- if (app_type == AppType.SYNAPSE) {
131- try {
132- // for contacts we can load the thumbnail because we expect it to be
133- // the avatar. For other types it'd be ridiculously small.
134- if (match.match_type == Synapse.MatchType.CONTACT && match.has_thumbnail) {
135- return new Gdk.Pixbuf.from_file_at_scale (match.thumbnail_path, size, size, true);
136- }
137-
138- var icon = Icon.new_for_string (icon_name);
139- var info = Gtk.IconTheme.get_default ().lookup_by_gicon (icon,
140- size, Gtk.IconLookupFlags.FORCE_SIZE);
141-
142- if (info == null)
143- throw new IconError.NOT_FOUND ("Not found");
144-
145- return info.load_icon ();
146- } catch (Error e) {
147- warning ("Failed to load icon: %s\n", e.message);
148- }
149-
150- try {
151- return Slingshot.icon_theme.load_icon ("application-default-icon",
152- size, Gtk.IconLookupFlags.FORCE_SIZE);
153- } catch (Error e) {
154- critical (e.message);
155- }
156- }
157-
158- Gdk.Pixbuf icon = null;
159- var flags = Gtk.IconLookupFlags.FORCE_SIZE;
160-
161- if (loadable_icon != null)
162- return get_loadable_icon ();
163-
164- IconLoadFallbackMethod[] fallbacks = {
165- new IconLoadFallbackMethod (() => {
166- try {
167- icon = Slingshot.icon_theme.load_icon (icon_name, size, flags);
168- } catch (Error e) {
169- warning ("Could not load icon. Falling back to method 2");
170- }
171- }),
172-
173- new IconLoadFallbackMethod (() => {
174- // Since the best method didn't work retry after some time
175- if (check_icon_again) {
176- // only recheck once
177- check_icon_again = false;
178-
179- Timeout.add_seconds (RECHECK_TIMEOUT, () => {
180- Slingshot.icon_theme.rescan_if_needed ();
181- update_icon ();
182- return false;
183- });
184- }
185- }),
186-
187- new IconLoadFallbackMethod (() => {
188- try {
189- if (icon_name.last_index_of (".") > 0) {
190- var name = icon_name[0:icon_name.last_index_of (".")];
191- icon = Slingshot.icon_theme.load_icon (name, size, flags);
192- }
193- } catch (Error e) {
194- warning ("Could not load icon. Falling back to method 3");
195- }
196- }),
197-
198- new IconLoadFallbackMethod (() => {
199- try {
200- icon = new Gdk.Pixbuf.from_file_at_scale (icon_name, size, size, false);
201- } catch (Error e) {
202- warning ("Could not load icon. Falling back to method 4");
203- }
204- }),
205-
206- new IconLoadFallbackMethod (() => {
207- try {
208- icon = Slingshot.icon_theme.load_icon ("application-default-icon", size, flags);
209- } catch (Error e) {
210- warning ("Could not load icon. Falling back to method 5");
211- }
212- }),
213-
214- new IconLoadFallbackMethod (() => {
215- try {
216- icon = Slingshot.icon_theme.load_icon ("gtk-missing-image", size, flags);
217- } catch (Error e) {
218- error ("Could not find a fallback icon to load");
219- }
220- })
221- };
222- foreach (IconLoadFallbackMethod fallback in fallbacks) {
223- fallback.load_icon ();
224- if (icon != null)
225- break;
226- }
227-
228- return icon;
229- }
230-
231- public Gdk.Pixbuf? get_loadable_icon () {
232- Gdk.Pixbuf? tmp_loadable_icon;
233- try {
234- var icon_stream = loadable_icon.load (0, null, null);
235- tmp_loadable_icon = new Gdk.Pixbuf.from_stream_at_scale (icon_stream, Slingshot.settings.icon_size,
236- Slingshot.settings.icon_size, true, null);
237- } catch (Error e) {
238- tmp_loadable_icon = null;
239- }
240- return tmp_loadable_icon;
241 }
242
243 public bool launch () {
244@@ -375,4 +231,4 @@
245 }
246 }
247 }
248-}
249\ No newline at end of file
250+}
251
252=== modified file 'src/Widgets/AppEntry.vala'
253--- src/Widgets/AppEntry.vala 2015-11-19 13:45:51 +0000
254+++ src/Widgets/AppEntry.vala 2015-12-22 17:09:38 +0000
255@@ -19,20 +19,35 @@
256 public class Slingshot.Widgets.AppEntry : Gtk.Button {
257 private static Gtk.Menu menu;
258
259+ public signal void app_launched ();
260+
261 public Gtk.Label app_label;
262- private Gdk.Pixbuf icon;
263+ public unowned string exec_name {
264+ get {
265+ return application.exec;
266+ }
267+ }
268+
269+ public unowned string app_name {
270+ get {
271+ return application.name;
272+ }
273+ }
274+
275+ public unowned string desktop_id {
276+ get {
277+ return application.desktop_id;
278+ }
279+ }
280+
281+ public unowned string desktop_path {
282+ get {
283+ return application.desktop_path;
284+ }
285+ }
286+
287 private new Gtk.Image image;
288-
289- public string exec_name;
290- public string app_name;
291- public string desktop_id;
292- public int icon_size;
293- public string desktop_path;
294-
295- public signal void app_launched ();
296-
297 private bool dragging = false; //prevent launching
298-
299 private Backend.App application;
300
301 #if HAS_PLANK
302@@ -50,7 +65,11 @@
303 private static Plank.DBus.Client plank_client;
304 #endif
305 private bool docked = false;
306- private string desktop_uri;
307+ private string desktop_uri {
308+ owned get {
309+ return File.new_for_path (desktop_path).get_uri ();
310+ }
311+ }
312 #endif
313
314 public AppEntry (Backend.App app) {
315@@ -58,18 +77,8 @@
316 Gtk.drag_source_set (this, Gdk.ModifierType.BUTTON1_MASK, {dnd},
317 Gdk.DragAction.COPY);
318
319- desktop_id = app.desktop_id;
320- desktop_path = app.desktop_path;
321-#if HAS_PLANK
322- desktop_uri = File.new_for_path (desktop_path).get_uri ();
323-#endif
324-
325 application = app;
326- app_name = app.name;
327 tooltip_text = app.description;
328- exec_name = app.exec;
329- icon_size = Slingshot.settings.icon_size;
330- icon = app.icon;
331
332 get_style_context ().add_class (Gtk.STYLE_CLASS_FLAT);
333
334@@ -81,8 +90,9 @@
335 app_label.set_single_line_mode (false);
336 app_label.set_ellipsize (Pango.EllipsizeMode.END);
337
338- image = new Gtk.Image.from_pixbuf (icon);
339- image.icon_size = icon_size;
340+ image = new Gtk.Image ();
341+ image.gicon = app.icon;
342+ image.pixel_size = 64;
343 image.margin_top = 12;
344
345 var grid = new Gtk.Grid ();
346@@ -94,7 +104,6 @@
347 grid.add (app_label);
348
349 add (grid);
350- set_size_request (Pixels.ITEM_SIZE, Pixels.ITEM_SIZE);
351
352 this.clicked.connect (launch_app);
353
354@@ -112,7 +121,7 @@
355
356 this.drag_begin.connect ( (ctx) => {
357 this.dragging = true;
358- Gtk.drag_set_icon_pixbuf (ctx, icon, 0, 0);
359+ Gtk.drag_set_icon_gicon (ctx, app.icon, 0, 0);
360 });
361
362 this.drag_end.connect ( () => {
363@@ -124,11 +133,9 @@
364 sel.set_uris ({File.new_for_path (desktop_path).get_uri ()});
365 });
366
367- app.icon_changed.connect (() => {
368- icon = app.icon;
369- image.set_from_pixbuf (icon);
370+ app.notify["icon"].connect (() => {
371+ ((Gtk.Image) image).gicon = app.icon;
372 });
373-
374 }
375
376 public override void get_preferred_width (out int minimum_width, out int natural_width) {
377
378=== modified file 'src/Widgets/SearchItem.vala'
379--- src/Widgets/SearchItem.vala 2015-12-19 14:14:07 +0000
380+++ src/Widgets/SearchItem.vala 2015-12-22 17:09:38 +0000
381@@ -59,7 +59,9 @@
382 name_label.use_markup = true;
383 ((Gtk.Misc) name_label).xalign = 0.0f;
384
385- icon = new Gtk.Image.from_pixbuf (app.load_icon (ICON_SIZE));
386+ icon = new Gtk.Image ();
387+ icon.gicon = app.icon;
388+ icon.pixel_size = ICON_SIZE;
389
390 // load a favicon if we're an internet page
391 var uri_match = app.match as Synapse.UriMatch;
392@@ -94,7 +96,7 @@
393 Gdk.DragAction.COPY);
394 this.drag_begin.connect ( (ctx) => {
395 this.dragging = true;
396- Gtk.drag_set_icon_pixbuf (ctx, app.icon, 0, 0);
397+ Gtk.drag_set_icon_gicon (ctx, app.icon, 0, 0);
398 });
399 this.drag_end.connect ( () => {
400 this.dragging = false;

Subscribers

People subscribed via source and target branches