Window icons

[francesca64]

Fixes #69

I ended up changing the design of this several times, so this fairly innocuous feature is the thing that finally burned me out.

• WindowBuilder::with_window_icon and Window::set_window_icon exist now, and are implemented on X11 and Windows (which are the only applicable platforms). I didn't put this behind platform ext traits, simply for ergonomic reasons.
• Icon::from_rgba is a checked way to create icons from raw RGBA data. It's not possible to create an icon that isn't valid (which is to say, an icon that can't be indexed as RGBA).
• With the icon_loading feature enabled, there's integration with the image crate. Icon::from_path, Icon::from_bytes (you can use this with include_bytes!), Icon::from_reader, From<image::DynamicImage>, and From<image::RgbaImage>.
• WindowAttributes and PlatformSpecificWindowBuilderAttributes are now moved to window constructors instead of borrowed. It didn't look like there was any reason for them to need to be borrowed, and taking ownership of the icon data gives us more opportunity to do things in-place. Though, it doesn't really matter, since on Windows we still have to clone for other reasons (which will probably be resolved in the future), and on X11 we can't use the data in-place either way.
• Windows additionally has WindowBuilderExt::with_taskbar_icon and WindowExt::set_taskbar_icon. It's likely better to use manifests for this.
• To set application icons on X11 (i.e. what shows up in task switchers), while it is possible to just add a second larger icon and have heuristics in some WMs/tools pick it up, implementing that both significantly increased complexity and led to a semantically misleading API with unreliable behavior. The right way to do this is using .desktop files, but we don't yet set (or expose) enough hints for those to be used. That should be easy to resolve in future work.
• It's become clear to me that we (as in, the Rust community, not necessarily winit) need good documentation (and probably tooling) for packaging cross-platform apps. While I don't think it makes sense for winit to do more with icons than what I've already done here, it's going to be hard to convince users of the merits of that attitude if they're left out in the cold to research icon metadata and conventions on every platform.

In my personal opinion, the example program I made for this is really cool. Someone should make a game played entirely in the window icon...

[edwin0cheng]

I tested in my windows machine, with different image formats.
It is so cool. Definitely someone should port some Snake game on the icon 😊

On the other hand, would the icon_loading feature be separated to new trait ? :

pub struct Icon { ...}

#[cfg(feature = "icon_loading")]
pub trait IconLoading { ...}

#[cfg(feature = "icon_loading")]
impl IconLoading for Icon ...

[edwin0cheng]

And could we use NSDockTile and applicationIconImage on macos ? If so, i would like to make an PR to implement it after this one is merged.

[francesca64]

@edwin0cheng what would be the benefits of using a trait for this?

applicationIconImage would have to be under platform ext with a different function name, since it has different semantics (and expected sizes) from window icons. Either way, it sounds like a welcome feature (so long as it's documented that it's intended for temporary icon changes).

[edwin0cheng]

• Just don't need to write #[cfg(feature = "icon_loading")] for each functions and make use winit::IconLoading; explicitly. But these are very subtle benefits, I am fine without them.
• So set_window_icon itself is not for temporary icon change ??

[francesca64]

Well, people using winit won't have to worry about the feature gate in their own code, though I suppose the example doesn't do a good job reflecting that.

What I mean about the icon being temporary is in regards to this blurb from the applicationIconImage docs:

Assign an image to this property when you want to temporarily change the app icon in the dock app tile.

My concern is that people might use this instead of making a .app bundle, rather than in supplement to it. I think winit should generally steer people towards following native conventions.

[elinorbgr]

My concern is that people might use this instead of making a .app bundle, rather than in supplement to it. I think winit should generally steer people towards following native conventions.

On this vein, I will add that the only way currently to set an icon for an app in wayland is to provide an appropriate .desktop file and set the proper app_id for the surface. Nothing is really possible (currently) to dynamically change the icon.

[icefoxen]

Tested, works for me on Windows 10 and Debian Linux.

[francesca64]

@icefoxen thanks for testing this! (Thanks @edwin0cheng, too!)

@vberger is setting the app_id currently possible? I still have to implement startup notification/etc. on X11, but after that, it would be good to document .desktop file usage.

[elinorbgr]

Allowing to set the app id is not yet exposed afaik, but would be a trivial fix via the unix extension trait.

I'm not familiar enough to know with X11 to know if/how this can be unified, maybe? The wayland app_id is defined as being the id of the associated .desktop file, as defined here: https://specifications.freedesktop.org/desktop-entry-spec/desktop-entry-spec-latest.html#desktop-file-id

[francesca64]

@vberger on X11, you have to set the WM_CLASS, so it's basically the same sans the .desktop extension. It's not exposed yet either (though I already have it done on another branch); at present, we actually set WM_CLASS to the initial window title, which is very surprising. I'm providing the binary name from argv[0] as a defualt value, since then things should work out of the box for the majority of people, though that's honestly mostly motivated by the ergonomic problems of using WindowBuilderExt.

[elinorbgr]

Okay, it should be the same thing then. I guess we should add some API to allow modifying it. Wayland allows to modify this at basically any time, and it's a very straightforward thing: just call the set_app_id method of the wayland window, exactly like set_title: https://github.com/tomaka/winit/blob/1e971030946a289eec598c0c40bff6ec8ba1190f/src/platform/linux/wayland/window.rs#L143-L145

So I guess the best would be to design some platform-specific API fitting the X11 requirements, and plug the wayland backend on that. What do you think?

[francesca64]

@vberger yeah, that sounds good. Anyway, I've finished rebasing the Windows DPI PR, so now I'll look into getting the DPI API situation sorted out.