Support more icon sizes

[danirabbit]

It seems icon sizes are unfortunately hardcoded to only be 64px or 128px here:

appstream-glib/client/as-compose.c

Lines 39 to 167 in cbe4aae

	static gboolean
	add_icons (AsApp *app,
	const gchar *icons_dir,
	guint min_icon_size,
	const gchar *prefix,
	const gchar *key,
	GError **error)
	{
	g_autofree gchar *fn_hidpi = NULL;
	g_autofree gchar *fn = NULL;
	g_autofree gchar *name_hidpi = NULL;
	g_autofree gchar *name = NULL;
	g_autofree gchar *icon_path = NULL;
	g_autofree gchar *icon_subdir = NULL;
	g_autofree gchar *icon_path_hidpi = NULL;
	g_autofree gchar *icon_subdir_hidpi = NULL;
	g_autoptr(AsIcon) icon_hidpi = NULL;
	g_autoptr(AsIcon) icon = NULL;
	g_autoptr(AsImage) im = NULL;
	g_autoptr(GdkPixbuf) pixbuf_hidpi = NULL;
	g_autoptr(GdkPixbuf) pixbuf = NULL;
	g_autoptr(GError) error_local = NULL;
	/* find 64x64 icon */
	fn = as_utils_find_icon_filename_full (prefix, key,
	AS_UTILS_FIND_ICON_NONE,
	error);
	if (fn == NULL) {
	g_prefix_error (error, "Failed to find icon: ");
	return FALSE;
	}
	/* load the icon */
	im = as_image_new ();
	if (!as_image_load_filename_full (im, fn,
	64, min_icon_size,
	AS_IMAGE_LOAD_FLAG_ALWAYS_RESIZE |
	AS_IMAGE_LOAD_FLAG_ONLY_SUPPORTED |
	AS_IMAGE_LOAD_FLAG_SHARPEN,
	error)) {
	g_prefix_error (error, "Failed to load icon: ");
	return FALSE;
	}
	pixbuf = g_object_ref (as_image_get_pixbuf (im));
	/* save in target directory */
	name = g_strdup_printf ("%s.png", as_app_get_id_filename (AS_APP (app)));
	icon = as_icon_new ();
	as_icon_set_pixbuf (icon, pixbuf);
	as_icon_set_name (icon, name);
	as_icon_set_kind (icon, AS_ICON_KIND_CACHED);
	as_icon_set_prefix (icon, as_app_get_icon_path (AS_APP (app)));
	as_app_add_icon (AS_APP (app), icon);
	icon_path = g_build_filename (icons_dir, "64x64", name, NULL);
	icon_subdir = g_path_get_dirname (icon_path);
	if (g_mkdir_with_parents (icon_subdir, 0755)) {
	int errsv = errno;
	g_set_error (error,
	AS_APP_ERROR,
	AS_APP_ERROR_FAILED,
	"failed to create %s: %s",
	icon_subdir,
	strerror (errsv));
	return FALSE;
	}
	/* TRANSLATORS: we've saving the icon file to disk */
	g_print ("%s %s\n", _("Saving icon"), icon_path);
	if (!gdk_pixbuf_save (pixbuf, icon_path, "png", error, NULL))
	return FALSE;
	/* try to get a HiDPI icon */
	fn_hidpi = as_utils_find_icon_filename_full (prefix, key,
	AS_UTILS_FIND_ICON_HI_DPI,
	NULL);
	if (fn_hidpi == NULL) {
	g_debug ("no HiDPI icon found with key %s in %s", key, prefix);
	return TRUE;
	}
	/* load the HiDPI icon */
	g_debug ("trying to load %s", fn_hidpi);
	if (!as_image_load_filename_full (im, fn_hidpi,
	128, 128,
	AS_IMAGE_LOAD_FLAG_ALWAYS_RESIZE |
	AS_IMAGE_LOAD_FLAG_SHARPEN,
	&error_local)) {
	g_debug ("failed to load HiDPI icon: %s", error_local->message);
	return TRUE;
	}
	pixbuf_hidpi = g_object_ref (as_image_get_pixbuf (im));
	if (gdk_pixbuf_get_width (pixbuf_hidpi) <= gdk_pixbuf_get_width (pixbuf) ||
	gdk_pixbuf_get_height (pixbuf_hidpi) <= gdk_pixbuf_get_height (pixbuf)) {
	g_debug ("HiDPI icon no larger than normal icon");
	return TRUE;
	}
	as_app_add_kudo_kind (AS_APP (app), AS_KUDO_KIND_HI_DPI_ICON);
	/* save icon */
	name_hidpi = g_strdup_printf ("%s.png", as_app_get_id_filename (AS_APP (app)));
	icon_hidpi = as_icon_new ();
	as_icon_set_pixbuf (icon_hidpi, pixbuf_hidpi);
	as_icon_set_name (icon_hidpi, name_hidpi);
	as_icon_set_kind (icon_hidpi, AS_ICON_KIND_CACHED);
	as_icon_set_prefix (icon_hidpi, as_app_get_icon_path (AS_APP (app)));
	as_app_add_icon (AS_APP (app), icon_hidpi);
	icon_path_hidpi = g_build_filename (icons_dir, "128x128", name_hidpi, NULL);
	icon_subdir_hidpi = g_path_get_dirname (icon_path_hidpi);
	if (g_mkdir_with_parents (icon_subdir_hidpi, 0755)) {
	int errsv = errno;
	g_set_error (error,
	AS_APP_ERROR,
	AS_APP_ERROR_FAILED,
	"failed to create %s: %s",
	icon_subdir_hidpi,
	strerror (errsv));
	return FALSE;
	}
	/* TRANSLATORS: we've saving the icon file to disk */
	g_print ("%s %s\n", _("Saving icon"), icon_path_hidpi);
	if (!gdk_pixbuf_save (pixbuf_hidpi, icon_path_hidpi, "png", error, NULL))
	return FALSE;
	return TRUE;
	}

