Add icon to WindowDesciptor #1163
Add icon to WindowDesciptor #1163goodbadwolf wants to merge 3 commits intobevyengine:mainfrom goodbadwolf:window-icon
Conversation
This PR adds an optional icon to WindowDescriptor and updates `window_settings` example to use it.
This also includes minor refactoring
DJMcNab
left a comment
DJMcNab
left a comment
There was a problem hiding this comment.
A good start, thanks
Future work would be to support changing the icon at runtime, although I'm not sure if winit supports that presently.
| /// Creates an `Icon` from 32bpp RGBA data. | ||
| /// | ||
| /// The length of `rgba` must be divisible by 4, and `width * height` must equal | ||
| /// `rgba.len() / 4`. Otherwise, this will icon will not be used. |
There was a problem hiding this comment.
| /// `rgba.len() / 4`. Otherwise, this will icon will not be used. | |
| /// `rgba.len() / 4`. Otherwise, the icon will not be used. |
| pub cursor_visible: bool, | ||
| pub cursor_locked: bool, | ||
| pub mode: WindowMode, | ||
| #[cfg(any(target_os = "windows", target_os = "linux"))] |
There was a problem hiding this comment.
Personally, I'd be happier to have the icon be silently ignored on platforms where it isn't supported.
In its current form this is a feature gate footgun which makes it less likely that applications would support e.g. darwin or android out of the gate.
Additionally, it seems reasonable that we might in future support making this change the website favicon, perhaps behind a feature gate
| #[cfg(any(target_os = "windows", target_os = "linux"))] | |
| /// The icon for the window. Only active on platforms which support it, which is currently Windows and X11 Linux. |
| match winit::window::Icon::from_rgba(icon.rgba.clone(), icon.width, icon.height) { | ||
| Ok(icon) => Some(icon), | ||
| Err(bad_icon) => { | ||
| println!( |
There was a problem hiding this comment.
This should probably use the tracing error! macro
| /// | ||
| /// The length of `rgba` must be divisible by 4, and `width * height` must equal | ||
| /// `rgba.len() / 4`. Otherwise, this will icon will not be used. | ||
| pub fn from_rgba(rgba: Vec<u8>, width: u32, height: u32) -> Self { |
There was a problem hiding this comment.
Ideally we'd support create an icon from any image type supported by the image crate. I'm not sure which crate that support should live in, however.
| vsync: true, | ||
| #[cfg(any(target_os = "windows", target_os = "linux"))] | ||
| icon: Some(Icon::from_rgba( | ||
| include_bytes!("bevy_icon.rgba").to_vec(), |
There was a problem hiding this comment.
I suppose if this used Cow<'static, [u8]> we could avoid the to_vec, in case another backend supported &'static [u8] icons for example
That being said, requiring the user to create the Vec to match the winit API is also reasonable.
|
If possible, I think it would be nice if we could load icons using the asset system (ex: make the window descriptor take a |
Moxinilian
added
C-Feature
|
I'm closing this due to inactivity and because I think this should use bevy_asset |
4 participants
This PR adds an optional icon to WindowDescriptor and updates
window_settingsexample to use it.Fixes #1031