ppy / osu-deploy

Deploy script for lazer
MIT License
45 stars 32 forks source link

Improve icons support on Linux #158

Open ravener opened 6 months ago

ravener commented 6 months ago

The icon packaged in the AppDir is probably a little too big (1024x1024), additionally proper desktop integration seems to fail when the icons are not available in the usual expected location which is normally /usr/share/icons/hicolor.

hicolor is simply the 'default' theme, since there is the concept of icon themes and other themes can apply a custom icon override if desired but hicolor is where apps install their default icons.

Currently integration with AppImageLauncher shows no icon for osu! and I have verified the same issue on other immature appimages that also failed to provide the additional icons in the expected directory (/usr/share/icons) where as major applications like Krita does so correctly.

Technically the AppImageLauncher is supposed to use the base AppDir icon and re-size it but it seems to be bugged

─❯ ail-cli integrate osu.AppImage                                            ─╯
Processing /home/ravener/osu.AppImage
Moving AppImage to integration directory
WARNING: No icons found at "usr/share/icons"
WARNING: Using .DirIcon as default app icon
ERROR: Unable to load image.
ERROR: No icon was generated for: /home/ravener/Applications/osu_e72e7ca7eb9bb530405a0f8263c2a923.AppImage
WARNING: Unable to resize the application icon into a 128x128 image: "Unable to load image.". It will be written unchanged.
WARNING: Unable to resize the application icon into a 256x256 image: "Unable to load image.". It will be written unchanged.

This happens to every app, so it's partly the tool's fault?. Yet many of the mature AppImages in the ecosystem like Krita and others do opt to put their icons in /usr/share/icons and that's typically how native packages do it, so not only does it workaround this bug with the tool but it just overall improves the general package structure to be more standard.

Let's take Krita as our example here on how to do it:

So in summary we should start including other size icons in /usr/share/icons/hicolor/[size]/apps/osu!.png inside the AppDir and I also recommend using a smaller size icon for the base AppDir icon that we currently have since 1024x1024 seems too unnecessarily large and we can have a 1024x1024 version of the icon in the aforementioned directories.

I would lend a hand in submitting a PR and such but I would like to hear from you first and I have no idea how do you prefer the icon resizing to be handled and which of the sizes will we choose to provide?

ravener commented 6 months ago

Quoting some relevant information from the Freedesktop.org Icon Theme Specification

In order to have a place for third party applications to install their icons there should always exist a theme called "hicolor". The data for the hicolor theme is available for download at: http://www.freedesktop.org/software/icon-theme/. Implementations are required to look in the "hicolor" theme if an icon was not found in the current theme.

Installing Application Icons

So, you're an application author, and want to install application icons so that they work in the KDE and Gnome menus. Minimally you should install a 48x48 icon in the hicolor theme. This means installing a PNG file in $prefix/share/icons/hicolor/48x48/apps. Optionally you can install icons in different sizes. For example, installing a svg icon in $prefix/share/icons/hicolor/scalable/apps means most desktops will have one icon that works for all sizes. You might even want to install icons with a look that matches other well known themes so your application will fit in with some specific desktop environment.

It is recommended that the icons installed in the hicolor theme look neutral, since it is a fallback theme that will be used in combination with some very different looking themes. But if you don't have any neutral icon, please install whatever icon you have in the hicolor theme so that all applications get at least some icon in all themes.

ravener commented 6 months ago

I'd also like to comment on this piece of code in the current AppRun

if [ ! -z "$APPIMAGE" ] && [ ! -z "$APPDIR" ]; then
    MD5=$(echo -n "file://$APPIMAGE" | md5sum | cut -d' ' -f1)
    cp "$APPDIR/osu!.png" "$HOME/.cache/thumbnails/normal/$MD5.png"
    cp "$APPDIR/osu!.png" "$HOME/.cache/thumbnails/large/$MD5.png"
    xdg-icon-resource forceupdate
fi

I see what you did there, it's a silly attempt to simulate as if the app has an icon but it feels wrong. Normally those thumbnail directories are filled via special 'thumbnailer' programs, these programs are for example used to generate image previews and so on, or otherwise manually set by the user through the file manager.

when I first saw that icon I was confused, 'did I accidentally install some program that thumbnails appimages? hmm but why does it not work on other appimages', it's just not expected behavior and I see no reason why osu! should behave any different from a usual AppImage, so I also propose we remove that snippet of code. Instead by complying with proper packaging standards we make it easy for users who want to integrate the appimage to do so, otherwise they can continue to just use it as a normal appimage like any other, without behaviors like this.

Following a standard packaging structure will also make it easy to re-use our efforts to create native .deb packages and such in the future if desired.

bdach commented 6 months ago

The icon is temporary. I'm not sure it's worth spending too much time on this until it becomes non-temporary.

ravener commented 6 months ago

It seems that the majority of desktop integration work I was trying to achieve has already been done on the flatpak package at https://github.com/flathub/sh.ppy.osu/ including icons, mime types (something i wanted to do next), appstream and all.

Since the flatpak also relies on the appimage by extracting it, i believe integrating some of those changes here will both enhance the appimage's ability to integrate and also simplify the script in the flatpak package since more will already be in the appimage.

Lemme know what you think about it, I'd be more than happy to assist with that.