Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add icon to WindowDesciptor #1163

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions crates/bevy_window/src/window.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,27 @@ pub enum WindowCommand {
},
}

#[derive(Debug, Clone)]
pub struct Icon {
pub rgba: Vec<u8>,
pub width: u32,
pub height: u32,
}

impl Icon {
/// 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.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// `rgba.len() / 4`. Otherwise, this will icon will not be used.
/// `rgba.len() / 4`. Otherwise, the icon will not be used.

pub fn from_rgba(rgba: Vec<u8>, width: u32, height: u32) -> Self {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Icon {
rgba,
width,
height,
}
}
}

/// Defines the way a window is displayed
/// The use_size option that is used in the Fullscreen variant
/// defines whether a videomode is chosen that best fits the width and height
Expand Down Expand Up @@ -383,6 +404,8 @@ pub struct WindowDescriptor {
pub cursor_visible: bool,
pub cursor_locked: bool,
pub mode: WindowMode,
#[cfg(any(target_os = "windows", target_os = "linux"))]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Suggested change
#[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.

pub icon: Option<Icon>,
#[cfg(target_arch = "wasm32")]
pub canvas: Option<String>,
}
Expand All @@ -400,6 +423,8 @@ impl Default for WindowDescriptor {
cursor_locked: false,
cursor_visible: true,
mode: WindowMode::Windowed,
#[cfg(any(target_os = "windows", target_os = "linux"))]
icon: None,
#[cfg(target_arch = "wasm32")]
canvas: None,
}
Expand Down
16 changes: 16 additions & 0 deletions crates/bevy_winit/src/winit_windows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,22 @@ impl WinitWindows {
}
}

#[cfg(any(target_os = "windows", target_os = "linux"))]
if let Some(icon) = &window_descriptor.icon {
let winit_icon =
match winit::window::Icon::from_rgba(icon.rgba.clone(), icon.width, icon.height) {
Ok(icon) => Some(icon),
Err(bad_icon) => {
println!(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably use the tracing error! macro

"Bad icon supplied for window and defaulting to none. Error: {:?}",
bad_icon
);
None
}
};
winit_window_builder = winit_window_builder.with_window_icon(winit_icon);
}

let winit_window = winit_window_builder.build(&event_loop).unwrap();

match winit_window.set_cursor_grab(window_descriptor.cursor_locked) {
Expand Down
Binary file added examples/window/bevy_icon.rgba
Binary file not shown.
8 changes: 8 additions & 0 deletions examples/window/window_settings.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
use bevy::prelude::*;
#[cfg(any(target_os = "windows", target_os = "linux"))]
use bevy::window::Icon;

/// This example illustrates how to customize the default window settings
fn main() {
Expand All @@ -8,6 +10,12 @@ fn main() {
width: 500.,
height: 300.,
vsync: true,
#[cfg(any(target_os = "windows", target_os = "linux"))]
icon: Some(Icon::from_rgba(
include_bytes!("bevy_icon.rgba").to_vec(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

32,
32,
)),
..Default::default()
})
.add_plugins(DefaultPlugins)
Expand Down