hughsie / appstream-glib

This library provides objects and helper methods to help reading and writing AppStream metadata.
GNU Lesser General Public License v2.1
65 stars 103 forks source link

Support more icon sizes #467

Open danirabbit opened 1 year ago

danirabbit commented 1 year ago

It seems icon sizes are unfortunately hardcoded to only be 64px or 128px here: https://github.com/hughsie/appstream-glib/blob/cbe4aae01a99154464cb865235689bf49d0a48a2/client/as-compose.c#L39-L167

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:

Screenshot from 2023-05-04 11 54 46

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: https://github.com/elementary/appcenter-reviews/issues/454

hughsie commented 1 year ago

Isn't this https://github.com/ximion/appstream-generator/issues/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 commented 1 year ago

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 commented 1 year ago

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

ximion commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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