egui-winit: Receive window scale factor and size via events

We found in our application (with custom `egui` renderer) that the
`screen_rect` in `RawInput` was mismatching with the `winit` events that
we processed earlier (in particular on resize on MacOS, which raises a
lot of small size changes).

One of the contributing factors was `egui-winit` reading the `Window`
size and scale factor over and over again separate from processed
events, causing a mismatch on some platforms because this value would
already change ahead of consuming a `Resized` event.  Secondly, some
existing event processing may rely on this value being consistent with
the event stream.

There is currently no guarantee -at least I don't see it in the
documentation- that `winit` ensures that `inner_size()` will always
return a consistent value until the user has consumed a `Resized` event.
This gets worse when certain functions (like `update_viewport_info()`)
are possibly called on a separate thread because both `egui::Context`
and `winit::Window` are thread-safe.

Those also have an extra draw-back of roundtripping to the event loop
on the main thread to query these values, as a `Window` is not actually
thread-safe on all platforms.

Author: MarijnS95

crates/eframe/src/native/glow_integration.rs

@@ -1049,6 +1049,7 @@ impl GlutinWindowContext {
 
             // Tell egui right away about native_pixels_per_point etc,
             // so that the app knows about it during app creation:
+            // TODO: update_viewport_info could take care of this right?
             let pixels_per_point = egui_winit::pixels_per_point(egui_ctx, window);
 
             egui_ctx.input_mut(|i| {

crates/eframe/src/native/wgpu_integration.rs

@@ -205,6 +205,7 @@ impl<'app> WgpuWinitApp<'app> {
         {
             // Tell egui right away about native_pixels_per_point etc,
             // so that the app knows about it during app creation:
+            // TODO: update_viewport_info could take care of this right?
             let pixels_per_point = egui_winit::pixels_per_point(&egui_ctx, &window);
 
             egui_ctx.input_mut(|i| {

crates/egui-winit/src/lib.rs

@@ -32,19 +32,9 @@ use winit::{
     window::{CursorGrabMode, Window, WindowButtons, WindowLevel},
 };
 
-pub fn screen_size_in_pixels(window: &Window) -> egui::Vec2 {
-    let size = if cfg!(target_os = "ios") {
-        // `outer_size` Includes the area behind the "dynamic island".
-        // It is up to the eframe user to make sure the dynamic island doesn't cover anything important.
-        // That will be easier once https://github.com/rust-windowing/winit/pull/3890 lands
-        window.outer_size()
-    } else {
-        window.inner_size()
-    };
-    egui::vec2(size.width as f32, size.height as f32)
-}
-
 /// Calculate the `pixels_per_point` for a given window, given the current egui zoom factor
+///
+/// Only use this during startup when a [`WindowEvent::ScaleFactorChanged`] was not yet delivered!
 pub fn pixels_per_point(egui_ctx: &egui::Context, window: &Window) -> f32 {
     let native_pixels_per_point = window.scale_factor() as f32;
     let egui_zoom_factor = egui_ctx.zoom_factor();
@@ -236,25 +226,25 @@ impl State {
 
         self.egui_input.time = Some(self.start_time.elapsed().as_secs_f64());
 
-        // On Windows, a minimized window will have 0 width and height.
-        // See: https://github.com/rust-windowing/winit/issues/208
-        // This solves an issue where egui window positions would be changed when minimizing on Windows.
-        let screen_size_in_pixels = screen_size_in_pixels(window);
-        let screen_size_in_points =
-            screen_size_in_pixels / pixels_per_point(&self.egui_ctx, window);
+        // // On Windows, a minimized window will have 0 width and height.
+        // // See: https://github.com/rust-windowing/winit/issues/208
+        // // This solves an issue where egui window positions would be changed when minimizing on Windows.
+        // let screen_size_in_pixels = screen_size_in_pixels(window);
+        // let screen_size_in_points =
+        //     screen_size_in_pixels / pixels_per_point(&self.egui_ctx, window);
 
-        self.egui_input.screen_rect = (screen_size_in_points.x > 0.0
-            && screen_size_in_points.y > 0.0)
-            .then(|| Rect::from_min_size(Pos2::ZERO, screen_size_in_points));
+        // self.egui_input.screen_rect = (screen_size_in_points.x > 0.0
+        //     && screen_size_in_points.y > 0.0)
+        //     .then(|| Rect::from_min_size(Pos2::ZERO, screen_size_in_points));
 
         // Tell egui which viewport is now active:
         self.egui_input.viewport_id = self.viewport_id;
 
-        self.egui_input
-            .viewports
-            .entry(self.viewport_id)
-            .or_default()
-            .native_pixels_per_point = Some(window.scale_factor() as f32);
+        // self.egui_input
+        //     .viewports
+        //     .entry(self.viewport_id)
+        //     .or_default()
+        //     .native_pixels_per_point = Some(window.scale_factor() as f32);
 
         self.egui_input.take()
     }
@@ -305,6 +295,31 @@ impl State {
                     consumed: false,
                 }
             }
+            WindowEvent::Resized(size) => {
+                // TODO: This event doesn't deliver outer size
+                // let size = if cfg!(target_os = "ios") {
+                //     // `outer_size` Includes the area behind the "dynamic island".
+                //     // It is up to the eframe user to make sure the dynamic island doesn't cover anything important.
+                //     // That will be easier once https://github.com/rust-windowing/winit/pull/3890 lands
+                //     window.outer_size()
+                // } else {
+                //     window.inner_size()
+                // };
+                let screen_size_in_pixels = egui::vec2(size.width as f32, size.height as f32);
+                let screen_size_in_points = screen_size_in_pixels / self.pixels_per_point();
+
+                // On Windows, a minimized window will have 0 width and height.
+                // See: https://github.com/rust-windowing/winit/issues/208
+                // This solves an issue where egui window positions would be changed when minimizing on Windows.
+                self.egui_input.screen_rect = (screen_size_in_points.x > 0.0
+                    && screen_size_in_points.y > 0.0)
+                    .then(|| Rect::from_min_size(Pos2::ZERO, screen_size_in_points));
+
+                EventResponse {
+                    repaint: true,
+                    consumed: false,
+                }
+            }
             WindowEvent::MouseInput { state, button, .. } => {
                 self.on_mouse_button_input(*state, *button);
                 EventResponse {
@@ -505,7 +520,6 @@ impl State {
             | WindowEvent::CursorEntered { .. }
             | WindowEvent::Destroyed
             | WindowEvent::Occluded(_)
-            | WindowEvent::Resized(_)
             | WindowEvent::Moved(_)
             | WindowEvent::TouchpadPressure { .. }
             | WindowEvent::CloseRequested => EventResponse {
@@ -546,11 +560,9 @@ impl State {
             }
 
             WindowEvent::PanGesture { delta, phase, .. } => {
-                let pixels_per_point = pixels_per_point(&self.egui_ctx, window);
-
                 self.egui_input.events.push(egui::Event::MouseWheel {
                     unit: egui::MouseWheelUnit::Point,
-                    delta: Vec2::new(delta.x, delta.y) / pixels_per_point,
+                    delta: Vec2::new(delta.x, delta.y) / self.pixels_per_point(),
                     phase: to_egui_touch_phase(*phase),
                     modifiers: self.egui_input.modifiers,
                 });
@@ -645,12 +657,8 @@ impl State {
         window: &Window,
         pos_in_pixels: winit::dpi::PhysicalPosition<f64>,
     ) {
-        let pixels_per_point = pixels_per_point(&self.egui_ctx, window);
-
-        let pos_in_points = egui::pos2(
-            pos_in_pixels.x as f32 / pixels_per_point,
-            pos_in_pixels.y as f32 / pixels_per_point,
-        );
+        let pos_in_points =
+            egui::pos2(pos_in_pixels.x as f32, pos_in_pixels.y as f32) / self.pixels_per_point();
         self.pointer_pos_in_points = Some(pos_in_points);
 
         if self.simulate_touch_screen {
@@ -675,17 +683,13 @@ impl State {
     }
 
     fn on_touch(&mut self, window: &Window, touch: &winit::event::Touch) {
-        let pixels_per_point = pixels_per_point(&self.egui_ctx, window);
-
         // Emit touch event
         self.egui_input.events.push(egui::Event::Touch {
             device_id: egui::TouchDeviceId(egui::epaint::util::hash(touch.device_id)),
             id: egui::TouchId::from(touch.id),
             phase: to_egui_touch_phase(touch.phase),
-            pos: egui::pos2(
-                touch.location.x as f32 / pixels_per_point,
-                touch.location.y as f32 / pixels_per_point,
-            ),
+            pos: egui::pos2(touch.location.x as f32, touch.location.y as f32)
+                / self.pixels_per_point(),
             force: match touch.force {
                 Some(winit::event::Force::Normalized(force)) => Some(force as f32),
                 Some(winit::event::Force::Calibrated {
@@ -740,30 +744,23 @@ impl State {
         delta: winit::event::MouseScrollDelta,
         phase: winit::event::TouchPhase,
     ) {
-        let pixels_per_point = pixels_per_point(&self.egui_ctx, window);
-
-        {
-            let (unit, delta) = match delta {
-                winit::event::MouseScrollDelta::LineDelta(x, y) => {
-                    (egui::MouseWheelUnit::Line, egui::vec2(x, y))
-                }
-                winit::event::MouseScrollDelta::PixelDelta(winit::dpi::PhysicalPosition {
-                    x,
-                    y,
-                }) => (
-                    egui::MouseWheelUnit::Point,
-                    egui::vec2(x as f32, y as f32) / pixels_per_point,
-                ),
-            };
-            let phase = to_egui_touch_phase(phase);
-            let modifiers = self.egui_input.modifiers;
-            self.egui_input.events.push(egui::Event::MouseWheel {
-                unit,
-                delta,
-                phase,
-                modifiers,
-            });
-        }
+        let (unit, delta) = match delta {
+            winit::event::MouseScrollDelta::LineDelta(x, y) => {
+                (egui::MouseWheelUnit::Line, egui::vec2(x, y))
+            }
+            winit::event::MouseScrollDelta::PixelDelta(winit::dpi::PhysicalPosition { x, y }) => (
+                egui::MouseWheelUnit::Point,
+                egui::vec2(x as f32, y as f32) / self.pixels_per_point(),
+            ),
+        };
+        let phase = to_egui_touch_phase(phase);
+        let modifiers = self.egui_input.modifiers;
+        self.egui_input.events.push(egui::Event::MouseWheel {
+            unit,
+            delta,
+            phase,
+            modifiers,
+        });
     }
 
     fn on_keyboard_input(&mut self, event: &winit::event::KeyEvent) {
@@ -917,8 +914,7 @@ impl State {
         }
 
         if let Some(ime) = ime {
-            let pixels_per_point = pixels_per_point(&self.egui_ctx, window);
-            let ime_rect_px = pixels_per_point * ime.rect;
+            let ime_rect_px = self.pixels_per_point() * ime.rect;
             if self.ime_rect_px != Some(ime_rect_px)
                 || self.egui_ctx.input(|i| !i.events.is_empty())
             {
@@ -973,6 +969,17 @@ impl State {
             self.current_cursor_icon = None;
         }
     }
+
+    /// Calculate the `pixels_per_point` for a given window, given the current egui zoom factor
+    fn pixels_per_point(&self) -> f32 {
+        self.egui_input
+            .viewports
+            .get(&self.viewport_id)
+            .expect("No viewport")
+            .native_pixels_per_point
+            .expect("ScaleFactorChanged must have been raised")
+            * self.egui_ctx.zoom_factor()
+    }
 }
 
 fn to_egui_touch_phase(phase: winit::event::TouchPhase) -> egui::TouchPhase {
@@ -1027,6 +1034,7 @@ pub fn update_viewport_info(
     is_init: bool,
 ) {
     profiling::function_scope!();
+    // TODO: Keep size and scale consistent with the event loop?
     let pixels_per_point = pixels_per_point(egui_ctx, window);
 
     let has_a_position = match window.is_minimized() {
@@ -1406,6 +1414,7 @@ fn process_viewport_command(
 
     log::trace!("Processing ViewportCommand::{command:?}");
 
+    // TODO: keep this consistent with the event loop instead of requerying it from the window
     let pixels_per_point = pixels_per_point(egui_ctx, window);
 
     match command {
@@ -1441,6 +1450,7 @@ fn process_viewport_command(
                 // because the linux backend converts physical to logical and back again.
                 // So let's just assume it worked:
 
+                // TODO: Don't call inner_size(), but use the returned size
                 info.inner_rect = inner_rect_in_points(window, pixels_per_point);
                 info.outer_rect = outer_rect_in_points(window, pixels_per_point);
             } else {