From 21835c31760f0c3e03d0d3db1f44b00ecb955734 Mon Sep 17 00:00:00 2001 From: rustbasic <127506429+rustbasic@users.noreply.github.com> Date: Sun, 31 Mar 2024 04:09:28 +0900 Subject: [PATCH] Fix `ViewportCommand::InnerSize` not resizing viewport on Wayland (#4211) --- crates/eframe/src/native/glow_integration.rs | 93 ++++++------ crates/eframe/src/native/wgpu_integration.rs | 149 +++++++++++-------- crates/egui-winit/src/lib.rs | 116 ++++++++------- 3 files changed, 198 insertions(+), 160 deletions(-) diff --git a/crates/eframe/src/native/glow_integration.rs b/crates/eframe/src/native/glow_integration.rs index 65e2094b886..823e0b0201a 100644 --- a/crates/eframe/src/native/glow_integration.rs +++ b/crates/eframe/src/native/glow_integration.rs @@ -23,8 +23,7 @@ use winit::{ use egui::{ epaint::ahash::HashMap, DeferredViewportUiCallback, ImmediateViewport, ViewportBuilder, - ViewportClass, ViewportId, ViewportIdMap, ViewportIdPair, ViewportIdSet, ViewportInfo, - ViewportOutput, + ViewportClass, ViewportId, ViewportIdMap, ViewportIdPair, ViewportInfo, ViewportOutput, }; #[cfg(feature = "accesskit")] use egui_winit::accesskit_winit; @@ -103,6 +102,7 @@ struct Viewport { ids: ViewportIdPair, class: ViewportClass, builder: ViewportBuilder, + deferred_commands: Vec, info: ViewportInfo, screenshot_requested: bool, @@ -554,7 +554,7 @@ impl GlowWinitRunning { let Some(window) = viewport.window.as_ref() else { return EventResult::Wait; }; - egui_winit::update_viewport_info(&mut viewport.info, &egui_ctx, window); + egui_winit::update_viewport_info(&mut viewport.info, &egui_ctx, window, false); let Some(egui_winit) = viewport.egui_winit.as_mut() else { return EventResult::Wait; @@ -640,6 +640,8 @@ impl GlowWinitRunning { viewport_output, } = full_output; + glutin.remove_viewports_not_in(&viewport_output); + let GlutinWindowContext { viewports, current_gl_context, @@ -649,6 +651,7 @@ impl GlowWinitRunning { let Some(viewport) = viewports.get_mut(&viewport_id) else { return EventResult::Wait; }; + viewport.info.events.clear(); // they should have been processed let window = viewport.window.clone().unwrap(); let gl_surface = viewport.gl_surface.as_ref().unwrap(); @@ -715,7 +718,7 @@ impl GlowWinitRunning { } } - glutin.handle_viewport_output(event_loop, &integration.egui_ctx, viewport_output); + glutin.handle_viewport_output(event_loop, &integration.egui_ctx, &viewport_output); integration.report_frame_time(frame_timer.total_time_sec()); // don't count auto-save time as part of regular frame time @@ -998,8 +1001,7 @@ impl GlutinWindowContext { if let Some(window) = &window { viewport_from_window.insert(window.id(), ViewportId::ROOT); window_from_viewport.insert(ViewportId::ROOT, window.id()); - info.minimized = window.is_minimized(); - info.maximized = Some(window.is_maximized()); + egui_winit::update_viewport_info(&mut info, egui_ctx, window, true); } let mut viewports = ViewportIdMap::default(); @@ -1009,6 +1011,7 @@ impl GlutinWindowContext { ids: ViewportIdPair::ROOT, class: ViewportClass::Root, builder: viewport_builder, + deferred_commands: vec![], info, screenshot_requested: false, viewport_ui_cb: None, @@ -1090,8 +1093,8 @@ impl GlutinWindowContext { &window, &viewport.builder, ); - viewport.info.minimized = window.is_minimized(); - viewport.info.maximized = Some(window.is_maximized()); + + egui_winit::update_viewport_info(&mut viewport.info, &self.egui_ctx, &window, true); viewport.window.insert(Arc::new(window)) }; @@ -1212,16 +1215,27 @@ impl GlutinWindowContext { self.gl_config.display().get_proc_address(addr) } + pub(crate) fn remove_viewports_not_in( + &mut self, + viewport_output: &ViewportIdMap, + ) { + // GC old viewports + self.viewports + .retain(|id, _| viewport_output.contains_key(id)); + self.viewport_from_window + .retain(|_, id| viewport_output.contains_key(id)); + self.window_from_viewport + .retain(|id, _| viewport_output.contains_key(id)); + } + fn handle_viewport_output( &mut self, event_loop: &EventLoopWindowTarget, egui_ctx: &egui::Context, - viewport_output: ViewportIdMap, + viewport_output: &ViewportIdMap, ) { crate::profile_function!(); - let active_viewports_ids: ViewportIdSet = viewport_output.keys().copied().collect(); - for ( viewport_id, ViewportOutput { @@ -1229,58 +1243,60 @@ impl GlutinWindowContext { class, builder, viewport_ui_cb, - commands, + mut commands, repaint_delay: _, // ignored - we listened to the repaint callback instead }, - ) in viewport_output + ) in viewport_output.clone() { let ids = ViewportIdPair::from_self_and_parent(viewport_id, parent); let viewport = initialize_or_update_viewport( - egui_ctx, &mut self.viewports, ids, class, builder, viewport_ui_cb, - self.focused_viewport, ); if let Some(window) = &viewport.window { + let old_inner_size = window.inner_size(); + let is_viewport_focused = self.focused_viewport == Some(viewport_id); + viewport.deferred_commands.append(&mut commands); + egui_winit::process_viewport_commands( egui_ctx, &mut viewport.info, - commands, + std::mem::take(&mut viewport.deferred_commands), window, is_viewport_focused, &mut viewport.screenshot_requested, ); + + // For Wayland : https://github.com/emilk/egui/issues/4196 + if cfg!(target_os = "linux") { + let new_inner_size = window.inner_size(); + if new_inner_size != old_inner_size { + self.resize(viewport_id, new_inner_size); + } + } } } // Create windows for any new viewports: self.initialize_all_windows(event_loop); - // GC old viewports - self.viewports - .retain(|id, _| active_viewports_ids.contains(id)); - self.viewport_from_window - .retain(|_, id| active_viewports_ids.contains(id)); - self.window_from_viewport - .retain(|id, _| active_viewports_ids.contains(id)); + self.remove_viewports_not_in(viewport_output); } } -fn initialize_or_update_viewport<'vp>( - egu_ctx: &egui::Context, - viewports: &'vp mut ViewportIdMap, +fn initialize_or_update_viewport( + viewports: &mut ViewportIdMap, ids: ViewportIdPair, class: ViewportClass, mut builder: ViewportBuilder, viewport_ui_cb: Option>, - focused_viewport: Option, -) -> &'vp mut Viewport { +) -> &mut Viewport { crate::profile_function!(); if builder.icon.is_none() { @@ -1298,6 +1314,7 @@ fn initialize_or_update_viewport<'vp>( ids, class, builder, + deferred_commands: vec![], info: Default::default(), screenshot_requested: false, viewport_ui_cb, @@ -1315,7 +1332,7 @@ fn initialize_or_update_viewport<'vp>( viewport.class = class; viewport.viewport_ui_cb = viewport_ui_cb; - let (delta_commands, recreate) = viewport.builder.patch(builder); + let (mut delta_commands, recreate) = viewport.builder.patch(builder); if recreate { log::debug!( @@ -1325,18 +1342,10 @@ fn initialize_or_update_viewport<'vp>( ); viewport.window = None; viewport.egui_winit = None; - } else if let Some(window) = &viewport.window { - let is_viewport_focused = focused_viewport == Some(ids.this); - egui_winit::process_viewport_commands( - egu_ctx, - &mut viewport.info, - delta_commands, - window, - is_viewport_focused, - &mut viewport.screenshot_requested, - ); } + viewport.deferred_commands.append(&mut delta_commands); + entry.into_mut() } } @@ -1366,13 +1375,11 @@ fn render_immediate_viewport( let mut glutin = glutin.borrow_mut(); initialize_or_update_viewport( - egui_ctx, &mut glutin.viewports, ids, ViewportClass::Immediate, builder, None, - None, ); if let Err(err) = glutin.initialize_window(viewport_id, event_loop) { @@ -1392,7 +1399,7 @@ fn render_immediate_viewport( let (Some(egui_winit), Some(window)) = (&mut viewport.egui_winit, &viewport.window) else { return; }; - egui_winit::update_viewport_info(&mut viewport.info, egui_ctx, window); + egui_winit::update_viewport_info(&mut viewport.info, egui_ctx, window, false); let mut raw_input = egui_winit.take_egui_input(window); raw_input.viewports = glutin @@ -1480,7 +1487,7 @@ fn render_immediate_viewport( egui_winit.handle_platform_output(window, platform_output); - glutin.handle_viewport_output(event_loop, egui_ctx, viewport_output); + glutin.handle_viewport_output(event_loop, egui_ctx, &viewport_output); } #[cfg(feature = "__screenshot")] diff --git a/crates/eframe/src/native/wgpu_integration.rs b/crates/eframe/src/native/wgpu_integration.rs index dd4debd8b7a..9e0022489e7 100644 --- a/crates/eframe/src/native/wgpu_integration.rs +++ b/crates/eframe/src/native/wgpu_integration.rs @@ -76,6 +76,7 @@ pub struct Viewport { ids: ViewportIdPair, class: ViewportClass, builder: ViewportBuilder, + deferred_commands: Vec, info: ViewportInfo, screenshot_requested: bool, @@ -154,13 +155,11 @@ impl WgpuWinitApp { } = &mut *running.shared.borrow_mut(); initialize_or_update_viewport( - egui_ctx, viewports, ViewportIdPair::ROOT, ViewportClass::Root, self.native_options.viewport.clone(), None, - None, ) .initialize_window(event_loop, egui_ctx, viewport_from_window, painter); } @@ -278,6 +277,9 @@ impl WgpuWinitApp { let mut viewport_from_window = HashMap::default(); viewport_from_window.insert(window.id(), ViewportId::ROOT); + let mut info = ViewportInfo::default(); + egui_winit::update_viewport_info(&mut info, &egui_ctx, &window, true); + let mut viewports = Viewports::default(); viewports.insert( ViewportId::ROOT, @@ -285,11 +287,8 @@ impl WgpuWinitApp { ids: ViewportIdPair::ROOT, class: ViewportClass::Root, builder, - info: ViewportInfo { - minimized: window.is_minimized(), - maximized: Some(window.is_maximized()), - ..Default::default() - }, + deferred_commands: vec![], + info, screenshot_requested: false, viewport_ui_cb: None, window: Some(window), @@ -603,7 +602,7 @@ impl WgpuWinitRunning { let Some(window) = window else { return EventResult::Wait; }; - egui_winit::update_viewport_info(info, &integration.egui_ctx, window); + egui_winit::update_viewport_info(info, &integration.egui_ctx, window, false); { crate::profile_scope!("set_window"); @@ -638,7 +637,7 @@ impl WgpuWinitRunning { // ------------------------------------------------------------ - let mut shared = shared.borrow_mut(); + let mut shared_mut = shared.borrow_mut(); let SharedState { egui_ctx, @@ -646,7 +645,17 @@ impl WgpuWinitRunning { painter, viewport_from_window, focused_viewport, - } = &mut *shared; + } = &mut *shared_mut; + + let FullOutput { + platform_output, + textures_delta, + shapes, + pixels_per_point, + viewport_output, + } = full_output; + + remove_viewports_not_in(viewports, painter, viewport_from_window, &viewport_output); let Some(viewport) = viewports.get_mut(&viewport_id) else { return EventResult::Wait; @@ -663,14 +672,6 @@ impl WgpuWinitRunning { return EventResult::Wait; }; - let FullOutput { - platform_output, - textures_delta, - shapes, - pixels_per_point, - viewport_output, - } = full_output; - egui_winit.handle_platform_output(window, platform_output); let clipped_primitives = egui_ctx.tessellate(shapes, pixels_per_point); @@ -700,8 +701,10 @@ impl WgpuWinitRunning { handle_viewport_output( &integration.egui_ctx, - viewport_output, + &viewport_output, viewports, + painter, + viewport_from_window, *focused_viewport, ); @@ -876,9 +879,7 @@ impl Viewport { painter.max_texture_side(), )); - self.info.minimized = window.is_minimized(); - self.info.maximized = Some(window.is_maximized()); - + egui_winit::update_viewport_info(&mut self.info, egui_ctx, &window, true); self.window = Some(window); } Err(err) => { @@ -933,15 +934,8 @@ fn render_immediate_viewport( .. } = &mut *shared.borrow_mut(); - let viewport = initialize_or_update_viewport( - egui_ctx, - viewports, - ids, - ViewportClass::Immediate, - builder, - None, - None, - ); + let viewport = + initialize_or_update_viewport(viewports, ids, ViewportClass::Immediate, builder, None); if viewport.window.is_none() { viewport.initialize_window(event_loop, egui_ctx, viewport_from_window, painter); } @@ -949,7 +943,7 @@ fn render_immediate_viewport( let (Some(window), Some(egui_winit)) = (&viewport.window, &mut viewport.egui_winit) else { return; }; - egui_winit::update_viewport_info(&mut viewport.info, egui_ctx, window); + egui_winit::update_viewport_info(&mut viewport.info, egui_ctx, window, false); let mut input = egui_winit.take_egui_input(window); input.viewports = viewports @@ -978,13 +972,14 @@ fn render_immediate_viewport( // ------------------------------------------ - let mut shared = shared.borrow_mut(); + let mut shared_mut = shared.borrow_mut(); let SharedState { viewports, painter, + viewport_from_window, focused_viewport, .. - } = &mut *shared; + } = &mut *shared_mut; let Some(viewport) = viewports.get_mut(&ids.this) else { return; @@ -1016,14 +1011,37 @@ fn render_immediate_viewport( egui_winit.handle_platform_output(window, platform_output); - handle_viewport_output(&egui_ctx, viewport_output, viewports, *focused_viewport); + handle_viewport_output( + &egui_ctx, + &viewport_output, + viewports, + painter, + viewport_from_window, + *focused_viewport, + ); +} + +pub(crate) fn remove_viewports_not_in( + viewports: &mut ViewportIdMap, + painter: &mut egui_wgpu::winit::Painter, + viewport_from_window: &mut HashMap, + viewport_output: &ViewportIdMap, +) { + let active_viewports_ids: ViewportIdSet = viewport_output.keys().copied().collect(); + + // Prune dead viewports: + viewports.retain(|id, _| active_viewports_ids.contains(id)); + viewport_from_window.retain(|_, id| active_viewports_ids.contains(id)); + painter.gc_viewports(&active_viewports_ids); } /// Add new viewports, and update existing ones: fn handle_viewport_output( egui_ctx: &egui::Context, - viewport_output: ViewportIdMap, + viewport_output: &ViewportIdMap, viewports: &mut ViewportIdMap, + painter: &mut egui_wgpu::winit::Painter, + viewport_from_window: &mut HashMap, focused_viewport: Option, ) { for ( @@ -1033,46 +1051,56 @@ fn handle_viewport_output( class, builder, viewport_ui_cb, - commands, + mut commands, repaint_delay: _, // ignored - we listened to the repaint callback instead }, - ) in viewport_output + ) in viewport_output.clone() { let ids = ViewportIdPair::from_self_and_parent(viewport_id, parent); - let viewport = initialize_or_update_viewport( - egui_ctx, - viewports, - ids, - class, - builder, - viewport_ui_cb, - focused_viewport, - ); + let viewport = + initialize_or_update_viewport(viewports, ids, class, builder, viewport_ui_cb); if let Some(window) = viewport.window.as_ref() { + let old_inner_size = window.inner_size(); + let is_viewport_focused = focused_viewport == Some(viewport_id); + viewport.deferred_commands.append(&mut commands); + egui_winit::process_viewport_commands( egui_ctx, &mut viewport.info, - commands, + std::mem::take(&mut viewport.deferred_commands), window, is_viewport_focused, &mut viewport.screenshot_requested, ); + + // For Wayland : https://github.com/emilk/egui/issues/4196 + if cfg!(target_os = "linux") { + let new_inner_size = window.inner_size(); + if new_inner_size != old_inner_size { + if let (Some(width), Some(height)) = ( + NonZeroU32::new(new_inner_size.width), + NonZeroU32::new(new_inner_size.height), + ) { + painter.on_window_resized(viewport_id, width, height); + } + } + } } } + + remove_viewports_not_in(viewports, painter, viewport_from_window, viewport_output); } -fn initialize_or_update_viewport<'vp>( - egui_ctx: &egui::Context, - viewports: &'vp mut Viewports, +fn initialize_or_update_viewport( + viewports: &mut Viewports, ids: ViewportIdPair, class: ViewportClass, mut builder: ViewportBuilder, viewport_ui_cb: Option>, - focused_viewport: Option, -) -> &'vp mut Viewport { +) -> &mut Viewport { crate::profile_function!(); if builder.icon.is_none() { @@ -1090,6 +1118,7 @@ fn initialize_or_update_viewport<'vp>( ids, class, builder, + deferred_commands: vec![], info: Default::default(), screenshot_requested: false, viewport_ui_cb, @@ -1106,7 +1135,7 @@ fn initialize_or_update_viewport<'vp>( viewport.ids.parent = ids.parent; viewport.viewport_ui_cb = viewport_ui_cb; - let (delta_commands, recreate) = viewport.builder.patch(builder); + let (mut delta_commands, recreate) = viewport.builder.patch(builder); if recreate { log::debug!( @@ -1116,18 +1145,10 @@ fn initialize_or_update_viewport<'vp>( ); viewport.window = None; viewport.egui_winit = None; - } else if let Some(window) = &viewport.window { - let is_viewport_focused = focused_viewport == Some(ids.this); - egui_winit::process_viewport_commands( - egui_ctx, - &mut viewport.info, - delta_commands, - window, - is_viewport_focused, - &mut viewport.screenshot_requested, - ); } + viewport.deferred_commands.append(&mut delta_commands); + entry.into_mut() } } diff --git a/crates/egui-winit/src/lib.rs b/crates/egui-winit/src/lib.rs index 3d8e9184fdc..5abfa0f6898 100644 --- a/crates/egui-winit/src/lib.rs +++ b/crates/egui-winit/src/lib.rs @@ -874,70 +874,62 @@ impl State { } } +pub fn inner_rect_in_points(window: &Window, pixels_per_point: f32) -> Option { + let inner_pos_px = window.inner_position().ok()?; + let inner_pos_px = egui::pos2(inner_pos_px.x as f32, inner_pos_px.y as f32); + + let inner_size_px = window.inner_size(); + let inner_size_px = egui::vec2(inner_size_px.width as f32, inner_size_px.height as f32); + + let inner_rect_px = egui::Rect::from_min_size(inner_pos_px, inner_size_px); + + Some(inner_rect_px / pixels_per_point) +} + +pub fn outer_rect_in_points(window: &Window, pixels_per_point: f32) -> Option { + let outer_pos_px = window.outer_position().ok()?; + let outer_pos_px = egui::pos2(outer_pos_px.x as f32, outer_pos_px.y as f32); + + let outer_size_px = window.outer_size(); + let outer_size_px = egui::vec2(outer_size_px.width as f32, outer_size_px.height as f32); + + let outer_rect_px = egui::Rect::from_min_size(outer_pos_px, outer_size_px); + + Some(outer_rect_px / pixels_per_point) +} + /// Update the given viewport info with the current state of the window. /// /// Call before [`State::take_egui_input`]. +/// +/// If this is called right after window creation, `is_init` should be `true`, otherwise `false`. pub fn update_viewport_info( viewport_info: &mut ViewportInfo, egui_ctx: &egui::Context, window: &Window, + is_init: bool, ) { crate::profile_function!(); let pixels_per_point = pixels_per_point(egui_ctx, window); let has_a_position = match window.is_minimized() { - None | Some(true) => false, - Some(false) => true, - }; - - let inner_pos_px = if has_a_position { - window - .inner_position() - .map(|pos| Pos2::new(pos.x as f32, pos.y as f32)) - .ok() - } else { - None - }; - - let outer_pos_px = if has_a_position { - window - .outer_position() - .map(|pos| Pos2::new(pos.x as f32, pos.y as f32)) - .ok() - } else { - None - }; - - let inner_size_px = if has_a_position { - let size = window.inner_size(); - Some(Vec2::new(size.width as f32, size.height as f32)) - } else { - None - }; - - let outer_size_px = if has_a_position { - let size = window.outer_size(); - Some(Vec2::new(size.width as f32, size.height as f32)) - } else { - None + Some(true) => false, + Some(false) | None => true, }; - let inner_rect_px = if let (Some(pos), Some(size)) = (inner_pos_px, inner_size_px) { - Some(Rect::from_min_size(pos, size)) + let inner_rect = if has_a_position { + inner_rect_in_points(window, pixels_per_point) } else { None }; - let outer_rect_px = if let (Some(pos), Some(size)) = (outer_pos_px, outer_size_px) { - Some(Rect::from_min_size(pos, size)) + let outer_rect = if has_a_position { + outer_rect_in_points(window, pixels_per_point) } else { None }; - let inner_rect = inner_rect_px.map(|r| r / pixels_per_point); - let outer_rect = outer_rect_px.map(|r| r / pixels_per_point); - let monitor_size = { crate::profile_scope!("monitor_size"); if let Some(monitor) = window.current_monitor() { @@ -948,21 +940,23 @@ pub fn update_viewport_info( } }; - viewport_info.focused = Some(window.has_focus()); - viewport_info.fullscreen = Some(window.fullscreen().is_some()); - viewport_info.inner_rect = inner_rect; - viewport_info.monitor_size = monitor_size; + viewport_info.title = Some(window.title()); viewport_info.native_pixels_per_point = Some(window.scale_factor() as f32); + + viewport_info.monitor_size = monitor_size; + viewport_info.inner_rect = inner_rect; viewport_info.outer_rect = outer_rect; - viewport_info.title = Some(window.title()); - if cfg!(target_os = "windows") { - // It's tempting to do this, but it leads to a deadlock on Mac when running + if is_init || !cfg!(target_os = "macos") { + // Asking for minimized/maximized state at runtime leads to a deadlock on Mac when running // `cargo run -p custom_window_frame`. // See https://github.com/emilk/egui/issues/3494 viewport_info.maximized = Some(window.is_maximized()); viewport_info.minimized = Some(window.is_minimized().unwrap_or(false)); } + + viewport_info.fullscreen = Some(window.fullscreen().is_some()); + viewport_info.focused = Some(window.has_focus()); } fn open_url_in_browser(_url: &str) { @@ -1319,11 +1313,27 @@ fn process_viewport_command( ViewportCommand::InnerSize(size) => { let width_px = pixels_per_point * size.x.max(1.0); let height_px = pixels_per_point * size.y.max(1.0); - if window - .request_inner_size(PhysicalSize::new(width_px, height_px)) - .is_some() - { - log::debug!("ViewportCommand::InnerSize ignored by winit"); + let requested_size = PhysicalSize::new(width_px, height_px); + if let Some(_returned_inner_size) = window.request_inner_size(requested_size) { + // On platforms where the size is entirely controlled by the user the + // applied size will be returned immediately, resize event in such case + // may not be generated. + // e.g. Linux + + // On platforms where resizing is disallowed by the windowing system, the current + // inner size is returned immediately, and the user one is ignored. + // e.g. Android, iOS, … + + // However, comparing the results is prone to numerical errors + // because the linux backend converts physical to logical and back again. + // So let's just assume it worked: + + info.inner_rect = inner_rect_in_points(window, pixels_per_point); + info.outer_rect = outer_rect_in_points(window, pixels_per_point); + } else { + // e.g. macOS, Windows + // The request went to the display system, + // and the actual size will be delivered later with the [`WindowEvent::Resized`]. } } ViewportCommand::BeginResize(direction) => {