eframe: Improve window centering (#2651)

* Create the winit::Window in one unified place

* Fix wrong unit of initial_window_pos

* Improve centering

crates/eframe/src/native/epi_integration.rs

@@ -49,12 +49,12 @@ pub fn read_window_info(window: &winit::window::Window, pixels_per_point: f32) -
     }
 }
 
-pub fn window_builder<E>(
+pub fn build_window<E>(
     event_loop: &EventLoopWindowTarget<E>,
     title: &str,
     native_options: &epi::NativeOptions,
     window_settings: Option<WindowSettings>,
-) -> winit::window::WindowBuilder {
+) -> Result<winit::window::Window, winit::error::OsError> {
     let epi::NativeOptions {
         always_on_top,
         maximized,
@@ -70,6 +70,7 @@ pub fn window_builder<E>(
         max_window_size,
         resizable,
         transparent,
+        centered,
         ..
     } = native_options;
 
@@ -105,13 +106,14 @@ pub fn window_builder<E>(
 
     window_builder = window_builder_drag_and_drop(window_builder, *drag_and_drop_support);
 
-    if let Some(mut window_settings) = window_settings {
+    let inner_size_points = if let Some(mut window_settings) = window_settings {
         // Restore pos/size from previous session
         window_settings.clamp_to_sane_values(largest_monitor_point_size(event_loop));
         window_builder = window_settings.initialize_window(window_builder);
+        window_settings.inner_size_points()
     } else {
         if let Some(pos) = *initial_window_pos {
-            window_builder = window_builder.with_position(winit::dpi::PhysicalPosition {
+            window_builder = window_builder.with_position(winit::dpi::LogicalPosition {
                 x: pos.x as f64,
                 y: pos.y as f64,
             });
@@ -122,9 +124,26 @@ pub fn window_builder<E>(
                 initial_window_size.at_most(largest_monitor_point_size(event_loop));
             window_builder = window_builder.with_inner_size(points_to_size(initial_window_size));
         }
+
+        *initial_window_size
+    };
+
+    if *centered {
+        if let Some(monitor) = event_loop.available_monitors().next() {
+            let monitor_size = monitor.size();
+            let inner_size = inner_size_points.unwrap_or(egui::Vec2 { x: 800.0, y: 600.0 });
+            if monitor_size.width > 0 && monitor_size.height > 0 {
+                let x = (monitor_size.width - inner_size.x as u32) / 2;
+                let y = (monitor_size.height - inner_size.y as u32) / 2;
+                window_builder = window_builder.with_position(winit::dpi::LogicalPosition {
+                    x: x as f64,
+                    y: y as f64,
+                });
+            }
+        }
     }
 
-    window_builder
+    window_builder.build(event_loop)
 }
 
 fn largest_monitor_point_size<E>(event_loop: &EventLoopWindowTarget<E>) -> egui::Vec2 {
@@ -153,7 +172,7 @@ fn window_builder_drag_and_drop(
     enable: bool,
 ) -> winit::window::WindowBuilder {
     use winit::platform::windows::WindowBuilderExtWindows as _;
-    window_builder.with_drag_and_drop(enable)
+    build_window.with_drag_and_drop(enable)
 }
 
 #[cfg(not(target_os = "windows"))]

crates/eframe/src/native/run.rs

@@ -290,27 +290,6 @@ fn run_and_exit(event_loop: EventLoop<UserEvent>, mut winit_app: impl WinitApp +
     })
 }
 
-fn center_window_pos(
-    monitor: Option<winit::monitor::MonitorHandle>,
-    native_options: &mut epi::NativeOptions,
-) {
-    // Get the current_monitor.
-    if let Some(monitor) = monitor {
-        let monitor_size = monitor.size();
-        let inner_size = native_options
-            .initial_window_size
-            .unwrap_or(egui::Vec2 { x: 800.0, y: 600.0 });
-        if monitor_size.width > 0 && monitor_size.height > 0 {
-            let x = (monitor_size.width - inner_size.x as u32) / 2;
-            let y = (monitor_size.height - inner_size.y as u32) / 2;
-            native_options.initial_window_pos = Some(egui::Pos2 {
-                x: x as _,
-                y: y as _,
-            });
-        }
-    }
-}
-
 // ----------------------------------------------------------------------------
 /// Run an egui app
 #[cfg(feature = "glow")]
@@ -516,8 +495,7 @@ mod glow_integration {
             let window_settings = epi_integration::load_window_settings(storage);
 
             let winit_window =
-                epi_integration::window_builder(event_loop, title, native_options, window_settings)
-                    .build(event_loop)?;
+                epi_integration::build_window(event_loop, title, native_options, window_settings)?;
             // a lot of the code below has been lifted from glutin example in their repo.
             let glutin_window_context =
                 unsafe { GlutinWindowContext::new(winit_window, native_options)? };
@@ -863,22 +841,13 @@ mod glow_integration {
         app_creator: epi::AppCreator,
     ) -> Result<()> {
         if native_options.run_and_return {
-            with_event_loop(native_options, |event_loop, mut native_options| {
-                if native_options.centered {
-                    center_window_pos(event_loop.available_monitors().next(), &mut native_options);
-                }
-
+            with_event_loop(native_options, |event_loop, native_options| {
                 let glow_eframe =
                     GlowWinitApp::new(event_loop, app_name, native_options, app_creator);
                 run_and_return(event_loop, glow_eframe)
             })
         } else {
             let event_loop = create_event_loop_builder(&mut native_options).build();
-
-            if native_options.centered {
-                center_window_pos(event_loop.available_monitors().next(), &mut native_options);
-            }
-
             let glow_eframe = GlowWinitApp::new(&event_loop, app_name, native_options, app_creator);
             run_and_exit(event_loop, glow_eframe);
         }
@@ -946,12 +915,9 @@ mod wgpu_integration {
             storage: Option<&dyn epi::Storage>,
             title: &str,
             native_options: &NativeOptions,
-        ) -> Result<winit::window::Window> {
+        ) -> std::result::Result<winit::window::Window, winit::error::OsError> {
             let window_settings = epi_integration::load_window_settings(storage);
-            Ok(
-                epi_integration::window_builder(event_loop, title, native_options, window_settings)
-                    .build(event_loop)?,
-            )
+            epi_integration::build_window(event_loop, title, native_options, window_settings)
         }
 
         #[allow(unsafe_code)]
@@ -1286,22 +1252,13 @@ mod wgpu_integration {
         app_creator: epi::AppCreator,
     ) -> Result<()> {
         if native_options.run_and_return {
-            with_event_loop(native_options, |event_loop, mut native_options| {
-                if native_options.centered {
-                    center_window_pos(event_loop.available_monitors().next(), &mut native_options);
-                }
-
+            with_event_loop(native_options, |event_loop, native_options| {
                 let wgpu_eframe =
                     WgpuWinitApp::new(event_loop, app_name, native_options, app_creator);
                 run_and_return(event_loop, wgpu_eframe)
             })
         } else {
             let event_loop = create_event_loop_builder(&mut native_options).build();
-
-            if native_options.centered {
-                center_window_pos(event_loop.available_monitors().next(), &mut native_options);
-            }
-
             let wgpu_eframe = WgpuWinitApp::new(&event_loop, app_name, native_options, app_creator);
             run_and_exit(event_loop, wgpu_eframe);
         }

crates/egui-winit/src/window_settings.rs

@@ -42,6 +42,10 @@ impl WindowSettings {
         }
     }
 
+    pub fn inner_size_points(&self) -> Option<egui::Vec2> {
+        self.inner_size_points
+    }
+
     pub fn initialize_window(
         &self,
         mut window: winit::window::WindowBuilder,