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

[Corentin Noël]

Use GIcon instead of Pixbuf loaded at startup

Use GIcon instead of Pixbuf loaded at startup

[Danielle Foré (danrabbit)]

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

review: Approve (ux)

[Rico Tzschichholz (ricotz)]

should "gtk_icon_theme_lookup_by_gicon_for_scale" be used?

[Corentin Noël (tintou)]

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)]

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

=== modified file 'src/Backend/App.vala'
--- src/Backend/App.vala	2014-10-18 01:57:49 +0000
+++ src/Backend/App.vala	2015-12-22 17:09:38 +0000
@@ -35,9 +35,8 @@
     public string description { get; private set; default = ""; }
     public string desktop_id { get; construct set; }
     public string exec { get; private set; }
-    public string icon_name { get; private set; default = ""; }
     public string[] keywords { get; private set;}
-    public Gdk.Pixbuf? icon { get; private set; default = null; }
+    public Icon icon { get; private set; default = new ThemedIcon ("application-default-icon"); }
     public double popularity { get; set; }
     public double relevancy { get; set; }
     public string desktop_path { get; private set; }
@@ -49,16 +48,9 @@
     public Synapse.Match? target { get; private set; default = null; }
     public Gee.ArrayList<string> actions { get; private set; default = null; }
     public Gee.HashMap<string, string> actions_map { get; private set; default = null; }
-    
-    public signal void icon_changed ();
+
     public signal void launched (App app);
 
-    // seconds to wait before retrying icon check
-    private const int RECHECK_TIMEOUT = 2;
-    private bool check_icon_again = true;
-
-    private LoadableIcon loadable_icon = null;
-
     // for FDO Desktop Actions
     // see http://standards.freedesktop.org/desktop-entry-spec/desktop-entry-spec-latest.html#extra-actions
     private const string DESKTOP_ACTION_KEY = "Actions";
@@ -75,9 +67,9 @@
         app_type = AppType.APP;
 
         unowned GLib.DesktopAppInfo info = entry.get_app_info ();
-        name = info.get_display_name ().dup ();
-        description = info.get_description ().dup () ?? name;
-        exec = info.get_commandline ().dup ();
+        name = info.get_display_name ();
+        description = info.get_description () ?? name;
+        exec = info.get_commandline ();
         desktop_id = entry.get_desktop_file_id ();
         desktop_path = entry.get_desktop_file_path ();
 #if HAVE_UNITY
@@ -85,19 +77,14 @@
 #endif
         categories = info.get_categories ();
         generic_name = info.get_generic_name ();
