From 853feea46480266cf6147159b4fe35abf695e59b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Max=20=E2=80=9CGoldstein=E2=80=9D=20Siling?= Date: Tue, 24 Jun 2025 14:44:56 +0300 Subject: [PATCH] Fix incorrect window sizes for non-resizable windows on Wayland (#7103) Using physical window sizes leads to all kinds of fun stuff: winit always uses scale factor 1.0 on start to convert it back to logical pixels and uses these logical pixels to set min/max size for non-resizeable windows. You're supposed to adjust size after getting a scale change event if you're using physical sizes, but adjusting min/max sizes doesn't seem to work on sway, so the window is stuck with an incorrect size. The scale factor we guessed might also be wrong even if there's only a single display since it doesn't take fractional scale into account. TL;DR: winit actually wants logical sizes in these methods (since Wayland in general operates mostly on logical sizes) and converting them back and forth is lossy. * Closes https://github.com/emilk/egui/issues/7095 * [x] I have followed the instructions in the PR template --------- Co-authored-by: Emil Ernerfeldt --- crates/eframe/src/native/glow_integration.rs | 2 - crates/egui-winit/src/lib.rs | 79 ++++++++------------ 2 files changed, 33 insertions(+), 48 deletions(-) diff --git a/crates/eframe/src/native/glow_integration.rs b/crates/eframe/src/native/glow_integration.rs index 946210c8..15ade4a3 100644 --- a/crates/eframe/src/native/glow_integration.rs +++ b/crates/eframe/src/native/glow_integration.rs @@ -959,7 +959,6 @@ impl GlutinWindowContext { .with_preference(glutin_winit::ApiPreference::FallbackEgl) .with_window_attributes(Some(egui_winit::create_winit_window_attributes( egui_ctx, - event_loop, viewport_builder.clone(), ))); @@ -1113,7 +1112,6 @@ impl GlutinWindowContext { log::debug!("Creating a window for viewport {viewport_id:?}"); let window_attributes = egui_winit::create_winit_window_attributes( &self.egui_ctx, - event_loop, viewport.builder.clone(), ); if window_attributes.transparent() diff --git a/crates/egui-winit/src/lib.rs b/crates/egui-winit/src/lib.rs index f205c310..b07d47b0 100644 --- a/crates/egui-winit/src/lib.rs +++ b/crates/egui-winit/src/lib.rs @@ -1566,8 +1566,7 @@ pub fn create_window( ) -> Result { profiling::function_scope!(); - let window_attributes = - create_winit_window_attributes(egui_ctx, event_loop, viewport_builder.clone()); + let window_attributes = create_winit_window_attributes(egui_ctx, viewport_builder.clone()); let window = event_loop.create_window(window_attributes)?; apply_viewport_builder_to_window(egui_ctx, &window, viewport_builder); Ok(window) @@ -1575,28 +1574,10 @@ pub fn create_window( pub fn create_winit_window_attributes( egui_ctx: &egui::Context, - event_loop: &ActiveEventLoop, viewport_builder: ViewportBuilder, ) -> winit::window::WindowAttributes { profiling::function_scope!(); - // We set sizes and positions in egui:s own ui points, which depends on the egui - // zoom_factor and the native pixels per point, so we need to know that here. - // We don't know what monitor the window will appear on though, but - // we'll try to fix that after the window is created in the call to `apply_viewport_builder_to_window`. - let native_pixels_per_point = event_loop - .primary_monitor() - .or_else(|| event_loop.available_monitors().next()) - .map_or_else( - || { - log::debug!("Failed to find a monitor - assuming native_pixels_per_point of 1.0"); - 1.0 - }, - |m| m.scale_factor() as f32, - ); - let zoom_factor = egui_ctx.zoom_factor(); - let pixels_per_point = zoom_factor * native_pixels_per_point; - let ViewportBuilder { title, position, @@ -1672,40 +1653,46 @@ pub fn create_winit_window_attributes( }) .with_active(active.unwrap_or(true)); + // Here and below: we create `LogicalSize` / `LogicalPosition` taking + // zoom factor into account. We don't have a good way to get physical size here, + // and trying to do it anyway leads to weird bugs on Wayland, see: + // https://github.com/emilk/egui/issues/7095#issuecomment-2920545377 + // https://github.com/rust-windowing/winit/issues/4266 + #[expect( + clippy::disallowed_types, + reason = "zoom factor is manually accounted for" + )] #[cfg(not(target_os = "ios"))] - if let Some(size) = inner_size { - window_attributes = window_attributes.with_inner_size(PhysicalSize::new( - pixels_per_point * size.x, - pixels_per_point * size.y, - )); - } + { + use winit::dpi::{LogicalPosition, LogicalSize}; + let zoom_factor = egui_ctx.zoom_factor(); - #[cfg(not(target_os = "ios"))] - if let Some(size) = min_inner_size { - window_attributes = window_attributes.with_min_inner_size(PhysicalSize::new( - pixels_per_point * size.x, - pixels_per_point * size.y, - )); - } + if let Some(size) = inner_size { + window_attributes = window_attributes + .with_inner_size(LogicalSize::new(zoom_factor * size.x, zoom_factor * size.y)); + } - #[cfg(not(target_os = "ios"))] - if let Some(size) = max_inner_size { - window_attributes = window_attributes.with_max_inner_size(PhysicalSize::new( - pixels_per_point * size.x, - pixels_per_point * size.y, - )); - } + if let Some(size) = min_inner_size { + window_attributes = window_attributes + .with_min_inner_size(LogicalSize::new(zoom_factor * size.x, zoom_factor * size.y)); + } - #[cfg(not(target_os = "ios"))] - if let Some(pos) = position { - window_attributes = window_attributes.with_position(PhysicalPosition::new( - pixels_per_point * pos.x, - pixels_per_point * pos.y, - )); + if let Some(size) = max_inner_size { + window_attributes = window_attributes + .with_max_inner_size(LogicalSize::new(zoom_factor * size.x, zoom_factor * size.y)); + } + + if let Some(pos) = position { + window_attributes = window_attributes.with_position(LogicalPosition::new( + zoom_factor * pos.x, + zoom_factor * pos.y, + )); + } } #[cfg(target_os = "ios")] { // Unused: + _ = egui_ctx; _ = pixels_per_point; _ = position; _ = inner_size;