From f16f41394291e7d18d9cf1b648ddfb0d652df04f Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 20:09:16 +0000 Subject: [PATCH] Fix #372: masking tile labels and long mask names stay inside their boxes The Create New Mask tiles painted their (untranslated) labels with an unconstrained Painter::text into tiles capped at 52 pt, so Background and Luminance ran past their tiles, and translations would more so. The grid now measures the translated labels: four 52 pt tiles when they fit (at 10.5 pt, or 9.5 pt before dropping a column), else three or two, a tile growing past 52 pt for a long label; only in a panel too narrow for even that is a label cut short (full text on hover). A mask row's name is cut short with an ellipsis before the eye (full name on hover). The Describe field places the translated Select button first and gives the field what is left, instead of reserving a fixed 64 pt. Status lines wrap explicitly. The painted label and name rects are registered (maskNewLabel:*, maskName:*), so the sidebar test checks the text, not just the widgets, at four panel widths with a long name and a long prompt. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01912WaK6Rmew3wxZuPyX7E8 --- crates/ui-egui/src/panels/masking.rs | 108 +++++++++++++++++++++------ crates/ui-egui/src/tests_panels.rs | 20 +++++ crates/ui-egui/src/widgets.rs | 9 +++ 3 files changed, 114 insertions(+), 23 deletions(-) diff --git a/crates/ui-egui/src/panels/masking.rs b/crates/ui-egui/src/panels/masking.rs index 31dbbac6f..b73e91d82 100644 --- a/crates/ui-egui/src/panels/masking.rs +++ b/crates/ui-egui/src/panels/masking.rs @@ -1,6 +1,6 @@ //! The Masking panel: create masks, list them, edit components and local adjustments. -use egui::{Align2, Rect, Sense, Stroke, pos2, vec2}; +use egui::{Rect, Sense, Stroke, pos2, vec2}; use lightcraft_catalog::PhotoId; use lightcraft_develop::{ControlSpec, LocalAdjustments, MaskShape, Section, Track}; use serde_json::json; @@ -10,7 +10,7 @@ use super::right::header; use crate::LightcraftApp; use crate::icons::{Icon, paint}; use crate::theme::Tokens; -use crate::widgets::{divider, icon_button, register, slider, text_button}; +use crate::widgets::{divider, icon_button, one_line, register, slider, text_button}; const fn spec(id: &'static str, label: &'static str, min: f64, max: f64, step: f64, decimals: u8, track: Track) -> ControlSpec { ControlSpec { id, label, section: Section::Light, min, max, default: 0.0, step, decimals, track } @@ -61,12 +61,29 @@ fn kind_label(s: &MaskShape) -> (&'static str, Icon) { const TILE: f32 = 52.0; const TILE_GAP: f32 = 6.0; +/// Room left and right of a tile's label. +const TILE_PAD: f32 = 3.0; -/// Columns and tile width of the Create New Mask grid in `width`: four tiles of up to 52 pt (at -/// least 48, so the labels fit), else three (narrowed if even those don't fit). -fn tile_layout(width: f32) -> (usize, f32) { - let tile = |n: f32| ((width - (n - 1.0) * TILE_GAP) / n).floor().min(TILE); - if tile(4.0) >= 48.0 { (4, tile(4.0)) } else { (3, tile(3.0).max(24.0)) } +/// Columns, tile width and label size of the Create New Mask grid in `width`, `label(size)` being +/// the widest label at a font size: four tiles of up to 52 pt (at least 48), else three, else two — +/// the first that fits the labels at 10.5 pt, or at 9.5 pt before a column is dropped. A tile grows +/// past 52 pt for a long (translated) label. In a panel too narrow even for that, two tiles whose +/// labels are cut short. +fn tile_layout(width: f32, label: impl Fn(f32) -> f32) -> (usize, f32, f32) { + let room = |n: usize| ((width - (n as f32 - 1.0) * TILE_GAP) / n as f32).floor(); + for n in [4, 3, 2] { + let w = room(n); + if n == 4 && w < 48.0 { + continue; + } + for size in [10.5, 9.5] { + let need = (label(size) + 2.0 * TILE_PAD).ceil(); + if need <= w { + return (n, w.min(TILE.max(need)), size); + } + } + } + (2, room(2).max(24.0), 9.5) } pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId) { @@ -88,15 +105,29 @@ pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId) { ("luminanceRange", "Luminance", Icon::Sliders), ("colorRange", "Color", Icon::Picker), ]; - // four 52 pt tiles a row when they fit, else as many as fit (at least three, shrunk) - let (cols, tile) = tile_layout(ui.available_width()); + // four 52 pt tiles a row when they and their labels fit, else fewer (wider for a long label) + let widest = |size: f32| { + let font = t.font(size); + tiles + .iter() + .map(|(_, l, _)| ui.painter().layout_no_wrap(crate::i18n::tr(l).to_string(), font.clone(), t.text_dim).size().x) + .fold(0.0, f32::max) + }; + let (cols, tile, size) = tile_layout(ui.available_width(), widest); egui::Grid::new("mask-tiles").spacing(vec2(TILE_GAP, TILE_GAP)).show(ui, |ui| { for (i, (kind, label, icon)) in tiles.iter().enumerate() { let (r, resp) = ui.allocate_exact_size(vec2(tile, 52.0), Sense::click()); register(ui.ctx(), format!("maskNew:{kind}"), r); ui.painter().rect_filled(r, 4.0, if resp.hovered() { t.hover } else { t.inset }); paint(ui.painter(), Rect::from_center_size(r.center() - vec2(0.0, 7.0), vec2(20.0, 20.0)), *icon, t.text_label); - ui.painter().text(pos2(r.center().x, r.bottom() - 9.0), Align2::CENTER_CENTER, *label, t.font(10.5), t.text_dim); + // (cut short only in a panel too narrow for the label: the full name on hover) + let label = crate::i18n::tr(label); + let g = one_line(ui.painter(), label, t.font(size), t.text_dim, tile - 2.0 * TILE_PAD); + let elided = g.elided; + let at = pos2(r.center().x, r.bottom() - 9.0) - g.size() / 2.0; + register(ui.ctx(), format!("maskNewLabel:{kind}"), Rect::from_min_size(at, g.size())); + ui.painter().galley(at, g, t.text_dim); + let resp = if elided { resp.on_hover_text(label) } else { resp }; if resp.clicked() { match *kind { "colorRange" => { @@ -136,7 +167,7 @@ pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId) { }; if let Some(status) = status { ui.add_space(4.0); - ui.label(egui::RichText::new(crate::i18n::tr(status)).color(t.text_dim)); + ui.add(egui::Label::new(egui::RichText::new(crate::i18n::tr(status)).color(t.text_dim)).wrap()); ui.ctx().request_repaint_after(std::time::Duration::from_millis(250)); } // the model download, while its dialog is closed @@ -207,13 +238,14 @@ pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId) { ); let icon = m.components.first().map(|c| kind_label(&c.shape).1).unwrap_or(Icon::Mask); paint(ui.painter(), Rect::from_min_size(r.min + vec2(8.0, 7.0), vec2(16.0, 16.0)), icon, t.text_label); - ui.painter().text( - pos2(r.left() + 32.0, r.center().y), - Align2::LEFT_CENTER, - &m.name, - t.font(13.0), - if m.visible { t.text } else { t.text_disabled }, - ); + // a long name is cut short (with …) before the eye, the full name on hover + let color = if m.visible { t.text } else { t.text_disabled }; + let g = one_line(ui.painter(), &m.name, t.font(13.0), color, r.width() - 32.0 - 32.0); + let elided = g.elided; + let at = pos2(r.left() + 32.0, r.center().y - g.size().y / 2.0); + register(ui.ctx(), format!("maskName:{}", m.id), Rect::from_min_size(at, g.size())); + ui.painter().galley(at, g, color); + let resp = if elided { resp.on_hover_text(&m.name) } else { resp }; // show / hide on hover (and always while hidden) // (the pointer test, not `hovered`: over the eye, the row itself no longer counts as hovered) if ui.rect_contains_pointer(r) || !m.visible { @@ -706,11 +738,15 @@ fn describe_field(app: &mut LightcraftApp, ui: &mut egui::Ui, new_mask: bool) { }; ui.label(crate::i18n::tr(prompt)); let mut submit = false; - ui.horizontal(|ui| { + // the (translated) Select button is placed first, at the right; the field takes what is left + ui.allocate_ui_with_layout(vec2(ui.available_width(), 24.0), egui::Layout::right_to_left(egui::Align::Center), |ui| { + if text_button(ui, "maskDescribeGo", crate::i18n::tr("Select"), false).clicked() { + submit = true; + } let r = ui.add( egui::TextEdit::singleline(&mut text) .hint_text(crate::i18n::tr("e.g. sky · the red car · car, road")) - .desired_width(ui.available_width() - 64.0), + .desired_width(ui.available_width()), ); register(ui.ctx(), "maskDescribe", r.rect); if !r.has_focus() && !r.lost_focus() && text.is_empty() { @@ -719,9 +755,6 @@ fn describe_field(app: &mut LightcraftApp, ui: &mut egui::Ui, new_mask: bool) { if r.lost_focus() && ui.input(|i| i.key_pressed(egui::Key::Enter)) { submit = true; } - if text_button(ui, "maskDescribeGo", crate::i18n::tr("Select"), false).clicked() { - submit = true; - } }); if ui.input(|i| i.key_pressed(egui::Key::Escape)) { app.ui.describe = None; @@ -813,3 +846,32 @@ fn brush_settings(app: &mut LightcraftApp, ui: &mut egui::Ui) { } } } + +#[cfg(test)] +mod tests { + use super::*; + + /// The grid keeps four 52 pt tiles when the labels fit, and gives a long (translated) label + /// room by dropping a column rather than drawing it past its tile. + #[test] + fn tile_labels_get_the_room_they_need() { + let short = |size: f32| size * 4.0; // "Luminance"-ish: 42 pt at 10.5 + let long = |size: f32| size * 5.6; // "Background"/"Hintergrund": 59 pt at 10.5, 53 at 9.5 + assert_eq!(tile_layout(224.0, short), (4, 51.0, 10.5)); + // too wide for four at either size: three tiles, wider than 52 so the label fits + let (n, w, size) = tile_layout(224.0, long); + assert_eq!((n, size), (3, 10.5)); + assert!(w >= long(10.5) + 2.0 * TILE_PAD && w <= (224.0 - 2.0 * TILE_GAP) / 3.0, "{w}"); + // a smaller label keeps a column: 9.5 pt fits where 10.5 doesn't + let mid = |size: f32| size * 4.6; // 48.3 at 10.5, 43.7 at 9.5 + assert_eq!(tile_layout(224.0, mid), (4, 51.0, 9.5)); + // a wide panel: tiles stay 52 pt (or as wide as the label) + assert_eq!(tile_layout(474.0, short).0, 4); + assert_eq!(tile_layout(474.0, short).1, TILE); + // far too narrow for any label: two tiles (labels cut short), never a negative width + let (n, w, _) = tile_layout(60.0, long); + assert_eq!(n, 2); + assert!(w >= 24.0); + assert_eq!(tile_layout(f32::NAN, long).0, 2); + } +} diff --git a/crates/ui-egui/src/tests_panels.rs b/crates/ui-egui/src/tests_panels.rs index ee4a0f8ab..cb2d8e305 100644 --- a/crates/ui-egui/src/tests_panels.rs +++ b/crates/ui-egui/src/tests_panels.rs @@ -117,7 +117,27 @@ fn masking_contents_fit_the_right_panel_at_any_width() { T, ); assert_eq!(r["ok"], true, "{r}"); + let long = "The person standing beside the very long fence across the background"; + let r = h.request("engine.execute", json!({"command": "mask.rename", "params": {"id": 1, "name": long}}), T); + assert_eq!(r["ok"], true, "{r}"); for width in [RIGHT_WIDTH.min, RIGHT_WIDTH.default, 330.0, RIGHT_WIDTH.max] { + // a long Describe prompt being typed: the field and its Select button stay in the panel + h.app.ui.describe = Some(("new".into(), long.repeat(2))); + h.step(); + h.step(); + assert_inside_right_panel(&h, &format!("width {width}, describing")); + let (field, go) = (widget(&h, "maskDescribe"), widget(&h, "button:maskDescribeGo")); + assert!(field.right() <= go.left() && go.right() <= widget(&h, "panel:right_panel").right(), "width {width}: {field:?} {go:?}"); + h.app.ui.describe = None; + // the painted text, not just the widgets: tile labels inside their tiles, a long mask + // name cut short before the eye + h.step(); + for (id, r) in h.app.widgets.iter().filter(|(id, _)| id.starts_with("maskNewLabel:")) { + let tile = widget(&h, &id.replace("maskNewLabel:", "maskNew:")); + assert!(tile.contains_rect(*r), "width {width}: {id} {r:?} outside its tile {tile:?}"); + } + let (name, row) = (widget(&h, "maskName:1"), widget(&h, "mask:1")); + assert!(name.right() <= row.right() - 27.0, "width {width}: the name {name:?} runs under the eye of {row:?}"); for (selected, tool) in [(false, ""), (true, ""), (true, "brush")] { let mask = if selected { json!(2) } else { serde_json::Value::Null }; h.request("engine.execute", json!({"command": "mask.select", "params": {"id": mask}}), T); diff --git a/crates/ui-egui/src/widgets.rs b/crates/ui-egui/src/widgets.rs index 5dcd9f499..c98baf41f 100644 --- a/crates/ui-egui/src/widgets.rs +++ b/crates/ui-egui/src/widgets.rs @@ -593,6 +593,15 @@ mod access_tests { } } +/// `text` laid out on one line, cut short with … when it is wider than `max` (the galley's +/// `elided` says whether it was): for text painted into a fixed box (a tile, a row before its +/// buttons), which a plain `Painter::text` would draw past. +pub(crate) fn one_line(painter: &egui::Painter, text: &str, font: egui::FontId, color: Color32, max: f32) -> std::sync::Arc { + let mut job = egui::text::LayoutJob::simple_singleline(text.to_string(), font, color); + job.wrap = egui::text::TextWrapping::truncate_at_width(max.max(1.0)); + painter.layout_job(job) +} + /// `text` shortened to fit `max` (as `width` measures it): leading folders drop first /// (`Users/me/Pictures/Lightroom` → `…/Pictures/Lightroom`) so the end of a path, which says the /// most, stays; a single name still too long loses its end (`2024-06-12 Tri…`: folders tend to