From 482dcacd37f94885dde6a31932591a78a8fbf2b4 Mon Sep 17 00:00:00 2001 From: OneSlips Date: Sun, 11 Oct 2026 22:38:34 +1000 Subject: [PATCH] UI: targeted adjustment says how to leave it; a wheel notch scrolls 100 points A Color Mixer Hue/Saturation/Luminance (or curve) targeted-adjustment tool left on takes over every click and drag on the photo, so click-to-zoom and panning stopped working and the canvas looked frozen (user report). Esc already turned it off, but nothing said the tool was on: the mixer's buttons look like tabs. Turning one on now shows a toast: drag up or down on the photo, Esc to stop. A mouse wheel notch scrolled egui's default 40 points; an open Edit panel is 2000+ points tall. Lines now scroll 100 points (theme::WHEEL_LINE_POINTS, set in apply_kind so every theme has it), about Lightroom's step; trackpads report points and are unaffected. What assumed a 40-point notch keeps its per-notch behaviour: wheel zoom (Ctrl+wheel and the loupe's plain wheel) uses theme::WHEEL_ZOOM_PER_POINT so a notch still zooms ~1.22x, and a focused slider's wheel nudge counts one notch (WHEEL_LINE_POINTS) per step. Tests: the tool announces itself and Esc leaves it (view unchanged); three wheel notches scroll at least 300 points (120 before); every theme sets the notch and keeps the zoom per notch; wheel nudges in notch units; the slider wheel-nudge test lets egui's smoothing deliver its 120 points. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/panels/edit.rs | 16 ++++++++++++++-- crates/ui-egui/src/tests_curve.rs | 17 +++++++++++++++++ crates/ui-egui/src/tests_masking.rs | 5 ++++- crates/ui-egui/src/tests_scroll.rs | 22 ++++++++++++++++++++++ crates/ui-egui/src/theme.rs | 28 ++++++++++++++++++++++++++++ crates/ui-egui/src/widgets.rs | 20 +++++++++++++------- 6 files changed, 98 insertions(+), 10 deletions(-) diff --git a/crates/ui-egui/src/panels/edit.rs b/crates/ui-egui/src/panels/edit.rs index ed2ee4635..ba2a6e89b 100644 --- a/crates/ui-egui/src/panels/edit.rs +++ b/crates/ui-egui/src/panels/edit.rs @@ -1058,7 +1058,19 @@ fn tat_button(app: &mut LightcraftApp, ui: &mut egui::Ui, tool: &str, tip: &str) let active = app.ui.tool == tool; let id = tool.replace(':', "-"); if crate::widgets::icon_button(ui, &id, Icon::Target, vec2(26.0, 26.0), active, true, tip).clicked() { - app.ui.tool = if active { String::new() } else { tool.to_string() }; + set_tat(app, ui.ctx(), tool, active); + } +} + +/// Turn targeted-adjustment tool `tool` off (`active`) or on. Turning it on says how to use and +/// leave it: while it is on, dragging on the photo adjusts instead of zooming or panning, which is +/// easy to miss when the button looks like a tab. +fn set_tat(app: &mut LightcraftApp, ctx: &egui::Context, tool: &str, active: bool) { + if active { + app.ui.tool.clear(); + } else { + app.ui.tool = tool.to_string(); + app.toast(ctx, crate::i18n::tr("Targeted adjustment on: drag up or down on the photo · Esc to stop")); } } @@ -1100,7 +1112,7 @@ fn mixer(app: &mut LightcraftApp, ui: &mut egui::Ui, d: &DevelopSettings) { .on_hover_text(crate::i18n::tr("Targeted adjustment: drag up/down on the photo to adjust the colours there")) .clicked() { - app.ui.tool = if active { String::new() } else { tool }; + set_tat(app, ui.ctx(), &tool, active); } } }); diff --git a/crates/ui-egui/src/tests_curve.rs b/crates/ui-egui/src/tests_curve.rs index ca5848bee..342e384bf 100644 --- a/crates/ui-egui/src/tests_curve.rs +++ b/crates/ui-egui/src/tests_curve.rs @@ -152,3 +152,20 @@ fn dragging_a_point_past_its_neighbour_is_clamped() { assert!(c[1].0 < c[2].0 && c[1].0 > 0.55, "clamped just left of its neighbour: {c:?}"); assert_eq!(c[2], (0.6, 0.6), "the neighbour stays: {c:?}"); } + +/// A targeted-adjustment tool takes over clicks and drags on the photo, so turning it on says so +/// (and how to leave), and Esc leaves it: one left on by mistake looked like a frozen canvas. +#[test] +fn targeted_adjustment_announces_itself_and_esc_turns_it_off() { + let mut h = curve_open("master"); + let r = h.request("ui.clickWidget", json!({"id": "icon:tat-curve"}), T); + assert_eq!(r["ok"], true, "{r}"); + h.step(); + assert_eq!(h.app.ui.tool, "tat:curve"); + assert!(h.app.ui.toast.as_ref().is_some_and(|t| t.0.contains("Esc")), "{:?}", h.app.ui.toast); + let r = h.request("ui.key", json!({"key": "esc"}), T); + assert_eq!(r["ok"], true, "{r}"); + h.step(); + assert!(h.app.ui.tool.is_empty(), "Esc turns the tool off: {:?}", h.app.ui.tool); + assert!(matches!(h.app.ui.view, crate::state::ViewMode::Detail), "the first Esc only leaves the tool"); +} diff --git a/crates/ui-egui/src/tests_masking.rs b/crates/ui-egui/src/tests_masking.rs index 8024c62c5..845baf2fa 100644 --- a/crates/ui-egui/src/tests_masking.rs +++ b/crates/ui-egui/src/tests_masking.rs @@ -1352,9 +1352,12 @@ fn a_selected_develop_slider_accepts_wheel_nudges() { // A wheel event over the selected control adjusts exposure instead of // scrolling the right sidebar (an unselected slider leaves scrolling alone). + // (120 points is more than one notch, `theme::WHEEL_LINE_POINTS`; egui smooths it over a few frames) let result = h.request("ui.scroll", json!({"dy": 120.0}), T); assert_eq!(result["ok"], true, "{result}"); - h.step(); + for _ in 0..10 { + h.step(); + } let after = lightcraft_develop::controls::get(&develop(&h), "light.exposure").expect("exposure"); assert!(after > before, "selected exposure did not increase: {before} -> {after}"); } diff --git a/crates/ui-egui/src/tests_scroll.rs b/crates/ui-egui/src/tests_scroll.rs index 1eff1ec7f..d04a818a4 100644 --- a/crates/ui-egui/src/tests_scroll.rs +++ b/crates/ui-egui/src/tests_scroll.rs @@ -208,3 +208,25 @@ fn filmstrip_shows_every_selected_photo() { assert_eq!(r["ok"], true, "{r}"); assert_eq!(fill(&mut h, second), active); } + +/// A mouse wheel notch (a line) scrolls `theme::WHEEL_LINE_POINTS`, not egui's default 40: long +/// panels took 50+ notches. +#[test] +fn a_wheel_notch_scrolls_a_hundred_points() { + let mut h = demo("photoGrid"); + let canvas = h.app.canvas_rect.expect("grid canvas"); + h.request("ui.move", json!({"x": canvas.center().x, "y": canvas.center().y}), T); + let y0 = grid_y(&h); + for _ in 0..3 { + h.events.push(egui::Event::MouseWheel { + unit: egui::MouseWheelUnit::Line, + delta: egui::vec2(0.0, -1.0), + phase: egui::TouchPhase::Move, + modifiers: egui::Modifiers::NONE, + }); + h.step(); + } + idle(&mut h, 120); + let moved = grid_y(&h) - y0; + assert!(moved >= 3.0 * crate::theme::WHEEL_LINE_POINTS - 1.0, "three notches scrolled {moved} points"); +} diff --git a/crates/ui-egui/src/theme.rs b/crates/ui-egui/src/theme.rs index a87a196ae..5ca7c2470 100644 --- a/crates/ui-egui/src/theme.rs +++ b/crates/ui-egui/src/theme.rs @@ -343,6 +343,13 @@ fn craft_font_name(f: &lightcraft_engine::CraftFont) -> String { format!("craft-fonts {} {}", f.family, f.style) } +/// Points one mouse-wheel notch (a line) scrolls. +pub const WHEEL_LINE_POINTS: f32 = 100.0; + +/// Zoom per wheel point (`exp(speed × points)`): egui's 1/200 at its stock 40-point notch, rescaled so +/// one notch still zooms the same amount now that it scrolls [`WHEEL_LINE_POINTS`]. +pub const WHEEL_ZOOM_PER_POINT: f32 = 40.0 / 200.0 / WHEEL_LINE_POINTS; + fn theme_id() -> egui::Id { egui::Id::new("lightcraft-theme") } @@ -362,6 +369,14 @@ pub fn apply(ctx: &egui::Context) { pub fn apply_kind(ctx: &egui::Context, kind: ThemeKind) { // Keep photo zoom shortcuts from resizing egui itself. ctx.options_mut(|o| o.zoom_with_keyboard = false); + // A mouse wheel notch scrolls 100 points (egui's default 40 took 50+ notches through an open + // Edit panel; Lightroom moves about this much per notch). Trackpads report points and are + // unaffected. + // A modifier-wheel (and the loupe's plain-wheel) zoom notch stays about 1.22x, as it was at 40 points. + ctx.options_mut(|o| { + o.input_options.line_scroll_speed = WHEEL_LINE_POINTS; + o.input_options.scroll_zoom_speed = WHEEL_ZOOM_PER_POINT; + }); let t = Tokens::for_kind(kind); ctx.data_mut(|d| { d.insert_temp(egui::Id::NULL, t); @@ -410,6 +425,19 @@ mod tests { use super::*; use crate::i18n::{Locale, set_language}; + /// Every theme scrolls 100 points a wheel notch, and a zoom notch stays egui's stock ~1.22x + /// (`exp(40 / 200)`) instead of growing with the longer scroll. + #[test] + fn a_wheel_notch_scrolls_further_but_zooms_as_before_in_every_theme() { + for kind in [ThemeKind::Charcoal, ThemeKind::Midnight, ThemeKind::Silver, ThemeKind::Paper] { + let ctx = egui::Context::default(); + apply_kind(&ctx, kind); + let (line, zoom) = ctx.options(|o| (o.input_options.line_scroll_speed, o.input_options.scroll_zoom_speed)); + assert_eq!(line, WHEEL_LINE_POINTS, "{kind:?}"); + assert!(((zoom * line).exp() - (40.0_f32 / 200.0).exp()).abs() < 1e-4, "{kind:?}: a notch zooms {}", (zoom * line).exp()); + } + } + /// #736: a Thai folder name drew boxes because only CJK craft-fonts faces were fallbacks. With a /// craft-fonts build that carries a Thai face, an English UI offers it in every family. #[test] diff --git a/crates/ui-egui/src/widgets.rs b/crates/ui-egui/src/widgets.rs index 0872b2ec4..cd86d5f38 100644 --- a/crates/ui-egui/src/widgets.rs +++ b/crates/ui-egui/src/widgets.rs @@ -369,10 +369,11 @@ pub fn slider(ui: &mut Ui, spec: &ControlSpec, value: f64, enabled: bool, label_ } /// Convert vertical wheel movement to bounded slider steps, keeping sub-notch -/// trackpad movement between frames without amplifying a single large event. +/// trackpad movement between frames without amplifying a single large event. One mouse-wheel +/// notch ([`crate::theme::WHEEL_LINE_POINTS`]) is one step. fn wheel_nudges(remainder: &mut f32, delta: f32) -> f64 { - const POINTS_PER_STEP: f32 = 40.0; - let total = (*remainder + delta).clamp(-400.0, 400.0); + const POINTS_PER_STEP: f32 = crate::theme::WHEEL_LINE_POINTS; + let total = (*remainder + delta).clamp(-10.0 * POINTS_PER_STEP, 10.0 * POINTS_PER_STEP); let steps = (total / POINTS_PER_STEP).trunc(); *remainder = total - steps * POINTS_PER_STEP; f64::from(steps) @@ -625,13 +626,18 @@ mod tests { #[test] fn wheel_steps_accumulate_small_movements_and_preserve_direction() { + // one mouse-wheel notch is one step, whatever a notch scrolls + let notch = crate::theme::WHEEL_LINE_POINTS; let mut remainder = 0.0; - assert_eq!(wheel_nudges(&mut remainder, 15.0), 0.0); - assert_eq!(wheel_nudges(&mut remainder, 25.0), 1.0); + assert_eq!(wheel_nudges(&mut remainder, notch * 0.375), 0.0); + assert_eq!(wheel_nudges(&mut remainder, notch * 0.625), 1.0); assert_eq!(remainder, 0.0); - assert_eq!(wheel_nudges(&mut remainder, -100.0), -2.0); - assert_eq!(wheel_nudges(&mut remainder, -20.0), -1.0); + assert_eq!(wheel_nudges(&mut remainder, -notch * 2.5), -2.0); + assert_eq!(wheel_nudges(&mut remainder, -notch * 0.5), -1.0); assert_eq!(remainder, 0.0); + assert_eq!(wheel_nudges(&mut remainder, notch), 1.0, "a single notch is a single nudge"); + assert_eq!(remainder, 0.0); + assert_eq!(wheel_nudges(&mut remainder, notch * 50.0), 10.0, "a single huge event is capped"); } /// Typed values (issue #322): signs, a decimal comma, units; on the control's steps, in range.