From 7d5a343c24794ef4f28bd1b38c1e8a07b06a97de Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Wed, 26 Apr 2023 14:54:35 +0200 Subject: [PATCH 1/2] Set Rerun viewer native app icon using eframe This was recently added to eframe --- crates/re_viewer/src/app.rs | 11 -- crates/re_viewer/src/app_icon.rs | 190 ------------------------------- crates/re_viewer/src/lib.rs | 2 - crates/re_viewer/src/native.rs | 20 ++++ 4 files changed, 20 insertions(+), 203 deletions(-) delete mode 100644 crates/re_viewer/src/app_icon.rs diff --git a/crates/re_viewer/src/app.rs b/crates/re_viewer/src/app.rs index c374cc5a1aa3..bce99cd35e5f 100644 --- a/crates/re_viewer/src/app.rs +++ b/crates/re_viewer/src/app.rs @@ -16,7 +16,6 @@ use re_smart_channel::Receiver; use re_ui::{toasts, Command}; use crate::{ - app_icon::setup_app_icon, misc::{AppOptions, Caches, RecordingConfig, ViewerContext}, ui::{data_ui::ComponentUiRegistry, Blueprint}, viewer_analytics::ViewerAnalytics, @@ -25,8 +24,6 @@ use crate::{ #[cfg(not(target_arch = "wasm32"))] use re_log_types::TimeRangeF; -use super::app_icon::AppIconStatus; - const WATERMARK: bool = false; // Nice for recording media material // ---------------------------------------------------------------------------- @@ -97,8 +94,6 @@ pub struct App { cmd_palette: re_ui::CommandPalette, analytics: ViewerAnalytics, - - icon_status: AppIconStatus, } impl App { @@ -155,8 +150,6 @@ impl App { cmd_palette: Default::default(), analytics, - - icon_status: AppIconStatus::NotSetTryAgain, } } @@ -439,10 +432,6 @@ impl eframe::App for App { self.ram_limit_warner.update(); } - if self.icon_status == AppIconStatus::NotSetTryAgain { - self.icon_status = setup_app_icon(); - } - if self.shutdown.load(std::sync::atomic::Ordering::Relaxed) { #[cfg(not(target_arch = "wasm32"))] frame.close(); diff --git a/crates/re_viewer/src/app_icon.rs b/crates/re_viewer/src/app_icon.rs deleted file mode 100644 index 5101065cacb6..000000000000 --- a/crates/re_viewer/src/app_icon.rs +++ /dev/null @@ -1,190 +0,0 @@ -/// In which state the app icon is (as far as we know). -#[derive(PartialEq, Eq)] -#[allow(dead_code)] -pub enum AppIconStatus { - /// We did not set it or failed to do it. In any case we won't try again. - NotSetIgnored, - - /// We haven't set the icon yet, we should try again next frame. - /// - /// This can happen repeatedly due to lazy window creation on some platforms. - NotSetTryAgain, - - /// We successfully set the icon and it should be visible now. - Set, -} - -/// Sets app icon at runtime. -/// -/// By setting the icon at runtime and not via resource files etc. we ensure that we'll get the chance -/// to set the same icon when the process/window is started from python (which sets its own icon ahead of us!). -/// -/// Since window creation can be lazy, call this every frame until it's either successfully or gave up. -/// (See [`AppIconStatus`]) -pub fn setup_app_icon() -> AppIconStatus { - crate::profile_function!(); - - #[cfg(target_os = "windows")] - return setup_app_icon_windows(); - - #[cfg(target_os = "macos")] - return setup_app_icon_mac(); - - #[allow(unreachable_code)] - AppIconStatus::NotSetIgnored -} - -/// Set icon for Windows applications. -#[cfg(target_os = "windows")] -#[allow(unsafe_code)] -fn setup_app_icon_windows() -> AppIconStatus { - use winapi::um::winuser; - - // We would get fairly far already with winit's `set_window_icon` (which is exposed to eframe) actually! - // However, it only sets ICON_SMALL, i.e. doesn't allow us to set a higher resolution icon for the task bar. - // Also, there is scaling issues, detailed below. - - // TODO(andreas): This does not set the task bar icon for when our application is started from python. - // Things tried so far: - // * Querying for an owning window and setting icon there (there doesn't seem to be an owning window) - // * using undocumented SetConsoleIcon method (successfully queried via GetProcAddress) - - // SAFETY: WinApi function without side-effects. - let window_handle = unsafe { winuser::GetActiveWindow() }; - if window_handle.is_null() { - // The Window isn't available yet. Try again later! - return AppIconStatus::NotSetTryAgain; - } - - fn create_hicon_with_scale( - unscaled_image: &image::DynamicImage, - target_size: i32, - ) -> winapi::shared::windef::HICON { - let image_scaled = image::imageops::resize( - unscaled_image, - target_size as _, - target_size as _, - image::imageops::Lanczos3, - ); - - // Creating transparent icons with WinApi is a huge mess. - // We'd need to go through CreateIconIndirect's ICONINFO struct which then - // takes a mask HBITMAP and a color HBITMAP and creating each of these is pain. - // Instead we workaround this by creating a png which CreateIconFromResourceEx magically understands. - // This is a pretty horrible hack as we spend a lot of time encoding and decoding, but at least the code is a lot shorter. - let mut image_scaled_bytes: Vec = Vec::new(); - if image_scaled - .write_to( - &mut std::io::Cursor::new(&mut image_scaled_bytes), - image::ImageOutputFormat::Png, - ) - .is_err() - { - return std::ptr::null_mut(); - } - - // SAFETY: Creating an HICON which should be readonly on our data. - unsafe { - winuser::CreateIconFromResourceEx( - image_scaled_bytes.as_mut_ptr(), - image_scaled_bytes.len() as u32, - 1, // Means this is an icon, not a cursor. - 0x00030000, // Version number of the HICON - target_size, // Note that this method can scale, but it does so *very* poorly. So let's avoid that! - target_size, - winuser::LR_DEFAULTCOLOR, - ) - } - } - - let Ok(unscaled_image) = image::load_from_memory(re_ui::icons::APP_ICON.png_bytes) else { - re_log::debug!("Failed to decode icon png data."); - return AppIconStatus::NotSetIgnored; - }; - - // Only setting ICON_BIG with the icon size for big icons (SM_CXICON) works fine - // but the scaling it does then for the small icon is pretty bad. - // Instead we set the correct sizes manually and take over the scaling ourselves. - // For this to work we first need to set the big icon and then the small one. - // - // Note that ICON_SMALL may be used even if we don't render a title bar as it may be used in alt+tab! - { - // SAFETY: WinAPI getter function with no known side effects. - let icon_size_big = unsafe { winuser::GetSystemMetrics(winuser::SM_CXICON) }; - let icon_big = create_hicon_with_scale(&unscaled_image, icon_size_big); - if icon_big.is_null() { - re_log::debug!("Failed to create HICON (for big icon) from embedded png data."); - return AppIconStatus::NotSetIgnored; // We could try independently with the small icon but what's the point, it would look bad! - } else { - // SAFETY: Unsafe WinApi function, takes objects previously created with WinAPI, all checked for null prior. - unsafe { - winuser::SendMessageW( - window_handle, - winuser::WM_SETICON, - winuser::ICON_BIG as usize, - icon_big as isize, - ); - } - } - } - { - // SAFETY: WinAPI getter function with no known side effects. - let icon_size_small = unsafe { winuser::GetSystemMetrics(winuser::SM_CXSMICON) }; - let icon_small = create_hicon_with_scale(&unscaled_image, icon_size_small); - if icon_small.is_null() { - re_log::debug!("Failed to create HICON (for small icon) from embedded png data."); - return AppIconStatus::NotSetIgnored; - } else { - // SAFETY: Unsafe WinApi function, takes objects previously created with WinAPI, all checked for null prior. - unsafe { - winuser::SendMessageW( - window_handle, - winuser::WM_SETICON, - winuser::ICON_SMALL as usize, - icon_small as isize, - ); - } - } - } - - // It _probably_ worked out. - AppIconStatus::Set -} - -/// Set icon & app title for `MacOS` applications. -#[cfg(target_os = "macos")] -#[allow(unsafe_code)] -fn setup_app_icon_mac() -> AppIconStatus { - use cocoa::{ - appkit::{NSApp, NSApplication, NSImage, NSMenu, NSWindow}, - base::{id, nil}, - foundation::{NSData, NSString}, - }; - use objc::{msg_send, sel, sel_impl}; - - let icon_data = &re_ui::icons::APP_ICON.png_bytes; - - // SAFETY: Accessing raw data from icon in a read-only manner. Icon data is static! - unsafe { - let app = NSApp(); - let data = NSData::dataWithBytes_length_( - nil, - icon_data.as_ptr().cast::(), - icon_data.len() as u64, - ); - let app_icon = NSImage::initWithData_(NSImage::alloc(nil), data); - app.setApplicationIconImage_(app_icon); - - // Change the title in the top bar - for python processes this would be again "python" otherwise. - let main_menu = app.mainMenu(); - let app_menu: id = msg_send![main_menu.itemAtIndex_(0), submenu]; - app_menu.setTitle_(NSString::alloc(nil).init_str(crate::APPLICATION_NAME)); - - // The title in the Dock apparently can't be changed. - // At least these people didn't figure it out either: - // https://stackoverflow.com/questions/69831167/qt-change-application-title-dynamically-on-macos - // https://stackoverflow.com/questions/28808226/changing-cocoa-app-icon-title-and-menu-labels-at-runtime - } - - AppIconStatus::Set -} diff --git a/crates/re_viewer/src/lib.rs b/crates/re_viewer/src/lib.rs index 3d762465ef03..0a9fc02add05 100644 --- a/crates/re_viewer/src/lib.rs +++ b/crates/re_viewer/src/lib.rs @@ -33,8 +33,6 @@ mod native; #[cfg(not(target_arch = "wasm32"))] pub use native::{run_native_app, run_native_viewer_with_messages}; -mod app_icon; - #[cfg(not(target_arch = "wasm32"))] pub use misc::profiler::Profiler; diff --git a/crates/re_viewer/src/native.rs b/crates/re_viewer/src/native.rs index c365c5829dc3..f174e3427f8d 100644 --- a/crates/re_viewer/src/native.rs +++ b/crates/re_viewer/src/native.rs @@ -11,6 +11,8 @@ pub fn run_native_app(app_creator: AppCreator) -> eframe::Result<()> { initial_window_size: Some([1600.0, 1200.0].into()), min_window_size: Some([320.0, 450.0].into()), // Should be high enough to fit the rerun menu + icon_data: icon_data(), + #[cfg(target_os = "macos")] fullsize_content: re_ui::FULLSIZE_CONTENT, @@ -40,6 +42,24 @@ pub fn run_native_app(app_creator: AppCreator) -> eframe::Result<()> { ) } +#[allow(clippy::unnecessary_wraps)] +fn icon_data() -> Option { + // We include the .png with `include_bytes`. If that fails, things are extremely broken. + match eframe::IconData::try_from_png_bytes(re_ui::icons::APP_ICON.png_bytes) { + Ok(icon_data) => Some(icon_data), + Err(err) => { + #[cfg(debug_assertions)] + panic!("Failed to load app icon: {err}"); + + #[cfg(not(debug_assertions))] + { + re_log::warn!("Failed to load app icon: {err}"); + None + } + } + } +} + pub fn run_native_viewer_with_messages( build_info: re_build_info::BuildInfo, app_env: crate::AppEnvironment, From dff74b3376a88be8f10b65fc524d3f1d85a47d4f Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Wed, 26 Apr 2023 15:46:36 +0200 Subject: [PATCH 2/2] Fix linux build --- Cargo.lock | 7 ++++--- Cargo.toml | 1 + crates/re_ui/src/icons.rs | 11 ----------- crates/re_viewer/Cargo.toml | 1 + crates/re_viewer/src/native.rs | 13 ++++++++++++- 5 files changed, 18 insertions(+), 15 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f112633a3352..ed0120052432 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4148,6 +4148,7 @@ dependencies = [ "anyhow", "arboard", "bytemuck", + "cfg-if", "cocoa", "console_error_panic_hook", "eframe", @@ -5840,7 +5841,7 @@ dependencies = [ "js-sys", "log", "naga", - "parking_lot 0.11.2", + "parking_lot 0.12.1", "profiling", "raw-window-handle", "smallvec", @@ -5864,7 +5865,7 @@ dependencies = [ "codespan-reporting", "log", "naga", - "parking_lot 0.11.2", + "parking_lot 0.12.1", "profiling", "raw-window-handle", "rustc-hash", @@ -5902,7 +5903,7 @@ dependencies = [ "metal", "naga", "objc", - "parking_lot 0.11.2", + "parking_lot 0.12.1", "profiling", "range-alloc", "raw-window-handle", diff --git a/Cargo.toml b/Cargo.toml index 955953fc76c3..b5c35e7a5280 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -54,6 +54,7 @@ ahash = "0.8" anyhow = "1.0" arrow2 = "0.16" arrow2_convert = "0.4.2" +cfg-if = "1.0" clap = "4.0" comfy-table = { version = "6.1", default-features = false } ctrlc = { version = "3.0", features = ["termination"] } diff --git a/crates/re_ui/src/icons.rs b/crates/re_ui/src/icons.rs index ecd81778f537..001432ee9f38 100644 --- a/crates/re_ui/src/icons.rs +++ b/crates/re_ui/src/icons.rs @@ -12,17 +12,6 @@ impl Icon { } } -#[cfg(target_os = "macos")] -pub const APP_ICON: Icon = Icon::new( - "app_icon_mac", - include_bytes!("../data/icons/app_icon_mac.png"), -); -#[cfg(target_os = "windows")] -pub const APP_ICON: Icon = Icon::new( - "app_icon_windows", - include_bytes!("../data/icons/app_icon_windows.png"), -); - pub const RERUN_MENU: Icon = Icon::new("rerun_menu", include_bytes!("../data/icons/rerun_menu.png")); diff --git a/crates/re_viewer/Cargo.toml b/crates/re_viewer/Cargo.toml index a0e65d963311..b2a4c81109a4 100644 --- a/crates/re_viewer/Cargo.toml +++ b/crates/re_viewer/Cargo.toml @@ -67,6 +67,7 @@ re_analytics = { workspace = true, optional = true } ahash.workspace = true anyhow.workspace = true bytemuck = { version = "1.11", features = ["extern_crate_alloc"] } +cfg-if.workspace = true eframe = { workspace = true, default-features = false, features = [ "default_fonts", "persistence", diff --git a/crates/re_viewer/src/native.rs b/crates/re_viewer/src/native.rs index f174e3427f8d..90bedf2f8814 100644 --- a/crates/re_viewer/src/native.rs +++ b/crates/re_viewer/src/native.rs @@ -44,8 +44,19 @@ pub fn run_native_app(app_creator: AppCreator) -> eframe::Result<()> { #[allow(clippy::unnecessary_wraps)] fn icon_data() -> Option { + cfg_if::cfg_if! { + if #[cfg(macos)] { + let app_icon_png_bytes = include_bytes!("../../re_ui/data/icons/app_icon_mac.png"); + } else if #[cfg(windows)] { + let app_icon_png_bytes = include_bytes!("../../re_ui/data/icons/app_icon_windows.png"); + } else { + // Use the same icon for X11 as for Windows, at least for now. + let app_icon_png_bytes = include_bytes!("../../re_ui/data/icons/app_icon_windows.png"); + } + }; + // We include the .png with `include_bytes`. If that fails, things are extremely broken. - match eframe::IconData::try_from_png_bytes(re_ui::icons::APP_ICON.png_bytes) { + match eframe::IconData::try_from_png_bytes(app_icon_png_bytes) { Ok(icon_data) => Some(icon_data), Err(err) => { #[cfg(debug_assertions)]