It would be nice to support more icon sizes so that app stores don't have blurry icons in lists etc. For example AppCenter uses 48px icons in lists. This means that apps from our system deb repo get sharp and properly scaled icons but apps from our flatpak remote do not. See for example this screenshot where the top and bottom icons have crisp lines and the middle icon is blurry:

It would also be nice if we used the actual scaled icons based on the icon theme spec since 128px@1x is not the same thing as 64px@2x for icons that are actually hinted and rendered for proper display scaling.

Related: elementary/appcenter-reviews#454

[hughsie]

Isn't this ximion/appstream-generator#56 ? In Fedora the policy is not to include the app unless a 64px or larger icon is present, and so I don't think we'd ever ship the 48px versions even if allowed by the specification.

[ximion]

Yes, and AFAIK Elementary is using appstream-generator for everything already...
For asgen, you also need a 64x64 (or larger) icon, but it will include 48x48px icons as well.

[hughsie]

There's zero appetite on making the AppStream metadata even larger on Fedora or RHEL -- it's already 20MB decompressed.

[ximion]

It's already extremely large on Debian too, and we don't have binary deltas for the icon tarballs yet. We do however have a CDN, and appstream-generator allows certain icon types to be served from a remote source only (so that's what we do with everything bigger than 64x64px).
I would probably serve everything from the web except for 64x64px icons, but there's some privacy concerns, I guess...

[danirabbit]

Ah yeah I suppose it is. It's been a long time. I guess maybe flat manager or something else is using appstream-glib instead of appstream-generator? @davidmhewitt would you be able to provide additional context here?

Just for clarity, I only want the ability to serve the icon sizes we use from our Flatpak remote. I don't want to change anything for anyone else :)

[davidmhewitt]

I guess maybe flat manager or something else is using appstream-glib instead of appstream-generator? @davidmhewitt would you be able to provide additional context here?

Ah, apologies, I seem to have misunderstood how the appstream publishing phase works on a flatpak remote. I had got the idea from somewhere that it was appstream-compose that merged all of the component metadata and copied the relevant icons to be served from the remote.

However, I'm now seeing that it seems to be flatpak itself that does that here:
https://github.com/flatpak/flatpak/blob/1cbff35386f4e6584646199a26fdfe82e72d732b/common/flatpak-utils.c#L5430-L5441

Is that correct?

[ximion]

Yes, looks like Flatpak hardcodes that, which isn't ideal... But also flatpak/flatpak-builder#517 still isn't solved, so without that addressed there wouldn't even be other icon sizes that Flatpak could consider.