-
-        if (info.get_icon () is ThemedIcon) {
-            icon_name = (info.get_icon () as ThemedIcon).get_names ()[0].dup ();
-        } else if (info.get_icon () is LoadableIcon) {
-            loadable_icon = info.get_icon () as LoadableIcon;
-            icon = get_loadable_icon ();
-        } else {
-            icon_name = "application-default-icon";
+        var desktop_icon = info.get_icon ();
+        if (desktop_icon != null) {
+            icon = desktop_icon;
         }
-        if (icon == null) {
-            update_icon ();
 
-            Slingshot.icon_theme.changed.connect (update_icon);
+        weak Gtk.IconTheme theme = Gtk.IconTheme.get_default ();
+        if (theme.lookup_by_gicon (icon, 64, Gtk.IconLookupFlags.GENERIC_FALLBACK|Gtk.IconLookupFlags.USE_BUILTIN) == null) {
+            icon = new ThemedIcon ("application-default-icon");
         }
     }
 
@@ -108,159 +95,28 @@
         description = _("Run this command…");
         exec = command;
         desktop_id = command;
-        icon_name = "system-run";
-
-        update_icon ();
-
+        icon = new ThemedIcon ("system-run");
     }
 
     public App.from_synapse_match (Synapse.Match match, Synapse.Match? target = null) {
-
         app_type = AppType.SYNAPSE;
 
         name = match.title;
         description = match.description;
-        icon_name = match.icon_name;
+        if (match.match_type == Synapse.MatchType.CONTACT && match.has_thumbnail) {
+            var file = File.new_for_path (match.thumbnail_path);
+            icon = new FileIcon (file);
+        } else if (match.icon_name != null) {
+            icon = new ThemedIcon (match.icon_name);
+        }
+
+        weak Gtk.IconTheme theme = Gtk.IconTheme.get_default ();
+        if (theme.lookup_by_gicon (icon, 64, Gtk.IconLookupFlags.GENERIC_FALLBACK|Gtk.IconLookupFlags.USE_BUILTIN) == null) {
+            icon = new ThemedIcon ("application-default-icon");
+        }
 
         this.match = match;
         this.target = target;
-
-        update_icon ();
-
-    }
-
-    ~App () {
-        Slingshot.icon_theme.changed.disconnect (update_icon);
-    }
-
-    public void update_icon () {
-        icon = load_icon (Slingshot.settings.icon_size);
-
-        icon_changed ();
-    }
-
-    private delegate void IconLoadFallback ();
-
-    private class IconLoadFallbackMethod {
-        public unowned IconLoadFallback load_icon;
-
-        public IconLoadFallbackMethod (IconLoadFallback fallback) {
-            load_icon = fallback;
-        }
-    }
-
-    public Gdk.Pixbuf? load_icon (int size) {
-        if (app_type == AppType.SYNAPSE) {
-            try {
-                // for contacts we can load the thumbnail because we expect it to be
-                // the avatar. For other types it'd be ridiculously small.
-                if (match.match_type == Synapse.MatchType.CONTACT && match.has_thumbnail) {
-                    return new Gdk.Pixbuf.from_file_at_scale (match.thumbnail_path, size, size, true);
-                }
-
-                var icon = Icon.new_for_string (icon_name);
-                var info = Gtk.IconTheme.get_default ().lookup_by_gicon (icon,
-                    size, Gtk.IconLookupFlags.FORCE_SIZE);
-
-                if (info == null)
-                    throw new IconError.NOT_FOUND ("Not found");
-
-                return info.load_icon ();
-            } catch (Error e) {
-                warning ("Failed to load icon: %s\n", e.message);
-            }
-
-            try {
-                return Slingshot.icon_theme.load_icon ("application-default-icon",
-                    size, Gtk.IconLookupFlags.FORCE_SIZE);
-            } catch (Error e) {
-                critical (e.message);
-            }
-        }
-
-        Gdk.Pixbuf icon = null;
-        var flags = Gtk.IconLookupFlags.FORCE_SIZE;
-
-        if (loadable_icon != null)
-            return get_loadable_icon ();
-
-        IconLoadFallbackMethod[] fallbacks = {
-            new IconLoadFallbackMethod (() => {
-                try {
-                    icon = Slingshot.icon_theme.load_icon (icon_name, size, flags);
-                } catch (Error e) {
-                    warning ("Could not load icon. Falling back to method 2");
-                }
-            }),
-
-            new IconLoadFallbackMethod (() => {
-                // Since the best method didn't work retry after some time
-                if (check_icon_again) {
-                    // only recheck once
-                    check_icon_again = false;
-
-                    Timeout.add_seconds (RECHECK_TIMEOUT, () => {
-                        Slingshot.icon_theme.rescan_if_needed ();
-                        update_icon ();
-                        return false;
-                    });
-                }
-            }),
-
-            new IconLoadFallbackMethod (() => {
-                try {
-                    if (icon_name.last_index_of (".") > 0) {
-                        var name = icon_name[0:icon_name.last_index_of (".")];
-                        icon = Slingshot.icon_theme.load_icon (name, size, flags);
-                    }
-                } catch (Error e) {
-                    warning ("Could not load icon. Falling back to method 3");
-                }
-            }),
-
-            new IconLoadFallbackMethod (() => {
-                try {
-                    icon = new Gdk.Pixbuf.from_file_at_scale (icon_name, size, size, false);
-                } catch (Error e) {
-                    warning ("Could not load icon. Falling back to method 4");
-                }
-            }),
-
-            new IconLoadFallbackMethod (() => {
-                try {
-                    icon = Slingshot.icon_theme.load_icon ("application-default-icon", size, flags);
-                 } catch (Error e) {
-                     warning ("Could not load icon. Falling back to method 5");
-                 }
-            }),
-
-            new IconLoadFallbackMethod (() => {
-                 try {
-                    icon = Slingshot.icon_theme.load_icon ("gtk-missing-image", size, flags);
-                 } catch (Error e) {
-                    error ("Could not find a fallback icon to load");
-                 }
-            })
-        };
-        foreach (IconLoadFallbackMethod fallback in fallbacks) {
-            fallback.load_icon ();
-            if (icon != null)
-                break;
-        }
-
-        return icon;
-    }
-
-    public Gdk.Pixbuf? get_loadable_icon () {
-        Gdk.Pixbuf? tmp_loadable_icon;
-        try {
-            var icon_stream = loadable_icon.load (0, null, null);
-            tmp_loadable_icon = new Gdk.Pixbuf.from_stream_at_scale (icon_stream, Slingshot.settings.icon_size,
-                                                                     Slingshot.settings.icon_size, true, null);
-        } catch (Error e) {
-            tmp_loadable_icon = null;
-        }
-        return tmp_loadable_icon;
     }
 
     public bool launch () {
@@ -375,4 +231,4 @@
             }
         }
     }
-}
\ No newline at end of file
+}

