From a677478b926e27f0b0238dbccff665b72d65901d Mon Sep 17 00:00:00 2001 From: Jason Baldwin <33934381+bitshiftza@users.noreply.github.com> Date: Fri, 9 Oct 2026 20:14:49 +0200 Subject: [PATCH] KEY-ZOOM: Cmd+= / Cmd+- / Cmd+0 zoom the photo, not egui's interface scale (#566) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit egui's zoom_with_keyboard was left on, so at the end of every frame it took the same keys as View ▸ Zoom In / Zoom Out / Zoom to Fit for its browser-style interface zoom. On macOS the native menu bar got the keys first; on Windows and Linux the whole UI scaled. theme::apply turns the option off for every context (desktop, headless, tests). Headless tests that pressed Cmd+Plus / Cmd+Minus to exercise the interface zoom's viewport handling set the zoom factor directly; the CLI snapshot test that did the same is dropped (the unit tests cover the mechanism). New tests_zoom_keys.rs: the shortcuts step the photo zoom while the zoom factor stays, and Ctrl+wheel over the photo zooms it in the Edit view. --- apps/lightcraft-cli/tests/cli.rs | 23 ------- crates/ui-egui/src/headless.rs | 26 ++++---- crates/ui-egui/src/lib.rs | 2 + crates/ui-egui/src/tests_zoom_keys.rs | 87 +++++++++++++++++++++++++++ crates/ui-egui/src/theme.rs | 5 ++ docs/control-protocol.md | 3 +- docs/parity.md | 2 +- 7 files changed, 108 insertions(+), 40 deletions(-) create mode 100644 crates/ui-egui/src/tests_zoom_keys.rs diff --git a/apps/lightcraft-cli/tests/cli.rs b/apps/lightcraft-cli/tests/cli.rs index 2009e818d..02570c7fa 100644 --- a/apps/lightcraft-cli/tests/cli.rs +++ b/apps/lightcraft-cli/tests/cli.rs @@ -233,29 +233,6 @@ fn snapshot_script_failure_exits_non_zero() { assert_eq!((d.width, d.height), (320, 200)); } -#[test] -fn snapshot_ui_zoom_keeps_requested_pixel_dimensions() { - let script = tmp("snap-ui-zoom.jsonl"); - std::fs::write( - &script, - format!("{}\n{}\n", json!({"method": "ui.key", "params": {"key": "Plus", "cmd": true}}), json!({"method": "ui.inspect"})), - ) - .unwrap(); - for (size, scale, expected) in [("400x240", "1", (400, 240)), ("345x200", "0.9", (311, 180))] { - let out = tmp(&format!("snap-ui-zoom-{scale}.png")); - let o = Command::new(BIN) - .args(["snapshot", "--script", script.to_str().unwrap(), "-o", out.to_str().unwrap(), "--size", size, "--scale", scale]) - .output() - .unwrap(); - assert!(o.status.success(), "{}", String::from_utf8_lossy(&o.stderr)); - let replies: Vec = String::from_utf8_lossy(&o.stdout).lines().map(|line| serde_json::from_str(line).unwrap()).collect(); - assert!(replies.iter().all(|reply| reply["ok"] == true), "{replies:?}"); - assert!(replies[1]["result"]["pixelsPerPoint"].as_f64().unwrap() > scale.parse::().unwrap()); - let decoded = lightcraft_codecs::decode(&std::fs::read(out).unwrap(), Default::default()).unwrap(); - assert_eq!((decoded.width, decoded.height), expected); - } -} - /// Headless snapshots default to CPU photo rendering as well as CPU UI rasterization. #[test] fn snapshot_defaults_to_cpu_without_gpu_environment_overrides() { diff --git a/crates/ui-egui/src/headless.rs b/crates/ui-egui/src/headless.rs index e6929269f..3dd0a06b2 100644 --- a/crates/ui-egui/src/headless.rs +++ b/crates/ui-egui/src/headless.rs @@ -453,20 +453,14 @@ mod tests { { let mut view = HeadlessView::new(); for frame in 0..9 { - let key = match frame { - 1 => Some(egui::Key::Plus), - 5 => Some(egui::Key::Minus), - _ => None, - }; - let mut raw = HeadlessView::raw_input(size, scale, frame as f64 / 60.0, vec![]); - if let Some(key) = key { - let modifiers = egui::Modifiers { command: true, ..Default::default() }; - raw.events.push(egui::Event::ModifiersChanged(modifiers)); - raw.events.extend(vec![ - egui::Event::Key { key, physical_key: None, pressed: true, repeat: false, modifiers }, - egui::Event::Key { key, physical_key: None, pressed: false, repeat: false, modifiers }, - ]); + // the interface scale changes between frames (Cmd+Plus / Cmd+Minus are the + // photo's zoom, issue #566: egui's own keyboard zoom is off) + match frame { + 1 => view.ctx.set_zoom_factor(1.1), + 5 => view.ctx.set_zoom_factor(1.0), + _ => {} } + let raw = HeadlessView::raw_input(size, scale, frame as f64 / 60.0, vec![]); view.run(raw, |ui| { ui.label("UI zoom changes layout, keeping the host viewport size"); }); @@ -489,7 +483,8 @@ mod tests { app.ui.settings.gpu = false; let mut h = Headless::new(app, [480.0, 320.0], 2.0); let t = Duration::from_secs(5); - assert_eq!(h.request("ui.key", json!({"key": "Plus", "cmd": true}), t)["ok"], true); + h.view.ctx.set_zoom_factor(1.1); + assert_eq!(h.request("ui.inspect", json!({}), t)["ok"], true); assert!(h.view.ctx.zoom_factor() > 1.0); let main = h.view.ctx.clone(); let screenshot = h.app.headless_screenshot(&main, true).unwrap(); @@ -538,7 +533,8 @@ mod tests { app.ui.settings.gpu = false; let mut h = Headless::new(app, [480.0, 320.0], 0.9); let t = Duration::from_secs(5); - assert_eq!(h.request("ui.key", json!({"key": "Minus", "cmd": true}), t)["ok"], true); + h.view.ctx.set_zoom_factor(0.9); + h.step(); assert!((h.view.ctx.zoom_factor() - 0.9).abs() < 0.001); assert_eq!(h.request("ui.resize", json!({"width": 150, "height": 150}), t)["ok"], true); assert_eq!(h.size, egui::vec2(135.0, 135.0)); diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 705818911..0416d144f 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -68,6 +68,8 @@ mod tests_titlebar; mod tests_unsaved; #[cfg(test)] mod tests_zoom; +#[cfg(test)] +mod tests_zoom_keys; use std::sync::mpsc::{Receiver, Sender}; diff --git a/crates/ui-egui/src/tests_zoom_keys.rs b/crates/ui-egui/src/tests_zoom_keys.rs new file mode 100644 index 000000000..3c1111217 --- /dev/null +++ b/crates/ui-egui/src/tests_zoom_keys.rs @@ -0,0 +1,87 @@ +//! Headless tests of the View ▸ Zoom shortcuts and modifier-wheel zoom: Cmd+= / Cmd+- / Cmd+0 +//! (Ctrl on Windows and Linux) zoom the photo and never egui's interface scale, which would +//! otherwise take the same keys at the end of the frame; Ctrl+wheel over the photo in the Edit +//! view zooms it. + +use std::time::Duration; + +use serde_json::json; + +use crate::headless::Headless; +use crate::state::Zoom; +use crate::{LightcraftApp, Services}; + +const T: Duration = Duration::from_secs(20); +const SETTLE: Duration = Duration::from_secs(120); + +fn edit_view() -> Headless { + let services = Services { png: None, ..Default::default() }; + let mut app = LightcraftApp::new(lightcraft_engine::Session::with_demo(), services); + app.ui.settings.gpu = false; + let mut h = Headless::new(app, [1400.0, 900.0], 1.0); + let r = h.request("ui.set", json!({"view": "detail", "right": "edit"}), T); + assert_eq!(r["ok"], true, "{r}"); + h.settle(SETTLE); + h +} + +fn key(h: &mut Headless, k: &str) { + let r = h.request("ui.key", json!({"key": k, "cmd": true}), T); + assert_eq!(r["ok"], true, "{r}"); + h.step(); + h.step(); +} + +#[test] +fn zoom_shortcuts_zoom_the_photo_and_leave_the_interface_scale_alone() { + let mut h = edit_view(); + assert_eq!(h.app.ui.zoom, Zoom::Fit); + let scale = h.view.ctx.zoom_factor(); + // Cmd+= (Ctrl+= on Windows / Linux) is View ▸ Zoom In: the photo, not the UI + key(&mut h, "="); + assert_eq!(h.app.ui.zoom, Zoom::Percent(50.0), "{:?}", h.app.ui.zoom); + assert_eq!(h.view.ctx.zoom_factor(), scale, "Cmd+= scaled the interface"); + // Cmd+Plus (a keypad or layout that reports `+`) is egui's other interface-zoom key + key(&mut h, "Plus"); + assert_eq!(h.view.ctx.zoom_factor(), scale, "Cmd+Plus scaled the interface"); + key(&mut h, "-"); + assert_eq!(h.app.ui.zoom, Zoom::Percent(25.0), "{:?}", h.app.ui.zoom); + assert_eq!(h.view.ctx.zoom_factor(), scale, "Cmd+- scaled the interface"); + key(&mut h, "-"); + assert_eq!(h.app.ui.zoom, Zoom::Fit, "{:?}", h.app.ui.zoom); + assert_eq!(h.view.ctx.zoom_factor(), scale, "Cmd+- scaled the interface"); + // Cmd+0 is Zoom to Fit; egui would reset its interface scale with the same key + h.app.run("view.navigate", json!({"zoom": {"percent": 200.0}})).unwrap(); + h.view.ctx.set_zoom_factor(1.2); + h.step(); + h.step(); + assert!((h.view.ctx.zoom_factor() - 1.2).abs() < 0.001); + key(&mut h, "0"); + assert_eq!(h.app.ui.zoom, Zoom::Fit, "{:?}", h.app.ui.zoom); + assert!((h.view.ctx.zoom_factor() - 1.2).abs() < 0.001, "Cmd+0 reset the interface scale"); +} + +#[test] +fn ctrl_wheel_over_the_photo_zooms_it_in_the_edit_view() { + let mut h = edit_view(); + let img = h.app.image_rect.unwrap(); + let canvas = h.app.canvas_rect.unwrap(); + assert!(canvas.contains(img.center()), "{img:?} outside {canvas:?}"); + let r = h.request("ui.move", json!({"x": img.center().x, "y": img.center().y}), T); + assert_eq!(r["ok"], true, "{r}"); + let r = h.request("ui.scroll", json!({"dy": 120.0, "ctrl": true, "cmd": true}), T); + assert_eq!(r["ok"], true, "{r}"); + for _ in 0..40 { + h.step(); + } + assert!(matches!(h.app.ui.zoom, Zoom::Percent(p) if p > 0.0), "Ctrl+wheel did not zoom: {:?}", h.app.ui.zoom); + assert!(h.app.image_rect.unwrap().width() > img.width(), "the photo did not grow"); + // a plain wheel over the photo pans (or does nothing at Fit): never zooms further + let zoomed = h.app.image_rect.unwrap(); + let r = h.request("ui.scroll", json!({"dy": 120.0}), T); + assert_eq!(r["ok"], true, "{r}"); + for _ in 0..40 { + h.step(); + } + assert!((h.app.image_rect.unwrap().width() - zoomed.width()).abs() < 0.01, "a plain wheel changed the zoom"); +} diff --git a/crates/ui-egui/src/theme.rs b/crates/ui-egui/src/theme.rs index b04ef1be2..0d7e1570c 100644 --- a/crates/ui-egui/src/theme.rs +++ b/crates/ui-egui/src/theme.rs @@ -216,6 +216,11 @@ fn craft_font_name(f: &lightcraft_engine::CraftFont) -> String { } pub fn apply(ctx: &egui::Context) { + // Cmd+= / Cmd+- / Cmd+0 (Ctrl on Windows and Linux) are View ▸ Zoom In / Zoom Out / Zoom to Fit + // for the photo. egui would take the same keys at the end of every frame for its browser-style + // interface zoom (issue #566); on macOS the native menu bar got them first, elsewhere the whole + // UI scaled. + ctx.options_mut(|o| o.zoom_with_keyboard = false); let t = Tokens::default(); ctx.data_mut(|d| d.insert_temp(egui::Id::NULL, t)); let mut v = Visuals::dark(); diff --git a/docs/control-protocol.md b/docs/control-protocol.md index 4247b559e..d97c92cb6 100644 --- a/docs/control-protocol.md +++ b/docs/control-protocol.md @@ -100,7 +100,8 @@ Headless screenshot dimensions must be finite and at least one logical point, wi positive scale. Rounded output must be at least one pixel per edge, at most 16,384 pixels per edge and 64 million pixels in total. `snapshot` and `ui.resize` reject requests outside these limits before layout or rasterization; supplied resize dimensions must be numbers. -UI zoom changes text and control sizes while preserving the snapshot viewport's requested +An interface zoom (egui's zoom factor; the keyboard never changes it — ⌘= / ⌘− / ⌘0 zoom the +photo, issue #566) changes text and control sizes while preserving the snapshot viewport's requested physical pixel dimensions. Later `ui.resize` requests use current egui points; validation and rasterization both convert them to native viewport points before rounding the output pixels. A native edge below one point after zooming out is valid if it rounds to at least diff --git a/docs/parity.md b/docs/parity.md index 4c27755b5..7b0e1bd45 100644 --- a/docs/parity.md +++ b/docs/parity.md @@ -656,7 +656,7 @@ From `06-shortcuts.md` part 1. Evidence is our binding; conflicts are explained | KEY-AUTO | Auto — ⇧A | P0 | ✅ | `cmd:develop.auto` | | | KEY-PHOTOSHOP | External editor — ⇧⌘E | P2 | ✅ | `cmd:photo.editInExternal` | ⇧⌘E as observed; the export dialog is ⇧E | | KEY-ROTATE | Rotate — ⌘[ / ⌘] | P0 | ✅ | `cmd:photo.rotateLeft`, `cmd:photo.rotateRight` | | -| KEY-ZOOM | Zoom in / out — ⌘= / ⌘− | P0 | ✅ | `cmd:view.zoomIn`, `cmd:view.zoomOut` | | +| KEY-ZOOM | Zoom in / out — ⌘= / ⌘− | P0 | ✅ | `cmd:view.zoomIn`, `cmd:view.zoomOut`, `crates/ui-egui/src/tests_zoom_keys.rs` | the keys zoom the photo only: egui's own interface zoom on ⌘= / ⌘− / ⌘0 (Ctrl on Windows and Linux, where no native menu bar took the keys first) is off (issue #566) | | KEY-SELECTALL | Select all — ⌘A | P0 | ✅ | `cmd:library.selectAll` | | | KEY-SELECTNONE | Select none — ⌘D | P0 | ✅ | `cmd:library.selectNone` | secondary binding (primary ⌘⇧A) | | KEY-PASTESELECTED | Paste selected — ⇧⌘V | P0 | ✅ | `cmd:dialog.pasteSettings` | |