From 7b0ec0c15bb86ee649feadce0ce8d81132feec38 Mon Sep 17 00:00:00 2001 From: ThisMad Date: Sat, 19 Sep 2026 15:48:25 +0200 Subject: [PATCH 1/3] feat(gpui): per-window content zoom, installed by MoonRoot from the theme MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Browser-style page zoom for a GPUI window. `Window::set_content_zoom` folds a zoom into the window's scale factor: every quad, path, glyph and GPU canvas renders at the combined density, `viewport_size` becomes the platform content size divided by the zoom, pointer input is divided once at the top of `dispatch_event`, and the four platform setters plus the IME geometry in `PlatformInputHandler` multiply on the way back — so no platform crate is touched. A zoom requested from a root view's render is parked and applied at the top of the next frame, which the request itself guarantees. `MoonScale` gains `zoom`; `MoonThemeConfig::set_zoom` guards it like `set_ui_scale`; `MoonRoot::render` installs the theme's zoom on its window the way it already installs the rem size, and `install_config` refreshes every window so a change reaches them all. `GpuFrameInfo` and `GpuCanvasTextContext` carry the zoom beside the combined factor for a canvas that keeps device density. Inherited widgets that clamped content positions against the platform window size now read `viewport_size()`. Documented in MOON_PATCH_QUEUE.md with what a re-sync drops; nine window tests and four theme/root tests pin the contracts. --- crates/moon-gpui/src/gpu_canvas.rs | 21 +- crates/moon-gpui/src/platform.rs | 32 +- crates/moon-gpui/src/window.rs | 191 +++++++- crates/moon-gpui/src/window/tests.rs | 442 ++++++++++++++++++ .../src/menu/context_menu.rs | 4 +- .../moon-ui-components/src/menu/popup_menu.rs | 6 +- .../src/moon/context_menu/tests.rs | 2 + .../src/moon/disclosure/tests.rs | 1 + .../src/moon/dropdown/popup/tests.rs | 1 + .../src/moon/dropdown/tests.rs | 12 + .../src/moon/popover/tests.rs | 2 + .../src/moon/segment/tests.rs | 2 + crates/moon-ui-components/src/moon/theme.rs | 55 ++- .../src/moon/theme/tests.rs | 52 +++ crates/moon-ui-components/src/root.rs | 46 +- .../moon-ui-components/src/window_border.rs | 6 +- .../themes/moon-graphite.toml | 2 + .../moon-ui-components/themes/moon-light.toml | 2 + .../themes/moon-terminal.toml | 2 + docs/GPU_CANVAS_TEXT_API.md | 3 +- docs/MOON_PATCH_QUEUE.md | 20 + docs/component-api-baseline.json | 16 + 22 files changed, 896 insertions(+), 24 deletions(-) create mode 100644 crates/moon-gpui/src/window/tests.rs diff --git a/crates/moon-gpui/src/gpu_canvas.rs b/crates/moon-gpui/src/gpu_canvas.rs index 633ae02..c0e1c86 100644 --- a/crates/moon-gpui/src/gpu_canvas.rs +++ b/crates/moon-gpui/src/gpu_canvas.rs @@ -64,8 +64,14 @@ pub struct GpuFrameInfo { pub now: Instant, /// Canvas bounds in logical pixels. pub bounds: Bounds, - /// Window scale factor used to convert logical pixels to device pixels. + /// Effective window scale factor, the platform factor times the content zoom: `bounds` + /// times this is the canvas in device pixels. pub scale_factor: f32, + /// The window's content zoom folded into `scale_factor`. A canvas that must keep device + /// density (a chart whose lines and captions should not follow UI zoom) sizes its own + /// geometry by `scale_factor / content_zoom` while still filling `bounds` times + /// `scale_factor` device pixels. + pub content_zoom: f32, /// Whether the platform currently expects a present to be possible. pub presentable: bool, } @@ -528,6 +534,7 @@ pub struct GpuCanvasTextContext<'a> { pub(crate) sprite_atlas: Arc, pub(crate) bounds: Bounds, pub(crate) scale_factor: f32, + pub(crate) content_zoom: f32, pub(crate) content_mask: ContentMask, pub(crate) background_appearance: WindowBackgroundAppearance, pub(crate) subpixel_rendering_supported: bool, @@ -545,6 +552,7 @@ impl<'a> GpuCanvasTextContext<'a> { sprite_atlas: Arc, bounds: Bounds, scale_factor: f32, + content_zoom: f32, content_mask: ContentMask, background_appearance: WindowBackgroundAppearance, subpixel_rendering_supported: bool, @@ -559,6 +567,7 @@ impl<'a> GpuCanvasTextContext<'a> { sprite_atlas, bounds, scale_factor, + content_zoom, content_mask, background_appearance, subpixel_rendering_supported, @@ -581,11 +590,18 @@ impl<'a> GpuCanvasTextContext<'a> { self.bounds } - /// Window scale factor used for the current frame. + /// Effective window scale factor for the current frame: the platform factor times the + /// content zoom. Logical text metrics and origins times this are device pixels. pub fn scale_factor(&self) -> f32 { self.scale_factor } + /// The window's content zoom folded into [`Self::scale_factor`]; see + /// [`GpuFrameInfo::content_zoom`]. + pub fn content_zoom(&self) -> f32 { + self.content_zoom + } + /// Effective text clip in scaled/device pixels. pub fn content_mask(&self) -> ContentMask { self.content_mask @@ -633,6 +649,7 @@ impl<'a> GpuCanvasTextContext<'a> { self.sprite_atlas.clone(), self.bounds, self.scale_factor, + self.content_zoom, self.content_mask, self.background_appearance, self.subpixel_rendering_supported, diff --git a/crates/moon-gpui/src/platform.rs b/crates/moon-gpui/src/platform.rs index 474b4e0..9c66fd5 100644 --- a/crates/moon-gpui/src/platform.rs +++ b/crates/moon-gpui/src/platform.rs @@ -1261,9 +1261,19 @@ impl PlatformInputHandler { .ok(); } + /// The platform-space bounds of a UTF-16 range, or `None` when the handler has none. + /// + /// Every platform layer positions its IME candidate window from this (and from + /// [`Self::selected_bounds`]), so the content-space bounds the handler reports are multiplied + /// by the window's content zoom here, once, rather than in each platform. pub fn bounds_for_range(&mut self, range_utf16: Range) -> Option> { self.cx - .update(|window, cx| self.handler.bounds_for_range(range_utf16, window, cx)) + .update(|window, cx| { + let zoom = window.content_zoom(); + self.handler + .bounds_for_range(range_utf16, window, cx) + .map(|bounds| bounds.map(|c| c * zoom)) + }) .ok() .flatten() } @@ -1312,14 +1322,24 @@ impl PlatformInputHandler { } } + /// The platform-space bounds the IME candidate window should sit beside, or `None`. + /// + /// Content-space bounds from the handler, multiplied by the window's content zoom like + /// [`Self::bounds_for_range`]. pub fn selected_bounds(&mut self, window: &mut Window, cx: &mut App) -> Option> { + let zoom = window.content_zoom(); let marked_range = self.handler.marked_text_range(window, cx); let selection = self.handler.selected_text_range(true, window, cx)?; Self::compute_ime_candidate_bounds(marked_range, &selection, |range| { self.handler.bounds_for_range(range, window, cx) }) + .map(|bounds| bounds.map(|c| c * zoom)) } + /// [`Self::selected_bounds`] for a caller without the window in hand. + /// + /// Composed from [`Self::bounds_for_range`], which already converts to platform space, so + /// nothing is multiplied again here. pub fn ime_candidate_bounds(&mut self) -> Option> { let marked_range = self.marked_text_range(); let selection = self.selected_text_range(true)?; @@ -1329,9 +1349,17 @@ impl PlatformInputHandler { } #[allow(unused)] + /// The character under a platform-space point, or `None`. + /// + /// The point arrives in the platform's logical pixels and is divided by the content zoom + /// before the handler, which lays text out in content space, sees it. pub fn character_index_for_point(&mut self, point: Point) -> Option { self.cx - .update(|window, cx| self.handler.character_index_for_point(point, window, cx)) + .update(|window, cx| { + let zoom = window.content_zoom(); + self.handler + .character_index_for_point(point.map(|c| c / zoom), window, cx) + }) .ok() .flatten() } diff --git a/crates/moon-gpui/src/window.rs b/crates/moon-gpui/src/window.rs index ba4987c..3dcf385 100644 --- a/crates/moon-gpui/src/window.rs +++ b/crates/moon-gpui/src/window.rs @@ -62,6 +62,8 @@ use uuid::Uuid; pub(crate) mod a11y; mod prompts; +#[cfg(test)] +mod tests; use self::a11y::A11y; #[cfg(not(target_family = "wasm"))] @@ -1257,7 +1259,16 @@ pub struct Window { mouse_hit_test: HitTest, modifiers: Modifiers, capslock: Capslock, + /// Effective scale factor: the platform's factor multiplied by [`Self::content_zoom`]. + /// Every primitive, glyph raster and layout conversion reads it through + /// [`Self::scale_factor`], so zoomed content renders at the combined density. scale_factor: f32, + /// Browser-style page zoom layered on the platform scale factor. Layout runs in content + /// space: `viewport_size` is the platform content size divided by this, pointer input is + /// divided by it on entry, and coordinates handed back to the platform are multiplied. + content_zoom: f32, + /// A zoom requested while a frame was being drawn; applied at the top of the next `draw`. + pending_content_zoom: Option, pub(crate) bounds_observers: SubscriberSet<(), AnyObserver>, appearance: WindowAppearance, pub(crate) appearance_observers: SubscriberSet<(), AnyObserver>, @@ -1881,6 +1892,10 @@ impl Window { modifiers, capslock, scale_factor, + // A window opens unzoomed, so the platform's content size and mouse position above + // are already content space; `set_content_zoom` reconciles them from then on. + content_zoom: 1.0, + pending_content_zoom: None, bounds_observers: SubscriberSet::new(), appearance, appearance_observers: SubscriberSet::new(), @@ -2147,11 +2162,15 @@ impl Window { /// Return the `WindowBounds` to indicate that how a window should be opened /// after it has been closed + /// + /// Screen space, in the platform's logical pixels: unaffected by [`Self::content_zoom`]. pub fn window_bounds(&self) -> WindowBounds { self.platform_window.window_bounds() } /// Return the `WindowBounds` excluding insets (Wayland and X11) + /// + /// Screen space, in the platform's logical pixels: unaffected by [`Self::content_zoom`]. pub fn inner_window_bounds(&self) -> WindowBounds { self.platform_window.inner_window_bounds() } @@ -2385,8 +2404,7 @@ impl Window { /// the platform window, then notifies observers. Normally called automatically /// by the platform's resize callback, but exposed publicly for test infrastructure. pub fn bounds_changed(&mut self, cx: &mut App) { - self.scale_factor = self.platform_window.scale_factor(); - self.viewport_size = self.platform_window.content_size(); + self.sync_platform_geometry(); self.display_id = self.platform_window.display().map(|display| display.id()); self.refresh(); @@ -2396,7 +2414,72 @@ impl Window { .retain(&(), |callback| callback(self, cx)); } + /// Recompute the effective scale factor and the content-space viewport from the platform + /// window and the current content zoom. + /// + /// The platform reports its own factor and content size; multiplying the zoom in here means a + /// DPI change (a window dragged to another monitor) keeps the zoom instead of dropping it. + fn sync_platform_geometry(&mut self) { + let zoom = self.content_zoom; + self.scale_factor = self.platform_window.scale_factor() * zoom; + self.viewport_size = self.platform_window.content_size().map(|d| d / zoom); + } + + /// Browser-style content zoom of this window, `1.0` by default. + /// + /// See [`Self::set_content_zoom`]. While a zoom requested during a draw is still pending, this + /// keeps reporting the applied value. + pub fn content_zoom(&self) -> f32 { + self.content_zoom + } + + /// Set the content zoom, scaling everything the window draws like page zoom in a browser. + /// + /// The effective [`Self::scale_factor`] becomes the platform factor times `zoom`, so every + /// quad, path, glyph and GPU canvas renders at the combined density; [`Self::viewport_size`] + /// shrinks or grows accordingly and pointer input arrives in that content space. Screen-space + /// queries ([`Self::bounds`], [`Self::window_bounds`]) are unaffected. + /// + /// Non-finite or non-positive values are ignored and an unchanged value is a no-op. Outside a + /// draw the change applies immediately: the geometry is recomputed, the window is refreshed + /// and bounds observers run, exactly as for a platform resize. During a draw — a root view's + /// `render` is the expected caller — the change is deferred to the top of the next frame so + /// the frame in progress stays consistent, and that frame is requested. + pub fn set_content_zoom(&mut self, zoom: f32, cx: &mut App) { + if !(zoom.is_finite() && zoom > 0.0) { + return; + } + if zoom == self.pending_content_zoom.unwrap_or(self.content_zoom) { + return; + } + if self.invalidator.not_drawing() { + self.apply_content_zoom(zoom, cx); + } else { + self.pending_content_zoom = Some(zoom); + // `draw` cleared the dirty flag before rendering, so this survives the frame and + // guarantees the one that applies the zoom. + self.invalidator.set_dirty(true); + } + } + + /// Install a validated zoom: geometry, the platform's client inset, a refresh and the bounds + /// observers, in that order. + fn apply_content_zoom(&mut self, zoom: f32, cx: &mut App) { + self.content_zoom = zoom; + self.sync_platform_geometry(); + if let Some(inset) = self.client_inset { + self.platform_window.set_client_inset(inset * zoom); + } + self.refresh(); + self.bounds_observers + .clone() + .retain(&(), |callback| callback(self, cx)); + } + /// Returns the bounds of the current window in the global coordinate space, which could span across multiple displays. + /// + /// Screen space, in the platform's logical pixels: unaffected by [`Self::content_zoom`]. Use + /// [`Self::viewport_size`] for anything laid out inside the window. pub fn bounds(&self) -> Bounds { self.platform_window.bounds() } @@ -2410,9 +2493,11 @@ impl Window { .render_to_image(&self.rendered_frame.scene) } - /// Set the content size of the window. + /// Set the content size of the window, in content space (the counterpart of + /// [`Self::viewport_size`]); the platform receives it multiplied by the content zoom. pub fn resize(&mut self, size: Size) { - self.platform_window.resize(size); + let zoom = self.content_zoom; + self.platform_window.resize(size.map(|d| d * zoom)); } /// Returns whether or not the window is currently fullscreen @@ -2439,7 +2524,8 @@ impl Window { self.appearance } - /// Returns the size of the drawable area within the window. + /// Returns the size of the drawable area within the window, in content space: the platform + /// content size divided by [`Self::content_zoom`]. pub fn viewport_size(&self) -> Size { self.viewport_size } @@ -2471,7 +2557,8 @@ impl Window { /// Opens the native title bar context menu, useful when implementing client side decorations (Wayland and X11) pub fn show_window_menu(&self, position: Point) { - self.platform_window.show_window_menu(position) + self.platform_window + .show_window_menu(position.map(|c| c * self.content_zoom)) } /// Handle window movement for Linux and macOS. @@ -2484,8 +2571,11 @@ impl Window { /// When using client side decorations, set this to the width of the invisible decorations (Wayland and X11) pub fn set_client_inset(&mut self, inset: Pixels) { + // Stored in content space, because `client_inset()` feeds layout; the platform gets the + // inset in its own pixels. self.client_inset = Some(inset); - self.platform_window.set_client_inset(inset); + self.platform_window + .set_client_inset(inset * self.content_zoom); } /// Returns the client_inset value by [`Self::set_client_inset`]. @@ -2511,7 +2601,8 @@ impl Window { /// Sets the position of the macOS traffic light buttons. #[cfg(target_os = "macos")] pub fn set_traffic_light_position(&self, position: Point) { - self.platform_window.set_traffic_light_position(position); + self.platform_window + .set_traffic_light_position(position.map(|c| c * self.content_zoom)); } /// Sets the application identifier. @@ -2556,7 +2647,8 @@ impl Window { /// The scale factor of the display associated with the window. For example, it could /// return 2.0 for a "retina" display, indicating that each logical pixel should actually - /// be rendered as two pixels on screen. + /// be rendered as two pixels on screen. Includes [`Self::content_zoom`]: glyphs rasterise and + /// quads snap at this combined factor, and logical pixels times it are device pixels. pub fn scale_factor(&self) -> f32 { self.scale_factor } @@ -2728,7 +2820,8 @@ impl Window { .is_action_available(action, node_id) } - /// The position of the mouse relative to the window. + /// The position of the mouse relative to the window, in content space (already divided by + /// [`Self::content_zoom`]). pub fn mouse_position(&self) -> Point { self.mouse_position } @@ -2805,6 +2898,7 @@ impl Window { let now = Instant::now(); let scale_factor = self.scale_factor(); + let content_zoom = self.content_zoom; let presentable = self.platform_window.can_present(); let text_system = self.text_system.clone(); let sprite_atlas = self.sprite_atlas.clone(); @@ -2838,6 +2932,7 @@ impl Window { now, bounds, scale_factor, + content_zoom, presentable, }; let wants_present = canvas.driver.frame(info).requests_present(); @@ -2856,6 +2951,7 @@ impl Window { sprite_atlas.clone(), bounds, scale_factor, + content_zoom, content_mask, background_appearance, subpixel_rendering_supported, @@ -2906,6 +3002,12 @@ impl Window { // This ensures that multiple test Apps have isolated arenas. let _arena_scope = ElementArenaScope::enter(&cx.element_arena); + // A zoom requested from the previous frame's render lands here, before the entities it + // invalidates are collected, so the frame about to be drawn is the one that shows it. + if let Some(zoom) = self.pending_content_zoom.take() { + self.apply_content_zoom(zoom, cx); + } + self.invalidate_entities(); cx.entities.clear_accessed(); debug_assert!(self.rendered_entity_stack.is_empty()); @@ -4834,12 +4936,81 @@ impl Window { .unwrap_or_else(|| action.name().to_string()) } + /// Bring a platform event's pointer coordinates into content space. + /// + /// Every platform hands `dispatch_event` positions in its own logical pixels; under a content + /// zoom the element tree is laid out in those pixels divided by the zoom, so the divide happens + /// once, here, before anything reads the event. Pixel scroll deltas follow the same rule; line + /// deltas and the pinch ratio are unitless. The match is exhaustive on purpose: a variant that + /// arrives with an upstream re-sync must be classified rather than silently passed through. + fn unzoom_input(&self, event: PlatformInput) -> PlatformInput { + let zoom = self.content_zoom; + if zoom == 1.0 { + return event; + } + let unzoom = |p: Point| p.map(|c| c / zoom); + match event { + PlatformInput::MouseMove(mut e) => { + e.position = unzoom(e.position); + PlatformInput::MouseMove(e) + } + PlatformInput::MouseDown(mut e) => { + e.position = unzoom(e.position); + PlatformInput::MouseDown(e) + } + PlatformInput::MouseUp(mut e) => { + e.position = unzoom(e.position); + PlatformInput::MouseUp(e) + } + PlatformInput::MousePressure(mut e) => { + e.position = unzoom(e.position); + PlatformInput::MousePressure(e) + } + PlatformInput::MouseExited(mut e) => { + e.position = unzoom(e.position); + PlatformInput::MouseExited(e) + } + PlatformInput::Pinch(mut e) => { + e.position = unzoom(e.position); + PlatformInput::Pinch(e) + } + PlatformInput::ScrollWheel(mut e) => { + e.position = unzoom(e.position); + if let crate::ScrollDelta::Pixels(delta) = e.delta { + e.delta = crate::ScrollDelta::Pixels(unzoom(delta)); + } + PlatformInput::ScrollWheel(e) + } + PlatformInput::FileDrop(FileDropEvent::Entered { position, paths }) => { + PlatformInput::FileDrop(FileDropEvent::Entered { + position: unzoom(position), + paths, + }) + } + PlatformInput::FileDrop(FileDropEvent::Pending { position }) => { + PlatformInput::FileDrop(FileDropEvent::Pending { + position: unzoom(position), + }) + } + PlatformInput::FileDrop(FileDropEvent::Submit { position }) => { + PlatformInput::FileDrop(FileDropEvent::Submit { + position: unzoom(position), + }) + } + event @ (PlatformInput::FileDrop(FileDropEvent::Exited) + | PlatformInput::KeyDown(_) + | PlatformInput::KeyUp(_) + | PlatformInput::ModifiersChanged(_)) => event, + } + } + /// Dispatch a mouse or keyboard event on the window. #[profiling::function] pub fn dispatch_event(&mut self, event: PlatformInput, cx: &mut App) -> DispatchEventResult { #[cfg(feature = "input-latency-histogram")] let dispatch_time = Instant::now(); let update_count_before = self.invalidator.update_count(); + let event = self.unzoom_input(event); // Track input modality for focus-visible styling and hover suppression. // Hover is suppressed during keyboard modality so that keyboard navigation // doesn't show hover highlights on the item under the mouse cursor. diff --git a/crates/moon-gpui/src/window/tests.rs b/crates/moon-gpui/src/window/tests.rs new file mode 100644 index 0000000..08d3333 --- /dev/null +++ b/crates/moon-gpui/src/window/tests.rs @@ -0,0 +1,442 @@ +//! Content-zoom contracts for `window.rs`. +//! +//! Browser-style page zoom folds into the window's scale factor. These tests pin the places it +//! must be honoured — geometry, pointer input and the deferred apply from a render — and the two +//! boundaries that cross back to platform space: IME geometry and GPU canvas frame info. + +use std::{ + cell::{Cell, RefCell}, + ops::Range, + rc::Rc, +}; + +use crate::{ + AnyWindowHandle, App, AppContext as _, AsyncWindowContext, Bounds, Context, FileDropEvent, + GpuCanvasDrawContext, GpuCanvasDriver, GpuCanvasHandle, GpuCanvasPrepareContext, + GpuFrameDecision, GpuFrameInfo, InputEvent as _, InputHandler, InteractiveElement as _, + IntoElement, MouseMoveEvent, Pixels, PlatformInput, PlatformInputHandler, Point, Render, + ScrollDelta, ScrollWheelEvent, Size, Styled as _, TestAppContext, TouchPhase, UTF16Selection, + Window, div, gpu_canvas, point, px, size, +}; + +/// A root view that draws nothing; the window's own state is what these tests read. +struct EmptyView; + +impl Render for EmptyView { + fn render(&mut self, _: &mut Window, _: &mut Context) -> impl IntoElement { + div() + } +} + +/// Open a test window on `build` and draw its first frame. +fn open_window( + build: impl FnOnce(&mut Window, &mut Context) -> V, +) -> (TestAppContext, AnyWindowHandle) { + let mut app = TestAppContext::single(); + let window = app.add_window(build); + let any: AnyWindowHandle = window.into(); + draw(&mut app, any); + (app, any) +} + +/// Draw one frame of `any`. +fn draw(app: &mut TestAppContext, any: AnyWindowHandle) { + app.update_window(any, |_, window, cx| { + window.draw(cx).clear(); + }) + .unwrap(); +} + +/// Set the content zoom of `any` from outside a draw. +fn zoom(app: &mut TestAppContext, any: AnyWindowHandle, zoom: f32) { + app.update_window(any, |_, window, cx| window.set_content_zoom(zoom, cx)) + .unwrap(); +} + +/// The platform factor of `any`, read before any zoom is set. +fn platform_factor(app: &mut TestAppContext, any: AnyWindowHandle) -> f32 { + app.update_window(any, |_, window, _| window.scale_factor()) + .unwrap() +} + +/// Catches dropping either term of `window.rs:sync_platform_geometry`: the interface would keep +/// rendering at the platform density, or lay out against a viewport the zoom no longer fits. +#[test] +fn content_zoom_multiplies_the_platform_factor_and_divides_the_viewport() { + let (mut app, any) = open_window(|_, _| EmptyView); + app.update_window(any, |_, window, cx| { + let factor = window.scale_factor(); + let viewport = window.viewport_size(); + window.set_content_zoom(2.0, cx); + assert_eq!(window.content_zoom(), 2.0); + assert_eq!(window.scale_factor(), factor * 2.0); + assert_eq!(window.viewport_size(), viewport.map(|d| d / 2.0)); + }) + .unwrap(); +} + +/// Catches a re-sync restoring the bare platform reads in `window.rs:bounds_changed`: the first +/// resize or monitor change would silently drop the zoom. +#[test] +fn content_zoom_survives_a_platform_resize() { + let (mut app, any) = open_window(|_, _| EmptyView); + let platform_factor = platform_factor(&mut app, any); + zoom(&mut app, any, 2.0); + app.simulate_window_resize(any, size(px(800.), px(600.))); + app.update_window(any, |_, window, _| { + assert_eq!(window.viewport_size(), size(px(400.), px(300.))); + assert_eq!(window.scale_factor(), platform_factor * 2.0); + }) + .unwrap(); +} + +/// Catches a missing arm in `window.rs:unzoom_input`: a click at zoom would land on the element +/// twice as far from the origin as the one under the pointer. +#[test] +fn pointer_positions_arrive_in_content_space() { + let (mut app, any) = open_window(|_, _| EmptyView); + zoom(&mut app, any, 2.0); + app.update_window(any, |_, window, cx| { + window.dispatch_event( + MouseMoveEvent { + position: point(px(100.), px(100.)), + modifiers: Default::default(), + pressed_button: None, + } + .to_platform_input(), + cx, + ); + assert_eq!(window.mouse_position(), point(px(50.), px(50.))); + + window.dispatch_event( + PlatformInput::FileDrop(FileDropEvent::Pending { + position: point(px(80.), px(40.)), + }), + cx, + ); + assert_eq!(window.mouse_position(), point(px(40.), px(20.))); + }) + .unwrap(); +} + +/// A 50 px square that records the scroll delta it receives. +struct ScrollProbe { + seen: Rc>>, +} + +impl Render for ScrollProbe { + fn render(&mut self, _: &mut Window, _: &mut Context) -> impl IntoElement { + let seen = self.seen.clone(); + div() + .id("scroll-probe") + .size(px(50.)) + .on_scroll_wheel(move |event, _, _| seen.set(Some(event.delta))) + } +} + +/// Catches scaling `Lines` or forgetting `Pixels` in `window.rs:unzoom_input`: a trackpad would +/// scroll twice as far as the finger moved, or a wheel notch would shrink with the zoom. +#[test] +fn pixel_scroll_deltas_are_divided_but_line_deltas_are_not() { + let seen: Rc>> = Rc::new(Cell::new(None)); + let (mut app, any) = open_window({ + let seen = seen.clone(); + move |_, _| ScrollProbe { seen } + }); + zoom(&mut app, any, 2.0); + draw(&mut app, any); + let scroll = |delta: ScrollDelta| { + ScrollWheelEvent { + position: point(px(20.), px(20.)), + delta, + modifiers: Default::default(), + touch_phase: TouchPhase::Moved, + } + .to_platform_input() + }; + app.update_window(any, |_, window, cx| { + window.dispatch_event(scroll(ScrollDelta::Pixels(point(px(10.), px(20.)))), cx); + assert!( + matches!(seen.get(), Some(ScrollDelta::Pixels(d)) if d == point(px(5.), px(10.))), + "pixel deltas are content space: {:?}", + seen.get() + ); + window.dispatch_event(scroll(ScrollDelta::Lines(point(0., 3.))), cx); + assert!( + matches!(seen.get(), Some(ScrollDelta::Lines(d)) if d == point(0., 3.)), + "line deltas are unitless: {:?}", + seen.get() + ); + }) + .unwrap(); +} + +/// Catches removing the guard in `window.rs:set_content_zoom`: a stored `0` would divide the +/// viewport by zero and lay the whole window out at infinity. +#[test] +fn an_impossible_content_zoom_is_ignored() { + let (mut app, any) = open_window(|_, _| EmptyView); + let platform_factor = platform_factor(&mut app, any); + zoom(&mut app, any, 2.0); + for impossible in [0.0, -1.0, f32::NAN, f32::INFINITY] { + zoom(&mut app, any, impossible); + app.update_window(any, |_, window, _| { + assert_eq!(window.content_zoom(), 2.0, "{impossible} must be ignored"); + assert_eq!(window.scale_factor(), platform_factor * 2.0); + }) + .unwrap(); + } +} + +/// A root view whose only job is to own a bounds observer. +struct BoundsCounter; + +impl Render for BoundsCounter { + fn render(&mut self, _: &mut Window, _: &mut Context) -> impl IntoElement { + div() + } +} + +/// Catches dropping the observer call in `window.rs:apply_content_zoom` (a consumer that measures +/// against the viewport never re-measures) or the no-op check in `set_content_zoom` (a root that +/// re-applies the theme zoom every frame would fire observers at frame rate). +#[test] +fn changing_content_zoom_notifies_bounds_observers_once() { + let count: Rc> = Rc::new(Cell::new(0)); + let (mut app, any) = open_window({ + let count = count.clone(); + move |window, cx: &mut Context| { + cx.observe_window_bounds(window, move |_, _, _| count.set(count.get() + 1)) + .detach(); + BoundsCounter + } + }); + assert_eq!(count.get(), 0, "drawing alone changes no bounds"); + zoom(&mut app, any, 2.0); + assert_eq!(count.get(), 1, "a new zoom is a bounds change"); + zoom(&mut app, any, 2.0); + assert_eq!(count.get(), 1, "the same zoom again is not"); +} + +/// A root view that, once armed, asks for a zoom from inside its own render, as a themed root +/// does, and records the zoom and viewport each render saw. +struct ZoomingView { + armed: Rc>, + renders: Rc)>>>, +} + +impl Render for ZoomingView { + fn render(&mut self, window: &mut Window, cx: &mut Context) -> impl IntoElement { + self.renders + .borrow_mut() + .push((window.content_zoom(), window.viewport_size())); + if self.armed.get() { + window.set_content_zoom(2.0, cx); + } + div() + } +} + +/// Catches applying the zoom mid-frame in `window.rs:set_content_zoom` (the root is laid out at +/// the old size and painted at the new density for one frame) or omitting `set_dirty` there (a +/// zoom set from a render would never get the frame that applies it). +#[test] +fn a_zoom_requested_while_drawing_lands_on_the_next_frame() { + let armed = Rc::new(Cell::new(false)); + let renders: Rc)>>> = Rc::new(RefCell::new(Vec::new())); + // Created disarmed, because the harness draws a new window itself; arming afterwards puts + // the two frames that matter under this test's own draws. + let (mut app, any) = open_window({ + let armed = armed.clone(); + let renders = renders.clone(); + move |_, _| ZoomingView { armed, renders } + }); + armed.set(true); + let frames_before = renders.borrow().len(); + app.update_window(any, |_, window, cx| { + let viewport = window.viewport_size(); + window.draw(cx).clear(); + assert_eq!( + window.content_zoom(), + 1.0, + "the frame in progress keeps its geometry" + ); + assert!( + window.invalidator.is_dirty(), + "the zoom must request the frame that applies it" + ); + window.draw(cx).clear(); + assert_eq!(window.content_zoom(), 2.0); + assert_eq!(window.viewport_size(), viewport.map(|d| d / 2.0)); + let seen = renders.borrow(); + assert_eq!( + &seen[frames_before..], + &[(1.0, viewport), (2.0, viewport.map(|d| d / 2.0))], + "the parking render saw the old geometry, the applying one the new" + ); + }) + .unwrap(); +} + +/// An input handler with a caret at (10, 10)-(20, 20) in content space that records the point +/// it is asked about. +struct CaretHandler { + seen_point: Rc>>>, +} + +impl InputHandler for CaretHandler { + fn selected_text_range( + &mut self, + _: bool, + _: &mut Window, + _: &mut App, + ) -> Option { + Some(UTF16Selection { + range: 0..0, + reversed: false, + }) + } + + fn marked_text_range(&mut self, _: &mut Window, _: &mut App) -> Option> { + None + } + + fn text_for_range( + &mut self, + _: Range, + _: &mut Option>, + _: &mut Window, + _: &mut App, + ) -> Option { + None + } + + fn replace_text_in_range( + &mut self, + _: Option>, + _: &str, + _: &mut Window, + _: &mut App, + ) { + } + + fn replace_and_mark_text_in_range( + &mut self, + _: Option>, + _: &str, + _: Option>, + _: &mut Window, + _: &mut App, + ) { + } + + fn unmark_text(&mut self, _: &mut Window, _: &mut App) {} + + fn bounds_for_range( + &mut self, + _: Range, + _: &mut Window, + _: &mut App, + ) -> Option> { + Some(Bounds { + origin: point(px(10.), px(10.)), + size: size(px(10.), px(10.)), + }) + } + + fn character_index_for_point( + &mut self, + point: Point, + _: &mut Window, + _: &mut App, + ) -> Option { + self.seen_point.set(Some(point)); + Some(0) + } +} + +/// Catches dropping either conversion in `platform.rs:PlatformInputHandler`: the IME candidate +/// window would float away from the caret at zoom, and a click into composed text would pick the +/// wrong character. +#[test] +fn ime_geometry_crosses_to_platform_space_and_back() { + let (mut app, any) = open_window(|_, _| EmptyView); + zoom(&mut app, any, 2.0); + let seen_point = Rc::new(Cell::new(None)); + let mut handler = PlatformInputHandler::new( + AsyncWindowContext::new_context(app.to_async(), any), + Box::new(CaretHandler { + seen_point: seen_point.clone(), + }), + ); + let platform_caret = Bounds { + origin: point(px(20.), px(20.)), + size: size(px(20.), px(20.)), + }; + + assert_eq!(handler.bounds_for_range(0..0), Some(platform_caret)); + assert_eq!( + handler.ime_candidate_bounds(), + Some(platform_caret), + "composed from the converted range, so converted exactly once" + ); + app.update_window(any, |_, window, cx| { + assert_eq!(handler.selected_bounds(window, cx), Some(platform_caret)); + }) + .unwrap(); + + assert_eq!( + handler.character_index_for_point(point(px(40.), px(40.))), + Some(0) + ); + assert_eq!(seen_point.get(), Some(point(px(20.), px(20.)))); +} + +/// A GPU canvas driver that records the factor and zoom of the frame info it receives. +struct FrameInfoProbe { + seen: Rc>>, +} + +impl GpuCanvasDriver for FrameInfoProbe { + fn frame(&mut self, info: GpuFrameInfo) -> GpuFrameDecision { + self.seen.set(Some((info.scale_factor, info.content_zoom))); + GpuFrameDecision::Skip + } + + fn prepare_gpu(&mut self, _: &mut GpuCanvasPrepareContext<'_>) -> anyhow::Result<()> { + Ok(()) + } + + fn draw(&mut self, _: &mut GpuCanvasDrawContext<'_>) -> anyhow::Result<()> { + Ok(()) + } +} + +/// A root view that paints one 50 px GPU canvas. +struct CanvasView { + handle: GpuCanvasHandle, +} + +impl Render for CanvasView { + fn render(&mut self, _: &mut Window, _: &mut Context) -> impl IntoElement { + gpu_canvas(self.handle.clone()).size(px(50.)) + } +} + +/// Catches dropping the `content_zoom` assignment where `window.rs:frame_gpu_canvases` builds the +/// frame info: a canvas that keeps device density (a chart) could no longer tell the zoom apart +/// from the platform factor and would silently zoom with the interface. +#[test] +fn gpu_canvas_frame_info_carries_the_zoom_beside_the_combined_factor() { + let seen = Rc::new(Cell::new(None)); + let handle = GpuCanvasHandle::new(FrameInfoProbe { seen: seen.clone() }); + let (mut app, any) = open_window(move |_, _| CanvasView { handle }); + let platform_factor = platform_factor(&mut app, any); + zoom(&mut app, any, 2.0); + draw(&mut app, any); + app.update_window(any, |_, window, _| { + window.frame_gpu_canvases(false); + }) + .unwrap(); + assert_eq!(seen.get(), Some((platform_factor * 2.0, 2.0))); +} diff --git a/crates/moon-ui-components/src/menu/context_menu.rs b/crates/moon-ui-components/src/menu/context_menu.rs index 73b5a55..3b839d3 100644 --- a/crates/moon-ui-components/src/menu/context_menu.rs +++ b/crates/moon-ui-components/src/menu/context_menu.rs @@ -181,8 +181,8 @@ impl Element for ContextMenu< deferred( anchored().child( div() - .w(window.bounds().size.width) - .h(window.bounds().size.height) + .w(window.viewport_size().width) + .h(window.viewport_size().height) .on_scroll_wheel(|_, _, cx| { cx.stop_propagation(); }) diff --git a/crates/moon-ui-components/src/menu/popup_menu.rs b/crates/moon-ui-components/src/menu/popup_menu.rs index c477c8a..87de6ef 100644 --- a/crates/moon-ui-components/src/menu/popup_menu.rs +++ b/crates/moon-ui-components/src/menu/popup_menu.rs @@ -1113,13 +1113,13 @@ impl PopupMenu { fn update_submenu_menu_anchor(&mut self, window: &Window) { let bounds = self.bounds; let max_width = self.max_width(); - let (anchor, left) = if max_width + bounds.origin.x > window.bounds().size.width { + let (anchor, left) = if max_width + bounds.origin.x > window.viewport_size().width { (Anchor::TopRight, -px(16.)) } else { (Anchor::TopLeft, bounds.size.width - px(8.)) }; - let is_bottom_pos = bounds.origin.y + bounds.size.height > window.bounds().size.height; + let is_bottom_pos = bounds.origin.y + bounds.size.height > window.viewport_size().height; self.submenu_anchor = if is_bottom_pos { (anchor.other_side_along(gpui::Axis::Vertical), left) } else { @@ -1354,7 +1354,7 @@ impl Render for PopupMenu { let items_count = self.menu_items.len(); let max_height = self.max_height.unwrap_or_else(|| { - let window_half_height = window.window_bounds().get_bounds().size.height * 0.5; + let window_half_height = window.viewport_size().height * 0.5; window_half_height.min(px(450.)) }); diff --git a/crates/moon-ui-components/src/moon/context_menu/tests.rs b/crates/moon-ui-components/src/moon/context_menu/tests.rs index 30c9ceb..40bc4d5 100644 --- a/crates/moon-ui-components/src/moon/context_menu/tests.rs +++ b/crates/moon-ui-components/src/moon/context_menu/tests.rs @@ -120,6 +120,7 @@ fn fitted_root_context_menu_grows_and_stays_inside_scaled_viewports(cx: &mut gpu font: 1.35, font_delta: 2.0, tier: Default::default(), + zoom: 1.0, }, ), ( @@ -129,6 +130,7 @@ fn fitted_root_context_menu_grows_and_stays_inside_scaled_viewports(cx: &mut gpu font: 0.9, font_delta: 2.0, tier: Default::default(), + zoom: 1.0, }, ), ] { diff --git a/crates/moon-ui-components/src/moon/disclosure/tests.rs b/crates/moon-ui-components/src/moon/disclosure/tests.rs index fc0d6f9..041c3f4 100644 --- a/crates/moon-ui-components/src/moon/disclosure/tests.rs +++ b/crates/moon-ui-components/src/moon/disclosure/tests.rs @@ -146,6 +146,7 @@ fn caret_geometry_scales_with_the_ui_token(cx: &mut gpui::TestAppContext) { font: 1.0, font_delta: 0.0, tier: Default::default(), + zoom: 1.0, }; }); diff --git a/crates/moon-ui-components/src/moon/dropdown/popup/tests.rs b/crates/moon-ui-components/src/moon/dropdown/popup/tests.rs index 7293f71..92537d2 100644 --- a/crates/moon-ui-components/src/moon/dropdown/popup/tests.rs +++ b/crates/moon-ui-components/src/moon/dropdown/popup/tests.rs @@ -153,6 +153,7 @@ fn submenu_flips_left_and_caps_height_at_viewport_edges(cx: &mut gpui::TestAppCo font: 1.0, font_delta: 2.0, tier: Default::default(), + zoom: 1.0, }; }); let window = cx.add_window(move |_, _| CascadeHarness { diff --git a/crates/moon-ui-components/src/moon/dropdown/tests.rs b/crates/moon-ui-components/src/moon/dropdown/tests.rs index 5f26547..91591af 100644 --- a/crates/moon-ui-components/src/moon/dropdown/tests.rs +++ b/crates/moon-ui-components/src/moon/dropdown/tests.rs @@ -414,6 +414,7 @@ fn fitted_dropdown_stays_inside_both_viewport_edges_at_independent_scales( font: 1.35, font_delta: 2.0, tier: Default::default(), + zoom: 1.0, }, ), ( @@ -423,6 +424,7 @@ fn fitted_dropdown_stays_inside_both_viewport_edges_at_independent_scales( font: 0.9, font_delta: 2.0, tier: Default::default(), + zoom: 1.0, }, ), ] { @@ -629,6 +631,7 @@ fn fitted_trigger_preserves_caret_at_independent_scale_extremes() { font, font_delta, tier: Default::default(), + zoom: 1.0, }; let font_size = 10.5; let text_scale = tokens.font(font_size) / font_size; @@ -674,6 +677,7 @@ fn scaled_trigger_uses_font_width_without_clipping_component_chrome() { font, font_delta, tier: Default::default(), + zoom: 1.0, }; let font_size = 10.5; let text_scale = tokens.font(font_size) / font_size; @@ -721,6 +725,7 @@ fn scaled_menu_width_retains_fitted_rows_at_independent_scale_extremes( font, font_delta, tier: Default::default(), + zoom: 1.0, }; let metrics = MoonPopupMenu::new("scaled-menu-test") .size(MoonSize::Sm) @@ -772,6 +777,7 @@ fn menu_max_height_distinguishes_ui_scaled_and_rendered_values() { font: 0.25, font_delta: 0.0, tier: Default::default(), + zoom: 1.0, }; assert_eq!( @@ -1353,6 +1359,7 @@ fn fitted_submenu_resolves_width_from_its_own_items(cx: &mut gpui::TestAppContex font: 0.75, font_delta: 4.0, tier: Default::default(), + zoom: 1.0, }; cx.update(|cx| { MoonTheme::global_mut(cx).scale = scale; @@ -1537,6 +1544,7 @@ fn pinned_header_wrapper_enforces_its_declared_height(cx: &mut gpui::TestAppCont font: 1.0, font_delta: 0.0, tier: Default::default(), + zoom: 1.0, }; }); let window = cx.add_window(|_, _| HeaderHeightHarness); @@ -1791,6 +1799,7 @@ fn pinned_header_scaling_shrinks_the_wrapper_when_the_clamp_engages(cx: &mut gpu font: 1.0, font_delta: 0.0, tier: Default::default(), + zoom: 1.0, }; }); let window = cx.add_window(|_, _| HeaderClampScalingHarness); @@ -1869,6 +1878,7 @@ fn tier_menu_text_follows_ui_zoom_never_font_scale() { font: 1.75, font_delta: 4.0, tier: Default::default(), + zoom: 1.0, }, ..MoonThemeTokens::default() }; @@ -1931,6 +1941,7 @@ fn tier_menu_width_follows_ui_zoom_never_font_delta(cx: &mut gpui::TestAppContex font, font_delta, tier: Default::default(), + zoom: 1.0, }, ..MoonThemeTokens::default() }; @@ -1989,6 +2000,7 @@ fn scaled_menu_width_at_a_tier_follows_ui_zoom_never_font_delta(cx: &mut gpui::T font, font_delta, tier: Default::default(), + zoom: 1.0, }, ..MoonThemeTokens::default() }; diff --git a/crates/moon-ui-components/src/moon/popover/tests.rs b/crates/moon-ui-components/src/moon/popover/tests.rs index dec2297..3b326fa 100644 --- a/crates/moon-ui-components/src/moon/popover/tests.rs +++ b/crates/moon-ui-components/src/moon/popover/tests.rs @@ -23,6 +23,7 @@ fn content_width_policies_reserve_scaled_popup_chrome() { font, font_delta, tier: Default::default(), + zoom: 1.0, }; let chrome = tokens.ui(POPOVER_PADDING) * 2.0 + POPOVER_BORDER * 2.0; @@ -76,6 +77,7 @@ fn intrinsic_popover_shrink_wraps_its_rendered_child(cx: &mut gpui::TestAppConte font: 0.25, font_delta: 0.0, tier: Default::default(), + zoom: 1.0, }; cx.update(|cx| { MoonTheme::global_mut(cx).scale = scale; diff --git a/crates/moon-ui-components/src/moon/segment/tests.rs b/crates/moon-ui-components/src/moon/segment/tests.rs index e3bc853..5acef3f 100644 --- a/crates/moon-ui-components/src/moon/segment/tests.rs +++ b/crates/moon-ui-components/src/moon/segment/tests.rs @@ -116,6 +116,7 @@ fn fitted_item_preserves_the_boundary_and_ellipsizes_one_past_it() { font, font_delta, tier: Default::default(), + zoom: 1.0, }; let min = tokens.font_width(34.0); let max = tokens.font_width(104.0); @@ -342,6 +343,7 @@ fn fitted_segment_width_survives_high_ui_low_font_render(cx: &mut gpui::TestAppC font: 0.25, font_delta: 0.0, tier: Default::default(), + zoom: 1.0, }; let item = cx.update(|cx| { MoonTheme::global_mut(cx).scale = scale; diff --git a/crates/moon-ui-components/src/moon/theme.rs b/crates/moon-ui-components/src/moon/theme.rs index e6272fe..ae114d7 100644 --- a/crates/moon-ui-components/src/moon/theme.rs +++ b/crates/moon-ui-components/src/moon/theme.rs @@ -24,16 +24,22 @@ pub struct MoonScale { pub font_delta: f32, /// Preferred component density; independent of geometry and text scaling. pub tier: MoonSize, + /// Browser-style page zoom applied to the whole window by `MoonRoot` through + /// `Window::set_content_zoom`: text, geometry, images and GPU canvases scale together and + /// every design value reaches the components unscaled. Independent of `ui` (geometry only) + /// and of `font` / `font_delta` (text only). + pub zoom: f32, } impl Default for MoonScale { - /// Return unscaled metrics and MoonUI's default medium tier. + /// Return unscaled metrics, MoonUI's default medium tier and no window zoom. fn default() -> Self { Self { ui: 1.0, font: 1.0, font_delta: 0.0, tier: MoonSize::default(), + zoom: 1.0, } } } @@ -101,6 +107,14 @@ impl MoonThemeTokens { self.scale.tier } + /// Return the window content zoom `MoonRoot` installs, unvalidated. + /// + /// No floor here, unlike [`Self::ui`]: `Window::set_content_zoom` rejects an impossible value + /// itself, and [`MoonThemeConfig::set_zoom`] never stores one. + pub fn zoom(&self) -> f32 { + self.scale.zoom + } + pub fn ui(&self, value: f32) -> f32 { value * self.scale.ui.max(0.25) } @@ -486,6 +500,29 @@ impl MoonThemeConfig { self.set_ui_scale(ui_scale); self } + + /// Set the window content zoom on both themes, refusing a value that cannot be a zoom. + /// + /// Same guard as [`Self::set_ui_scale`], for the same reason: a stored zero or a negative + /// number would not fail loudly, it would render every window at nothing and take the + /// settings screen that could repair it along. Zero, negatives and non-finite values are + /// replaced by the default; any positive value is stored verbatim, because a consumer may be + /// persisting a deliberate choice outside whatever range its own slider offers. + pub fn set_zoom(&mut self, zoom: f32) { + let zoom = if zoom.is_finite() && zoom > 0.0 { + zoom + } else { + MoonScale::default().zoom + }; + self.dark.scale.zoom = zoom; + self.light.scale.zoom = zoom; + } + + /// Return this configuration with the supplied window content zoom on both themes. + pub fn with_zoom(mut self, zoom: f32) -> Self { + self.set_zoom(zoom); + self + } } #[derive(Clone, Debug)] @@ -536,10 +573,15 @@ impl MoonTheme { } } + /// Install `config` as the active theme and re-render every open window. + /// + /// The refresh is what carries a changed [`MoonScale::zoom`] to windows other than the one + /// the caller is in: each `MoonRoot` applies the theme's zoom to its window on render. pub fn install_config(config: MoonThemeConfig, cx: &mut App) { let theme = Self::from_config(config); theme.sync_base_theme(cx); cx.set_global(theme); + cx.refresh_windows(); } pub fn load_toml(path: impl AsRef) -> Result { @@ -557,6 +599,16 @@ impl MoonTheme { cx.try_global::() } + /// The window content zoom of the active theme, `1.0` when no theme is installed. + /// + /// Read directly from the global rather than through [`Self::active_tokens`], because + /// `MoonRoot` asks every frame and the tokens clone two `SharedString`s. + pub fn content_zoom(cx: &App) -> f32 { + cx.try_global::() + .map(|theme| theme.scale.zoom) + .unwrap_or(1.0) + } + pub fn global_mut(cx: &mut App) -> &mut Self { if !cx.has_global::() { cx.set_global(Self::default()); @@ -573,6 +625,7 @@ impl MoonTheme { let next = Self::from_config(config); next.sync_base_theme(cx); *Self::global_mut(cx) = next; + cx.refresh_windows(); } pub fn active_tokens(cx: &App) -> MoonThemeTokens { diff --git a/crates/moon-ui-components/src/moon/theme/tests.rs b/crates/moon-ui-components/src/moon/theme/tests.rs index 3bf3d4a..ce8b477 100644 --- a/crates/moon-ui-components/src/moon/theme/tests.rs +++ b/crates/moon-ui-components/src/moon/theme/tests.rs @@ -219,3 +219,55 @@ fn tier_band_bases_are_compact_only() { assert_eq!(sm.tier_band_base(standard, |m| m.line_height), standard); } } + +/// Catches allowing non-positive or non-finite values in `theme.rs:MoonThemeConfig::set_zoom`, +/// which would render every window at nothing, the settings screen that could repair it included. +#[test] +fn an_impossible_zoom_is_replaced_rather_than_stored() { + for impossible in [0.0_f32, -1.0, f32::NAN, f32::INFINITY] { + let cfg = MoonThemeConfig::moon_terminal().with_zoom(impossible); + + assert_eq!( + cfg.dark.scale.zoom, + MoonScale::default().zoom, + "a zoom of {impossible} cannot be rendered; it must not be stored" + ); + assert_eq!( + cfg.light.scale.zoom, + MoonScale::default().zoom, + "both themes must be guarded, not just the dark one" + ); + } +} + +/// Catches clamping positive values in `theme.rs:MoonThemeConfig::set_zoom`, which would +/// overwrite a zoom a consumer persisted outside the range of its own slider. +#[test] +fn an_unusual_but_positive_zoom_is_stored_verbatim() { + for kept in [0.5_f32, 0.8, 1.75, 3.0] { + let cfg = MoonThemeConfig::moon_terminal().with_zoom(kept); + + assert_eq!( + cfg.dark.scale.zoom, kept, + "a positive zoom of {kept} is a legitimate choice; the guard is not a clamp" + ); + assert_eq!(cfg.light.scale.zoom, kept, "both themes take the zoom"); + } +} + +/// Catches a `MoonScale::default` zoom other than one, or a dropped serde default on the new +/// field: every theme file written before it existed would open its windows zoomed. +#[test] +fn legacy_theme_toml_defaults_zoom_to_one() { + let config: MoonThemeConfig = toml::from_str( + "[dark.scale] +ui = 1.2 +[light.scale] +font = 1.1 +", + ) + .unwrap(); + assert_eq!(config.dark.zoom(), 1.0); + assert_eq!(config.light.zoom(), 1.0); + assert_eq!(MoonThemeTokens::default().zoom(), 1.0); +} diff --git a/crates/moon-ui-components/src/root.rs b/crates/moon-ui-components/src/root.rs index 4cf1626..c781374 100644 --- a/crates/moon-ui-components/src/root.rs +++ b/crates/moon-ui-components/src/root.rs @@ -3,7 +3,7 @@ use crate::{ dialog::{ANIMATION_DURATION, Dialog}, focus_trap::FocusTrapManager, input::{Copy, InputState}, - moon::MoonDialog, + moon::{MoonDialog, MoonTheme}, native_menu::FallbackMenuOverlay, notification::{Notification, NotificationList}, sheet::Sheet, @@ -734,6 +734,14 @@ impl Styled for MoonRoot { impl Render for MoonRoot { fn render(&mut self, window: &mut Window, cx: &mut Context) -> impl IntoElement { window.set_rem_size(cx.theme().font_size); + // The theme's zoom reaches every window through its root, the same way the rem size + // does: a new window adopts it on its first frame, and a theme change reaches the others + // on the frame `MoonTheme::install_config` requests. `Window` defers a change made here + // to the next frame and ignores an equal value, so this is cheap to ask every render. + let zoom = MoonTheme::content_zoom(cx); + if window.content_zoom() != zoom { + window.set_content_zoom(zoom, cx); + } let sheet_layer = self.render_sheet_layer_inline(window, cx); let dialog_layer = self.render_dialog_layer_inline(window, cx); @@ -818,6 +826,42 @@ mod tests { assert!(root.read_with(cx, |root, _| root.bordered)); } + /// Catches dropping the zoom sync in `root.rs:MoonRoot::render`: a theme zoom would change + /// the tokens and never reach the window, so nothing on screen would scale. + #[gpui::test] + fn root_applies_the_theme_zoom_to_its_window(cx: &mut TestAppContext) { + cx.update(crate::init); + cx.update(|cx| { + MoonTheme::install_config( + crate::moon::MoonThemeConfig::moon_terminal().with_zoom(2.0), + cx, + ) + }); + + let (_root, cx) = cx.add_window_view(|window, cx| { + let view = cx.new(|_| TestView); + Root::new(view, window, cx).bordered(false) + }); + // The first render parks the zoom; the next frame applies it. Two draws cover both + // whichever the harness has already run. + let (zoom, factor, platform_factor) = cx.update(|window, cx| { + let platform_factor = window.scale_factor() / window.content_zoom(); + window.draw(cx).clear(); + window.draw(cx).clear(); + ( + window.content_zoom(), + window.scale_factor(), + platform_factor, + ) + }); + assert_eq!(zoom, 2.0, "the root installs the theme zoom on its window"); + assert_eq!( + factor, + platform_factor * 2.0, + "and the window renders at it" + ); + } + struct FocusProbeView { control: FocusHandle, } diff --git a/crates/moon-ui-components/src/window_border.rs b/crates/moon-ui-components/src/window_border.rs index b4e1b64..991f95d 100644 --- a/crates/moon-ui-components/src/window_border.rs +++ b/crates/moon-ui-components/src/window_border.rs @@ -115,7 +115,9 @@ impl RenderOnce for WindowBorder { if matches!(decorations, Decorations::Client { .. }) { window.set_client_inset(platform_inset); } - let window_size = window.window_bounds().get_bounds().size; + // Content space, like `mouse_position()` and the hit zones laid out below; the platform + // window size would be off by the content zoom. + let window_size = window.viewport_size(); div() .id("window-backdrop") @@ -144,7 +146,7 @@ impl RenderOnce for WindowBorder { if tiling.top && tiling.bottom && tiling.left && tiling.right { return; } - let size = window.window_bounds().get_bounds().size; + let size = window.viewport_size(); let pos = window.mouse_position(); let insets = client_frame_insets(platform_inset, &tiling); diff --git a/crates/moon-ui-components/themes/moon-graphite.toml b/crates/moon-ui-components/themes/moon-graphite.toml index 7c52a26..1d6a65c 100644 --- a/crates/moon-ui-components/themes/moon-graphite.toml +++ b/crates/moon-ui-components/themes/moon-graphite.toml @@ -61,6 +61,7 @@ hairline = 1.0 ui = 1.0 font = 1.0 font_delta = 0.0 +zoom = 1.0 [dark.typography] font_family = "Inter" @@ -130,6 +131,7 @@ hairline = 1.0 ui = 1.0 font = 1.0 font_delta = 0.0 +zoom = 1.0 [light.typography] font_family = "Inter" diff --git a/crates/moon-ui-components/themes/moon-light.toml b/crates/moon-ui-components/themes/moon-light.toml index e0ae9d4..e378e7a 100644 --- a/crates/moon-ui-components/themes/moon-light.toml +++ b/crates/moon-ui-components/themes/moon-light.toml @@ -61,6 +61,7 @@ hairline = 1.0 ui = 1.0 font = 1.0 font_delta = 0.0 +zoom = 1.0 [dark.typography] font_family = "Inter" @@ -130,6 +131,7 @@ hairline = 1.0 ui = 1.0 font = 1.0 font_delta = 0.0 +zoom = 1.0 [light.typography] font_family = "Inter" diff --git a/crates/moon-ui-components/themes/moon-terminal.toml b/crates/moon-ui-components/themes/moon-terminal.toml index daf7830..bbf4f6a 100644 --- a/crates/moon-ui-components/themes/moon-terminal.toml +++ b/crates/moon-ui-components/themes/moon-terminal.toml @@ -61,6 +61,7 @@ hairline = 1.0 ui = 1.0 font = 1.0 font_delta = 0.0 +zoom = 1.0 [dark.typography] font_family = "Inter" @@ -130,6 +131,7 @@ hairline = 1.0 ui = 1.0 font = 1.0 font_delta = 0.0 +zoom = 1.0 [light.typography] font_family = "Inter" diff --git a/docs/GPU_CANVAS_TEXT_API.md b/docs/GPU_CANVAS_TEXT_API.md index 281c3b0..7ad8d75 100644 --- a/docs/GPU_CANVAS_TEXT_API.md +++ b/docs/GPU_CANVAS_TEXT_API.md @@ -295,7 +295,8 @@ crates/moon-gpui-wgpu/src/wgpu_renderer.rs 1. вызывает `driver.frame(info)`; 2. понимает, будет ли этот tick реально представлен; 3. если будет, вызывает `driver.prepare_text(ctx)` для всех canvas-ов окна; -4. даёт `prepare_text(ctx)` доступ к `ctx.bounds()`, `ctx.scale_factor()`, +4. даёт `prepare_text(ctx)` доступ к `ctx.bounds()`, `ctx.scale_factor()` (платформенный + фактор, умноженный на content zoom окна; `ctx.content_zoom()` отдаёт множитель отдельно), `ctx.content_mask()`, `ctx.canvas_layer()` и `ctx.text_layer()`; 5. кладет glyph sprites в `text_layer`, а не обязательно в слой самого canvas. diff --git a/docs/MOON_PATCH_QUEUE.md b/docs/MOON_PATCH_QUEUE.md index 117e2c1..03328a9 100644 --- a/docs/MOON_PATCH_QUEUE.md +++ b/docs/MOON_PATCH_QUEUE.md @@ -68,6 +68,26 @@ cargo xtask transform --zed-tag v0.0.0 --zed-path R:\test\_zed_gpui_base_84b753 the bare `last_input_modality` field and its `KeyDown(_)` arm; keep the state struct and its unit tests. + + - `Window::content_zoom` / `set_content_zoom`: browser-style page zoom per + window. The `scale_factor` field holds the platform factor times the zoom, + `viewport_size` the platform content size divided by it, both recomputed + by `sync_platform_geometry` from `bounds_changed` and from a zoom change. + Pointer input is divided on entry (`unzoom_input`, the first statement of + `dispatch_event`: positions, `ScrollDelta::Pixels` and file-drop + positions), and `resize`, `set_client_inset`, `show_window_menu`, + `set_traffic_light_position` multiply on the way to the platform, as do + `PlatformInputHandler::{bounds_for_range, selected_bounds}` while + `character_index_for_point` divides, so every platform's IME geometry + converts in one place. A zoom requested while drawing is parked in + `pending_content_zoom` and applied at the top of the next `draw`. + `GpuFrameInfo` and `GpuCanvasTextContext` carry `content_zoom` beside the + combined `scale_factor` for a canvas that keeps device density. A re-sync + drops the two fields, the helper, the `unzoom_input` call, the block in + `draw`, the four setters' multiplications, the `PlatformInputHandler` + conversions and the GPU canvas fields; restore all of them and keep + `window/tests.rs`. + 2. Zed bugfix candidates kept separate from `gpu_canvas` when possible: - Windows DPI/restore-bounds behavior - Linux/X11 borderless decoration fallback diff --git a/docs/component-api-baseline.json b/docs/component-api-baseline.json index 112bd43..9445d96 100644 --- a/docs/component-api-baseline.json +++ b/docs/component-api-baseline.json @@ -4505,6 +4505,10 @@ "file": "crates/moon-ui-components/src/moon/theme.rs", "signature": "pub fn base_mono_font_size(&self) -> f32" }, + { + "file": "crates/moon-ui-components/src/moon/theme.rs", + "signature": "pub fn content_zoom(cx: &App) -> f32" + }, { "file": "crates/moon-ui-components/src/moon/theme.rs", "signature": "pub fn fit_band(&self, base_height: f32, base_line_height: f32) -> f32" @@ -4593,6 +4597,10 @@ "file": "crates/moon-ui-components/src/moon/theme.rs", "signature": "pub fn set_ui_scale(&mut self, ui_scale: f32)" }, + { + "file": "crates/moon-ui-components/src/moon/theme.rs", + "signature": "pub fn set_zoom(&mut self, zoom: f32)" + }, { "file": "crates/moon-ui-components/src/moon/theme.rs", "signature": "pub fn table_header_base(&self) -> f32" @@ -4637,6 +4645,14 @@ "file": "crates/moon-ui-components/src/moon/theme.rs", "signature": "pub fn with_ui_scale(mut self, ui_scale: f32) -> Self" }, + { + "file": "crates/moon-ui-components/src/moon/theme.rs", + "signature": "pub fn with_zoom(mut self, zoom: f32) -> Self" + }, + { + "file": "crates/moon-ui-components/src/moon/theme.rs", + "signature": "pub fn zoom(&self) -> f32" + }, { "file": "crates/moon-ui-components/src/moon/theme.rs", "signature": "pub struct MoonScale" From c64970992b70d30ab75819663982e3e4bb1ae50b Mon Sep 17 00:00:00 2001 From: ThisMad Date: Sat, 19 Sep 2026 18:46:04 +0200 Subject: [PATCH 2/3] fix(gpui): review follow-ups for content zoom Two clean-context reviews of the zoom patch, all findings above nit level: - the accessibility click fallback multiplies the node's content-space centre before it synthesises platform events, which `dispatch_event` divides again; - an immediate `set_content_zoom` cancels a zoom parked by an earlier render and rescales the tracked `mouse_position`, so the frame that applies the zoom hovers the right element; - `resize` stays in the platform's pixels like `bounds()`, so a size saved from `window_bounds()` restores unchanged; - `Root::new` installs the theme's zoom before the first frame; - the macOS native context menu multiplies its position into AppKit view points, and the editor popovers clamp against `viewport_size()` (TrackedFork drift: menu 3 -> 4, input and text_area 16 -> 19, mirror baseline regenerated); - the retained text cache key includes `content_zoom`; - a theme file's impossible zoom is normalized on install. Three more window tests and a theme test pin them; the patch-queue entry now lists everything a re-sync drops. --- crates/moon-gpui/src/gpu_canvas.rs | 4 + crates/moon-gpui/src/window.rs | 26 +++-- crates/moon-gpui/src/window/tests.rs | 108 +++++++++++++++++- .../component-manifest.json | 6 +- .../src/input/popovers/code_action_menu.rs | 2 +- .../src/input/popovers/completion_menu.rs | 4 +- .../src/input/popovers/hover_popover.rs | 6 +- crates/moon-ui-components/src/moon/theme.rs | 23 +++- .../src/moon/theme/tests.rs | 20 +++- .../src/native_menu/macos.rs | 6 +- crates/moon-ui-components/src/root.rs | 9 +- docs/MOON_PATCH_QUEUE.md | 30 +++-- docs/component-mirror-baseline.json | 41 ++++--- 13 files changed, 223 insertions(+), 62 deletions(-) diff --git a/crates/moon-gpui/src/gpu_canvas.rs b/crates/moon-gpui/src/gpu_canvas.rs index c0e1c86..4acbc75 100644 --- a/crates/moon-gpui/src/gpu_canvas.rs +++ b/crates/moon-gpui/src/gpu_canvas.rs @@ -154,6 +154,9 @@ struct GpuCanvasRetainedTextCacheKey { bounds: Bounds, content_mask: ContentMask, scale_factor_bits: u32, + /// Keyed beside the combined factor: a consumer that keeps device density sizes by + /// `scale_factor / content_zoom`, which a zoom change offset by a DPI change leaves unchanged. + content_zoom_bits: u32, background_appearance: WindowBackgroundAppearance, subpixel_rendering_supported: bool, text_rendering_mode: TextRenderingMode, @@ -637,6 +640,7 @@ impl<'a> GpuCanvasTextContext<'a> { bounds: self.bounds, content_mask: self.content_mask, scale_factor_bits: self.scale_factor.to_bits(), + content_zoom_bits: self.content_zoom.to_bits(), background_appearance: self.background_appearance, subpixel_rendering_supported: self.subpixel_rendering_supported, text_rendering_mode: self.text_rendering_mode, diff --git a/crates/moon-gpui/src/window.rs b/crates/moon-gpui/src/window.rs index 3dcf385..8ffeff2 100644 --- a/crates/moon-gpui/src/window.rs +++ b/crates/moon-gpui/src/window.rs @@ -2462,11 +2462,18 @@ impl Window { } } - /// Install a validated zoom: geometry, the platform's client inset, a refresh and the bounds - /// observers, in that order. + /// Install a validated zoom: geometry, the tracked pointer, the platform's client inset, a + /// refresh and the bounds observers, in that order. fn apply_content_zoom(&mut self, zoom: f32, cx: &mut App) { + // An immediate apply supersedes a zoom parked by an earlier render; otherwise the next + // `draw` would take the stale request and win over this one. + self.pending_content_zoom = None; + let previous = self.content_zoom; self.content_zoom = zoom; self.sync_platform_geometry(); + // The tracked pointer is content space too. Until the next platform event it would + // otherwise hover and cursor-style the element at the old position in the new space. + self.mouse_position = self.mouse_position.map(|c| c * previous / zoom); if let Some(inset) = self.client_inset { self.platform_window.set_client_inset(inset * zoom); } @@ -2493,11 +2500,11 @@ impl Window { .render_to_image(&self.rendered_frame.scene) } - /// Set the content size of the window, in content space (the counterpart of - /// [`Self::viewport_size`]); the platform receives it multiplied by the content zoom. + /// Set the content size of the window, in the platform's logical pixels: the counterpart of + /// [`Self::bounds`]`.size`, not of [`Self::viewport_size`], so a size saved from + /// [`Self::window_bounds`] restores unchanged under content zoom. pub fn resize(&mut self, size: Size) { - let zoom = self.content_zoom; - self.platform_window.resize(size.map(|d| d * zoom)); + self.platform_window.resize(size); } /// Returns whether or not the window is currently fullscreen @@ -6026,7 +6033,9 @@ impl Window { match request.action { accesskit::Action::Click => { if let Some(bounds) = self.a11y.node_bounds.get(&request.target_node).copied() { - let center = bounds.center(); + // Node bounds are content space; the synthetic events below are platform + // events by contract, and `dispatch_event` divides them by the zoom again. + let center = bounds.center().map(|c| c * self.content_zoom); let mouse_down = PlatformInput::MouseDown(crate::MouseDownEvent { button: MouseButton::Left, position: center, @@ -6260,7 +6269,8 @@ impl Window { self.modifiers = modifiers; } - /// For testing: simulate a mouse move event to the given position. + /// For testing: simulate a mouse move event to the given position, in the platform's logical + /// pixels as a real event would be (divided by the content zoom on the way in). /// This dispatches the event through the normal event handling path, /// which will trigger hover states and tooltips. #[cfg(any(test, feature = "test-support"))] diff --git a/crates/moon-gpui/src/window/tests.rs b/crates/moon-gpui/src/window/tests.rs index 08d3333..3693e73 100644 --- a/crates/moon-gpui/src/window/tests.rs +++ b/crates/moon-gpui/src/window/tests.rs @@ -15,8 +15,8 @@ use crate::{ GpuCanvasDrawContext, GpuCanvasDriver, GpuCanvasHandle, GpuCanvasPrepareContext, GpuFrameDecision, GpuFrameInfo, InputEvent as _, InputHandler, InteractiveElement as _, IntoElement, MouseMoveEvent, Pixels, PlatformInput, PlatformInputHandler, Point, Render, - ScrollDelta, ScrollWheelEvent, Size, Styled as _, TestAppContext, TouchPhase, UTF16Selection, - Window, div, gpu_canvas, point, px, size, + ScrollDelta, ScrollWheelEvent, Size, StatefulInteractiveElement as _, Styled as _, + TestAppContext, TouchPhase, UTF16Selection, Window, div, gpu_canvas, point, px, size, }; /// A root view that draws nothing; the window's own state is what these tests read. @@ -59,18 +59,34 @@ fn platform_factor(app: &mut TestAppContext, any: AnyWindowHandle) -> f32 { .unwrap() } -/// Catches dropping either term of `window.rs:sync_platform_geometry`: the interface would keep -/// rendering at the platform density, or lay out against a viewport the zoom no longer fits. +/// Catches dropping either term of `window.rs:sync_platform_geometry` (the interface would keep +/// rendering at the platform density, or lay out against a viewport the zoom no longer fits) or +/// leaving `mouse_position` in the old space in `apply_content_zoom` (the frame that applies the +/// zoom hovers the element at the old position in the new space). #[test] fn content_zoom_multiplies_the_platform_factor_and_divides_the_viewport() { let (mut app, any) = open_window(|_, _| EmptyView); app.update_window(any, |_, window, cx| { + window.dispatch_event( + MouseMoveEvent { + position: point(px(100.), px(100.)), + modifiers: Default::default(), + pressed_button: None, + } + .to_platform_input(), + cx, + ); let factor = window.scale_factor(); let viewport = window.viewport_size(); window.set_content_zoom(2.0, cx); assert_eq!(window.content_zoom(), 2.0); assert_eq!(window.scale_factor(), factor * 2.0); assert_eq!(window.viewport_size(), viewport.map(|d| d / 2.0)); + assert_eq!( + window.mouse_position(), + point(px(50.), px(50.)), + "the tracked pointer follows the space it is compared in" + ); }) .unwrap(); } @@ -90,8 +106,8 @@ fn content_zoom_survives_a_platform_resize() { .unwrap(); } -/// Catches a missing arm in `window.rs:unzoom_input`: a click at zoom would land on the element -/// twice as far from the origin as the one under the pointer. +/// Catches an arm of `window.rs:unzoom_input` moved into its pass-through group: a click at zoom +/// would land on the element twice as far from the origin as the one under the pointer. #[test] fn pointer_positions_arrive_in_content_space() { let (mut app, any) = open_window(|_, _| EmptyView); @@ -392,6 +408,86 @@ fn ime_geometry_crosses_to_platform_space_and_back() { assert_eq!(seen_point.get(), Some(point(px(20.), px(20.)))); } +/// Catches `window.rs:apply_content_zoom` leaving a parked request behind: the next `draw` would +/// take the stale zoom and override the one applied since. +#[test] +fn an_immediate_zoom_supersedes_a_parked_one() { + let armed = Rc::new(Cell::new(false)); + let renders: Rc)>>> = Rc::new(RefCell::new(Vec::new())); + let (mut app, any) = open_window({ + let armed = armed.clone(); + let renders = renders.clone(); + move |_, _| ZoomingView { armed, renders } + }); + armed.set(true); + draw(&mut app, any); + armed.set(false); + zoom(&mut app, any, 1.5); + draw(&mut app, any); + app.update_window(any, |_, window, _| { + assert_eq!( + window.content_zoom(), + 1.5, + "the zoom applied outside the draw wins over the one a render parked before it" + ); + }) + .unwrap(); +} + +/// A 50 px square that counts its clicks, for the accessibility click fallback. +struct ClickProbe { + clicks: Rc>, +} + +impl Render for ClickProbe { + fn render(&mut self, _: &mut Window, _: &mut Context) -> impl IntoElement { + let clicks = self.clicks.clone(); + div() + .id("click-probe") + .size(px(50.)) + .on_click(move |_, _, _| clicks.set(clicks.get() + 1)) + } +} + +/// Catches `window.rs:handle_a11y_action` handing a content-space centre to `dispatch_event`, +/// which divides it by the zoom again: a screen-reader click at zoom would land on the wrong +/// element, or on nothing. +#[test] +fn an_accessibility_click_lands_on_its_node_at_zoom() { + let clicks = Rc::new(Cell::new(0)); + let (mut app, any) = open_window({ + let clicks = clicks.clone(); + move |_, _| ClickProbe { clicks } + }); + zoom(&mut app, any, 2.0); + draw(&mut app, any); + app.update_window(any, |_, window, cx| { + let node = accesskit::NodeId(7); + window.a11y.node_bounds.insert( + node, + Bounds { + origin: point(px(0.), px(0.)), + size: size(px(50.), px(50.)), + }, + ); + window.handle_a11y_action( + accesskit::ActionRequest { + action: accesskit::Action::Click, + target_tree: accesskit::TreeId::ROOT, + target_node: node, + data: None, + }, + cx, + ); + }) + .unwrap(); + assert_eq!( + clicks.get(), + 1, + "the synthetic click must reach the node it was aimed at" + ); +} + /// A GPU canvas driver that records the factor and zoom of the frame info it receives. struct FrameInfoProbe { seen: Rc>>, diff --git a/crates/moon-ui-components/component-manifest.json b/crates/moon-ui-components/component-manifest.json index f402870..d7f9835 100644 --- a/crates/moon-ui-components/component-manifest.json +++ b/crates/moon-ui-components/component-manifest.json @@ -348,7 +348,7 @@ "public_path": "moon_ui::MoonInput", "upstream_ref": "Longbridge::input@cda0fc7fd4e4809dd2a8bae0337ac43b49e9f675", "fork_reason": "Reviewed TrackedFork drift: Moon tone/theme hooks plus UTF-8, mask, number, and search hardening live in base input; Longbridge editor engine remains source-owned", - "donor_drift_budget": 16, + "donor_drift_budget": 19, "contracts": ["input.utf8_boundary_clamp", "input.mask_contract", "gallery.visual_coverage"] }, { @@ -685,7 +685,7 @@ "public_path": "moon_ui::MoonTextArea", "upstream_ref": "Longbridge::input@cda0fc7fd4e4809dd2a8bae0337ac43b49e9f675", "fork_reason": "Reviewed TrackedFork drift: MoonTextArea shares the reviewed Moon input engine drift; Longbridge multiline input behavior remains source-owned", - "donor_drift_budget": 16, + "donor_drift_budget": 19, "contracts": ["gallery.visual_coverage"] }, { @@ -770,7 +770,7 @@ "public_path": "moon_ui::MoonNativeMenu", "upstream_ref": "Longbridge::native_menu@cda0fc7fd4e4809dd2a8bae0337ac43b49e9f675", "fork_reason": "Reviewed TrackedFork drift: Moon native-menu fallback/window ownership hooks live in base native menu; Longbridge native menu behavior remains source-owned", - "donor_drift_budget": 3, + "donor_drift_budget": 4, "contracts": ["gallery.visual_coverage"] }, { diff --git a/crates/moon-ui-components/src/input/popovers/code_action_menu.rs b/crates/moon-ui-components/src/input/popovers/code_action_menu.rs index 0bb654b..78f5d3d 100644 --- a/crates/moon-ui-components/src/input/popovers/code_action_menu.rs +++ b/crates/moon-ui-components/src/input/popovers/code_action_menu.rs @@ -318,7 +318,7 @@ impl Render for CodeActionMenu { return Empty.into_any_element(); }; - let max_width = MAX_MENU_WIDTH.min(window.bounds().size.width - pos.x); + let max_width = MAX_MENU_WIDTH.min(window.viewport_size().width - pos.x); deferred( editor_popover("code-action-menu", cx) diff --git a/crates/moon-ui-components/src/input/popovers/completion_menu.rs b/crates/moon-ui-components/src/input/popovers/completion_menu.rs index b98e926..d3d8aa1 100644 --- a/crates/moon-ui-components/src/input/popovers/completion_menu.rs +++ b/crates/moon-ui-components/src/input/popovers/completion_menu.rs @@ -408,11 +408,11 @@ impl Render for CompletionMenu { .selected_item() .and_then(|item| item.documentation.clone()); - let max_width = MAX_MENU_WIDTH.min(window.bounds().size.width - pos.x); + let max_width = MAX_MENU_WIDTH.min(window.viewport_size().width - pos.x); let abs_pos = self.editor.read(cx).input_bounds.origin + pos; let vertical_layout = abs_pos.x + MAX_MENU_WIDTH + POPOVER_GAP + MAX_MENU_WIDTH + POPOVER_GAP - > window.bounds().size.width; + > window.viewport_size().width; deferred( div() diff --git a/crates/moon-ui-components/src/input/popovers/hover_popover.rs b/crates/moon-ui-components/src/input/popovers/hover_popover.rs index 96d2c3b..a1cc9db 100644 --- a/crates/moon-ui-components/src/input/popovers/hover_popover.rs +++ b/crates/moon-ui-components/src/input/popovers/hover_popover.rs @@ -180,9 +180,9 @@ impl Element for Popover { let max_width = self .width_limit .end - .min(window.bounds().size.width - SNAP_TO_EDGE * 2) + .min(window.viewport_size().width - SNAP_TO_EDGE * 2) .max(px(200.)); - let max_height = (window.bounds().size.height - SNAP_TO_EDGE * 2).min(px(320.)); + let max_height = (window.viewport_size().height - SNAP_TO_EDGE * 2).min(px(320.)); let mut popover = deferred( div() @@ -204,7 +204,7 @@ impl Element for Popover { let popover_size = popover.layout_as_root(AvailableSpace::min_size(), window, cx); const SNAP_TO_EDGE: Pixels = px(8.); let top_space = trigger_bounds.top() - SNAP_TO_EDGE; - let right_space = window.bounds().size.width - trigger_bounds.left() - SNAP_TO_EDGE; + let right_space = window.viewport_size().width - trigger_bounds.left() - SNAP_TO_EDGE; let mut pos = point( trigger_bounds.left(), diff --git a/crates/moon-ui-components/src/moon/theme.rs b/crates/moon-ui-components/src/moon/theme.rs index ae114d7..727b1ce 100644 --- a/crates/moon-ui-components/src/moon/theme.rs +++ b/crates/moon-ui-components/src/moon/theme.rs @@ -507,13 +507,11 @@ impl MoonThemeConfig { /// number would not fail loudly, it would render every window at nothing and take the /// settings screen that could repair it along. Zero, negatives and non-finite values are /// replaced by the default; any positive value is stored verbatim, because a consumer may be - /// persisting a deliberate choice outside whatever range its own slider offers. + /// persisting a deliberate choice outside whatever range its own slider offers. No range + /// beyond that is enforced here: a consumer that persists the value keeps it within the range + /// its own control offers. pub fn set_zoom(&mut self, zoom: f32) { - let zoom = if zoom.is_finite() && zoom > 0.0 { - zoom - } else { - MoonScale::default().zoom - }; + let zoom = normalized_zoom(zoom); self.dark.scale.zoom = zoom; self.light.scale.zoom = zoom; } @@ -525,6 +523,15 @@ impl MoonThemeConfig { } } +/// `zoom` when it can be a zoom, the default when it cannot; see [`MoonThemeConfig::set_zoom`]. +fn normalized_zoom(zoom: f32) -> f32 { + if zoom.is_finite() && zoom > 0.0 { + zoom + } else { + MoonScale::default().zoom + } +} + #[derive(Clone, Debug)] pub struct MoonTheme { pub mode: ThemeMode, @@ -550,6 +557,10 @@ impl MoonTheme { let mut config = config; config.dark.palette = config.dark.palette.with_legacy_defaults(); config.light.palette = config.light.palette.with_legacy_defaults(); + // A theme file bypasses `set_zoom`. An impossible zoom is normalized here for the reason + // that setter refuses one, so the tokens never report a zoom no window will take. + config.dark.scale.zoom = normalized_zoom(config.dark.scale.zoom); + config.light.scale.zoom = normalized_zoom(config.light.scale.zoom); let tokens = match config.mode { ThemeMode::Light => config.light.clone(), ThemeMode::Dark | ThemeMode::System => config.dark.clone(), diff --git a/crates/moon-ui-components/src/moon/theme/tests.rs b/crates/moon-ui-components/src/moon/theme/tests.rs index ce8b477..381b233 100644 --- a/crates/moon-ui-components/src/moon/theme/tests.rs +++ b/crates/moon-ui-components/src/moon/theme/tests.rs @@ -1,6 +1,6 @@ //! Guards scale validity and density propagation through theme loading and mode selection. -use super::{MoonScale, MoonThemeConfig, MoonThemeTokens}; +use super::{MoonScale, MoonTheme, MoonThemeConfig, MoonThemeTokens}; use crate::moon::{ colors::MoonColors, tokens::{MoonPalette, contrast_ratio}, @@ -271,3 +271,21 @@ font = 1.1 assert_eq!(config.light.zoom(), 1.0); assert_eq!(MoonThemeTokens::default().zoom(), 1.0); } + +/// Catches `theme.rs:MoonTheme::from_config` installing a theme file's impossible zoom: a +/// hand-edited `zoom = 0` bypasses `set_zoom`, every window would refuse it, and the tokens would +/// still report 0 to consumers. +#[test] +fn an_impossible_zoom_in_a_theme_file_is_normalized_on_install() { + let config: MoonThemeConfig = toml::from_str( + "[dark.scale] +zoom = 0.0 +[light.scale] +zoom = -2.0 +", + ) + .unwrap(); + let theme = MoonTheme::from_config(config); + assert_eq!(theme.scale.zoom, 1.0); + assert_eq!(theme.config.light.zoom(), 1.0); +} diff --git a/crates/moon-ui-components/src/native_menu/macos.rs b/crates/moon-ui-components/src/native_menu/macos.rs index 93fe86d..069ab5d 100644 --- a/crates/moon-ui-components/src/native_menu/macos.rs +++ b/crates/moon-ui-components/src/native_menu/macos.rs @@ -58,6 +58,9 @@ pub(super) fn show( // Inherent `Window::window_handle` (GPUI's `AnyWindowHandle`), not the // `raw_window_handle::HasWindowHandle` trait method in scope below. let handle = Window::window_handle(window); + // AppKit positions the menu in view points, the platform's logical pixels, while `position` + // is a GPUI content-space point; under content zoom the two differ by the zoom. + let position = position.map(|c| c * window.content_zoom()); cx.spawn(async move |cx| { let action = run_menu(view_ptr, &items, position); @@ -94,7 +97,8 @@ fn run_menu( let mut actions: Vec<&Box> = Vec::new(); let ns_menu = build_menu(items, &target, mtm, &mut actions); - // `position` is window-relative, logical pixels, origin top-left (GPUI). + // `position` is window-relative, in the platform's logical pixels (`show` applied the content + // zoom), origin top-left (GPUI). // AppKit view coordinates have their origin at the bottom-left, so flip y. let height = view.bounds().size.height; let location = NSPoint::new( diff --git a/crates/moon-ui-components/src/root.rs b/crates/moon-ui-components/src/root.rs index c781374..fd6b93f 100644 --- a/crates/moon-ui-components/src/root.rs +++ b/crates/moon-ui-components/src/root.rs @@ -154,6 +154,9 @@ impl ActiveDialog { impl MoonRoot { /// Create a new Root view. pub fn new(view: impl Into, window: &mut Window, cx: &mut Context) -> Self { + // Outside a draw, so this applies at once and the window's first frame is already at + // the theme's zoom; `render` below keeps it current afterwards. + window.set_content_zoom(MoonTheme::content_zoom(cx), cx); Self { style: StyleRefinement::default(), view: view.into(), @@ -735,9 +738,9 @@ impl Render for MoonRoot { fn render(&mut self, window: &mut Window, cx: &mut Context) -> impl IntoElement { window.set_rem_size(cx.theme().font_size); // The theme's zoom reaches every window through its root, the same way the rem size - // does: a new window adopts it on its first frame, and a theme change reaches the others - // on the frame `MoonTheme::install_config` requests. `Window` defers a change made here - // to the next frame and ignores an equal value, so this is cheap to ask every render. + // does: `new` installs it before the first frame, and a theme change reaches every open + // window on the frame `MoonTheme::install_config` requests. `Window` defers a change made + // here to the next frame and ignores an equal value, so this is cheap to ask every render. let zoom = MoonTheme::content_zoom(cx); if window.content_zoom() != zoom { window.set_content_zoom(zoom, cx); diff --git a/docs/MOON_PATCH_QUEUE.md b/docs/MOON_PATCH_QUEUE.md index 03328a9..8b92280 100644 --- a/docs/MOON_PATCH_QUEUE.md +++ b/docs/MOON_PATCH_QUEUE.md @@ -73,19 +73,27 @@ cargo xtask transform --zed-tag v0.0.0 --zed-path R:\test\_zed_gpui_base_84b753 window. The `scale_factor` field holds the platform factor times the zoom, `viewport_size` the platform content size divided by it, both recomputed by `sync_platform_geometry` from `bounds_changed` and from a zoom change. - Pointer input is divided on entry (`unzoom_input`, the first statement of - `dispatch_event`: positions, `ScrollDelta::Pixels` and file-drop - positions), and `resize`, `set_client_inset`, `show_window_menu`, - `set_traffic_light_position` multiply on the way to the platform, as do + Pointer input is divided on entry (`unzoom_input`, at the top of + `dispatch_event` before anything reads the event: positions, + `ScrollDelta::Pixels` and file-drop positions), and `set_client_inset`, + `show_window_menu`, `set_traffic_light_position` and the accessibility + click fallback multiply on the way back, as do `PlatformInputHandler::{bounds_for_range, selected_bounds}` while `character_index_for_point` divides, so every platform's IME geometry - converts in one place. A zoom requested while drawing is parked in - `pending_content_zoom` and applied at the top of the next `draw`. - `GpuFrameInfo` and `GpuCanvasTextContext` carry `content_zoom` beside the - combined `scale_factor` for a canvas that keeps device density. A re-sync - drops the two fields, the helper, the `unzoom_input` call, the block in - `draw`, the four setters' multiplications, the `PlatformInputHandler` - conversions and the GPU canvas fields; restore all of them and keep + converts in one place. `resize`, like `bounds()`, stays in the platform's + pixels so a saved size restores unchanged. A zoom requested while drawing + is parked in `pending_content_zoom` and applied at the top of the next + `draw`; an immediate apply cancels a parked one and rescales the tracked + pointer. `GpuFrameInfo` and `GpuCanvasTextContext` carry `content_zoom` + beside the combined `scale_factor` for a canvas that keeps device density, + and the retained text cache key includes it. A re-sync drops the two + fields and their init in `Window::new`, `sync_platform_geometry`, + `content_zoom` / `set_content_zoom` / `apply_content_zoom`, `unzoom_input` + and its call, the block in `draw`, the setters' and the a11y click's + multiplications, the `PlatformInputHandler` conversions, the + `content_zoom` field and parameter of `GpuFrameInfo` / + `GpuCanvasTextContext` (forwarded by `draw_retained_text_layer`) and the + `#[cfg(test)] mod tests;` line; restore all of them and keep `window/tests.rs`. 2. Zed bugfix candidates kept separate from `gpu_canvas` when possible: diff --git a/docs/component-mirror-baseline.json b/docs/component-mirror-baseline.json index fa4ff10..5ca1b6a 100644 --- a/docs/component-mirror-baseline.json +++ b/docs/component-mirror-baseline.json @@ -437,7 +437,7 @@ "local_paths": [ "input" ], - "local_hash": "66b8196a2c8bdc79", + "local_hash": "5c656e0ef4caf7de", "local_files": [ { "path": "input/blink_cursor.rs", @@ -576,13 +576,13 @@ }, { "path": "input/popovers/code_action_menu.rs", - "hash": "5758e26153bf8aa1", - "bytes": 9480 + "hash": "6a04a1d08c6b2915", + "bytes": 9482 }, { "path": "input/popovers/completion_menu.rs", - "hash": "040d5b8e3cb38d6b", - "bytes": 14755 + "hash": "6eade17c8cc5254b", + "bytes": 14759 }, { "path": "input/popovers/diagnostic_popover.rs", @@ -591,8 +591,8 @@ }, { "path": "input/popovers/hover_popover.rs", - "hash": "63639b9e8cf89fb5", - "bytes": 8814 + "hash": "1fe6156956a05589", + "bytes": 8820 }, { "path": "input/popovers/mod.rs", @@ -816,7 +816,10 @@ "input/mask_pattern.rs", "input/mod.rs", "input/number_input.rs", + "input/popovers/code_action_menu.rs", + "input/popovers/completion_menu.rs", "input/popovers/diagnostic_popover.rs", + "input/popovers/hover_popover.rs", "input/popovers/mod.rs", "input/search.rs", "input/state.rs" @@ -928,7 +931,7 @@ "local_paths": [ "native_menu" ], - "local_hash": "d8df00f5803a0e99", + "local_hash": "36617a8b4a31a71e", "local_files": [ { "path": "native_menu/fallback.rs", @@ -937,8 +940,8 @@ }, { "path": "native_menu/macos.rs", - "hash": "e3d981192ccf16c6", - "bytes": 6234 + "hash": "685cb0856791d46e", + "bytes": 6539 }, { "path": "native_menu/mod.rs", @@ -976,6 +979,7 @@ ], "donor_changed_files": [ "native_menu/fallback.rs", + "native_menu/macos.rs", "native_menu/mod.rs", "native_menu/windows.rs" ] @@ -1495,7 +1499,7 @@ "local_paths": [ "input" ], - "local_hash": "66b8196a2c8bdc79", + "local_hash": "5c656e0ef4caf7de", "local_files": [ { "path": "input/blink_cursor.rs", @@ -1634,13 +1638,13 @@ }, { "path": "input/popovers/code_action_menu.rs", - "hash": "5758e26153bf8aa1", - "bytes": 9480 + "hash": "6a04a1d08c6b2915", + "bytes": 9482 }, { "path": "input/popovers/completion_menu.rs", - "hash": "040d5b8e3cb38d6b", - "bytes": 14755 + "hash": "6eade17c8cc5254b", + "bytes": 14759 }, { "path": "input/popovers/diagnostic_popover.rs", @@ -1649,8 +1653,8 @@ }, { "path": "input/popovers/hover_popover.rs", - "hash": "63639b9e8cf89fb5", - "bytes": 8814 + "hash": "1fe6156956a05589", + "bytes": 8820 }, { "path": "input/popovers/mod.rs", @@ -1874,7 +1878,10 @@ "input/mask_pattern.rs", "input/mod.rs", "input/number_input.rs", + "input/popovers/code_action_menu.rs", + "input/popovers/completion_menu.rs", "input/popovers/diagnostic_popover.rs", + "input/popovers/hover_popover.rs", "input/popovers/mod.rs", "input/search.rs", "input/state.rs" From 89c417c6ad97fbc535f0fe70c14fddfc24f09de4 Mon Sep 17 00:00:00 2001 From: ThisMad Date: Mon, 21 Sep 2026 10:51:29 +0200 Subject: [PATCH 3/3] fix(toggle,checkbox,radio): centre the thumb, mark and dot on whole device pixels Layout rounds every authored length to device pixels on its own. At a fractional scale factor a box, the part centred in it and the inset between them round apart: a 20px track, a 16px thumb and a 2px gap at 80 % land as 16, 13 and 2, so the thumb rests on the bottom edge of its track with all the space above it. The checkbox mark drifted by up to two pixels the same way, because its inset also took a flat 1px off for a border that layout draws at a whole number of device pixels. `foundation::snap_centered` snaps the box and the inset and takes the part as what is left between them, so the inset is equal on every side at any factor; `snap_border` gives the border the width layout draws it at. The toggle sizes its thumb through it, the checkbox its mark, the radio its dot. Each control gets a sweep over every 5 % scale factor from 50 % to 300 % that asserts equal space on every side; each was proven red against the unsnapped code. The checkbox is a tracked donor file, so the mirror baseline moves. --- crates/moon-ui-components/src/checkbox.rs | 44 ++++++++++++-- .../moon-ui-components/src/moon/foundation.rs | 33 +++++++++++ crates/moon-ui-components/src/moon/radio.rs | 7 ++- .../src/moon/radio/tests.rs | 34 +++++++++++ crates/moon-ui-components/src/moon/toggle.rs | 17 +++++- .../src/moon/toggle/tests.rs | 58 +++++++++++++++++++ docs/component-mirror-baseline.json | 6 +- 7 files changed, 188 insertions(+), 11 deletions(-) diff --git a/crates/moon-ui-components/src/checkbox.rs b/crates/moon-ui-components/src/checkbox.rs index a9eb2d9..d92b3e5 100644 --- a/crates/moon-ui-components/src/checkbox.rs +++ b/crates/moon-ui-components/src/checkbox.rs @@ -3,6 +3,7 @@ use std::{rc::Rc, time::Duration}; use crate::{ Disableable, Selectable, Sizable, Size, StyledExt as _, moon::MoonTone, + moon::foundation::{snap_border, snap_centered}, moon::{MoonColors, MoonPalette, MoonTheme, MoonThemeTokens, rgba_from, svg::moon_svg}, text::Text, tooltip::ComponentTooltip, @@ -606,16 +607,19 @@ fn checkbox_mark( window: &mut Window, cx: &mut App, ) -> Option { + // The mark takes the size that centres on whole device pixels, so a fractional scale factor + // cannot round it towards one corner. Absolute insets start inside the box's 1px border + // (`border_1` on both the checkbox and radio boxes), so centring within the outer box takes + // that border back off, at the width layout draws it. + let mark = snap_centered(metrics.box_size, metrics.mark_size, window); + let offset = mark.inset - snap_border(px(1.), window); fading_mark(id.clone(), checked, window, cx, || { - // Absolute insets start inside the box's 1px border (`border_1` on both the checkbox and - // radio boxes), so centring within the outer box takes that border back off. - let offset = (metrics.box_size - metrics.mark_size) * 0.5 - px(1.); moon_svg(icon) .debug_selector(|| format!("{id}:mark")) .absolute() .top(offset) .left(offset) - .size(metrics.mark_size) + .size(mark.inner) .when_some(metrics.mark_stroke, |mark, stroke| { mark.stroke_width(stroke) }) @@ -957,6 +961,38 @@ mod tests { } } + /// Catches `checkbox_mark` placing the mark from unsnapped lengths again (dropping + /// `snap_centered`), or taking a flat 1px off for the box's border instead of the width layout + /// draws it at (`snap_border`). Layout rounds the box, the mark, the inset and the border to + /// device pixels separately, so at 150 % a small box left two pixels over the mark and four + /// under it. At every scale factor the mark must keep the same space on all four sides. + #[gpui::test] + fn test_checked_mark_stays_centred_at_any_scale_factor(cx: &mut gpui::TestAppContext) { + cx.update(crate::init); + for size in [Size::Small, Size::Medium] { + let window = cx.add_window(move |_, _| CheckedCheckboxHarness { size }); + let mut cx = gpui::VisualTestContext::from_window(window.into(), cx); + cx.run_until_parked(); + // Every 5 % step from 50 % to 300 %; the test platform's own factor is 2.0. + for factor in (10..=60).map(|step| step as f32 * 0.05) { + cx.update(|window, cx| { + window.set_content_zoom(factor / 2.0, cx); + window.draw(cx).clear(); + }); + let box_bounds = cx.debug_bounds("probe:box").expect("box must render"); + let mark = cx.debug_bounds("probe:mark").expect("mark must render"); + let device = |length: gpui::Pixels| (length.as_f32() * factor).round(); + let sides = [ + device(mark.top() - box_bounds.top()), + device(box_bounds.bottom() - mark.bottom()), + device(mark.left() - box_bounds.left()), + device(box_bounds.right() - mark.right()), + ]; + assert_eq!(sides, [sides[0]; 4], "{size:?} factor={factor:.2}"); + } + } + } + struct DescribedCheckboxHarness { size: Size, } diff --git a/crates/moon-ui-components/src/moon/foundation.rs b/crates/moon-ui-components/src/moon/foundation.rs index f6fffb1..1073fa0 100644 --- a/crates/moon-ui-components/src/moon/foundation.rs +++ b/crates/moon-ui-components/src/moon/foundation.rs @@ -135,6 +135,39 @@ pub fn moon_cubic_bezier(x1: f32, y1: f32, x2: f32, y2: f32) -> impl Fn(f32) -> } } +/// A box, the part centred in it and the inset between them, each on whole device pixels. +#[derive(Clone, Copy, Debug, PartialEq)] +pub(crate) struct Centered { + pub outer: Pixels, + pub inner: Pixels, + pub inset: Pixels, +} + +/// Sizes `inner` so that it centres in `outer` on whole device pixels. +/// +/// Layout rounds every authored length to device pixels on its own. At a fractional scale factor +/// a box, the part centred in it and the inset between them then round apart: 20, 16 and 2 at a +/// factor of 0.8 land as 16, 13 and 2, which leaves two pixels above the part and one under it. +/// Snapping the box and the inset, and taking the part as what is left between them, keeps the +/// inset the same on both sides at any factor; the part gives up at most a pixel for it. +pub(crate) fn snap_centered(outer: Pixels, inner: Pixels, window: &Window) -> Centered { + let outer = window.pixel_snap(outer); + let inset = window.pixel_snap((outer - inner) * 0.5); + Centered { + outer, + inner: outer - inset * 2.0, + inset, + } +} + +/// A border width as layout draws it: on whole device pixels, and never thinner than one. +pub(crate) fn snap_border(width: Pixels, window: &Window) -> Pixels { + if width <= px(0.) { + return px(0.); + } + window.pixel_snap(width).max(px(1.) / window.scale_factor()) +} + pub fn selected_background(p: MoonPalette) -> Background { linear_gradient( 90.0, diff --git a/crates/moon-ui-components/src/moon/radio.rs b/crates/moon-ui-components/src/moon/radio.rs index 01c3003..bfda052 100644 --- a/crates/moon-ui-components/src/moon/radio.rs +++ b/crates/moon-ui-components/src/moon/radio.rs @@ -10,7 +10,7 @@ use crate::checkbox::{ use super::{ checkbox::tier_size, colors::MoonColors, - foundation::MoonSize, + foundation::{MoonSize, snap_centered}, theme::{MoonTheme, MoonThemeTokens}, tokens::{MoonRect, MoonTone}, }; @@ -209,10 +209,13 @@ impl RenderOnce for MoonRadio { .description .filter(|description| !description.is_empty()); let has_text = label.is_some() || description.is_some(); + // The dot takes the size that centres on whole device pixels inside the circle, so a + // fractional scale factor cannot leave it a pixel off the middle. + let dot_size = snap_centered(choice.box_size, metrics.dot_size, window).inner; let dot = fading_mark(state_id, checked, window, cx, || { div() .debug_selector(|| format!("{}:dot", self.id)) - .size(metrics.dot_size) + .size(dot_size) .rounded_full() .bg(colors.mark) }); diff --git a/crates/moon-ui-components/src/moon/radio/tests.rs b/crates/moon-ui-components/src/moon/radio/tests.rs index 9c910ba..0642904 100644 --- a/crates/moon-ui-components/src/moon/radio/tests.rs +++ b/crates/moon-ui-components/src/moon/radio/tests.rs @@ -282,6 +282,40 @@ fn radio_without_text_is_its_circle_with_a_centred_dot(cx: &mut TestAppContext) } } +/// Catches `MoonRadio::render` sizing the dot on its own again instead of through +/// `snap_centered`. Layout rounds the circle and the dot to device pixels separately, so at 110 % +/// a 16px circle and a 6px dot land as 18 and 7: an odd pixel is left over and the dot sits off +/// the middle. At every scale factor the dot must keep the same space on all four sides. +#[gpui::test] +fn the_dot_stays_centred_at_any_scale_factor(cx: &mut TestAppContext) { + cx.update(crate::init); + for tier in [MoonSize::Sm, MoonSize::Md] { + let window = cx.add_window(move |_, _| ProbedRadioHarness { + size: tier, + label: None, + }); + let mut cx = VisualTestContext::from_window(window.into(), cx); + cx.run_until_parked(); + // Every 5 % step from 50 % to 300 %; the test platform's own factor is 2.0. + for factor in (10..=60).map(|step| step as f32 * 0.05) { + cx.update(|window, cx| { + window.set_content_zoom(factor / 2.0, cx); + window.draw(cx).clear(); + }); + let circle = cx.debug_bounds("probed:box").expect("circle must render"); + let dot = cx.debug_bounds("probed:dot").expect("dot must render"); + let device = |length: Pixels| (length.as_f32() * factor).round(); + let sides = [ + device(dot.top() - circle.top()), + device(circle.bottom() - dot.bottom()), + device(dot.left() - circle.left()), + device(circle.right() - dot.right()), + ]; + assert_eq!(sides, [sides[0]; 4], "{tier:?} factor={factor:.2}"); + } + } +} + /// Catches changing tier metrics or applying legacy font scaling, which enlarges tier labels. #[test] fn radio_metrics_match_designer_reference() { diff --git a/crates/moon-ui-components/src/moon/toggle.rs b/crates/moon-ui-components/src/moon/toggle.rs index eb80d32..e88be47 100644 --- a/crates/moon-ui-components/src/moon/toggle.rs +++ b/crates/moon-ui-components/src/moon/toggle.rs @@ -7,7 +7,7 @@ use crate::checkbox::{ChoiceColors, MoonCheckboxMetrics, choice_text_column}; use super::{ colors::MoonColors, - foundation::{MoonSize, moon_cubic_bezier, moon_shadow_sm}, + foundation::{MoonSize, moon_cubic_bezier, moon_shadow_sm, snap_centered}, theme::{MoonTheme, MoonThemeTokens}, tokens::{MoonPalette, MoonRect, MoonTone, rgba_from}, }; @@ -216,6 +216,19 @@ impl MoonToggleMetrics { } } + /// These metrics with the track on whole device pixels and the thumb sized from what the + /// track and the gap leave (`snap_centered`), so the thumb keeps one gap above, below and at + /// the end it rests against at any scale factor instead of rounding a pixel towards one edge. + fn snapped(self, window: &Window) -> Self { + let track = snap_centered(px(self.track_height), px(self.thumb_size), window); + Self { + track_width: window.pixel_snap(px(self.track_width)).as_f32(), + track_height: track.outer.as_f32(), + thumb_size: track.inner.as_f32(), + ..self + } + } + /// The track's cap radius. fn radius(self) -> f32 { self.track_height * 0.5 @@ -660,7 +673,7 @@ impl RenderOnce for MoonToggle { let size = self .size .unwrap_or_else(|| MoonToggleSize::density_default(&tokens)); - let metrics = size.resolve(self.variant, &tokens); + let metrics = size.resolve(self.variant, &tokens).snapped(window); let choice = metrics.choice(); let p = tokens.palette; let roles = MoonColors::active(cx); diff --git a/crates/moon-ui-components/src/moon/toggle/tests.rs b/crates/moon-ui-components/src/moon/toggle/tests.rs index b9d0eba..957b84f 100644 --- a/crates/moon-ui-components/src/moon/toggle/tests.rs +++ b/crates/moon-ui-components/src/moon/toggle/tests.rs @@ -856,3 +856,61 @@ fn a_slim_thumb_is_a_ring_around_a_face_rather_than_a_bordered_disc(cx: &mut gpu ); } } + +/// The scale factors a window can take: every 5 % step from 50 % to 300 %, which covers the +/// content zoom's range on a 1x display and on the common fractional and 2x ones. +fn scale_factors() -> impl Iterator { + (10..=60).map(|step| step as f32 * 0.05) +} + +/// Catches `MoonToggle::render` going back to the unsnapped metrics (dropping `snapped`), or +/// `snap_centered` sizing the thumb on its own again. Layout rounds every length to device pixels +/// separately, so at 80 % a 20px track, a 16px thumb and a 2px gap land as 16, 13 and 2: the +/// thumb rests on the track's bottom edge with all the space above it. At every scale factor the +/// thumb must keep exactly one gap above it, below it and at the end it rests against. +#[gpui::test] +fn the_thumb_keeps_one_gap_on_every_side_at_any_scale_factor(cx: &mut gpui::TestAppContext) { + cx.update(crate::init); + for (variant, tier) in [ + (MoonToggleVariant::Default, MoonSize::Sm), + (MoonToggleVariant::Default, MoonSize::Md), + (MoonToggleVariant::Slim, MoonSize::Sm), + (MoonToggleVariant::Slim, MoonSize::Md), + ] { + for checked in [false, true] { + let mut cx = render_toggle( + cx, + SizedToggleHarness { + variant, + ..SizedToggleHarness::bare(tier, checked) + }, + ); + for factor in scale_factors() { + cx.update(|window, cx| { + // The test platform's own factor is 2.0; the zoom makes up the rest. + window.set_content_zoom(factor / 2.0, cx); + window.draw(cx).clear(); + assert!((window.scale_factor() - factor).abs() < 1e-4); + }); + let track = cx.debug_bounds("sized:track").expect("track must render"); + let thumb = cx.debug_bounds("sized:thumb").expect("thumb must render"); + let device = |length: gpui::Pixels| (length.as_f32() * factor).round(); + let above = device(thumb.top() - track.top()); + let below = device(track.bottom() - thumb.bottom()); + let resting = if checked { + device(track.right() - thumb.right()) + } else { + device(thumb.left() - track.left()) + }; + let context = format!("{variant:?} {tier:?} checked={checked} factor={factor:.2}"); + assert_eq!(above, below, "{context}"); + assert_eq!(resting, above, "{context}"); + assert_eq!( + device(thumb.size.width), + device(thumb.size.height), + "{context}" + ); + } + } + } +} diff --git a/docs/component-mirror-baseline.json b/docs/component-mirror-baseline.json index 0ee8d4a..b0ad754 100644 --- a/docs/component-mirror-baseline.json +++ b/docs/component-mirror-baseline.json @@ -173,12 +173,12 @@ "local_paths": [ "checkbox.rs" ], - "local_hash": "1db0967787119d5c", + "local_hash": "dbf68eda8a74c5d1", "local_files": [ { "path": "checkbox.rs", - "hash": "dbca0db1c887d6e5", - "bytes": 52233 + "hash": "3044316dc40e01cd", + "bytes": 54366 } ], "donor_hash": "c665cd46d9f5aa2a",