From 5b4c04212c49ab41f3b4ea946e112f14609fd4e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20Correa=20G=C3=B3mez?= Date: Tue, 7 Feb 2023 01:12:37 +0100 Subject: [PATCH 1/4] gs-app: Make sure that themed icons exist before providing them And check for those before checking for other icons, since they are expected to be provided in any possible size. Fixes #2063 --- lib/gs-app.c | 79 +++++++++++++++++++++++++++++----------------------- 1 file changed, 44 insertions(+), 35 deletions(-) diff --git a/lib/gs-app.c b/lib/gs-app.c index 7b7d7f24f..06a5ba777 100644 --- a/lib/gs-app.c +++ b/lib/gs-app.c @@ -1880,6 +1880,33 @@ gs_app_set_developer_name (GsApp *app, const gchar *developer_name) _g_set_str (&priv->developer_name, developer_name); } +static GtkIconTheme * +get_icon_theme (void) +{ + GtkIconTheme *theme; + GdkDisplay *display = gdk_display_get_default (); + + if (display != NULL) { + theme = g_object_ref (gtk_icon_theme_get_for_display (display)); + } else { + const gchar *test_search_path; + + /* This fallback path is needed for the unit tests, + * which run without a screen, and in an environment + * where the XDG dir variables don’t point to the system + * datadir which contains the system icon theme. */ + theme = gtk_icon_theme_new (); + + test_search_path = g_getenv ("GS_SELF_TEST_ICON_THEME_PATH"); + if (test_search_path != NULL) { + g_auto(GStrv) dirs = g_strsplit (test_search_path, ":", -1); + gtk_icon_theme_set_search_path (theme, (const char * const *) dirs); + } + } + + return theme; +} + /** * gs_app_get_icon_for_size: * @app: a #GsApp @@ -1926,7 +1953,22 @@ gs_app_get_icon_for_size (GsApp *app, g_debug ("Looking for icon for %s, at size %u×%u, with fallback %s", gs_app_get_id (app), size, scale, fallback_icon_name); - /* See if there’s an icon the right size, or the first one which is too + /* If there’s a themed icon with no width set, use that, as typically + * themed icons are available in any given size. */ + for (guint i = 0; priv->icons != NULL && i < priv->icons->len; i++) { + GIcon *icon = priv->icons->pdata[i]; + guint icon_width = gs_icon_get_width (icon); + + if (icon_width == 0 && G_IS_THEMED_ICON (icon)) { + g_autoptr(GtkIconTheme) theme = get_icon_theme (); + if (gtk_icon_theme_has_gicon (theme, icon)) { + g_debug ("Found themed icon"); + return g_object_ref (icon); + } + } + } + + /* See if there’s an icon of the right size, or the first one which is too * big which could be scaled down. Note that the icons array may be * lazily created. */ for (guint i = 0; priv->icons != NULL && i < priv->icons->len; i++) { @@ -1956,18 +1998,6 @@ gs_app_get_icon_for_size (GsApp *app, return g_object_ref (icon); } - g_debug ("Found no icons of the right size; checking themed icons"); - - /* If there’s a themed icon with no width set, use that, as typically - * themed icons are available in all the right sizes. */ - for (guint i = 0; priv->icons != NULL && i < priv->icons->len; i++) { - GIcon *icon = priv->icons->pdata[i]; - guint icon_width = gs_icon_get_width (icon); - - if (icon_width == 0 && G_IS_THEMED_ICON (icon)) - return g_object_ref (icon); - } - if (scale > 1) { g_debug ("Retrying at scale 1"); return gs_app_get_icon_for_size (app, size, 1, fallback_icon_name); @@ -4676,28 +4706,7 @@ calculate_key_colors (GsApp *app) pb_small = gdk_pixbuf_new_from_stream_at_scale (icon_stream, 32, 32, TRUE, NULL, NULL); } else if (G_IS_THEMED_ICON (icon_small)) { g_autoptr(GtkIconPaintable) icon_paintable = NULL; - g_autoptr(GtkIconTheme) theme = NULL; - GdkDisplay *display; - - display = gdk_display_get_default (); - if (display != NULL) { - theme = g_object_ref (gtk_icon_theme_get_for_display (display)); - } else { - const gchar *test_search_path; - - /* This fallback path is needed for the unit tests, - * which run without a screen, and in an environment - * where the XDG dir variables don’t point to the system - * datadir which contains the system icon theme. */ - theme = gtk_icon_theme_new (); - - test_search_path = g_getenv ("GS_SELF_TEST_ICON_THEME_PATH"); - if (test_search_path != NULL) { - g_auto(GStrv) dirs = g_strsplit (test_search_path, ":", -1); - gtk_icon_theme_set_search_path (theme, (const char * const *)dirs); - - } - } + g_autoptr(GtkIconTheme) theme = get_icon_theme (); icon_paintable = gtk_icon_theme_lookup_by_gicon (theme, icon_small, 32, 1, -- GitLab From 760b6273ae21de7e0f85d16d1ce0ac1cea2a18b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20Correa=20G=C3=B3mez?= Date: Tue, 7 Feb 2023 01:13:58 +0100 Subject: [PATCH 2/4] gs-app: Improve comment on why checking if local icons exist --- lib/gs-app.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/gs-app.c b/lib/gs-app.c index 06a5ba777..870eaba07 100644 --- a/lib/gs-app.c +++ b/lib/gs-app.c @@ -1981,7 +1981,10 @@ gs_app_get_icon_for_size (GsApp *app, g_debug ("\tConsidering icon of type %s (%s), width %u×%u", G_OBJECT_TYPE_NAME (icon), icon_str, icon_width, icon_scale); - /* Appstream only guarantees the 64x64@1 cached icon is present, ignore other icons that aren't installed. */ + /* To avoid excessive I/O, the loading of AppStream data does + * not verify the existence of cached icons, which we do now. + * Since AppStream only guarantees that the 64x64@1 cached icon + * is present, ignore other icons if they do not exist. */ if (G_IS_FILE_ICON (icon) && !(icon_width == 64 && icon_height == 64 && icon_scale == 1)) { GFile *file = g_file_icon_get_file (G_FILE_ICON (icon)); if (!g_file_query_exists (file, NULL)) { -- GitLab From eaebf88d04710937d567e4ca4699027865a644ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20Correa=20G=C3=B3mez?= Date: Tue, 7 Feb 2023 01:33:27 +0100 Subject: [PATCH 3/4] gs-app: Optimize icon search by only running icon loop once --- lib/gs-app.c | 35 +++++++++++++++++------------------ 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/lib/gs-app.c b/lib/gs-app.c index 870eaba07..47c0c69fb 100644 --- a/lib/gs-app.c +++ b/lib/gs-app.c @@ -1945,6 +1945,7 @@ gs_app_get_icon_for_size (GsApp *app, const gchar *fallback_icon_name) { GsAppPrivate *priv = gs_app_get_instance_private (app); + g_autoptr(GIcon) candidate_icon = NULL; g_return_val_if_fail (GS_IS_APP (app), NULL); g_return_val_if_fail (size > 0, NULL); @@ -1953,12 +1954,18 @@ gs_app_get_icon_for_size (GsApp *app, g_debug ("Looking for icon for %s, at size %u×%u, with fallback %s", gs_app_get_id (app), size, scale, fallback_icon_name); - /* If there’s a themed icon with no width set, use that, as typically - * themed icons are available in any given size. */ for (guint i = 0; priv->icons != NULL && i < priv->icons->len; i++) { GIcon *icon = priv->icons->pdata[i]; + g_autofree gchar *icon_str = g_icon_to_string (icon); guint icon_width = gs_icon_get_width (icon); + guint icon_height = gs_icon_get_height (icon); + guint icon_scale = gs_icon_get_scale (icon); + g_debug ("\tConsidering icon of type %s (%s), width %u×%u", + G_OBJECT_TYPE_NAME (icon), icon_str, icon_width, icon_scale); + + /* If there’s a themed icon with no width set, use that, as + * typically themed icons are available in any given size. */ if (icon_width == 0 && G_IS_THEMED_ICON (icon)) { g_autoptr(GtkIconTheme) theme = get_icon_theme (); if (gtk_icon_theme_has_gicon (theme, icon)) { @@ -1966,20 +1973,6 @@ gs_app_get_icon_for_size (GsApp *app, return g_object_ref (icon); } } - } - - /* See if there’s an icon of the right size, or the first one which is too - * big which could be scaled down. Note that the icons array may be - * lazily created. */ - for (guint i = 0; priv->icons != NULL && i < priv->icons->len; i++) { - GIcon *icon = priv->icons->pdata[i]; - g_autofree gchar *icon_str = g_icon_to_string (icon); - guint icon_width = gs_icon_get_width (icon); - guint icon_height = gs_icon_get_height (icon); - guint icon_scale = gs_icon_get_scale (icon); - - g_debug ("\tConsidering icon of type %s (%s), width %u×%u", - G_OBJECT_TYPE_NAME (icon), icon_str, icon_width, icon_scale); /* To avoid excessive I/O, the loading of AppStream data does * not verify the existence of cached icons, which we do now. @@ -1997,10 +1990,16 @@ gs_app_get_icon_for_size (GsApp *app, if (icon_width == 0 || icon_width * icon_scale < size * scale) continue; - if (icon_width * icon_scale >= size * scale) - return g_object_ref (icon); + /* See if there’s an icon of the right size, or the first one which is too + * big which could be scaled down. Note that the icons array may be + * lazily created. */ + if (candidate_icon == NULL && (icon_width * icon_scale >= size * scale)) + candidate_icon = g_object_ref (icon); } + if (candidate_icon != NULL) + return g_object_ref (candidate_icon); + if (scale > 1) { g_debug ("Retrying at scale 1"); return gs_app_get_icon_for_size (app, size, 1, fallback_icon_name); -- GitLab