=== modified file 'src/Widgets/AppEntry.vala'
--- src/Widgets/AppEntry.vala	2015-11-19 13:45:51 +0000
+++ src/Widgets/AppEntry.vala	2015-12-22 17:09:38 +0000
@@ -19,20 +19,35 @@
 public class Slingshot.Widgets.AppEntry : Gtk.Button {
     private static Gtk.Menu menu;
 
+    public signal void app_launched ();
+
     public Gtk.Label app_label;
-    private Gdk.Pixbuf icon;
+    public unowned string exec_name {
+        get {
+            return application.exec;
+        }
+    }
+
+    public unowned string app_name {
+        get {
+            return application.name;
+        }
+    }
+
+    public unowned string desktop_id {
+        get {
+            return application.desktop_id;
+        }
+    }
+
+    public unowned string desktop_path {
+        get {
+            return application.desktop_path;
+        }
+    }
+
     private new Gtk.Image image;
-
-    public string exec_name;
-    public string app_name;
-    public string desktop_id;
-    public int icon_size;
-    public string desktop_path;
-
-    public signal void app_launched ();
-
     private bool dragging = false; //prevent launching
-
     private Backend.App application;
 
 #if HAS_PLANK
@@ -50,7 +65,11 @@
     private static Plank.DBus.Client plank_client;
 #endif
     private bool docked = false;
-    private string desktop_uri;
+    private string desktop_uri {
+        owned get {
+            return File.new_for_path (desktop_path).get_uri ();
+        }
+    }
 #endif
 
     public AppEntry (Backend.App app) {
@@ -58,18 +77,8 @@
         Gtk.drag_source_set (this, Gdk.ModifierType.BUTTON1_MASK, {dnd},
                              Gdk.DragAction.COPY);
 
-        desktop_id = app.desktop_id;
-        desktop_path = app.desktop_path;
-#if HAS_PLANK
-        desktop_uri = File.new_for_path (desktop_path).get_uri ();
-#endif
-
         application = app;
-        app_name = app.name;
         tooltip_text = app.description;
-        exec_name = app.exec;
-        icon_size = Slingshot.settings.icon_size;
-        icon = app.icon;
 
         get_style_context ().add_class (Gtk.STYLE_CLASS_FLAT);
 
@@ -81,8 +90,9 @@
         app_label.set_single_line_mode (false);
         app_label.set_ellipsize (Pango.EllipsizeMode.END);
 
-        image = new Gtk.Image.from_pixbuf (icon);
-        image.icon_size = icon_size;
+        image = new Gtk.Image ();
+        image.gicon = app.icon;
+        image.pixel_size = 64;
         image.margin_top = 12;
 
         var grid = new Gtk.Grid ();
@@ -94,7 +104,6 @@
         grid.add (app_label);
 
         add (grid);
-        set_size_request (Pixels.ITEM_SIZE, Pixels.ITEM_SIZE);
 
         this.clicked.connect (launch_app);
 
@@ -112,7 +121,7 @@
 
         this.drag_begin.connect ( (ctx) => {
             this.dragging = true;
-            Gtk.drag_set_icon_pixbuf (ctx, icon, 0, 0);
+            Gtk.drag_set_icon_gicon (ctx, app.icon, 0, 0);
         });
 
         this.drag_end.connect ( () => {
@@ -124,11 +133,9 @@
             sel.set_uris ({File.new_for_path (desktop_path).get_uri ()});
         });
 
-        app.icon_changed.connect (() => {
-            icon = app.icon;
-            image.set_from_pixbuf (icon);
+        app.notify["icon"].connect (() => {
+            ((Gtk.Image) image).gicon = app.icon;
         });
-
     }
 
     public override void get_preferred_width (out int minimum_width, out int natural_width) {

=== modified file 'src/Widgets/SearchItem.vala'
--- src/Widgets/SearchItem.vala	2015-12-19 14:14:07 +0000
+++ src/Widgets/SearchItem.vala	2015-12-22 17:09:38 +0000
@@ -59,7 +59,9 @@
             name_label.use_markup = true;
             ((Gtk.Misc) name_label).xalign = 0.0f;
 
-            icon = new Gtk.Image.from_pixbuf (app.load_icon (ICON_SIZE));
+            icon = new Gtk.Image ();
+            icon.gicon = app.icon;
+            icon.pixel_size = ICON_SIZE;
 
             // load a favicon if we're an internet page
             var uri_match = app.match as Synapse.UriMatch;
@@ -94,7 +96,7 @@
                 Gdk.DragAction.COPY);
                 this.drag_begin.connect ( (ctx) => {
                     this.dragging = true;
-                    Gtk.drag_set_icon_pixbuf (ctx, app.icon, 0, 0);
+                    Gtk.drag_set_icon_gicon (ctx, app.icon, 0, 0);
                 });
                 this.drag_end.connect ( () => {
                     this.dragging = false;