diff --git a/crates/ely_app/src/shell/web_surface.rs b/crates/ely_app/src/shell/web_surface.rs index 7fa35a9..5197ff5 100644 --- a/crates/ely_app/src/shell/web_surface.rs +++ b/crates/ely_app/src/shell/web_surface.rs @@ -102,8 +102,9 @@ impl WebSurfaceStore { tab_id: &TabId, requested_url: &str, delta: Point, + scale_factor: f32, ) -> bool { - let Some(delta) = WebSurfaceScrollDelta::from_point(delta) else { + let Some(delta) = WebSurfaceScrollDelta::from_point(delta, scale_factor) else { return false; }; @@ -130,8 +131,13 @@ impl WebSurfaceStore { true } - pub(super) fn record_viewport_size(&mut self, tab_id: &TabId, bounds: Bounds) -> bool { - let Some(size) = WebSurfaceSize::from_bounds(bounds) else { + pub(super) fn record_viewport_size( + &mut self, + tab_id: &TabId, + bounds: Bounds, + scale_factor: f32, + ) -> bool { + let Some(size) = WebSurfaceSize::from_bounds(bounds, scale_factor) else { return false; }; let surface = self.surface_mut(tab_id); @@ -162,13 +168,15 @@ impl WebSurfaceStore { &mut self, tab_id: &TabId, position: Point, + scale_factor: f32, ) -> bool { let surface = self.surfaces.get_mut(tab_id).filter(|surface| surface.viewport_bounds.is_some()); let Some(surface) = surface else { return false; }; let bounds = surface.viewport_bounds.expect("viewport_bounds checked above"); - let Some(point) = WebSurfaceClickPoint::from_window_position(bounds, position) else { + let Some(point) = WebSurfaceClickPoint::from_window_position(bounds, position, scale_factor) + else { return false; }; surface.hover_point = Some(point); @@ -180,13 +188,15 @@ impl WebSurfaceStore { tab_id: &TabId, requested_url: &str, position: Point, + scale_factor: f32, ) -> bool { let Some(bounds) = self.surfaces.get(tab_id).and_then(|surface| surface.viewport_bounds) else { return false; }; - let Some(point) = WebSurfaceClickPoint::from_window_position(bounds, position) else { + let Some(point) = WebSurfaceClickPoint::from_window_position(bounds, position, scale_factor) + else { return false; }; diff --git a/crates/ely_app/src/shell/web_surface_controller.rs b/crates/ely_app/src/shell/web_surface_controller.rs index 3fe1c8e..824e489 100644 --- a/crates/ely_app/src/shell/web_surface_controller.rs +++ b/crates/ely_app/src/shell/web_surface_controller.rs @@ -52,9 +52,10 @@ impl ElyShell { &mut self, tab_id: TabId, bounds: Bounds, + scale_factor: f32, cx: &mut Context, ) { - if self.web_surfaces.record_viewport_size(&tab_id, bounds) { + if self.web_surfaces.record_viewport_size(&tab_id, bounds, scale_factor) { cx.notify(); } } @@ -64,9 +65,15 @@ impl ElyShell { tab_id: TabId, requested_url: String, delta: Point, + scale_factor: f32, cx: &mut Context, ) { - if self.web_surfaces.record_scroll_delta(&tab_id, requested_url.as_str(), delta) { + if self.web_surfaces.record_scroll_delta( + &tab_id, + requested_url.as_str(), + delta, + scale_factor, + ) { cx.notify(); } } @@ -75,9 +82,10 @@ impl ElyShell { &mut self, tab_id: TabId, position: Point, + scale_factor: f32, cx: &mut Context, ) { - if self.web_surfaces.record_hover_point(&tab_id, position) { + if self.web_surfaces.record_hover_point(&tab_id, position, scale_factor) { cx.notify(); } } @@ -91,7 +99,13 @@ impl ElyShell { cx: &mut Context, ) { self.focus_handle.focus(window); - if self.web_surfaces.record_click_point(&tab_id, requested_url.as_str(), position) { + let scale_factor = window.scale_factor(); + if self.web_surfaces.record_click_point( + &tab_id, + requested_url.as_str(), + position, + scale_factor, + ) { cx.notify(); } } diff --git a/crates/ely_app/src/shell/web_surface_geometry.rs b/crates/ely_app/src/shell/web_surface_geometry.rs index b089b86..c32877b 100644 --- a/crates/ely_app/src/shell/web_surface_geometry.rs +++ b/crates/ely_app/src/shell/web_surface_geometry.rs @@ -7,10 +7,10 @@ pub(super) struct WebSurfaceSize { } impl WebSurfaceSize { - pub(super) fn from_bounds(bounds: Bounds) -> Option { + pub(super) fn from_bounds(bounds: Bounds, scale_factor: f32) -> Option { Some(Self { - width: viewport_dimension(bounds.size.width)?, - height: viewport_dimension(bounds.size.height)?, + width: viewport_dimension(bounds.size.width, scale_factor)?, + height: viewport_dimension(bounds.size.height, scale_factor)?, }) } } @@ -25,10 +25,11 @@ impl WebSurfaceClickPoint { pub(super) fn from_window_position( bounds: Bounds, position: Point, + scale_factor: f32, ) -> Option { Some(Self { - x: click_coordinate(position.x, bounds.origin.x, bounds.size.width)?, - y: click_coordinate(position.y, bounds.origin.y, bounds.size.height)?, + x: click_coordinate(position.x, bounds.origin.x, bounds.size.width, scale_factor)?, + y: click_coordinate(position.y, bounds.origin.y, bounds.size.height, scale_factor)?, }) } @@ -82,9 +83,9 @@ pub(super) struct WebSurfaceScrollDelta { } impl WebSurfaceScrollDelta { - pub(super) fn from_point(delta: Point) -> Option { - let x = scroll_dimension(delta.x)?; - let y = scroll_dimension(delta.y)?; + pub(super) fn from_point(delta: Point, scale_factor: f32) -> Option { + let x = scroll_dimension(delta.x, scale_factor)?; + let y = scroll_dimension(delta.y, scale_factor)?; if x == 0 && y == 0 { return None; } @@ -105,8 +106,8 @@ impl WebSurfaceScrollDelta { } } -fn viewport_dimension(pixels: Pixels) -> Option { - let value = f32::from(pixels.round()); +fn viewport_dimension(pixels: Pixels, scale_factor: f32) -> Option { + let value = (f32::from(pixels) * positive_scale_or_one(scale_factor)).round(); if !value.is_finite() || value < 1.0 || value > u32::MAX as f32 { return None; } @@ -114,8 +115,8 @@ fn viewport_dimension(pixels: Pixels) -> Option { Some(value as u32) } -fn scroll_dimension(pixels: Pixels) -> Option { - let value = f32::from(pixels.round()); +fn scroll_dimension(pixels: Pixels, scale_factor: f32) -> Option { + let value = (f32::from(pixels) * positive_scale_or_one(scale_factor)).round(); if !value.is_finite() { return None; } @@ -129,14 +130,24 @@ fn scroll_dimension(pixels: Pixels) -> Option { Some(value as i32) } -fn click_coordinate(position: Pixels, origin: Pixels, size: Pixels) -> Option { +fn click_coordinate( + position: Pixels, + origin: Pixels, + size: Pixels, + scale_factor: f32, +) -> Option { let relative = f32::from(position) - f32::from(origin); let size = f32::from(size); if !relative.is_finite() || !size.is_finite() || relative < 0.0 || relative >= size { return None; } - Some(relative.floor() as u32) + let scaled = (relative * positive_scale_or_one(scale_factor)).floor(); + if !scaled.is_finite() || scaled < 0.0 || scaled > u32::MAX as f32 { + return None; + } + + Some(scaled as u32) } fn positive_scroll_component(current: i32, delta: i32) -> i32 { @@ -149,3 +160,12 @@ fn combined_scroll_delta(current: i32, next: i32) -> i32 { let value = i64::from(current) + i64::from(next); value.clamp(i64::from(i32::MIN), i64::from(i32::MAX)) as i32 } + +/// Guard against a zero/negative/NaN scale factor reaching the +/// arithmetic above. GPUI's `Window::scale_factor` returns the real +/// device pixel ratio (1.0 on standard displays, 2.0 on Retina), so a +/// non-positive value would mean the platform reported nonsense; we +/// fall back to 1.0 rather than collapsing every coordinate to zero. +fn positive_scale_or_one(scale_factor: f32) -> f32 { + if scale_factor.is_finite() && scale_factor > 0.0 { scale_factor } else { 1.0 } +} diff --git a/crates/ely_app/src/shell/web_surface_live_site_tests.rs b/crates/ely_app/src/shell/web_surface_live_site_tests.rs index b177815..54c1abf 100644 --- a/crates/ely_app/src/shell/web_surface_live_site_tests.rs +++ b/crates/ely_app/src/shell/web_surface_live_site_tests.rs @@ -101,7 +101,7 @@ fn render_web_surface_frame( for attempt in 0..LIVE_SITE_RENDER_ATTEMPTS { let tab = web_tab(profile_id.clone(), case.url)?; - assert!(store.record_viewport_size(tab.id(), live_surface_bounds()), "{}", case.url); + assert!(store.record_viewport_size(tab.id(), live_surface_bounds(), 1.0), "{}", case.url); store.ensure_surface(&tab, ProfileDataMode::Transient, &[]); match wait_for_ready_frame(store, tab.id(), case) { diff --git a/crates/ely_app/src/shell/web_surface_tests.rs b/crates/ely_app/src/shell/web_surface_tests.rs index c5426ef..557d766 100644 --- a/crates/ely_app/src/shell/web_surface_tests.rs +++ b/crates/ely_app/src/shell/web_surface_tests.rs @@ -10,8 +10,13 @@ fn typed_text_enters_pending_input_after_clicked_viewport() -> Result<(), Box Result<(), Box> let mut store = WebSurfaceStore::new(); let tab = web_tab("https://example.com/list")?; - assert!(store.record_viewport_size(tab.id(), web_bounds())); - assert!(store.record_scroll_delta(tab.id(), tab.url().as_str(), point(px(0.0), px(140.0)))); - assert!(store.record_scroll_delta(tab.id(), tab.url().as_str(), point(px(0.0), px(60.0)))); + assert!(store.record_viewport_size(tab.id(), web_bounds(), 1.0)); + assert!(store.record_scroll_delta( + tab.id(), + tab.url().as_str(), + point(px(0.0), px(140.0)), + 1.0, + )); + assert!(store.record_scroll_delta( + tab.id(), + tab.url().as_str(), + point(px(0.0), px(60.0)), + 1.0, + )); let input = store.take_pending_input(tab.id(), tab.url().as_str()); @@ -43,9 +58,9 @@ fn viewport_size_changes_after_stable_second_measurement() -> Result<(), Box Result<(), Box Result<(), Box Result<(), Box> { + let mut store = WebSurfaceStore::new(); + let tab = web_tab("https://example.com/form")?; + let url = tab.url().as_str(); + + assert!(store.record_viewport_size(tab.id(), web_bounds(), 2.0)); + assert!(store.record_click_point(tab.id(), url, point(px(160.0), px(120.0)), 2.0)); + assert!(store.record_typed_text(tab.id(), url, "h")); + assert!(store.record_scroll_delta(tab.id(), url, point(px(0.0), px(140.0)), 2.0)); + + let input = store.take_pending_input(tab.id(), url); + + assert_eq!( + input.scroll_delta.map(|delta| (delta.x(), delta.y())), + Some((0, 280)), + "wheel delta of 140 logical px must be 280 device px on Retina", + ); + assert_eq!(input.scroll_offset.y(), 280, "scroll offset accumulates in device px"); + assert_eq!( + input.click_point, + None, + "scroll drops the buffered click — its viewport coords are stale", + ); + Ok(()) +} + /// Locks the precondition that `record_typed_text` requires a prior /// click to have established keyboard focus. Without this guard, a /// future refactor could silently start buffering stray keystrokes @@ -105,7 +153,7 @@ fn typing_without_a_prior_click_is_rejected() -> Result<(), Box> { let tab = web_tab("https://example.com/form")?; let url = tab.url().as_str(); - assert!(store.record_viewport_size(tab.id(), web_bounds())); + assert!(store.record_viewport_size(tab.id(), web_bounds(), 1.0)); assert!( !store.record_typed_text(tab.id(), url, "x"), "typing must fail until a click establishes keyboard focus on this tab and url", diff --git a/crates/ely_app/src/shell/web_surface_view.rs b/crates/ely_app/src/shell/web_surface_view.rs index dd7f09d..11f20ab 100644 --- a/crates/ely_app/src/shell/web_surface_view.rs +++ b/crates/ely_app/src/shell/web_surface_view.rs @@ -129,22 +129,26 @@ fn render_input_overlay( }); cx.stop_propagation(); }) - .on_mouse_move(move |event, _window, cx| { + .on_mouse_move(move |event, window, cx| { + let scale_factor = window.scale_factor(); hover_entity.update(cx, |shell, cx| { shell.hover_external_web_viewport( hover_tab_id.clone(), event.position, + scale_factor, cx, ); }); }) .on_scroll_wheel(move |event, window, cx| { let delta = event.delta.pixel_delta(window.line_height()); + let scale_factor = window.scale_factor(); scroll_entity.update(cx, |shell, cx| { shell.scroll_external_web_viewport( scroll_tab_id.clone(), scroll_url.clone(), delta, + scale_factor, cx, ); }); @@ -154,9 +158,10 @@ fn render_input_overlay( fn render_viewport_tracker(tab_id: TabId, state_entity: Entity) -> impl IntoElement { canvas( - move |bounds, _window: &mut Window, cx: &mut App| { + move |bounds, window: &mut Window, cx: &mut App| { + let scale_factor = window.scale_factor(); state_entity.update(cx, |shell, cx| { - shell.record_external_web_viewport(tab_id, bounds, cx); + shell.record_external_web_viewport(tab_id, bounds, scale_factor, cx); }); }, |_, _, _, _| {},