From 53c9dd109ddfe6edb1dd657a1487d9f233d27aba Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Thu, 23 Jan 2025 03:59:34 +0100 Subject: [PATCH 01/10] Improve debug-output on resize --- crates/eframe/src/web/app_runner.rs | 10 +++++++++ crates/eframe/src/web/events.rs | 35 ++++++++++++++++++++++++++--- crates/eframe/src/web/mod.rs | 3 +++ 3 files changed, 45 insertions(+), 3 deletions(-) diff --git a/crates/eframe/src/web/app_runner.rs b/crates/eframe/src/web/app_runner.rs index 6d11069f891..09770b852d1 100644 --- a/crates/eframe/src/web/app_runner.rs +++ b/crates/eframe/src/web/app_runner.rs @@ -216,6 +216,16 @@ impl AppRunner { let canvas_size = super::canvas_size_in_points(self.canvas(), self.egui_ctx()); let mut raw_input = self.input.new_frame(canvas_size); + if super::DEBUG_RESIZE { + log::info!( + "egui running at canvas size: {}x{} points, DPR: {}, zoom_factor: {}", + canvas_size.x, + canvas_size.y, + super::native_pixels_per_point(), + self.egui_ctx.zoom_factor(), + ); + } + self.app.raw_input_hook(&self.egui_ctx, &mut raw_input); let full_output = self.egui_ctx.run(raw_input, |egui_ctx| { diff --git a/crates/eframe/src/web/events.rs b/crates/eframe/src/web/events.rs index 414e5be2383..7d7769d5dd9 100644 --- a/crates/eframe/src/web/events.rs +++ b/crates/eframe/src/web/events.rs @@ -2,7 +2,7 @@ use super::{ button_from_mouse_event, location_hash, modifiers_from_kb_event, modifiers_from_mouse_event, modifiers_from_wheel_event, pos_from_mouse_event, prefers_color_scheme_dark, primary_touch_pos, push_touches, text_from_keyboard_event, theme_from_dark_mode, translate_key, AppRunner, - Closure, JsCast, JsValue, WebRunner, + Closure, JsCast, JsValue, WebRunner, DEBUG_RESIZE, }; use web_sys::EventTarget; @@ -363,10 +363,26 @@ fn install_window_events(runner_ref: &WebRunner, window: &EventTarget) -> Result runner.save(); })?; - // NOTE: resize is handled by `ResizeObserver` below + runner_ref.add_event_listener(window, "resize", move |_: web_sys::Event, runner| { + // NOTE: we also use the `ResizeObserver`, but it doesn't always trigger when + // the Device Pixel Ratio (DPR) changes, so we need to subscribe to the "resize" event as well. + if DEBUG_RESIZE { + log::debug!( + "Resize event, canvas size: {}x{}, DPR: {}", + runner.canvas().width(), + runner.canvas().height(), + web_sys::window().unwrap().device_pixel_ratio() + ); + } + // TODO: trigger resize_observer.observe + // runner.needs_repaint.repaint_asap(); + })?; + for event_name in &["load", "pagehide", "pageshow"] { runner_ref.add_event_listener(window, event_name, move |_: web_sys::Event, runner| { - // log::debug!("{event_name:?}"); + if DEBUG_RESIZE { + log::debug!("{event_name:?}"); + } runner.needs_repaint.repaint_asap(); })?; } @@ -835,6 +851,12 @@ pub(crate) fn install_resize_observer(runner_ref: &WebRunner) -> Result<(), JsVa return; } }; + if DEBUG_RESIZE { + log::info!( + "ResizeObserver: new canvas size: {width}x{height}, DPR: {}", + web_sys::window().unwrap().device_pixel_ratio() + ); + } canvas.set_width(width); canvas.set_height(height); @@ -878,6 +900,10 @@ fn get_display_size(resize_observer_entries: &js_sys::Array) -> Result<(u32, u32 width = size.inline_size(); height = size.block_size(); dpr = 1.0; // no need to apply + + if DEBUG_RESIZE { + log::info!("devicePixelContentBoxSize {width}x{height}"); + } } else if JsValue::from_str("contentBoxSize").js_in(entry.as_ref()) { let content_box_size = entry.content_box_size(); let idx0 = content_box_size.at(0); @@ -892,6 +918,9 @@ fn get_display_size(resize_observer_entries: &js_sys::Array) -> Result<(u32, u32 width = size.inline_size(); height = size.block_size(); } + if DEBUG_RESIZE { + log::info!("contentBoxSize {width}x{height}"); + } } else { // legacy let content_rect = entry.content_rect(); diff --git a/crates/eframe/src/web/mod.rs b/crates/eframe/src/web/mod.rs index 911c453f224..74371fa35b1 100644 --- a/crates/eframe/src/web/mod.rs +++ b/crates/eframe/src/web/mod.rs @@ -51,6 +51,9 @@ use input::{ // ---------------------------------------------------------------------------- +/// Debug browser resizing? +const DEBUG_RESIZE: bool = false; + pub(crate) fn string_from_js_value(value: &JsValue) -> String { value.as_string().unwrap_or_else(|| format!("{value:#?}")) } From 9686401c86c737fbd4349750b83cd1e1e499f2df Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Thu, 23 Jan 2025 04:19:27 +0100 Subject: [PATCH 02/10] Refactor how ResizeObserver is initialized --- crates/eframe/src/web/events.rs | 103 ++++++++++++++++------------ crates/eframe/src/web/web_runner.rs | 64 +++++++---------- 2 files changed, 86 insertions(+), 81 deletions(-) diff --git a/crates/eframe/src/web/events.rs b/crates/eframe/src/web/events.rs index 7d7769d5dd9..94efe6bd904 100644 --- a/crates/eframe/src/web/events.rs +++ b/crates/eframe/src/web/events.rs @@ -829,59 +829,76 @@ fn install_drag_and_drop(runner_ref: &WebRunner, target: &EventTarget) -> Result Ok(()) } -/// Install a `ResizeObserver` to observe changes to the size of the canvas. -/// -/// This is the only way to ensure a canvas size change without an associated window `resize` event -/// actually results in a resize of the canvas. +/// A `ResizeObserver` is used to observe changes to the size of the canvas. /// /// The resize observer is called the by the browser at `observe` time, instead of just on the first actual resize. /// We use that to trigger the first `request_animation_frame` _after_ updating the size of the canvas to the correct dimensions, /// to avoid [#4622](https://github.com/emilk/egui/issues/4622). -pub(crate) fn install_resize_observer(runner_ref: &WebRunner) -> Result<(), JsValue> { - let closure = Closure::wrap(Box::new({ - let runner_ref = runner_ref.clone(); - move |entries: js_sys::Array| { - // Only call the wrapped closure if the egui code has not panicked - if let Some(mut runner_lock) = runner_ref.try_lock() { - let canvas = runner_lock.canvas(); - let (width, height) = match get_display_size(&entries) { - Ok(v) => v, - Err(err) => { - log::error!("{}", super::string_from_js_value(&err)); - return; - } - }; +pub struct ResizeObserverContext { + observer: web_sys::ResizeObserver, + + // Kept so it is not dropped until we are done with it. + _closure: Closure, +} + +impl Drop for ResizeObserverContext { + fn drop(&mut self) { + self.observer.disconnect(); + } +} + +impl ResizeObserverContext { + pub fn new(runner_ref: &WebRunner) -> Result { + let closure = Closure::wrap(Box::new({ + let runner_ref = runner_ref.clone(); + move |entries: js_sys::Array| { if DEBUG_RESIZE { - log::info!( - "ResizeObserver: new canvas size: {width}x{height}, DPR: {}", - web_sys::window().unwrap().device_pixel_ratio() - ); + log::info!("ResizeObserverContext callback"); + } + // Only call the wrapped closure if the egui code has not panicked + if let Some(mut runner_lock) = runner_ref.try_lock() { + let canvas = runner_lock.canvas(); + let (width, height) = match get_display_size(&entries) { + Ok(v) => v, + Err(err) => { + log::error!("{}", super::string_from_js_value(&err)); + return; + } + }; + if DEBUG_RESIZE { + log::info!( + "ResizeObserver: new canvas size: {width}x{height}, DPR: {}", + web_sys::window().unwrap().device_pixel_ratio() + ); + } + canvas.set_width(width); + canvas.set_height(height); + + // force an immediate repaint + runner_lock.needs_repaint.repaint_asap(); + paint_if_needed(&mut runner_lock); + drop(runner_lock); + // we rely on the resize observer to trigger the first `request_animation_frame`: + if let Err(err) = runner_ref.request_animation_frame() { + log::error!("{}", super::string_from_js_value(&err)); + }; } - canvas.set_width(width); - canvas.set_height(height); - - // force an immediate repaint - runner_lock.needs_repaint.repaint_asap(); - paint_if_needed(&mut runner_lock); - drop(runner_lock); - // we rely on the resize observer to trigger the first `request_animation_frame`: - if let Err(err) = runner_ref.request_animation_frame() { - log::error!("{}", super::string_from_js_value(&err)); - }; } - } - }) as Box); + }) as Box); - let observer = web_sys::ResizeObserver::new(closure.as_ref().unchecked_ref())?; - let options = web_sys::ResizeObserverOptions::new(); - options.set_box(web_sys::ResizeObserverBoxOptions::ContentBox); - if let Some(runner_lock) = runner_ref.try_lock() { - observer.observe_with_options(runner_lock.canvas(), &options); - drop(runner_lock); - runner_ref.set_resize_observer(observer, closure); + let observer = web_sys::ResizeObserver::new(closure.as_ref().unchecked_ref())?; + + Ok(Self { + observer, + _closure: closure, + }) } - Ok(()) + pub fn observe(&self, canvas: &web_sys::HtmlCanvasElement) { + let options = web_sys::ResizeObserverOptions::new(); + options.set_box(web_sys::ResizeObserverBoxOptions::ContentBox); + self.observer.observe_with_options(canvas, &options); + } } // Code ported to Rust from: diff --git a/crates/eframe/src/web/web_runner.rs b/crates/eframe/src/web/web_runner.rs index 6cbc371f34c..52bff0c2a87 100644 --- a/crates/eframe/src/web/web_runner.rs +++ b/crates/eframe/src/web/web_runner.rs @@ -2,9 +2,13 @@ use std::{cell::RefCell, rc::Rc}; use wasm_bindgen::prelude::*; -use crate::{epi, App}; +use crate::{epi, web::DEBUG_RESIZE, App}; -use super::{events, text_agent::TextAgent, AppRunner, PanicHandler}; +use super::{ + events::{self, ResizeObserverContext}, + text_agent::TextAgent, + AppRunner, PanicHandler, +}; /// This is how `eframe` runs your web application /// @@ -18,7 +22,7 @@ pub struct WebRunner { /// If we ever panic during running, this `RefCell` is poisoned. /// So before we use it, we need to check [`Self::panic_handler`]. - runner: Rc>>, + app_runner: Rc>>, /// In case of a panic, unsubscribe these. /// They have to be in a separate `Rc` so that we don't need to pass them to @@ -39,7 +43,7 @@ impl WebRunner { Self { panic_handler, - runner: Rc::new(RefCell::new(None)), + app_runner: Rc::new(RefCell::new(None)), events_to_unsubscribe: Rc::new(RefCell::new(Default::default())), frame: Default::default(), resize_observer: Default::default(), @@ -58,28 +62,33 @@ impl WebRunner { ) -> Result<(), JsValue> { self.destroy(); - let text_agent = TextAgent::attach(self)?; - - let runner = AppRunner::new(canvas, web_options, app_creator, text_agent).await?; - { // Make sure the canvas can be given focus. // https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/tabindex - runner.canvas().set_tab_index(0); + canvas.set_tab_index(0); // Don't outline the canvas when it has focus: - runner.canvas().style().set_property("outline", "none")?; + canvas.style().set_property("outline", "none")?; } - self.runner.replace(Some(runner)); + let text_agent = TextAgent::attach(self)?; { - events::install_event_handlers(self)?; + let resize_observer = events::ResizeObserverContext::new(self)?; - // The resize observer handles calling `request_animation_frame` to start the render loop. - events::install_resize_observer(self)?; + // This will (eventually) result in a `request_animation_frame` to start the render loop. + resize_observer.observe(&canvas); + + self.resize_observer.replace(Some(resize_observer)); + } + + { + let app_runner = AppRunner::new(canvas, web_options, app_creator, text_agent).await?; + self.app_runner.replace(Some(app_runner)); } + events::install_event_handlers(self)?; + Ok(()) } @@ -109,10 +118,7 @@ impl WebRunner { } } - if let Some(context) = self.resize_observer.take() { - context.resize_observer.disconnect(); - drop(context.closure); - } + self.resize_observer.replace(None); } /// Shut down eframe and clean up resources. @@ -124,7 +130,7 @@ impl WebRunner { window.cancel_animation_frame(frame.id).ok(); } - if let Some(runner) = self.runner.replace(None) { + if let Some(runner) = self.app_runner.replace(None) { runner.destroy(); } } @@ -138,7 +144,7 @@ impl WebRunner { self.unsubscribe_from_all_events(); None } else { - let lock = self.runner.try_borrow_mut().ok()?; + let lock = self.app_runner.try_borrow_mut().ok()?; std::cell::RefMut::filter_map(lock, |lock| -> Option<&mut AppRunner> { lock.as_mut() }) .ok() } @@ -226,19 +232,6 @@ impl WebRunner { Ok(()) } - - pub(crate) fn set_resize_observer( - &self, - resize_observer: web_sys::ResizeObserver, - closure: Closure, - ) { - self.resize_observer - .borrow_mut() - .replace(ResizeObserverContext { - resize_observer, - closure, - }); - } } // ---------------------------------------------------------------------------- @@ -253,11 +246,6 @@ struct AnimationFrameRequest { _closure: Closure Result<(), JsValue>>, } -struct ResizeObserverContext { - resize_observer: web_sys::ResizeObserver, - closure: Closure, -} - struct TargetEvent { target: web_sys::EventTarget, event_name: String, From 9d849232c064e02351dbb21aa748d4712c3d716c Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Thu, 23 Jan 2025 10:44:35 +0100 Subject: [PATCH 03/10] Use latest DPR when calculating pixels_per_point for egui --- crates/eframe/src/web/mod.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/eframe/src/web/mod.rs b/crates/eframe/src/web/mod.rs index 74371fa35b1..57db34c02ca 100644 --- a/crates/eframe/src/web/mod.rs +++ b/crates/eframe/src/web/mod.rs @@ -155,7 +155,10 @@ fn canvas_content_rect(canvas: &web_sys::HtmlCanvasElement) -> egui::Rect { } fn canvas_size_in_points(canvas: &web_sys::HtmlCanvasElement, ctx: &egui::Context) -> egui::Vec2 { - let pixels_per_point = ctx.pixels_per_point(); + // ctx.pixels_per_point can be outdated + + let pixels_per_point = ctx.zoom_factor() * native_pixels_per_point(); + egui::vec2( canvas.width() as f32 / pixels_per_point, canvas.height() as f32 / pixels_per_point, From 8a818b0254130236a588fe7f7930d50e92bfcb00 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Thu, 23 Jan 2025 10:44:53 +0100 Subject: [PATCH 04/10] Add `add_event_listener_ex` with more features --- crates/eframe/src/web/web_runner.rs | 39 +++++++++++++++++++++++------ 1 file changed, 32 insertions(+), 7 deletions(-) diff --git a/crates/eframe/src/web/web_runner.rs b/crates/eframe/src/web/web_runner.rs index 52bff0c2a87..9f06428753b 100644 --- a/crates/eframe/src/web/web_runner.rs +++ b/crates/eframe/src/web/web_runner.rs @@ -172,20 +172,45 @@ impl WebRunner { event_name: &'static str, mut closure: impl FnMut(E, &mut AppRunner) + 'static, ) -> Result<(), wasm_bindgen::JsValue> { - let runner_ref = self.clone(); + let options = web_sys::AddEventListenerOptions::default(); + self.add_event_listener_ex( + target, + event_name, + &options, + move |event, app_runner, _web_runner| closure(event, app_runner), + ) + } + + /// Convenience function to reduce boilerplate and ensure that all event handlers + /// are dealt with in the same way. + /// + /// All events added with this method will automatically be unsubscribed on panic, + /// or when [`Self::destroy`] is called. + pub fn add_event_listener_ex( + &self, + target: &web_sys::EventTarget, + event_name: &'static str, + options: &web_sys::AddEventListenerOptions, + mut closure: impl FnMut(E, &mut AppRunner, &WebRunner) + 'static, + ) -> Result<(), wasm_bindgen::JsValue> { + let web_runner = self.clone(); // Create a JS closure based on the FnMut provided let closure = Closure::wrap(Box::new(move |event: web_sys::Event| { // Only call the wrapped closure if the egui code has not panicked - if let Some(mut runner_lock) = runner_ref.try_lock() { + if let Some(mut runner_lock) = web_runner.try_lock() { // Cast the event to the expected event type let event = event.unchecked_into::(); - closure(event, &mut runner_lock); + closure(event, &mut runner_lock, &web_runner); } }) as Box); // Add the event listener to the target - target.add_event_listener_with_callback(event_name, closure.as_ref().unchecked_ref())?; + target.add_event_listener_with_callback_and_add_event_listener_options( + event_name, + closure.as_ref().unchecked_ref(), + options, + )?; let handle = TargetEvent { target: target.clone(), @@ -214,13 +239,13 @@ impl WebRunner { let window = web_sys::window().unwrap(); let closure = Closure::once({ - let runner_ref = self.clone(); + let web_runner = self.clone(); move || { // We can paint now, so clear the animation frame. // This drops the `closure` and allows another // animation frame to be scheduled - let _ = runner_ref.frame.take(); - events::paint_and_schedule(&runner_ref) + let _ = web_runner.frame.take(); + events::paint_and_schedule(&web_runner) } }); From aedc15520475133274d20fb1ea6eafdbd697fe1e Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Thu, 23 Jan 2025 10:45:17 +0100 Subject: [PATCH 05/10] Install DPR listener --- crates/eframe/Cargo.toml | 1 + crates/eframe/src/web/events.rs | 78 ++++++++++++++++++++++----------- 2 files changed, 54 insertions(+), 25 deletions(-) diff --git a/crates/eframe/Cargo.toml b/crates/eframe/Cargo.toml index 21f81d9e3c7..883ec7d5916 100644 --- a/crates/eframe/Cargo.toml +++ b/crates/eframe/Cargo.toml @@ -211,6 +211,7 @@ percent-encoding = "2.1" wasm-bindgen.workspace = true wasm-bindgen-futures.workspace = true web-sys = { workspace = true, features = [ + "AddEventListenerOptions", "BinaryType", "Blob", "BlobPropertyBag", diff --git a/crates/eframe/src/web/events.rs b/crates/eframe/src/web/events.rs index 94efe6bd904..a79e8de8462 100644 --- a/crates/eframe/src/web/events.rs +++ b/crates/eframe/src/web/events.rs @@ -1,10 +1,13 @@ +use ron::options; +use web_sys::EventTarget; + use super::{ button_from_mouse_event, location_hash, modifiers_from_kb_event, modifiers_from_mouse_event, - modifiers_from_wheel_event, pos_from_mouse_event, prefers_color_scheme_dark, primary_touch_pos, - push_touches, text_from_keyboard_event, theme_from_dark_mode, translate_key, AppRunner, - Closure, JsCast, JsValue, WebRunner, DEBUG_RESIZE, + modifiers_from_wheel_event, native_pixels_per_point, pos_from_mouse_event, + prefers_color_scheme_dark, primary_touch_pos, push_touches, text_from_keyboard_event, + theme_from_dark_mode, translate_key, AppRunner, Closure, JsCast, JsValue, WebRunner, + DEBUG_RESIZE, }; -use web_sys::EventTarget; // TODO(emilk): there are more calls to `prevent_default` and `stop_propagation` // than what is probably needed. @@ -357,29 +360,18 @@ fn install_copy_cut_paste(runner_ref: &WebRunner, target: &EventTarget) -> Resul Ok(()) } -fn install_window_events(runner_ref: &WebRunner, window: &EventTarget) -> Result<(), JsValue> { +fn install_window_events(web_runner: &WebRunner, window: &EventTarget) -> Result<(), JsValue> { // Save-on-close - runner_ref.add_event_listener(window, "onbeforeunload", |_: web_sys::Event, runner| { + web_runner.add_event_listener(window, "onbeforeunload", |_: web_sys::Event, runner| { runner.save(); })?; - runner_ref.add_event_listener(window, "resize", move |_: web_sys::Event, runner| { - // NOTE: we also use the `ResizeObserver`, but it doesn't always trigger when - // the Device Pixel Ratio (DPR) changes, so we need to subscribe to the "resize" event as well. - if DEBUG_RESIZE { - log::debug!( - "Resize event, canvas size: {}x{}, DPR: {}", - runner.canvas().width(), - runner.canvas().height(), - web_sys::window().unwrap().device_pixel_ratio() - ); - } - // TODO: trigger resize_observer.observe - // runner.needs_repaint.repaint_asap(); - })?; + // We want to handle the case of dragging the browser from one monitor to another, + // which can cause the DPR to change without any resize event (e.g. Safari). + install_dpr_change_event(web_runner); - for event_name in &["load", "pagehide", "pageshow"] { - runner_ref.add_event_listener(window, event_name, move |_: web_sys::Event, runner| { + for event_name in &["load", "pagehide", "pageshow", "resize"] { + web_runner.add_event_listener(window, event_name, move |_: web_sys::Event, runner| { if DEBUG_RESIZE { log::debug!("{event_name:?}"); } @@ -387,7 +379,7 @@ fn install_window_events(runner_ref: &WebRunner, window: &EventTarget) -> Result })?; } - runner_ref.add_event_listener(window, "hashchange", |_: web_sys::Event, runner| { + web_runner.add_event_listener(window, "hashchange", |_: web_sys::Event, runner| { // `epi::Frame::info(&self)` clones `epi::IntegrationInfo`, but we need to modify the original here runner.frame.info.web_info.location.hash = location_hash(); runner.needs_repaint.repaint_asap(); // tell the user about the new hash @@ -396,6 +388,39 @@ fn install_window_events(runner_ref: &WebRunner, window: &EventTarget) -> Result Ok(()) } +fn install_dpr_change_event(web_runner: &WebRunner) -> Result<(), JsValue> { + let original_dpr = native_pixels_per_point(); + + let window = web_sys::window().unwrap(); + let media_query_list = window + .match_media(&format!("(resolution: {original_dpr}dppx)")) + .unwrap() + .unwrap(); + + let closure = move |_: web_sys::Event, app_runner: &mut AppRunner, web_runner: &WebRunner| { + let new_dpr = native_pixels_per_point(); + log::debug!("Device Pixel Ratio changed from {original_dpr} to {new_dpr}"); + + if true { + // Explicitly resize canvas to match the new DPR. + // This is a bit ugly, but I haven't found a better way to do it. + let canvas = app_runner.canvas(); + canvas.set_width((canvas.width() as f32 * new_dpr / original_dpr).round() as _); + canvas.set_height((canvas.height() as f32 * new_dpr / original_dpr).round() as _); + log::debug!("Resized canvas to {}x{}", canvas.width(), canvas.height()); + } + + // It may be tempting to call `resize_observer.observe(&canvas)` here, + // but unfortunately this has no effect. + + install_dpr_change_event(web_runner); + }; + + let mut options = web_sys::AddEventListenerOptions::default(); + options.set_once(true); + web_runner.add_event_listener_ex(&media_query_list, "change", &options, closure) +} + fn install_color_scheme_change_event( runner_ref: &WebRunner, window: &web_sys::Window, @@ -853,7 +878,7 @@ impl ResizeObserverContext { let runner_ref = runner_ref.clone(); move |entries: js_sys::Array| { if DEBUG_RESIZE { - log::info!("ResizeObserverContext callback"); + // log::info!("ResizeObserverContext callback"); } // Only call the wrapped closure if the egui code has not panicked if let Some(mut runner_lock) = runner_ref.try_lock() { @@ -895,6 +920,9 @@ impl ResizeObserverContext { } pub fn observe(&self, canvas: &web_sys::HtmlCanvasElement) { + if DEBUG_RESIZE { + log::info!("Calling observe on canvas…"); + } let options = web_sys::ResizeObserverOptions::new(); options.set_box(web_sys::ResizeObserverBoxOptions::ContentBox); self.observer.observe_with_options(canvas, &options); @@ -919,7 +947,7 @@ fn get_display_size(resize_observer_entries: &js_sys::Array) -> Result<(u32, u32 dpr = 1.0; // no need to apply if DEBUG_RESIZE { - log::info!("devicePixelContentBoxSize {width}x{height}"); + // log::info!("devicePixelContentBoxSize {width}x{height}"); } } else if JsValue::from_str("contentBoxSize").js_in(entry.as_ref()) { let content_box_size = entry.content_box_size(); From 389ededdbca6e97c77ca6d5ce7ba48081c141a91 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Thu, 23 Jan 2025 10:45:25 +0100 Subject: [PATCH 06/10] Add `is_safari_browser` --- crates/eframe/src/web/mod.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/crates/eframe/src/web/mod.rs b/crates/eframe/src/web/mod.rs index 57db34c02ca..e016ed8d2b2 100644 --- a/crates/eframe/src/web/mod.rs +++ b/crates/eframe/src/web/mod.rs @@ -52,7 +52,7 @@ use input::{ // ---------------------------------------------------------------------------- /// Debug browser resizing? -const DEBUG_RESIZE: bool = false; +const DEBUG_RESIZE: bool = true; // TODO: revert pub(crate) fn string_from_js_value(value: &JsValue) -> String { value.as_string().unwrap_or_else(|| format!("{value:#?}")) @@ -358,3 +358,10 @@ pub fn percent_decode(s: &str) -> String { .decode_utf8_lossy() .to_string() } + +/// Are we running inside the Safari browser? +pub fn is_safari_browser() -> bool { + web_sys::window() + .map(|window| window.has_own_property(&JsValue::from("safari"))) + .unwrap_or(false) +} From b5fd7d2ae14453fa05d631f8da6ff30472c49fa9 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Thu, 23 Jan 2025 10:45:33 +0100 Subject: [PATCH 07/10] Better logging --- crates/eframe/src/web/app_runner.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/crates/eframe/src/web/app_runner.rs b/crates/eframe/src/web/app_runner.rs index 09770b852d1..76ae1761f25 100644 --- a/crates/eframe/src/web/app_runner.rs +++ b/crates/eframe/src/web/app_runner.rs @@ -59,7 +59,7 @@ impl AppRunner { egui_ctx.options_mut(|o| { // On web by default egui follows the zoom factor of the browser, - // and lets the browser handle the zoom shortscuts. + // and lets the browser handle the zoom shortcuts. // A user can still zoom egui separately by calling [`egui::Context::set_zoom_factor`]. o.zoom_with_keyboard = false; o.zoom_factor = 1.0; @@ -218,11 +218,13 @@ impl AppRunner { if super::DEBUG_RESIZE { log::info!( - "egui running at canvas size: {}x{} points, DPR: {}, zoom_factor: {}", - canvas_size.x, - canvas_size.y, + "egui running at canvas size: {}x{}, DPR: {}, zoom_factor: {}. egui size: {}x{} points", + self.canvas().width(), + self.canvas().height(), super::native_pixels_per_point(), self.egui_ctx.zoom_factor(), + canvas_size.x, + canvas_size.y, ); } From 123c2134e840d1944717fd8919307a0f5459fe07 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Thu, 23 Jan 2025 10:48:00 +0100 Subject: [PATCH 08/10] Clippy fixes --- crates/eframe/src/web/events.rs | 14 ++++++++++---- crates/eframe/src/web/mod.rs | 6 ++---- crates/eframe/src/web/web_runner.rs | 4 ++-- 3 files changed, 14 insertions(+), 10 deletions(-) diff --git a/crates/eframe/src/web/events.rs b/crates/eframe/src/web/events.rs index a79e8de8462..9c6156bbfb3 100644 --- a/crates/eframe/src/web/events.rs +++ b/crates/eframe/src/web/events.rs @@ -1,6 +1,7 @@ -use ron::options; use web_sys::EventTarget; +use crate::web::string_from_js_value; + use super::{ button_from_mouse_event, location_hash, modifiers_from_kb_event, modifiers_from_mouse_event, modifiers_from_wheel_event, native_pixels_per_point, pos_from_mouse_event, @@ -368,7 +369,7 @@ fn install_window_events(web_runner: &WebRunner, window: &EventTarget) -> Result // We want to handle the case of dragging the browser from one monitor to another, // which can cause the DPR to change without any resize event (e.g. Safari). - install_dpr_change_event(web_runner); + install_dpr_change_event(web_runner)?; for event_name in &["load", "pagehide", "pageshow", "resize"] { web_runner.add_event_listener(window, event_name, move |_: web_sys::Event, runner| { @@ -413,10 +414,15 @@ fn install_dpr_change_event(web_runner: &WebRunner) -> Result<(), JsValue> { // It may be tempting to call `resize_observer.observe(&canvas)` here, // but unfortunately this has no effect. - install_dpr_change_event(web_runner); + if let Err(err) = install_dpr_change_event(web_runner) { + log::error!( + "Failed to install DPR change event: {}", + string_from_js_value(&err) + ); + } }; - let mut options = web_sys::AddEventListenerOptions::default(); + let options = web_sys::AddEventListenerOptions::default(); options.set_once(true); web_runner.add_event_listener_ex(&media_query_list, "change", &options, closure) } diff --git a/crates/eframe/src/web/mod.rs b/crates/eframe/src/web/mod.rs index e016ed8d2b2..3dc7d7f8b3b 100644 --- a/crates/eframe/src/web/mod.rs +++ b/crates/eframe/src/web/mod.rs @@ -52,7 +52,7 @@ use input::{ // ---------------------------------------------------------------------------- /// Debug browser resizing? -const DEBUG_RESIZE: bool = true; // TODO: revert +const DEBUG_RESIZE: bool = false; pub(crate) fn string_from_js_value(value: &JsValue) -> String { value.as_string().unwrap_or_else(|| format!("{value:#?}")) @@ -361,7 +361,5 @@ pub fn percent_decode(s: &str) -> String { /// Are we running inside the Safari browser? pub fn is_safari_browser() -> bool { - web_sys::window() - .map(|window| window.has_own_property(&JsValue::from("safari"))) - .unwrap_or(false) + web_sys::window().is_some_and(|window| window.has_own_property(&JsValue::from("safari"))) } diff --git a/crates/eframe/src/web/web_runner.rs b/crates/eframe/src/web/web_runner.rs index 9f06428753b..80c9d021dae 100644 --- a/crates/eframe/src/web/web_runner.rs +++ b/crates/eframe/src/web/web_runner.rs @@ -2,7 +2,7 @@ use std::{cell::RefCell, rc::Rc}; use wasm_bindgen::prelude::*; -use crate::{epi, web::DEBUG_RESIZE, App}; +use crate::{epi, App}; use super::{ events::{self, ResizeObserverContext}, @@ -191,7 +191,7 @@ impl WebRunner { target: &web_sys::EventTarget, event_name: &'static str, options: &web_sys::AddEventListenerOptions, - mut closure: impl FnMut(E, &mut AppRunner, &WebRunner) + 'static, + mut closure: impl FnMut(E, &mut AppRunner, &Self) + 'static, ) -> Result<(), wasm_bindgen::JsValue> { let web_runner = self.clone(); From 90f7d78b407a9fa7824771206985c7c409dbc1ef Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Thu, 23 Jan 2025 10:51:55 +0100 Subject: [PATCH 09/10] Remove some unwraps --- crates/eframe/src/web/events.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/crates/eframe/src/web/events.rs b/crates/eframe/src/web/events.rs index 9c6156bbfb3..93d4e456635 100644 --- a/crates/eframe/src/web/events.rs +++ b/crates/eframe/src/web/events.rs @@ -393,10 +393,14 @@ fn install_dpr_change_event(web_runner: &WebRunner) -> Result<(), JsValue> { let original_dpr = native_pixels_per_point(); let window = web_sys::window().unwrap(); - let media_query_list = window - .match_media(&format!("(resolution: {original_dpr}dppx)")) - .unwrap() - .unwrap(); + let Some(media_query_list) = + window.match_media(&format!("(resolution: {original_dpr}dppx)"))? + else { + log::error!( + "Failed to create MediaQueryList: eframe won't be able to detect changes in DPR" + ); + return Ok(()); + }; let closure = move |_: web_sys::Event, app_runner: &mut AppRunner, web_runner: &WebRunner| { let new_dpr = native_pixels_per_point(); From 12a769e3e6dc28180952c177700bdf9140d093cc Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Thu, 23 Jan 2025 11:03:34 +0100 Subject: [PATCH 10/10] Don't subscribe to "resize" event, and explain why --- crates/eframe/src/web/events.rs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/crates/eframe/src/web/events.rs b/crates/eframe/src/web/events.rs index 93d4e456635..733e2e561d8 100644 --- a/crates/eframe/src/web/events.rs +++ b/crates/eframe/src/web/events.rs @@ -361,18 +361,20 @@ fn install_copy_cut_paste(runner_ref: &WebRunner, target: &EventTarget) -> Resul Ok(()) } -fn install_window_events(web_runner: &WebRunner, window: &EventTarget) -> Result<(), JsValue> { +fn install_window_events(runner_ref: &WebRunner, window: &EventTarget) -> Result<(), JsValue> { // Save-on-close - web_runner.add_event_listener(window, "onbeforeunload", |_: web_sys::Event, runner| { + runner_ref.add_event_listener(window, "onbeforeunload", |_: web_sys::Event, runner| { runner.save(); })?; // We want to handle the case of dragging the browser from one monitor to another, // which can cause the DPR to change without any resize event (e.g. Safari). - install_dpr_change_event(web_runner)?; + install_dpr_change_event(runner_ref)?; - for event_name in &["load", "pagehide", "pageshow", "resize"] { - web_runner.add_event_listener(window, event_name, move |_: web_sys::Event, runner| { + // No need to subscribe to "resize": we already subscribe to the canvas + // size using a ResizeObserver, and we also subscribe to DPR changes of the monitor. + for event_name in &["load", "pagehide", "pageshow"] { + runner_ref.add_event_listener(window, event_name, move |_: web_sys::Event, runner| { if DEBUG_RESIZE { log::debug!("{event_name:?}"); } @@ -380,7 +382,7 @@ fn install_window_events(web_runner: &WebRunner, window: &EventTarget) -> Result })?; } - web_runner.add_event_listener(window, "hashchange", |_: web_sys::Event, runner| { + runner_ref.add_event_listener(window, "hashchange", |_: web_sys::Event, runner| { // `epi::Frame::info(&self)` clones `epi::IntegrationInfo`, but we need to modify the original here runner.frame.info.web_info.location.hash = location_hash(); runner.needs_repaint.repaint_asap(); // tell the user about the new hash