From b0f184da333862b9c08ab178110ecfe699b52239 Mon Sep 17 00:00:00 2001 From: Nathaniel Ledford Date: Fri, 16 Oct 2026 16:03:02 -0400 Subject: [PATCH 01/11] LR-BEHAV-BG-TASKS: one progress bar, spinner and busy button Progress bars were five raw egui::ProgressBar calls with fixed widths (240, 260, 380), one hand-drawn bar in the activity stack, and a stock spinner; text-only waits had no sign of life. progress.rs now has Progress (a clamped, NaN-safe fraction or a gentle sweep when unknown, fills the layout's width, announced as a progress indicator with its percentage and registered by id) and a vector-arc spinner. Button::busy shows the spinner in the icon's place, keeps the width, ignores clicks and is announced busy. Thumbnails still being made draw a themed skeleton colour (a Tokens field, static) in the grid, the filmstrip and Compare, instead of a literal grey that was nearly black on the light themes. Co-Authored-By: Claude --- crates/ui-egui/src/button.rs | 44 +++++-- crates/ui-egui/src/lib.rs | 3 + crates/ui-egui/src/panels/activity.rs | 23 +--- crates/ui-egui/src/panels/compare.rs | 2 +- crates/ui-egui/src/panels/denoise.rs | 28 +++-- crates/ui-egui/src/panels/detail.rs | 3 + crates/ui-egui/src/panels/dialogs.rs | 3 +- crates/ui-egui/src/panels/faces.rs | 13 +- crates/ui-egui/src/panels/grid.rs | 2 +- crates/ui-egui/src/panels/masking.rs | 16 ++- crates/ui-egui/src/progress.rs | 174 ++++++++++++++++++++++++++ crates/ui-egui/src/sync.rs | 2 +- crates/ui-egui/src/tests_progress.rs | 167 ++++++++++++++++++++++++ crates/ui-egui/src/theme.rs | 7 ++ docs/parity-checklist.md | 3 +- docs/whats-new.md | 7 ++ 16 files changed, 443 insertions(+), 54 deletions(-) create mode 100644 crates/ui-egui/src/progress.rs create mode 100644 crates/ui-egui/src/tests_progress.rs diff --git a/crates/ui-egui/src/button.rs b/crates/ui-egui/src/button.rs index f46156ed9..a3f63a95b 100644 --- a/crates/ui-egui/src/button.rs +++ b/crates/ui-egui/src/button.rs @@ -188,6 +188,7 @@ pub struct Button<'a> { kind: Kind, size: Size, enabled: bool, + busy: bool, widget: Option, tooltip: Option<&'a str>, min_width: f32, @@ -204,6 +205,7 @@ impl<'a> Button<'a> { kind: Kind::default(), size: Size::default(), enabled: true, + busy: false, widget: None, tooltip: None, min_width: 0.0, @@ -256,6 +258,14 @@ impl<'a> Button<'a> { self } + /// Working on what it was pressed for: a spinner takes the icon's place (or sits before the + /// text, inside the button's own padding, so the button keeps its width), a click does nothing, + /// and screen readers hear it is busy. + pub fn busy(mut self, busy: bool) -> Self { + self.busy = busy; + self + } + /// Registered under this id (`button:…`) for scripts, tests and `ui.clickWidget`; unique on /// screen, as every registered id is. pub fn widget(mut self, id: impl Into) -> Self { @@ -331,14 +341,19 @@ impl<'a> Button<'a> { }; // a button in a disabled part of the window is disabled too let enabled = self.enabled && ui.is_enabled(); - let resp = ui.interact(rect, id, if enabled { Sense::click() } else { Sense::hover() }); + let resp = ui.interact(rect, id, if enabled && !self.busy { Sense::click() } else { Sense::hover() }); let words = crate::i18n::tr(self.name.or(self.text).unwrap_or_default()).to_string(); resp.widget_info(|| egui::WidgetInfo::labeled(egui::WidgetType::Button, enabled, words.clone())); + if self.busy { + ui.ctx().accesskit_node_builder(resp.id, |node| node.set_busy()); + } if let Some(w) = &self.widget { crate::widgets::register(ui.ctx(), w.clone(), rect); } let state = if !enabled { State::Disabled + } else if self.busy { + State::Idle } else if resp.is_pointer_button_down_on() { State::Pressed } else if resp.hovered() { @@ -362,16 +377,27 @@ impl<'a> Button<'a> { } let galley = self.galley(ui); let icon_s = self.size.icon(); - let content_w = match (&galley, self.icon) { - (Some(g), Some(_)) => icon_s + self.size.gap() + g.size().x, - (Some(g), None) => g.size().x, - (None, Some(_)) => icon_s, - (None, None) => 0.0, + // busy: the spinner is the icon's slot, or, with text alone, a slot as big as the padding + // allows, so the button never grows + let slot = + if self.busy && self.icon.is_none() && galley.is_some() { icon_s.min(self.size.padding() * 2.0 - self.size.gap()) } else { icon_s }; + let has_slot = self.icon.is_some() || (self.busy && galley.is_some()); + let content_w = match (&galley, has_slot) { + (Some(g), true) => slot + self.size.gap() + g.size().x, + (Some(g), false) => g.size().x, + (None, true) => slot, + (None, false) => 0.0, }; let mut x = rect.center().x - content_w / 2.0; - if let Some(icon) = self.icon { - paint(p, Rect::from_center_size(pos2(x + icon_s / 2.0, rect.center().y), vec2(icon_s, icon_s)), icon, lk.content); - x += icon_s + self.size.gap(); + if has_slot { + let slot_rect = Rect::from_center_size(pos2(x + slot / 2.0, rect.center().y), vec2(slot, slot)); + if self.busy { + crate::progress::paint_spinner(p, slot_rect, lk.content, ui.input(|i| i.time)); + ui.ctx().request_repaint_after(std::time::Duration::from_millis(33)); + } else if let Some(icon) = self.icon { + paint(p, slot_rect, icon, lk.content); + } + x += slot + self.size.gap(); } if let Some(g) = galley { let at = pos2(x, rect.center().y - g.size().y / 2.0); diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 1b2517907..a9238bd25 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -31,6 +31,7 @@ mod model_setup; pub mod panels; pub mod pick; pub mod print_view; +pub mod progress; pub mod recheck; pub mod region; pub mod reload; @@ -147,6 +148,8 @@ mod tests_profile_menu; #[cfg(test)] mod tests_profiles_layout; #[cfg(test)] +mod tests_progress; +#[cfg(test)] mod tests_quit_unsaved; #[cfg(test)] mod tests_recheck; diff --git a/crates/ui-egui/src/panels/activity.rs b/crates/ui-egui/src/panels/activity.rs index 2e4137fda..ed3c57cf8 100644 --- a/crates/ui-egui/src/panels/activity.rs +++ b/crates/ui-egui/src/panels/activity.rs @@ -3,7 +3,7 @@ //! that can be cancelled. The tasks come from the engine's registry ([`lightcraft_engine::activity`]); ✕ runs //! `activity.cancel` like any other frontend would. -use egui::{Align, Layout, Margin, Rect, RichText, Sense, Stroke, pos2, vec2}; +use egui::{Align, Layout, Margin, RichText, Sense, Stroke, pos2, vec2}; use lightcraft_engine::activity::{TaskInfo, Unit}; use serde_json::json; @@ -136,7 +136,7 @@ fn row(ui: &mut egui::Ui, t: &Tokens, task: &TaskInfo, width: f32) -> bool { clicked = resp.on_hover_text(tr("Cancel")).clicked(); } }); - bar(ui, t, task, width); + bar(ui, task, width); let line = detail_line(task); if !line.is_empty() { ui.add(egui::Label::new(RichText::new(line).font(t.font(11.0)).color(t.text_dim)).truncate()); @@ -147,20 +147,7 @@ fn row(ui: &mut egui::Ui, t: &Tokens, task: &TaskInfo, width: f32) -> bool { } /// The progress bar; while the amount of work isn't known, a segment sweeps across. -fn bar(ui: &mut egui::Ui, t: &Tokens, task: &TaskInfo, width: f32) { - let (rect, _) = ui.allocate_exact_size(vec2(width, 5.0), Sense::hover()); - let p = ui.painter(); - p.rect_filled(rect, 2.5, t.field); - let fill = if task.total > 0 { - let frac = (task.done as f64 / task.total as f64).clamp(0.0, 1.0) as f32; - Rect::from_min_size(rect.min, vec2(rect.width() * frac, rect.height())) - } else { - let phase = (ui.input(|i| i.time) * 0.7).fract() as f32; - let w = rect.width() * 0.3; - let x = rect.left() + (rect.width() + w) * phase - w; - Rect::from_min_max(pos2(x.max(rect.left()), rect.top()), pos2((x + w).min(rect.right()), rect.bottom())) - }; - if fill.width() > 0.0 { - p.rect_filled(fill, 2.5, if task.cancelling { t.text_dim } else { t.accent }); - } +fn bar(ui: &mut egui::Ui, task: &TaskInfo, width: f32) { + let fraction = (task.total > 0).then(|| (task.done as f64 / task.total as f64) as f32); + crate::progress::Progress::new(fraction).width(width).dimmed(task.cancelling).show(ui); } diff --git a/crates/ui-egui/src/panels/compare.rs b/crates/ui-egui/src/panels/compare.rs index bb7937640..cae016159 100644 --- a/crates/ui-egui/src/panels/compare.rs +++ b/crates/ui-egui/src/panels/compare.rs @@ -279,7 +279,7 @@ fn photo_tile(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId, slot: Slo display = super::detail::fit_rect(img_area, texture_aspect, zoom, native, ppp, app.ui.pan); p.image(tex.tex.id(), display, uv, Color32::WHITE); } else { - p.rect_filled(img.intersect(img_area), 0.0, Color32::from_gray(38)); + p.rect_filled(img.intersect(img_area), 0.0, Tokens::get(ui.ctx()).skeleton); } if photo.flag == Flag::Reject { p.rect_filled(display, 0.0, Color32::from_black_alpha(110)); diff --git a/crates/ui-egui/src/panels/denoise.rs b/crates/ui-egui/src/panels/denoise.rs index 437a378a7..d07a04178 100644 --- a/crates/ui-egui/src/panels/denoise.rs +++ b/crates/ui-egui/src/panels/denoise.rs @@ -168,7 +168,12 @@ pub fn detail_status(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId, am let now = ui.ctx().input(|i| i.time); app.caches.denoise.next_pump = app.caches.denoise.next_pump.min(now + 0.05); ui.ctx().request_repaint_after(std::time::Duration::from_millis(60)); - egui::Frame::NONE.inner_margin(pad).show(ui, |ui| note(ui, "Waiting to start…", t.text_dim)); + egui::Frame::NONE.inner_margin(pad).show(ui, |ui| { + ui.horizontal(|ui| { + crate::progress::spinner(ui, 12.0); + note(ui, "Waiting to start…", t.text_dim); + }) + }); } else if amount > 0.0 { egui::Frame::NONE.inner_margin(pad).show(ui, |ui| { note(ui, "Its denoised picture is not made yet, and pictures are not made on their own (Settings, AI Denoise).", t.text_dim); @@ -199,8 +204,7 @@ pub fn detail_status(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId, am } else { tr("Starting…").to_string() }; - let bar = ui.add(egui::ProgressBar::new(frac).desired_width(ui.available_width().min(240.0)).text(RichText::new(text).size(11.0))); - register(ui.ctx(), "denoise:progress", bar.rect); + crate::progress::Progress::new(if total > 0 { Some(frac) } else { None }).label(text).widget("denoise:progress").show(ui); }); } PhotoState::Failed { why, unsupported } => { @@ -266,7 +270,12 @@ pub fn settings_tab(app: &mut LightcraftApp, ui: &mut egui::Ui, t: &Tokens) { } if let Some(job) = downloads.iter().find(|d| d["id"] == "local-install") { match job["state"].as_str() { - Some("installing") => hint(ui, t, "Checking and installing the model…"), + Some("installing") => { + ui.horizontal(|ui| { + crate::progress::spinner(ui, 12.0); + hint(ui, t, "Checking and installing the model…"); + }); + } Some("failed") => hint(ui, t, job["error"].as_str().unwrap_or("Model installation failed")), _ => {} } @@ -333,8 +342,7 @@ fn work_section(app: &mut LightcraftApp, ui: &mut egui::Ui, t: &Tokens, list: &V done = done, total = total ); - let bar = ui.add(egui::ProgressBar::new(frac).desired_width(380.0).text(RichText::new(text).size(11.5))); - register(ui.ctx(), "denoise:settingsProgress", bar.rect); + crate::progress::Progress::new(Some(frac)).label(text).widget("denoise:settingsProgress").show(ui); } None if queued > 0 => hint(ui, t, &count(queued, "photo waiting", "photos waiting")), None => { @@ -444,11 +452,13 @@ fn model_row(app: &mut LightcraftApp, ui: &mut egui::Ui, t: &Tokens, m: &Value, } else { tr_format!("{} of {} from {host}", mb(Some(got)), mb(Some(total)), host = host) }; - let bar = ui.add(egui::ProgressBar::new(frac).desired_width(260.0).text(RichText::new(text).font(t.font(11.5)))); - register(ui.ctx(), format!("denoise:progress:{id}"), bar.rect); + crate::progress::Progress::new(Some(frac)).label(text).widget(format!("denoise:progress:{id}")).show(ui); } (Some("done" | "installing"), _) => { - ui.label(RichText::new(tr("Downloaded and checked. Installing…")).font(t.font(11.5)).color(t.text_dim)); + ui.horizontal(|ui| { + crate::progress::spinner(ui, 12.0); + ui.label(RichText::new(tr("Downloaded and checked. Installing…")).font(t.font(11.5)).color(t.text_dim)); + }); } (Some("failed"), Some(d)) => { let why = sentence(d["error"].as_str().unwrap_or("The download failed")); diff --git a/crates/ui-egui/src/panels/detail.rs b/crates/ui-egui/src/panels/detail.rs index b95d7b056..3a15e3f79 100644 --- a/crates/ui-egui/src/panels/detail.rs +++ b/crates/ui-egui/src/panels/detail.rs @@ -2859,6 +2859,9 @@ pub(crate) fn filmstrip(app: &mut LightcraftApp, ui: &mut egui::Ui, r: Rect) { } let img_area = Rect::from_min_max(cr.min + vec2(10.0, 22.0), cr.max - vec2(10.0, 8.0)); super::grid::request_thumb(app, *id, (256.0 * ppp.min(2.0) / 2.0) as usize * 2, 8); + if app.renderer.thumb(*id).is_none() { + p.rect_filled(img_area, 0.0, t.skeleton); + } if let Some(tex) = app.renderer.thumb(*id) { let [tw, th] = tex.size; let s = (img_area.width() / tw as f32).min(img_area.height() / th as f32); diff --git a/crates/ui-egui/src/panels/dialogs.rs b/crates/ui-egui/src/panels/dialogs.rs index 894ad2bd5..061826ba8 100644 --- a/crates/ui-egui/src/panels/dialogs.rs +++ b/crates/ui-egui/src/panels/dialogs.rs @@ -1666,8 +1666,7 @@ fn sam_model_body(app: &mut LightcraftApp, ui: &mut egui::Ui, error: Option<&str ui.add_space(4.0); let frac = if d.total > 0 { d.done as f64 / d.total as f64 } else { 0.0 }; let text = format!("{:.2} / {:.2} GB · {}", gb(d.done), gb(d.total), d.file); - let r = ui.add(egui::ProgressBar::new(frac as f32).text(text)); - crate::widgets::register(ui.ctx(), "progress:samDownload", r.rect); + crate::progress::Progress::new(Some(frac as f32)).label(text).widget("progress:samDownload").show(ui); let r = ui.button(crate::i18n::tr("Cancel Download")); crate::widgets::register(ui.ctx(), "button:samCancel", r.rect); if r.clicked() { diff --git a/crates/ui-egui/src/panels/faces.rs b/crates/ui-egui/src/panels/faces.rs index f1eb97caf..4f6febb93 100644 --- a/crates/ui-egui/src/panels/faces.rs +++ b/crates/ui-egui/src/panels/faces.rs @@ -312,10 +312,7 @@ fn scan_progress(app: &mut LightcraftApp, ui: &mut egui::Ui, t: &Tokens) { return; } let text = format!("Scanning photos for faces: {left} left · {faces} faces so far"); - let bar = ui.add( - egui::ProgressBar::new((1.0 - left as f32 / peak as f32).clamp(0.0, 1.0)).desired_width(380.0).text(RichText::new(text).font(t.font(11.5))), - ); - register(ui.ctx(), "faces:scanProgress", bar.rect); + crate::progress::Progress::new(Some(1.0 - left as f32 / peak as f32)).label(text).widget("faces:scanProgress").show(ui); } fn model_row(app: &mut LightcraftApp, ui: &mut egui::Ui, t: &Tokens, m: &Value, dl: Option<&Value>, can_run: bool) { @@ -367,11 +364,13 @@ fn model_row(app: &mut LightcraftApp, ui: &mut egui::Ui, t: &Tokens, m: &Value, } else { format!("{} of {} from {host}", mb(Some(got)), mb(Some(total))) }; - let bar = ui.add(egui::ProgressBar::new(frac).desired_width(260.0).text(RichText::new(text).font(t.font(11.5)))); - register(ui.ctx(), format!("faces:progress:{id}"), bar.rect); + crate::progress::Progress::new(Some(frac)).label(text).widget(format!("faces:progress:{id}")).show(ui); } (Some("done"), _) => { - ui.label(RichText::new(crate::i18n::tr("Downloaded and checked. Installing…")).font(t.font(11.5)).color(t.text_dim)); + ui.horizontal(|ui| { + crate::progress::spinner(ui, 12.0); + ui.label(RichText::new(crate::i18n::tr("Downloaded and checked. Installing…")).font(t.font(11.5)).color(t.text_dim)); + }); } (Some("failed"), Some(d)) => { let why = sentence(d["error"].as_str().unwrap_or("The download failed")); diff --git a/crates/ui-egui/src/panels/grid.rs b/crates/ui-egui/src/panels/grid.rs index a1184892c..9e934f9e8 100644 --- a/crates/ui-egui/src/panels/grid.rs +++ b/crates/ui-egui/src/panels/grid.rs @@ -800,7 +800,7 @@ fn cell(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId, r: Rect, square } } else { let ph = img_rect.shrink(if square { 20.0 } else { 0.0 }); - p.rect_filled(ph, 0.0, Color32::from_gray(38)); + p.rect_filled(ph, 0.0, t.skeleton); // not drawn yet (or unreadable): the selection is still visible if active { p.rect_stroke(ph, 0.0, Stroke::new(2.0, Color32::WHITE), StrokeKind::Outside); diff --git a/crates/ui-egui/src/panels/masking.rs b/crates/ui-egui/src/panels/masking.rs index b0371c422..54ede6bc3 100644 --- a/crates/ui-egui/src/panels/masking.rs +++ b/crates/ui-egui/src/panels/masking.rs @@ -169,7 +169,10 @@ pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId) { }; if let Some(status) = status { ui.add_space(4.0); - ui.add(egui::Label::new(egui::RichText::new(crate::i18n::tr(status)).color(t.text_dim)).wrap()); + ui.horizontal_top(|ui| { + crate::progress::spinner(ui, 14.0); + 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 @@ -178,10 +181,13 @@ pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId) { ui.add_space(4.0); ui.horizontal(|ui| { let pct = if download.total > 0 { download.done as f64 / download.total as f64 } else { 0.0 }; - let r = ui.add( - egui::ProgressBar::new(pct as f32).desired_width(ui.available_width() - 70.0).text(format!("SAM 3: {} %", (pct * 100.0).floor())), - ); - register(ui.ctx(), "maskSamProgress", r.rect); + // the bar takes what the Details button leaves + let details = ui.spacing().interact_size.x + 30.0; + crate::progress::Progress::new(Some(pct as f32)) + .label(format!("SAM 3: {} %", (pct * 100.0).floor())) + .width((ui.available_width() - details).max(96.0)) + .widget("maskSamProgress") + .show(ui); if text_button(ui, "maskSamDetails", crate::i18n::tr("Details"), false).clicked() { app.offer_sam_download(None); } diff --git a/crates/ui-egui/src/progress.rs b/crates/ui-egui/src/progress.rs new file mode 100644 index 000000000..01ec92a1b --- /dev/null +++ b/crates/ui-egui/src/progress.rs @@ -0,0 +1,174 @@ +//! Work in progress, shown one way everywhere: a [`Progress`] bar (a known fraction, or a gentle +//! sweep when nothing is known yet) and a [`spinner`] for short waits beside text or in a busy +//! [`crate::button::Button`]. It replaces `egui::ProgressBar` and `ui.spinner()`. +//! +//! ```ignore +//! Progress::new(Some(done as f32 / total as f32)).label(text).widget("denoise:progress").show(ui); +//! Progress::new(None).label(tr("Starting…")).show(ui); +//! ui.horizontal(|ui| { spinner(ui, 14.0); ui.label(tr("Selecting…")); }); +//! ``` +//! +//! A bar announces itself to screen readers as a progress indicator with its percentage. Motion +//! asks for repaints only while it is on screen. There is no reduced-motion setting in the app, +//! so the motion is kept slow and small: one gentle sweep, one slow turning arc. + +use std::f32::consts::TAU; + +use egui::{Color32, Pos2, Rect, Response, Sense, Shape, Stroke, Ui, pos2, vec2}; + +use crate::theme::Tokens; + +/// The narrowest a bar that fills the available width is drawn. +const MIN_WIDTH: f32 = 96.0; +/// Height of a bar that carries text. +const HEIGHT_WITH_TEXT: f32 = 18.0; +/// Height of a bar without text. +const HEIGHT_PLAIN: f32 = 6.0; +/// Seconds an indeterminate bar takes to sweep across and back. +const SWEEP_SECONDS: f64 = 1.8; +/// Seconds a spinner takes for one turn. +const TURN_SECONDS: f64 = 1.2; +/// How often moving things redraw. +const FRAME: std::time::Duration = std::time::Duration::from_millis(33); + +/// `fraction` as a number from 0 to 1: NaN counts as nothing done, and out-of-range values stop at +/// the ends. +pub fn clamp_fraction(fraction: f32) -> f32 { + if fraction.is_nan() { 0.0 } else { fraction.clamp(0.0, 1.0) } +} + +/// A progress bar, built and then shown with [`Progress::show`]. +#[derive(Clone, Debug)] +pub struct Progress { + fraction: Option, + label: Option, + widget: Option, + width: Option, + dimmed: bool, +} + +impl Progress { + /// `Some(fraction)` of the work done (clamped to 0..=1, NaN as 0), or `None` while it is not + /// known how much there is. + pub fn new(fraction: Option) -> Self { + Progress { fraction: fraction.map(clamp_fraction), label: None, widget: None, width: None, dimmed: false } + } + + /// The fraction drawn, once clamped (`None` for an indeterminate bar). + pub fn fraction(&self) -> Option { + self.fraction + } + + /// Text on the bar (a percentage, a count, a file's name), already translated. + pub fn label(mut self, text: impl Into) -> Self { + self.label = Some(text.into()); + self + } + + /// Registered under this id for scripts, tests and `ui.clickWidget` (e.g. `progress:samDownload`). + pub fn widget(mut self, id: impl Into) -> Self { + self.widget = Some(id.into()); + self + } + + /// Drawn in the quiet text colour instead of the accent (work that is being stopped). + pub fn dimmed(mut self, dimmed: bool) -> Self { + self.dimmed = dimmed; + self + } + + /// This wide; by default the bar fills the width the layout has left. + pub fn width(mut self, width: f32) -> Self { + self.width = Some(width); + self + } + + /// Show it where the layout puts it. + pub fn show(self, ui: &mut Ui) -> Response { + let t = Tokens::get(ui.ctx()); + let label = self.label.as_deref().filter(|s| !s.is_empty()); + let width = self.width.unwrap_or_else(|| ui.available_size_before_wrap().x.max(MIN_WIDTH)); + let height = if label.is_some() { HEIGHT_WITH_TEXT } else { HEIGHT_PLAIN }; + let (rect, resp) = ui.allocate_exact_size(vec2(width, height), Sense::hover()); + resp.widget_info(|| { + let mut info = match label { + Some(l) => egui::WidgetInfo::labeled(egui::WidgetType::ProgressIndicator, ui.is_enabled(), l), + None => egui::WidgetInfo::new(egui::WidgetType::ProgressIndicator), + }; + info.value = self.fraction.map(|f| f64::from(f * 100.0).floor()); + info + }); + if let Some(w) = &self.widget { + crate::widgets::register(ui.ctx(), w.clone(), rect); + } + if !ui.is_rect_visible(rect) { + return resp; + } + let p = ui.painter_at(rect); + let radius = height / 2.0; + p.rect_filled(rect, radius, t.inset); + p.rect_stroke(rect, radius, Stroke::new(1.0, t.field_border), egui::StrokeKind::Inside); + let inner = rect.shrink(1.0); + let fill = match self.fraction { + Some(f) => Rect::from_min_size(inner.min, vec2(inner.width() * f, inner.height())), + None => { + // a short bar sweeping across and back, eased so it slows at the ends + let phase = (ui.input(|i| i.time) / SWEEP_SECONDS).fract() as f32; + let eased = 0.5 - 0.5 * (phase * TAU).cos(); + let len = inner.width() * 0.3; + let x = inner.left() + (inner.width() - len) * eased; + ui.ctx().request_repaint_after(FRAME); + Rect::from_min_size(pos2(x, inner.top()), vec2(len, inner.height())) + } + }; + if fill.width() > 0.0 { + p.with_clip_rect(inner).rect_filled(fill, radius - 1.0, if self.dimmed { t.text_dim } else { t.accent }); + } + if let Some(l) = label { + // the text reads on the track in the theme's colour, and in white where the fill is + let font = t.font(11.5); + let on_track = p.layout_no_wrap(l.to_string(), font.clone(), t.text); + let on_fill = p.layout_no_wrap(l.to_string(), font, Color32::WHITE); + let at = pos2(rect.center().x - on_track.size().x / 2.0, rect.center().y - on_track.size().y / 2.0); + let clipped = ui.painter_at(rect); + clipped.galley(at, on_track, t.text); + clipped.with_clip_rect(rect.intersect(fill)).galley(at, on_fill, Color32::WHITE); + } + resp + } +} + +/// The arc a spinner draws at `time` seconds: points on a circle of `radius` around `center`, +/// covering three quarters of the turn and rotating once per [`TURN_SECONDS`]. +fn arc_points(center: Pos2, radius: f32, time: f64) -> Vec { + let start = ((time / TURN_SECONDS).fract() as f32) * TAU; + let steps = 24; + (0..=steps) + .map(|i| { + let a = start + TAU * 0.75 * (i as f32 / steps as f32); + center + vec2(a.cos(), a.sin()) * radius + }) + .collect() +} + +/// Draw a spinner of `size` points centred in `rect`, in `color`, at `time` seconds. Callers that +/// draw by hand (a busy button) use this; it asks for no repaint itself. +pub fn paint_spinner(p: &egui::Painter, rect: Rect, color: Color32, time: f64) { + let size = rect.width().min(rect.height()); + let width = (size / 8.0).max(1.5); + let radius = ((size - width) / 2.0).max(0.5); + p.add(Shape::line(arc_points(rect.center(), radius, time), Stroke::new(width, color))); +} + +/// A small turning arc beside text that says what is being waited for. It is drawn as a vector +/// arc, not a glyph, and asks for repaints only while it is on screen. +pub fn spinner(ui: &mut Ui, size: f32) -> Response { + let t = Tokens::get(ui.ctx()); + let (rect, resp) = ui.allocate_exact_size(vec2(size, size), Sense::hover()); + resp.widget_info(|| egui::WidgetInfo::new(egui::WidgetType::ProgressIndicator)); + if ui.is_rect_visible(rect) { + paint_spinner(ui.painter(), rect, t.text_dim, ui.input(|i| i.time)); + ui.ctx().request_repaint_after(FRAME); + } + resp +} diff --git a/crates/ui-egui/src/sync.rs b/crates/ui-egui/src/sync.rs index 0473a38fa..e349f694b 100644 --- a/crates/ui-egui/src/sync.rs +++ b/crates/ui-egui/src/sync.rs @@ -169,7 +169,7 @@ pub fn body(app: &LightcraftApp, ui: &mut egui::Ui, dlg: &mut Dialog) { ui.add_space(6.0); let Some(c) = counts.as_ref() else { ui.horizontal(|ui| { - ui.spinner(); + crate::progress::spinner(ui, 14.0); match progress(app) { Some(ScanStage::Listing(folders, files)) => ui.label(listing_text(folders, files)), Some(ScanStage::Checking(done, total)) if total > 0 => ui.label(format!("{} {done} / {total}", crate::i18n::tr("Scanning…"))), diff --git a/crates/ui-egui/src/tests_progress.rs b/crates/ui-egui/src/tests_progress.rs new file mode 100644 index 000000000..bd2c28e28 --- /dev/null +++ b/crates/ui-egui/src/tests_progress.rs @@ -0,0 +1,167 @@ +//! Progress bars, spinners and busy buttons: the fraction is always a sane number, a bar is +//! announced as progress with its percentage and reachable by its id, a busy button is announced +//! busy, ignores clicks and keeps its width, and a thumbnail's placeholder stands apart from what +//! it sits on in every theme. + +use egui::{Pos2, Rect, pos2, vec2}; + +use crate::button::{Button, contrast}; +use crate::icons::Icon; +use crate::progress::{Progress, clamp_fraction, spinner}; +use crate::theme::{ThemeKind, Tokens}; + +/// What a screen reader gets for one node. +#[derive(Debug, Clone, PartialEq)] +struct Node { + role: String, + label: String, + busy: bool, + value: Option, +} + +/// Run three frames of `draw` (pointer `events` in the second); return the nodes of the last +/// frame with an accessibility tree, the automation ids registered in the last frame, and what +/// `draw` returned. +fn frames(events: Vec, mut draw: impl FnMut(&mut egui::Ui) -> T) -> (Vec, Vec<(String, Rect)>, T) { + let ctx = egui::Context::default(); + crate::theme::install_fonts(&ctx); + ctx.enable_accesskit(); + let (mut nodes, mut ids, mut last) = (Vec::new(), Vec::new(), T::default()); + for frame in 0..3 { + let raw = egui::RawInput { + screen_rect: Some(Rect::from_min_size(Pos2::ZERO, vec2(800.0, 600.0))), + events: if frame == 1 { events.clone() } else { Vec::new() }, + ..Default::default() + }; + let mut out = ctx.run_ui(raw, |ui| last = draw(ui)); + out.textures_delta.clear(); + ids = crate::widgets::take_registry(&ctx); + if let Some(update) = out.platform_output.accesskit_update.take() { + nodes = update + .nodes + .iter() + .map(|(_, n)| Node { + role: format!("{:?}", n.role()), + label: n.label().unwrap_or_default().to_string(), + busy: n.is_busy(), + value: n.numeric_value(), + }) + .collect(); + } + } + (nodes, ids, last) +} + +#[test] +fn the_fraction_is_always_between_nothing_and_all() { + assert_eq!(clamp_fraction(f32::NAN), 0.0); + assert_eq!(clamp_fraction(-3.0), 0.0); + assert_eq!(clamp_fraction(7.5), 1.0); + assert_eq!(clamp_fraction(f32::INFINITY), 1.0); + assert_eq!(clamp_fraction(f32::NEG_INFINITY), 0.0); + assert_eq!(clamp_fraction(0.25), 0.25); + assert_eq!(Progress::new(Some(f32::NAN)).fraction(), Some(0.0)); + assert_eq!(Progress::new(Some(2.0)).fraction(), Some(1.0)); + assert_eq!(Progress::new(None).fraction(), None); +} + +#[test] +fn a_bar_is_progress_with_a_value_and_reachable_by_its_id() { + let (nodes, ids, _) = frames(Vec::new(), |ui| { + Progress::new(Some(0.42)).label("Making the denoised picture… 42%").widget("denoise:progress").show(ui); + }); + let bar = nodes.iter().find(|n| n.role == "ProgressIndicator").unwrap_or_else(|| panic!("no progress node: {nodes:?}")); + assert_eq!(bar.label, "Making the denoised picture… 42%"); + assert_eq!(bar.value, Some(42.0)); + assert!(ids.iter().any(|(id, r)| id == "denoise:progress" && r.width() > 0.0), "{ids:?}"); +} + +#[test] +fn an_indeterminate_bar_says_progress_without_a_value_and_is_drawn() { + let (nodes, ids, width) = frames(Vec::new(), |ui| Progress::new(None).label("Starting…").widget("p:x").show(ui).rect.width()); + let bar = nodes.iter().find(|n| n.role == "ProgressIndicator").unwrap_or_else(|| panic!("{nodes:?}")); + assert_eq!(bar.value, None); + assert!(ids.iter().any(|(id, _)| id == "p:x")); + assert!(width >= 96.0, "fills the width the layout has"); +} + +#[test] +fn a_bar_fills_the_layout_unless_given_a_width() { + let (_, _, (fill, fixed, tiny_room)) = frames(Vec::new(), |ui| { + let fill = Progress::new(Some(0.5)).show(ui).rect.width(); + let fixed = Progress::new(Some(0.5)).width(150.0).show(ui).rect.width(); + let tiny_room = ui.scope(|ui| { + ui.set_max_width(40.0); + Progress::new(Some(0.5)).show(ui).rect.width() + }); + (fill, fixed, tiny_room.inner) + }); + assert!(fill > 300.0, "fills: {fill}"); + assert_eq!(fixed, 150.0); + assert!(tiny_room >= 96.0, "never a sliver: {tiny_room}"); +} + +#[test] +fn a_spinner_takes_the_room_it_is_given() { + let (_, _, size) = frames(Vec::new(), |ui| spinner(ui, 14.0).rect.size()); + assert_eq!(size, vec2(14.0, 14.0)); +} + +fn click_at(at: Pos2) -> Vec { + vec![ + egui::Event::PointerMoved(at), + egui::Event::PointerButton { pos: at, button: egui::PointerButton::Primary, pressed: true, modifiers: Default::default() }, + egui::Event::PointerButton { pos: at, button: egui::PointerButton::Primary, pressed: false, modifiers: Default::default() }, + ] +} + +#[test] +fn a_busy_button_ignores_clicks_and_is_announced_busy() { + let place = Rect::from_min_size(Pos2::ZERO, vec2(120.0, 30.0)); + let click = |busy: bool| { + let mut clicked = false; + let (nodes, _, ()) = frames(click_at(pos2(40.0, 15.0)), |ui| clicked |= Button::new("Analyze").busy(busy).show_at(ui, place).clicked()); + (nodes, clicked) + }; + let (nodes, clicked) = click(true); + assert!(!clicked, "a busy button does nothing"); + let node = nodes.iter().find(|n| n.role == "Button" && n.label == "Analyze").unwrap_or_else(|| panic!("{nodes:?}")); + assert!(node.busy, "announced busy"); + + let (nodes, clicked) = click(false); + assert!(clicked, "the same button, not busy, is clicked"); + assert!(!nodes.iter().any(|n| n.busy), "{nodes:?}"); +} + +#[test] +fn a_button_keeps_its_width_while_busy() { + for (text, icon) in [("Analyze", None), ("Analyze", Some(Icon::Trash)), ("", Some(Icon::Trash))] { + for small in [false, true] { + let (_, _, (idle, busy)) = frames(Vec::new(), |ui| { + let mut make = |b: bool| { + let mut btn = if text.is_empty() { Button::icon_only(Icon::Trash, "Remove") } else { Button::new(text) }; + if let Some(i) = icon { + btn = btn.icon(i); + } + if small { + btn = btn.small(); + } + btn.busy(b).show(ui).rect.size() + }; + (make(false), make(true)) + }); + assert_eq!(idle, busy, "{text:?} {icon:?} small={small}"); + } + } +} + +#[test] +fn the_thumbnail_placeholder_stands_apart_in_every_theme() { + for kind in ThemeKind::ALL { + let t = Tokens::for_kind(kind); + for (name, behind) in [("grid", t.grid_bg), ("cell", t.cell), ("canvas", t.canvas)] { + let c = contrast(t.skeleton, behind); + assert!(c >= 1.1, "{kind:?}: the placeholder on the {name} is {c:.2}:1"); + } + } +} diff --git a/crates/ui-egui/src/theme.rs b/crates/ui-egui/src/theme.rs index 57ee987d1..132797a3c 100644 --- a/crates/ui-egui/src/theme.rs +++ b/crates/ui-egui/src/theme.rs @@ -44,6 +44,9 @@ pub struct Tokens { pub grid_bg: Color32, pub cell: Color32, pub cell_selected: Color32, + /// A thumbnail's place while its picture is still being made: static, apart from the grid, the + /// cells and the canvas in every theme. + pub skeleton: Color32, pub divider: Color32, pub inset: Color32, pub field: Color32, @@ -125,6 +128,7 @@ impl Default for Tokens { grid_bg: Color32::from_rgb(0x0f, 0x0f, 0x0f), cell: Color32::from_rgb(0x1c, 0x1c, 0x1c), cell_selected: Color32::from_rgb(0x2d, 0x2d, 0x2d), + skeleton: Color32::from_rgb(0x26, 0x26, 0x26), divider: Color32::from_rgb(0x1c, 0x1c, 0x1c), inset: Color32::from_rgb(0x23, 0x23, 0x23), field: Color32::from_rgb(0x22, 0x22, 0x22), @@ -172,6 +176,7 @@ impl Tokens { grid_bg: rgb(0x080808), cell: rgb(0x121212), cell_selected: rgb(0x262626), + skeleton: rgb(0x202020), divider: rgb(0x0e0e0e), inset: rgb(0x181818), field: rgb(0x161616), @@ -189,6 +194,7 @@ impl Tokens { grid_bg: rgb(0xd9d9d9), cell: rgb(0xcbcbcb), cell_selected: rgb(0xb4b4b4), + skeleton: rgb(0xb8b8b8), divider: rgb(0xc4c4c4), inset: rgb(0xdcdcdc), field: rgb(0xf6f6f6), @@ -219,6 +225,7 @@ impl Tokens { grid_bg: rgb(0xf2f2f2), cell: rgb(0xe8e8e8), cell_selected: rgb(0xd2d2d2), + skeleton: rgb(0xd6d6d6), divider: rgb(0xe0e0e0), inset: rgb(0xf0f0f0), field: rgb(0xffffff), diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index 99f17561e..729ef0aeb 100644 --- a/docs/parity-checklist.md +++ b/docs/parity-checklist.md @@ -461,7 +461,7 @@ differences and missing bindings are in [ui-parity.md](ui-parity.md#shortcuts). | LR-BEHAV-BATCH | Batch apply to selection | P0 | ✅ | `cmd:photo.rate`, `cmd:develop.paste`, `cmd:preset.apply`, `cmd:app.export` | | | LR-BEHAV-PREVIEW-HOVER | Hover previews | P1 | ✅ | `crates/ui-egui/src/panels/presets.rs`, `crates/ui-egui/src/panels/profiles.rs`, `crates/ui-egui/src/panels/right.rs` (`versions`), `crates/ui-egui/src/panels/detail.rs` | presets, profile menu, profile browser and versions preview in the loupe | | LR-BEHAV-PROGRESSIVE | Progressive rendering | P0 | ✅ | `crates/engine/src/media.rs`, `crates/preview/src/lib.rs` | | -| LR-BEHAV-BG-TASKS | Background tasks | P0 | ✅ | `cmd:activity.list`, `cmd:activity.cancel`, `crates/engine/src/activity.rs`, `crates/ui-egui/src/panels/activity.rs`, `docs/background-tasks.md`, `crates/preview/src/lib.rs` (`JobPool`) | renders off the UI thread; exports run several photos of a batch side by side (`export_parallelism`, memory-gated; files, names and order as one at a time, #496); one activity stack in the top-left corner under the top bar, as in Lightroom Classic (issue #345): imports, folder scans, exports (and the contact sheet PDF), Synchronize Folder (its scan and its run), Build / Smart Previews, Lightroom catalog inspect / import, Photo Merge, model downloads (SAM 3, face, AI Denoise), the face scan, AI Denoise, Find Missing Photos and Reload from Disk (`cmd:photo.reload`, its files decoded on a worker thread, cancellable; `wait: true` for agents) each show a row (name, progress bar or a sweep while the amount isn't known, count and current item) once 0.5 s old, three rows then "+N more"; ✕ cancels the jobs that can stop ("Stopping…" until they do); quitting asks while a cancellable task runs; agents and `ui.inspect` (`activity`) see the same list. No pause (Lightroom can pause some tasks) | +| LR-BEHAV-BG-TASKS | Background tasks | P0 | ✅ | `cmd:activity.list`, `cmd:activity.cancel`, `crates/engine/src/activity.rs`, `crates/ui-egui/src/panels/activity.rs`, `crates/ui-egui/src/progress.rs`, `docs/background-tasks.md`, `crates/preview/src/lib.rs` (`JobPool`) | renders off the UI thread; exports run several photos of a batch side by side (`export_parallelism`, memory-gated; files, names and order as one at a time, #496); one activity stack in the top-left corner under the top bar, as in Lightroom Classic (issue #345): imports, folder scans, exports (and the contact sheet PDF), Synchronize Folder (its scan and its run), Build / Smart Previews, Lightroom catalog inspect / import, Photo Merge, model downloads (SAM 3, face, AI Denoise), the face scan, AI Denoise, Find Missing Photos and Reload from Disk (`cmd:photo.reload`, its files decoded on a worker thread, cancellable; `wait: true` for agents) each show a row (name, progress bar or a sweep while the amount isn't known, count and current item) once 0.5 s old, three rows then "+N more"; ✕ cancels the jobs that can stop ("Stopping…" until they do); quitting asks while a cancellable task runs; agents and `ui.inspect` (`activity`) see the same list. No pause (Lightroom can pause some tasks) | | LR-BEHAV-OFFLINE | Offline editing | P1 | ✅ | | local-first: everything works offline | | LR-BEHAV-WEB-SAFETY | Library safety in the browser build | P1 | 🟡 | `cmd:file.backupLibrary`, `cmd:file.restoreLibrary`, `apps/lightcraft-web/src/safety.rs`, `apps/lightcraft-web/src/backup.rs`, `apps/lightcraft-web/src/files.rs` | web only (experimental): Back Up Library (zip of the catalog and every stored original) and a non-destructive Restore; failed saves show as unsaved and are retried; a photo that can't be stored isn't added; one tab per library (Web Locks); notices for storage that isn't persistent and for a library that can't open; `?reset` asks first; a panic shows a message. Missing: `?safe` start, deleting originals of removed photos, zip64 (backups > 4 GB) | | LR-BEHAV-GPU | GPU acceleration | P0 | ✅ | `crates/gpu/src/render.rs`, `cmd:app.gpu`, `docs/gpu-pipeline.md`, `crates/ui-egui/src/panels/notices.rs` | CPU fallback per render on device limits, out of memory, device errors / loss, incomplete or black results (`perf.gpuReason`, `perf.gpuFallback`; each render reports its own reason — `lightcraft_gpu::try_render`, `gpuFallback` per exported file — issue #675); short submissions for slow iGPUs; backend choice (DX12 only on Windows, `LIGHTCRAFT_GPU_BACKEND` / `WGPU_BACKEND`), device created after the window opens, crash sentinel (issue #136); a GPU lost while LightCraft runs (driver reset) stops it with that message and turns GPU rendering off for the next start, which says why (issue #1337; the window's renderer can't survive the lost device); quitting waits (up to 2 s) for renders in flight and starts no new GPU work, so none is inside the driver when the process exits (`crates/gpu/src/exit.rs`, issue #620; seen crashing only on Linux + NVIDIA, other drivers unverified); with GPU rendering on and no adapter found at all (e.g. no Vulkan loader on NixOS outside `nix develop`), the desktop app says once per session that it renders on the CPU and points to Help ▸ System Info (`gpu_notice`; not when the GPU is turned off or only a software adapter exists); no tiling beyond the buffer limit (CPU) | @@ -927,3 +927,4 @@ Latin stays upright. Advanced Japanese composition remains open. Regression cove | 2026-10-16 | minor | LR-VIEW-PHOTOGRID: when the photos shown change by those an edit touched (an import's batch, a capture time, a crop), the grid reads only those photos' dates and shapes again instead of every photo of the list (a 16-photo batch at 181,749 photos: grid frame 101 ms → 7 ms); status unchanged | | 2026-10-19 | minor | LRC-LIB-LIBFOLDERS: the sidebar trees indent by a narrower step (14 / 12 / 10 / 8 pt) when the deepest open row would not fit the panel, instead of cutting its name or scrolling sideways at once (issue #2298); status unchanged (✅) | | 2026-10-15 | patch | LR-EXP-DIM: Width & Height (`fit: widthHeight`) limits the width and the height as the photo stands, beside Dimensions (W × H, either orientation), as in Lightroom (#1620); status unchanged (✅) | +| 2026-10-16 | patch | LR-BEHAV-BG-TASKS: progress bars and spinners come from one component (`progress.rs`): the activity stack, model downloads, denoise and the face scan share it; busy buttons and a themed thumbnail placeholder added; status unchanged | diff --git a/docs/whats-new.md b/docs/whats-new.md index cffff83e2..e621351bf 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -2,6 +2,13 @@ ## 2026-10-21 +### Progress looks the same everywhere +- Download, scan, denoise and activity progress now use one bar that fills the width it is given, with its text on + it, and a gentle sweep while the amount of work isn't known yet. Waits that only said "Selecting…", "Waiting to + start…" or "Installing…" now show a small turning arc beside the text, and a thumbnail still being made shows a + quiet placeholder that fits the theme in the grid, the filmstrip and Compare. Screen readers hear a progress + indicator with its percentage. + ### Sharper resampling in HDR and panorama merges - Merged photos (HDR and panorama) are resampled with a sharper filter (Lanczos-3 instead of bilinear). Moving a photo by half a pixel used to lose a third of its finest detail; now it keeps most of it, without halos around From bbd9a35237eb50dc72860877610443f992530b12 Mon Sep 17 00:00:00 2001 From: Nathaniel Ledford Date: Fri, 16 Oct 2026 16:39:43 -0400 Subject: [PATCH 02/11] LR-BEHAV-BG-TASKS: a busy text button keeps its padding The spinner in a text-only busy button is smaller and sits closer to the text, so the text no longer touches the button's border while the width stays the same. Co-Authored-By: Claude --- crates/ui-egui/src/button.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/crates/ui-egui/src/button.rs b/crates/ui-egui/src/button.rs index a3f63a95b..3dbd368b1 100644 --- a/crates/ui-egui/src/button.rs +++ b/crates/ui-egui/src/button.rs @@ -377,13 +377,13 @@ impl<'a> Button<'a> { } let galley = self.galley(ui); let icon_s = self.size.icon(); - // busy: the spinner is the icon's slot, or, with text alone, a slot as big as the padding - // allows, so the button never grows - let slot = - if self.busy && self.icon.is_none() && galley.is_some() { icon_s.min(self.size.padding() * 2.0 - self.size.gap()) } else { icon_s }; - let has_slot = self.icon.is_some() || (self.busy && galley.is_some()); + // busy: the spinner is the icon's slot; with text alone it is a smaller one in the button's + // own padding (a little tighter), so the button never grows + let spinner_only = self.busy && self.icon.is_none() && galley.is_some(); + let (slot, gap) = if spinner_only { ((self.size.padding() * 2.0 - 12.0).clamp(8.0, icon_s), 4.0) } else { (icon_s, self.size.gap()) }; + let has_slot = self.icon.is_some() || spinner_only; let content_w = match (&galley, has_slot) { - (Some(g), true) => slot + self.size.gap() + g.size().x, + (Some(g), true) => slot + gap + g.size().x, (Some(g), false) => g.size().x, (None, true) => slot, (None, false) => 0.0, @@ -397,7 +397,7 @@ impl<'a> Button<'a> { } else if let Some(icon) = self.icon { paint(p, slot_rect, icon, lk.content); } - x += slot + self.size.gap(); + x += slot + gap; } if let Some(g) = galley { let at = pos2(x, rect.center().y - g.size().y / 2.0); From 05b653995b7d425012193723ea0624229c197058 Mon Sep 17 00:00:00 2001 From: Nathaniel Ledford Date: Fri, 16 Oct 2026 17:04:42 -0400 Subject: [PATCH 03/11] LR-BEHAV-BG-TASKS: busy buttons where work starts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Install from file… (denoise) shows busy while the model installs, and the describe-mask Select button while the AI works. Co-Authored-By: Claude --- crates/ui-egui/src/panels/denoise.rs | 15 +++++++-------- crates/ui-egui/src/panels/masking.rs | 3 ++- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/crates/ui-egui/src/panels/denoise.rs b/crates/ui-egui/src/panels/denoise.rs index d07a04178..08ad533f0 100644 --- a/crates/ui-egui/src/panels/denoise.rs +++ b/crates/ui-egui/src/panels/denoise.rs @@ -249,8 +249,12 @@ pub fn settings_tab(app: &mut LightcraftApp, ui: &mut egui::Ui, t: &Tokens) { .map(|v| v["downloads"].as_array().cloned().unwrap_or_default()) .unwrap_or_default(); heading(ui, t, "Models"); - let r = ui.add_enabled(runtime && app.services.pick_denoise_model.is_some(), egui::Button::new(tr("Install from file…"))); - register(ui.ctx(), "denoise:installFile", r.rect); + let installing = downloads.iter().any(|d| d["id"] == "local-install" && d["state"].as_str() == Some("installing")); + let r = crate::button::Button::new("Install from file…") + .enabled(runtime && app.services.pick_denoise_model.is_some()) + .busy(installing) + .widget("denoise:installFile") + .show(ui); if r.clicked() && let Some(path) = app.services.pick_denoise_model.as_mut().and_then(|f| f().into_iter().next()) { @@ -270,12 +274,7 @@ pub fn settings_tab(app: &mut LightcraftApp, ui: &mut egui::Ui, t: &Tokens) { } if let Some(job) = downloads.iter().find(|d| d["id"] == "local-install") { match job["state"].as_str() { - Some("installing") => { - ui.horizontal(|ui| { - crate::progress::spinner(ui, 12.0); - hint(ui, t, "Checking and installing the model…"); - }); - } + Some("installing") => hint(ui, t, "Checking and installing the model…"), Some("failed") => hint(ui, t, job["error"].as_str().unwrap_or("Model installation failed")), _ => {} } diff --git a/crates/ui-egui/src/panels/masking.rs b/crates/ui-egui/src/panels/masking.rs index 54ede6bc3..20e15e3a3 100644 --- a/crates/ui-egui/src/panels/masking.rs +++ b/crates/ui-egui/src/panels/masking.rs @@ -783,7 +783,8 @@ fn describe_field(app: &mut LightcraftApp, ui: &mut egui::Ui, new_mask: bool) { let mut submit = false; // 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() { + let working = app.session.segmenter.analyzing() || app.session.segmenter.busy(); + if crate::button::Button::new("Select").small().busy(working).widget("button:maskDescribeGo").show(ui).clicked() { submit = true; } let r = ui.add( From 3de9654e39564bf997c25bc8c8b4c769d14d00bf Mon Sep 17 00:00:00 2001 From: Nathaniel Ledford Date: Fri, 16 Oct 2026 17:14:29 -0400 Subject: [PATCH 04/11] LR-BEHAV-BG-TASKS: review fixes for progress and busy buttons Model-row bars keep Cancel in its place, a busy button stays focusable and draws its spinner only when visible, the skeleton colour stands apart from selected cells too, the filmstrip shows the failed-thumbnail mark, and more placeholders and waits use the skeleton token and the spinner. Co-Authored-By: Claude --- crates/ui-egui/src/button.rs | 19 ++++++++++++++++--- crates/ui-egui/src/import.rs | 2 +- crates/ui-egui/src/panels/denoise.rs | 6 +++++- crates/ui-egui/src/panels/detail.rs | 10 +++++++--- crates/ui-egui/src/panels/dialogs.rs | 14 ++++++++++++-- crates/ui-egui/src/panels/faces.rs | 9 ++++++++- crates/ui-egui/src/print_view.rs | 3 ++- crates/ui-egui/src/progress.rs | 2 ++ crates/ui-egui/src/tests_progress.rs | 16 +++++++++++++++- crates/ui-egui/src/theme.rs | 8 ++++---- 10 files changed, 72 insertions(+), 17 deletions(-) diff --git a/crates/ui-egui/src/button.rs b/crates/ui-egui/src/button.rs index 3dbd368b1..4e23f1c16 100644 --- a/crates/ui-egui/src/button.rs +++ b/crates/ui-egui/src/button.rs @@ -341,7 +341,18 @@ impl<'a> Button<'a> { }; // a button in a disabled part of the window is disabled too let enabled = self.enabled && ui.is_enabled(); - let resp = ui.interact(rect, id, if enabled && !self.busy { Sense::click() } else { Sense::hover() }); + let resp = ui.interact( + rect, + id, + if !enabled { + Sense::hover() + } else if self.busy { + // still reachable by the keyboard, but nothing to press + Sense::focusable_noninteractive() + } else { + Sense::click() + }, + ); let words = crate::i18n::tr(self.name.or(self.text).unwrap_or_default()).to_string(); resp.widget_info(|| egui::WidgetInfo::labeled(egui::WidgetType::Button, enabled, words.clone())); if self.busy { @@ -392,8 +403,10 @@ impl<'a> Button<'a> { if has_slot { let slot_rect = Rect::from_center_size(pos2(x + slot / 2.0, rect.center().y), vec2(slot, slot)); if self.busy { - crate::progress::paint_spinner(p, slot_rect, lk.content, ui.input(|i| i.time)); - ui.ctx().request_repaint_after(std::time::Duration::from_millis(33)); + if ui.is_rect_visible(rect) { + crate::progress::paint_spinner(p, slot_rect, lk.content, ui.input(|i| i.time)); + ui.ctx().request_repaint_after(std::time::Duration::from_millis(33)); + } } else if let Some(icon) = self.icon { paint(p, slot_rect, icon, lk.content); } diff --git a/crates/ui-egui/src/import.rs b/crates/ui-egui/src/import.rs index 323d8d902..36300bcda 100644 --- a/crates/ui-egui/src/import.rs +++ b/crates/ui-egui/src/import.rs @@ -1336,7 +1336,7 @@ fn candidate_cell(app: &mut LightcraftApp, ui: &mut egui::Ui, d: &mut ImportDial crate::widgets::announce(&resp, c.name.clone()); register(ui.ctx(), format!("import:{i}"), img); let p = ui.painter(); - p.rect_filled(img, 3.0, Color32::from_gray(30)); + p.rect_filled(img, 3.0, t.skeleton); // thumbnail (background job; the grid only asks for the cells in view) let slot = Slot::Import(i as u32); if !app.renderer.textures.contains_key(&slot) diff --git a/crates/ui-egui/src/panels/denoise.rs b/crates/ui-egui/src/panels/denoise.rs index 08ad533f0..38ef6a78c 100644 --- a/crates/ui-egui/src/panels/denoise.rs +++ b/crates/ui-egui/src/panels/denoise.rs @@ -451,7 +451,11 @@ fn model_row(app: &mut LightcraftApp, ui: &mut egui::Ui, t: &Tokens, m: &Value, } else { tr_format!("{} of {} from {host}", mb(Some(got)), mb(Some(total)), host = host) }; - crate::progress::Progress::new(Some(frac)).label(text).widget(format!("denoise:progress:{id}")).show(ui); + crate::progress::Progress::new(Some(frac)) + .label(text) + .width(crate::progress::ROW_WIDTH.min(ui.available_width())) + .widget(format!("denoise:progress:{id}")) + .show(ui); } (Some("done" | "installing"), _) => { ui.horizontal(|ui| { diff --git a/crates/ui-egui/src/panels/detail.rs b/crates/ui-egui/src/panels/detail.rs index 3a15e3f79..9b72e65de 100644 --- a/crates/ui-egui/src/panels/detail.rs +++ b/crates/ui-egui/src/panels/detail.rs @@ -1323,7 +1323,7 @@ fn navigator(app: &mut LightcraftApp, ui: &mut egui::Ui, canvas: Rect, img: Rect p.rect_filled(frame.expand(5.0), 4.0, Color32::from_black_alpha(190)); match tex { Some(t) => p.image(t.tex.id(), frame, Rect::from_min_max(pos2(0.0, 0.0), pos2(1.0, 1.0)), Color32::WHITE), - None => p.rect_filled(frame, 0.0, Color32::from_gray(40)), + None => p.rect_filled(frame, 0.0, Tokens::get(ui.ctx()).skeleton), }; // the visible part of the photo, in normalized image coordinates let vis = canvas.intersect(img); @@ -2859,10 +2859,14 @@ pub(crate) fn filmstrip(app: &mut LightcraftApp, ui: &mut egui::Ui, r: Rect) { } let img_area = Rect::from_min_max(cr.min + vec2(10.0, 22.0), cr.max - vec2(10.0, 8.0)); super::grid::request_thumb(app, *id, (256.0 * ppp.min(2.0) / 2.0) as usize * 2, 8); - if app.renderer.thumb(*id).is_none() { + let thumb = app.renderer.thumb(*id); + if thumb.is_none() { p.rect_filled(img_area, 0.0, t.skeleton); + if app.renderer.failure(Slot::Thumb(*id)).is_some() { + p.text(img_area.center(), Align2::CENTER_CENTER, "!", t.semibold(14.0), t.text_dim); + } } - if let Some(tex) = app.renderer.thumb(*id) { + if let Some(tex) = thumb { let [tw, th] = tex.size; let s = (img_area.width() / tw as f32).min(img_area.height() / th as f32); let fr = Rect::from_center_size(img_area.center(), vec2(tw as f32 * s, th as f32 * s)); diff --git a/crates/ui-egui/src/panels/dialogs.rs b/crates/ui-egui/src/panels/dialogs.rs index 061826ba8..3ad982cf5 100644 --- a/crates/ui-egui/src/panels/dialogs.rs +++ b/crates/ui-egui/src/panels/dialogs.rs @@ -133,7 +133,14 @@ fn watermark_preview( Ok(img) => img, Err(e) => { let note = if e.is_some() { "Preview unavailable" } else { "Loading preview…" }; - let r = ui.label(egui::RichText::new(crate::i18n::tr(note)).color(t.text_dim)); + let r = ui + .horizontal(|ui| { + if e.is_none() { + crate::progress::spinner(ui, 12.0); + } + ui.label(egui::RichText::new(crate::i18n::tr(note)).color(t.text_dim)) + }) + .inner; if let Some(e) = e { r.on_hover_text(e); } @@ -611,7 +618,10 @@ pub fn show(app: &mut LightcraftApp, ctx: &egui::Context) { }); } None => { - ui.label(egui::RichText::new(crate::i18n::tr("Checking names…")).color(t.text_dim)); + ui.horizontal(|ui| { + crate::progress::spinner(ui, 12.0); + ui.label(egui::RichText::new(crate::i18n::tr("Checking names…")).color(t.text_dim)); + }); } } let more = total.saturating_sub(RENAME_PREVIEW_ROWS); diff --git a/crates/ui-egui/src/panels/faces.rs b/crates/ui-egui/src/panels/faces.rs index 4f6febb93..9bc39a4b0 100644 --- a/crates/ui-egui/src/panels/faces.rs +++ b/crates/ui-egui/src/panels/faces.rs @@ -190,6 +190,9 @@ fn gpu_section(app: &mut LightcraftApp, ui: &mut egui::Ui, t: &Tokens, gpu: &Val }; ui.horizontal(|ui| { ui.label(RichText::new(crate::i18n::tr(label)).color(t.text_dim)); + if st["checking"] == true { + crate::progress::spinner(ui, 12.0); + } let r = ui.label(text); register(ui.ctx(), format!("faces:device:{what}"), r.rect); if let Some(why) = why { @@ -364,7 +367,11 @@ fn model_row(app: &mut LightcraftApp, ui: &mut egui::Ui, t: &Tokens, m: &Value, } else { format!("{} of {} from {host}", mb(Some(got)), mb(Some(total))) }; - crate::progress::Progress::new(Some(frac)).label(text).widget(format!("faces:progress:{id}")).show(ui); + crate::progress::Progress::new(Some(frac)) + .label(text) + .width(crate::progress::ROW_WIDTH.min(ui.available_width())) + .widget(format!("faces:progress:{id}")) + .show(ui); } (Some("done"), _) => { ui.horizontal(|ui| { diff --git a/crates/ui-egui/src/print_view.rs b/crates/ui-egui/src/print_view.rs index a18e1f691..037d619c7 100644 --- a/crates/ui-egui/src/print_view.rs +++ b/crates/ui-egui/src/print_view.rs @@ -856,7 +856,8 @@ fn paint_cell( clip.add(egui::Shape::mesh(mesh)); } None => { - clip.rect_filled(shown, 0.0, if photo.is_some() { Color32::from_gray(52) } else { Color32::from_gray(200) }); + let t = Tokens::get(painter.ctx()); + clip.rect_filled(shown, 0.0, if photo.is_some() { t.skeleton } else { t.track }); } } if im.stroke_enabled && im.stroke_width_pt > 0.0 { diff --git a/crates/ui-egui/src/progress.rs b/crates/ui-egui/src/progress.rs index 01ec92a1b..ed2964395 100644 --- a/crates/ui-egui/src/progress.rs +++ b/crates/ui-egui/src/progress.rs @@ -18,6 +18,8 @@ use egui::{Color32, Pos2, Rect, Response, Sense, Shape, Stroke, Ui, pos2, vec2}; use crate::theme::Tokens; +/// A bar in a model row: it leaves the row's Cancel button its place. +pub const ROW_WIDTH: f32 = 260.0; /// The narrowest a bar that fills the available width is drawn. const MIN_WIDTH: f32 = 96.0; /// Height of a bar that carries text. diff --git a/crates/ui-egui/src/tests_progress.rs b/crates/ui-egui/src/tests_progress.rs index bd2c28e28..ebdbda756 100644 --- a/crates/ui-egui/src/tests_progress.rs +++ b/crates/ui-egui/src/tests_progress.rs @@ -133,6 +133,20 @@ fn a_busy_button_ignores_clicks_and_is_announced_busy() { assert!(!nodes.iter().any(|n| n.busy), "{nodes:?}"); } +#[test] +fn a_busy_button_stays_reachable_by_the_keyboard() { + let place = Rect::from_min_size(Pos2::ZERO, vec2(120.0, 30.0)); + let mut asked = false; + let (_, _, focused) = frames(Vec::new(), |ui| { + let r = Button::new("Analyze").busy(true).show_at(ui, place); + if !std::mem::replace(&mut asked, true) { + r.request_focus(); + } + r.has_focus() + }); + assert!(focused, "focus is kept while it works"); +} + #[test] fn a_button_keeps_its_width_while_busy() { for (text, icon) in [("Analyze", None), ("Analyze", Some(Icon::Trash)), ("", Some(Icon::Trash))] { @@ -159,7 +173,7 @@ fn a_button_keeps_its_width_while_busy() { fn the_thumbnail_placeholder_stands_apart_in_every_theme() { for kind in ThemeKind::ALL { let t = Tokens::for_kind(kind); - for (name, behind) in [("grid", t.grid_bg), ("cell", t.cell), ("canvas", t.canvas)] { + for (name, behind) in [("grid", t.grid_bg), ("cell", t.cell), ("selected cell", t.cell_selected), ("canvas", t.canvas)] { let c = contrast(t.skeleton, behind); assert!(c >= 1.1, "{kind:?}: the placeholder on the {name} is {c:.2}:1"); } diff --git a/crates/ui-egui/src/theme.rs b/crates/ui-egui/src/theme.rs index 132797a3c..95e042e39 100644 --- a/crates/ui-egui/src/theme.rs +++ b/crates/ui-egui/src/theme.rs @@ -128,7 +128,7 @@ impl Default for Tokens { grid_bg: Color32::from_rgb(0x0f, 0x0f, 0x0f), cell: Color32::from_rgb(0x1c, 0x1c, 0x1c), cell_selected: Color32::from_rgb(0x2d, 0x2d, 0x2d), - skeleton: Color32::from_rgb(0x26, 0x26, 0x26), + skeleton: Color32::from_rgb(0x25, 0x25, 0x25), divider: Color32::from_rgb(0x1c, 0x1c, 0x1c), inset: Color32::from_rgb(0x23, 0x23, 0x23), field: Color32::from_rgb(0x22, 0x22, 0x22), @@ -176,7 +176,7 @@ impl Tokens { grid_bg: rgb(0x080808), cell: rgb(0x121212), cell_selected: rgb(0x262626), - skeleton: rgb(0x202020), + skeleton: rgb(0x1d1d1d), divider: rgb(0x0e0e0e), inset: rgb(0x181818), field: rgb(0x161616), @@ -194,7 +194,7 @@ impl Tokens { grid_bg: rgb(0xd9d9d9), cell: rgb(0xcbcbcb), cell_selected: rgb(0xb4b4b4), - skeleton: rgb(0xb8b8b8), + skeleton: rgb(0xbfbfbf), divider: rgb(0xc4c4c4), inset: rgb(0xdcdcdc), field: rgb(0xf6f6f6), @@ -225,7 +225,7 @@ impl Tokens { grid_bg: rgb(0xf2f2f2), cell: rgb(0xe8e8e8), cell_selected: rgb(0xd2d2d2), - skeleton: rgb(0xd6d6d6), + skeleton: rgb(0xdddddd), divider: rgb(0xe0e0e0), inset: rgb(0xf0f0f0), field: rgb(0xffffff), From 316f82dbd2b0b7e764ea9e6e842f4d3f9ef3c7ef Mon Sep 17 00:00:00 2001 From: Nathaniel Ledford Date: Tue, 20 Oct 2026 07:27:23 -0400 Subject: [PATCH 05/11] LR-BEHAV-BG-TASKS: name the failed filmstrip thumbnail, firmer light placeholders, paper print cells A failed thumbnail in the detail view's filmstrip is announced with the file name and "This photo can't be opened" (the painted ! had no name). The skeleton placeholder is set further off the surfaces in Silver and Paper (1.3 / 1.2 : 1 on the grid before, about 1.7 / 1.6 after) and the test floor for the light themes is 1.15. The print preview's empty and loading cells go back to fixed greys: its page is paper in every theme. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/panels/detail.rs | 13 ++++++++++--- crates/ui-egui/src/panels/grid.rs | 6 ++++-- crates/ui-egui/src/print_view.rs | 5 +++-- crates/ui-egui/src/tests_progress.rs | 20 +++++++++++++++++++- crates/ui-egui/src/theme.rs | 4 ++-- 5 files changed, 38 insertions(+), 10 deletions(-) diff --git a/crates/ui-egui/src/panels/detail.rs b/crates/ui-egui/src/panels/detail.rs index 9b72e65de..d5b069c41 100644 --- a/crates/ui-egui/src/panels/detail.rs +++ b/crates/ui-egui/src/panels/detail.rs @@ -2836,7 +2836,8 @@ pub(crate) fn filmstrip(app: &mut LightcraftApp, ui: &mut egui::Ui, r: Rect) { continue; } let resp = ui.interact(cr, egui::Id::new(("film", id.0)), Sense::click()); - crate::widgets::announce(&resp, app.session.catalog.photo(*id).map(|p| p.file_name.clone()).unwrap_or_default()); + let failed = app.renderer.thumb(*id).is_none() && app.renderer.failure(Slot::Thumb(*id)).is_some(); + crate::widgets::announce(&resp, film_spoken(app.session.catalog.photo(*id).map_or("", |p| p.file_name.as_str()), failed)); register(ui.ctx(), format!("film:{}", id.0), cr); let state = app.session.selection.state_of(*id); let sel = state == lightcraft_engine::SelectionState::Active; @@ -2862,8 +2863,8 @@ pub(crate) fn filmstrip(app: &mut LightcraftApp, ui: &mut egui::Ui, r: Rect) { let thumb = app.renderer.thumb(*id); if thumb.is_none() { p.rect_filled(img_area, 0.0, t.skeleton); - if app.renderer.failure(Slot::Thumb(*id)).is_some() { - p.text(img_area.center(), Align2::CENTER_CENTER, "!", t.semibold(14.0), t.text_dim); + if failed { + p.text(img_area.center(), Align2::CENTER_CENTER, "!", t.semibold(14.0), t.text); } } if let Some(tex) = thumb { @@ -2934,6 +2935,12 @@ pub(crate) fn filmstrip(app: &mut LightcraftApp, ui: &mut egui::Ui, r: Rect) { }); } +/// What a screen reader says for a filmstrip thumbnail: the file, and why there is no picture +/// when the thumbnail failed (the painted `!` has no name of its own). +pub(crate) fn film_spoken(file_name: &str, failed: bool) -> String { + if failed { format!("{file_name}, {}", crate::i18n::tr("This photo can't be opened")) } else { file_name.to_string() } +} + /// Rating, flag and edited badges along a filmstrip thumbnail's bottom edge (Settings → Interface). fn film_badges(p: &egui::Painter, t: &Tokens, fr: Rect, ph: &lightcraft_catalog::Photo) { use crate::icons::{Icon, paint}; diff --git a/crates/ui-egui/src/panels/grid.rs b/crates/ui-egui/src/panels/grid.rs index 9e934f9e8..1e8cb47fa 100644 --- a/crates/ui-egui/src/panels/grid.rs +++ b/crates/ui-egui/src/panels/grid.rs @@ -805,11 +805,13 @@ fn cell(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId, r: Rect, square if active { p.rect_stroke(ph, 0.0, Stroke::new(2.0, Color32::WHITE), StrokeKind::Outside); } else if selected { - p.rect_stroke(ph, 0.0, Stroke::new(2.0, Color32::from_gray(170)), StrokeKind::Outside); + // a light theme's skeleton is itself mid-grey: the ring takes the dim text colour + let ring = if t.skeleton.r() > 0x80 { t.text_dim } else { Color32::from_gray(170) }; + p.rect_stroke(ph, 0.0, Stroke::new(2.0, ring), StrokeKind::Outside); } if app.renderer.failure(Slot::Thumb(id)).is_some() { // unreadable / missing file - p.text(ph.center(), Align2::CENTER_CENTER, "!", t.semibold(18.0), t.text_dim); + p.text(ph.center(), Align2::CENTER_CENTER, "!", t.semibold(18.0), t.text); } } // labels and badges diff --git a/crates/ui-egui/src/print_view.rs b/crates/ui-egui/src/print_view.rs index 037d619c7..816e4600a 100644 --- a/crates/ui-egui/src/print_view.rs +++ b/crates/ui-egui/src/print_view.rs @@ -856,8 +856,9 @@ fn paint_cell( clip.add(egui::Shape::mesh(mesh)); } None => { - let t = Tokens::get(painter.ctx()); - clip.rect_filled(shown, 0.0, if photo.is_some() { t.skeleton } else { t.track }); + // the page is paper in every theme (its colour is the print's, not the theme's), so + // these stay fixed greys: a photo still to come, and an empty cell + clip.rect_filled(shown, 0.0, if photo.is_some() { Color32::from_gray(52) } else { Color32::from_gray(200) }); } } if im.stroke_enabled && im.stroke_width_pt > 0.0 { diff --git a/crates/ui-egui/src/tests_progress.rs b/crates/ui-egui/src/tests_progress.rs index ebdbda756..22eb3a70a 100644 --- a/crates/ui-egui/src/tests_progress.rs +++ b/crates/ui-egui/src/tests_progress.rs @@ -175,7 +175,25 @@ fn the_thumbnail_placeholder_stands_apart_in_every_theme() { let t = Tokens::for_kind(kind); for (name, behind) in [("grid", t.grid_bg), ("cell", t.cell), ("selected cell", t.cell_selected), ("canvas", t.canvas)] { let c = contrast(t.skeleton, behind); - assert!(c >= 1.1, "{kind:?}: the placeholder on the {name} is {c:.2}:1"); + // the light themes' surfaces are close together, so the placeholder has to be set + // further off them to read at all + let floor = if matches!(kind, ThemeKind::Silver | ThemeKind::Paper) { 1.15 } else { 1.1 }; + assert!(c >= floor, "{kind:?}: the placeholder on the {name} is {c:.2}:1"); } + let mark = contrast(t.text, t.skeleton); + assert!(mark >= 3.0, "{kind:?}: the failure mark on the placeholder is {mark:.2}:1"); } } + +#[test] +fn a_failed_filmstrip_thumbnail_is_announced_with_its_reason() { + let spoken = |failed: bool| { + let (nodes, _, _) = frames(Vec::new(), |ui| { + let resp = ui.interact(Rect::from_min_size(pos2(10.0, 10.0), vec2(120.0, 80.0)), egui::Id::new("film-test"), egui::Sense::click()); + crate::widgets::announce(&resp, crate::panels::detail::film_spoken("IMG_0001.jpg", failed)); + }); + nodes.into_iter().find(|n| n.role == "Button").map(|n| n.label) + }; + assert_eq!(spoken(false).as_deref(), Some("IMG_0001.jpg")); + assert_eq!(spoken(true).as_deref(), Some("IMG_0001.jpg, This photo can't be opened")); +} diff --git a/crates/ui-egui/src/theme.rs b/crates/ui-egui/src/theme.rs index 95e042e39..6c2bac6ac 100644 --- a/crates/ui-egui/src/theme.rs +++ b/crates/ui-egui/src/theme.rs @@ -194,7 +194,7 @@ impl Tokens { grid_bg: rgb(0xd9d9d9), cell: rgb(0xcbcbcb), cell_selected: rgb(0xb4b4b4), - skeleton: rgb(0xbfbfbf), + skeleton: rgb(0xa6a6a6), divider: rgb(0xc4c4c4), inset: rgb(0xdcdcdc), field: rgb(0xf6f6f6), @@ -225,7 +225,7 @@ impl Tokens { grid_bg: rgb(0xf2f2f2), cell: rgb(0xe8e8e8), cell_selected: rgb(0xd2d2d2), - skeleton: rgb(0xdddddd), + skeleton: rgb(0xbcbcbc), divider: rgb(0xe0e0e0), inset: rgb(0xf0f0f0), field: rgb(0xffffff), From ee2559c60f38eaeac808cedb916aebaefc33d8ef Mon Sep 17 00:00:00 2001 From: Nathaniel Ledford Date: Wed, 21 Oct 2026 15:00:31 -0400 Subject: [PATCH 06/11] LR-BEHAV-BG-TASKS: unknown totals sweep, denoise bars keep their widths, Enter waits for the segmenter --- crates/ui-egui/src/panels/denoise.rs | 30 ++++++++++++++++++---------- crates/ui-egui/src/panels/dialogs.rs | 3 +-- crates/ui-egui/src/panels/faces.rs | 3 +-- crates/ui-egui/src/panels/masking.rs | 21 ++++++++++++++++--- crates/ui-egui/src/progress.rs | 6 ++++++ crates/ui-egui/src/tests_progress.rs | 23 ++++++++++++++++++++- 6 files changed, 67 insertions(+), 19 deletions(-) diff --git a/crates/ui-egui/src/panels/denoise.rs b/crates/ui-egui/src/panels/denoise.rs index 38ef6a78c..69e31dead 100644 --- a/crates/ui-egui/src/panels/denoise.rs +++ b/crates/ui-egui/src/panels/denoise.rs @@ -141,6 +141,22 @@ pub fn detail_toggle(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId, on }); } +/// The bar under the Denoise slider while a photo's picture is made: at most 240 px wide, and +/// sweeping while the number of tiles is not known yet. +pub(crate) fn detail_bar(ui: &mut egui::Ui, done: u64, total: u64) -> egui::Response { + let fraction = crate::progress::fraction_of(done, total); + let text = match fraction { + Some(f) => tr_format!("Making the denoised picture… {}%", (f * 100.0).round() as u32), + None => tr("Starting…").to_string(), + }; + crate::progress::Progress::new(fraction).label(text).width(ui.available_width().min(240.0)).widget("denoise:progress").show(ui) +} + +/// The bar in Settings, AI Denoise: 380 px wide however wide the tab is. +pub(crate) fn settings_bar(ui: &mut egui::Ui, done: u64, total: u64, text: String) -> egui::Response { + crate::progress::Progress::new(crate::progress::fraction_of(done, total)).label(text).width(380.0).widget("denoise:settingsProgress").show(ui) +} + /// The line under the Denoise slider for the photo `id`, whose Amount is `amount`. pub fn detail_status(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId, amount: f64) { let t = Tokens::get(ui.ctx()); @@ -198,13 +214,7 @@ pub fn detail_status(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId, am } PhotoState::Running { done, total } => { egui::Frame::NONE.inner_margin(pad).show(ui, |ui| { - let frac = if total > 0 { (done as f32 / total as f32).clamp(0.0, 1.0) } else { 0.0 }; - let text = if total > 0 { - tr_format!("Making the denoised picture… {}%", (frac * 100.0).round() as u32) - } else { - tr("Starting…").to_string() - }; - crate::progress::Progress::new(if total > 0 { Some(frac) } else { None }).label(text).widget("denoise:progress").show(ui); + detail_bar(ui, done as u64, total as u64); }); } PhotoState::Failed { why, unsupported } => { @@ -329,7 +339,6 @@ fn work_section(app: &mut LightcraftApp, ui: &mut egui::Ui, t: &Tokens, list: &V let (running, queued) = (app.caches.denoise.running, app.caches.denoise.queued); match running { Some((done, total)) => { - let frac = if total > 0 { (done as f32 / total as f32).clamp(0.0, 1.0) } else { 0.0 }; let name = status["running"]["photo"] .as_u64() .and_then(|p| app.session.catalog.photo(PhotoId(p)).map(|p| p.file_name.clone())) @@ -341,7 +350,7 @@ fn work_section(app: &mut LightcraftApp, ui: &mut egui::Ui, t: &Tokens, list: &V done = done, total = total ); - crate::progress::Progress::new(Some(frac)).label(text).widget("denoise:settingsProgress").show(ui); + settings_bar(ui, done, total, text); } None if queued > 0 => hint(ui, t, &count(queued, "photo waiting", "photos waiting")), None => { @@ -445,13 +454,12 @@ fn model_row(app: &mut LightcraftApp, ui: &mut egui::Ui, t: &Tokens, m: &Value, match (dl_state, dl) { (Some("running"), Some(d)) => { let (got, total) = (d["bytes"].as_u64().unwrap_or(0), d["total"].as_u64().unwrap_or(0)); - let frac = if total > 0 { (got as f32 / total as f32).clamp(0.0, 1.0) } else { 0.0 }; let text = if total > 0 && got >= total { tr("Checking…").to_string() } else { tr_format!("{} of {} from {host}", mb(Some(got)), mb(Some(total)), host = host) }; - crate::progress::Progress::new(Some(frac)) + crate::progress::Progress::new(crate::progress::fraction_of(got, total)) .label(text) .width(crate::progress::ROW_WIDTH.min(ui.available_width())) .widget(format!("denoise:progress:{id}")) diff --git a/crates/ui-egui/src/panels/dialogs.rs b/crates/ui-egui/src/panels/dialogs.rs index 3ad982cf5..833523ec7 100644 --- a/crates/ui-egui/src/panels/dialogs.rs +++ b/crates/ui-egui/src/panels/dialogs.rs @@ -1674,9 +1674,8 @@ fn sam_model_body(app: &mut LightcraftApp, ui: &mut egui::Ui, error: Option<&str let seg = &app.session.segmenter; if d.running { ui.add_space(4.0); - let frac = if d.total > 0 { d.done as f64 / d.total as f64 } else { 0.0 }; let text = format!("{:.2} / {:.2} GB · {}", gb(d.done), gb(d.total), d.file); - crate::progress::Progress::new(Some(frac as f32)).label(text).widget("progress:samDownload").show(ui); + crate::progress::Progress::new(crate::progress::fraction_of(d.done, d.total)).label(text).widget("progress:samDownload").show(ui); let r = ui.button(crate::i18n::tr("Cancel Download")); crate::widgets::register(ui.ctx(), "button:samCancel", r.rect); if r.clicked() { diff --git a/crates/ui-egui/src/panels/faces.rs b/crates/ui-egui/src/panels/faces.rs index 9bc39a4b0..902c0fd84 100644 --- a/crates/ui-egui/src/panels/faces.rs +++ b/crates/ui-egui/src/panels/faces.rs @@ -361,13 +361,12 @@ fn model_row(app: &mut LightcraftApp, ui: &mut egui::Ui, t: &Tokens, m: &Value, match (dl_state, dl) { (Some("running"), Some(d)) => { let (got, total) = (d["bytes"].as_u64().unwrap_or(0), d["total"].as_u64().unwrap_or(0)); - let frac = if total > 0 { (got as f32 / total as f32).clamp(0.0, 1.0) } else { 0.0 }; let text = if total > 0 && got >= total { "Checking…".to_string() } else { format!("{} of {} from {host}", mb(Some(got)), mb(Some(total))) }; - crate::progress::Progress::new(Some(frac)) + crate::progress::Progress::new(crate::progress::fraction_of(got, total)) .label(text) .width(crate::progress::ROW_WIDTH.min(ui.available_width())) .widget(format!("faces:progress:{id}")) diff --git a/crates/ui-egui/src/panels/masking.rs b/crates/ui-egui/src/panels/masking.rs index 20e15e3a3..cab821913 100644 --- a/crates/ui-egui/src/panels/masking.rs +++ b/crates/ui-egui/src/panels/masking.rs @@ -183,8 +183,9 @@ pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId) { let pct = if download.total > 0 { download.done as f64 / download.total as f64 } else { 0.0 }; // the bar takes what the Details button leaves let details = ui.spacing().interact_size.x + 30.0; - crate::progress::Progress::new(Some(pct as f32)) - .label(format!("SAM 3: {} %", (pct * 100.0).floor())) + crate::progress::Progress::new(crate::progress::fraction_of(download.done, download.total)) + // no percentage to say while the size is unknown + .label(if download.total > 0 { format!("SAM 3: {} %", (pct * 100.0).floor()) } else { String::new() }) .width((ui.available_width() - details).max(96.0)) .widget("maskSamProgress") .show(ui); @@ -767,6 +768,11 @@ pub(crate) fn start_describe(app: &mut LightcraftApp, op: &str) { } /// The Describe field: type what to select ("sky", "the red car") and press Return. +/// Enter in the Describe field is the Select button: it starts nothing while the segmenter works. +fn enter_submits(working: bool, lost_focus: bool, enter_pressed: bool) -> bool { + !working && lost_focus && enter_pressed +} + fn describe_field(app: &mut LightcraftApp, ui: &mut egui::Ui, new_mask: bool) { let Some((op, mut text)) = app.ui.describe.clone() else { return }; // a new mask's field sits under the tiles; one that combines, under the selected mask @@ -796,7 +802,8 @@ fn describe_field(app: &mut LightcraftApp, ui: &mut egui::Ui, new_mask: bool) { if !r.has_focus() && !r.lost_focus() && text.is_empty() { r.request_focus(); } - if r.lost_focus() && ui.input(|i| i.key_pressed(egui::Key::Enter)) { + // Enter is the Select button: nothing starts while the segmenter is working + if enter_submits(working, r.lost_focus(), ui.input(|i| i.key_pressed(egui::Key::Enter))) { submit = true; } }); @@ -904,6 +911,14 @@ fn brush_settings(app: &mut LightcraftApp, ui: &mut egui::Ui) { mod tests { use super::*; + #[test] + fn enter_in_the_describe_field_starts_nothing_while_the_segmenter_works() { + assert!(enter_submits(false, true, true)); + assert!(!enter_submits(true, true, true)); + assert!(!enter_submits(false, false, true)); + assert!(!enter_submits(false, true, false)); + } + #[test] fn masking_context_actions_use_catalog_translations() { let saved = crate::i18n::language(); diff --git a/crates/ui-egui/src/progress.rs b/crates/ui-egui/src/progress.rs index ed2964395..f80f81783 100644 --- a/crates/ui-egui/src/progress.rs +++ b/crates/ui-egui/src/progress.rs @@ -39,6 +39,12 @@ pub fn clamp_fraction(fraction: f32) -> f32 { if fraction.is_nan() { 0.0 } else { fraction.clamp(0.0, 1.0) } } +/// `done` of `total` as a fraction, or `None` while the total is not known (0): an unknown total +/// is a bar that sweeps, not one stuck at 0 %. +pub fn fraction_of(done: u64, total: u64) -> Option { + (total > 0).then(|| clamp_fraction(done as f32 / total as f32)) +} + /// A progress bar, built and then shown with [`Progress::show`]. #[derive(Clone, Debug)] pub struct Progress { diff --git a/crates/ui-egui/src/tests_progress.rs b/crates/ui-egui/src/tests_progress.rs index 22eb3a70a..8ceb61109 100644 --- a/crates/ui-egui/src/tests_progress.rs +++ b/crates/ui-egui/src/tests_progress.rs @@ -7,7 +7,7 @@ use egui::{Pos2, Rect, pos2, vec2}; use crate::button::{Button, contrast}; use crate::icons::Icon; -use crate::progress::{Progress, clamp_fraction, spinner}; +use crate::progress::{Progress, clamp_fraction, fraction_of, spinner}; use crate::theme::{ThemeKind, Tokens}; /// What a screen reader gets for one node. @@ -65,6 +65,15 @@ fn the_fraction_is_always_between_nothing_and_all() { assert_eq!(Progress::new(None).fraction(), None); } +#[test] +fn an_unknown_total_is_an_indeterminate_bar_not_an_empty_one() { + assert_eq!(fraction_of(0, 0), None); + assert_eq!(fraction_of(5, 0), None); + assert_eq!(fraction_of(0, 10), Some(0.0)); + assert_eq!(fraction_of(5, 10), Some(0.5)); + assert_eq!(fraction_of(20, 10), Some(1.0)); +} + #[test] fn a_bar_is_progress_with_a_value_and_reachable_by_its_id() { let (nodes, ids, _) = frames(Vec::new(), |ui| { @@ -197,3 +206,15 @@ fn a_failed_filmstrip_thumbnail_is_announced_with_its_reason() { assert_eq!(spoken(false).as_deref(), Some("IMG_0001.jpg")); assert_eq!(spoken(true).as_deref(), Some("IMG_0001.jpg, This photo can't be opened")); } + +#[test] +fn the_denoise_bars_keep_their_widths_in_a_wide_container() { + let (_, ids, (detail, settings, unknown)) = frames(Vec::new(), |ui| { + let detail = crate::panels::denoise::detail_bar(ui, 3, 10).rect.width(); + let settings = crate::panels::denoise::settings_bar(ui, 3, 10, "Denoising a.jpg: 3 of 10 tiles".into()).rect.width(); + let unknown = crate::panels::denoise::detail_bar(ui, 0, 0).rect.width(); + (detail, settings, unknown) + }); + assert_eq!((detail, settings, unknown), (240.0, 380.0, 240.0)); + assert!(ids.iter().any(|(id, _)| id == "denoise:settingsProgress")); +} From 1e65f52e7939579876ad72abf64439a39434e63f Mon Sep 17 00:00:00 2001 From: Nathaniel Ledford Date: Wed, 21 Oct 2026 17:56:57 -0400 Subject: [PATCH 07/11] LR-BEHAV-BG-TASKS: the SAM download bar keeps its name while the size is unknown --- crates/ui-egui/src/panels/masking.rs | 21 +++++++++++++-------- crates/ui-egui/src/tests_progress.rs | 13 +++++++++++++ 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/crates/ui-egui/src/panels/masking.rs b/crates/ui-egui/src/panels/masking.rs index cab821913..fc2ff8ef3 100644 --- a/crates/ui-egui/src/panels/masking.rs +++ b/crates/ui-egui/src/panels/masking.rs @@ -180,15 +180,9 @@ pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId) { if download.running && app.ui.dialog.is_none() { ui.add_space(4.0); ui.horizontal(|ui| { - let pct = if download.total > 0 { download.done as f64 / download.total as f64 } else { 0.0 }; // the bar takes what the Details button leaves let details = ui.spacing().interact_size.x + 30.0; - crate::progress::Progress::new(crate::progress::fraction_of(download.done, download.total)) - // no percentage to say while the size is unknown - .label(if download.total > 0 { format!("SAM 3: {} %", (pct * 100.0).floor()) } else { String::new() }) - .width((ui.available_width() - details).max(96.0)) - .widget("maskSamProgress") - .show(ui); + sam_download_bar(ui, download.done, download.total, (ui.available_width() - details).max(96.0)); if text_button(ui, "maskSamDetails", crate::i18n::tr("Details"), false).clicked() { app.offer_sam_download(None); } @@ -767,12 +761,23 @@ pub(crate) fn start_describe(app: &mut LightcraftApp, op: &str) { app.ui.describe = Some((op.to_string(), String::new())); } -/// The Describe field: type what to select ("sky", "the red car") and press Return. +/// The SAM model's download, `width` wide. The label is a percentage once the size is known and +/// just the model's name before, so the bar keeps its height and its accessible name. +pub(crate) fn sam_download_bar(ui: &mut egui::Ui, done: u64, total: u64, width: f32) -> egui::Response { + let fraction = crate::progress::fraction_of(done, total); + let label = match fraction { + Some(f) => format!("SAM 3: {} %", (f64::from(f) * 100.0).floor()), + None => "SAM 3".to_string(), + }; + crate::progress::Progress::new(fraction).label(label).width(width).widget("maskSamProgress").show(ui) +} + /// Enter in the Describe field is the Select button: it starts nothing while the segmenter works. fn enter_submits(working: bool, lost_focus: bool, enter_pressed: bool) -> bool { !working && lost_focus && enter_pressed } +/// The Describe field: type what to select ("sky", "the red car") and press Return. fn describe_field(app: &mut LightcraftApp, ui: &mut egui::Ui, new_mask: bool) { let Some((op, mut text)) = app.ui.describe.clone() else { return }; // a new mask's field sits under the tiles; one that combines, under the selected mask diff --git a/crates/ui-egui/src/tests_progress.rs b/crates/ui-egui/src/tests_progress.rs index 8ceb61109..8319278cc 100644 --- a/crates/ui-egui/src/tests_progress.rs +++ b/crates/ui-egui/src/tests_progress.rs @@ -207,6 +207,19 @@ fn a_failed_filmstrip_thumbnail_is_announced_with_its_reason() { assert_eq!(spoken(true).as_deref(), Some("IMG_0001.jpg, This photo can't be opened")); } +#[test] +fn the_sam_bar_keeps_its_name_and_height_while_the_size_is_unknown() { + let (nodes, _, (known, unknown)) = frames(Vec::new(), |ui| { + let known = crate::panels::masking::sam_download_bar(ui, 5, 10, 200.0).rect.height(); + let unknown = crate::panels::masking::sam_download_bar(ui, 0, 0, 200.0).rect.height(); + (known, unknown) + }); + assert_eq!(known, unknown); + let bars: Vec<_> = nodes.iter().filter(|n| n.role == "ProgressIndicator").collect(); + assert_eq!(bars.len(), 2, "{nodes:?}"); + assert!(bars.iter().all(|b| b.label.starts_with("SAM 3")), "{bars:?}"); +} + #[test] fn the_denoise_bars_keep_their_widths_in_a_wide_container() { let (_, ids, (detail, settings, unknown)) = frames(Vec::new(), |ui| { From f36abff3e19360427076f8f74a40681656777e2f Mon Sep 17 00:00:00 2001 From: Nathaniel Ledford Date: Wed, 21 Oct 2026 18:50:44 -0400 Subject: [PATCH 08/11] LR-BEHAV-BG-TASKS: a detail pass counts as the segmenter working --- crates/ui-egui/src/panels/masking.rs | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/crates/ui-egui/src/panels/masking.rs b/crates/ui-egui/src/panels/masking.rs index fc2ff8ef3..517452e06 100644 --- a/crates/ui-egui/src/panels/masking.rs +++ b/crates/ui-egui/src/panels/masking.rs @@ -772,6 +772,11 @@ pub(crate) fn sam_download_bar(ui: &mut egui::Ui, done: u64, total: u64, width: crate::progress::Progress::new(fraction).label(label).width(width).widget("maskSamProgress").show(ui) } +/// Whether the segmenter has work (loading, analyzing, a request, a detail pass): nothing new starts then. +fn segmenter_working(analyzing: bool, busy: bool, detail_busy: bool) -> bool { + analyzing || busy || detail_busy +} + /// Enter in the Describe field is the Select button: it starts nothing while the segmenter works. fn enter_submits(working: bool, lost_focus: bool, enter_pressed: bool) -> bool { !working && lost_focus && enter_pressed @@ -794,7 +799,8 @@ fn describe_field(app: &mut LightcraftApp, ui: &mut egui::Ui, new_mask: bool) { let mut submit = false; // 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| { - let working = app.session.segmenter.analyzing() || app.session.segmenter.busy(); + let seg = &app.session.segmenter; + let working = segmenter_working(seg.analyzing(), seg.busy(), seg.detail_busy()); if crate::button::Button::new("Select").small().busy(working).widget("button:maskDescribeGo").show(ui).clicked() { submit = true; } @@ -916,6 +922,14 @@ fn brush_settings(app: &mut LightcraftApp, ui: &mut egui::Ui) { mod tests { use super::*; + #[test] + fn a_detail_pass_counts_as_the_segmenter_working() { + assert!(segmenter_working(false, false, true)); + assert!(segmenter_working(true, false, false)); + assert!(segmenter_working(false, true, false)); + assert!(!segmenter_working(false, false, false)); + } + #[test] fn enter_in_the_describe_field_starts_nothing_while_the_segmenter_works() { assert!(enter_submits(false, true, true)); From 0457d38a76cf1a29693fe15bd0a118b440229464 Mon Sep 17 00:00:00 2001 From: Nathaniel Ledford Date: Thu, 22 Oct 2026 11:13:29 -0400 Subject: [PATCH 09/11] LR-BEHAV-BG-TASKS: a failed filmstrip thumbnail with no file name is announced without a leading comma --- crates/ui-egui/src/panels/detail.rs | 9 +++++++-- crates/ui-egui/src/tests_progress.rs | 6 ++++++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/crates/ui-egui/src/panels/detail.rs b/crates/ui-egui/src/panels/detail.rs index d5b069c41..5a206844a 100644 --- a/crates/ui-egui/src/panels/detail.rs +++ b/crates/ui-egui/src/panels/detail.rs @@ -2936,9 +2936,14 @@ pub(crate) fn filmstrip(app: &mut LightcraftApp, ui: &mut egui::Ui, r: Rect) { } /// What a screen reader says for a filmstrip thumbnail: the file, and why there is no picture -/// when the thumbnail failed (the painted `!` has no name of its own). +/// when the thumbnail failed (the painted `!` has no name of its own). The name is empty when the +/// catalog no longer has the photo (or its file name is blank); the reason then stands alone. pub(crate) fn film_spoken(file_name: &str, failed: bool) -> String { - if failed { format!("{file_name}, {}", crate::i18n::tr("This photo can't be opened")) } else { file_name.to_string() } + match (file_name.is_empty(), failed) { + (_, false) => file_name.to_string(), + (true, true) => crate::i18n::tr("This photo can't be opened").to_string(), + (false, true) => format!("{file_name}, {}", crate::i18n::tr("This photo can't be opened")), + } } /// Rating, flag and edited badges along a filmstrip thumbnail's bottom edge (Settings → Interface). diff --git a/crates/ui-egui/src/tests_progress.rs b/crates/ui-egui/src/tests_progress.rs index 8319278cc..7d906bc81 100644 --- a/crates/ui-egui/src/tests_progress.rs +++ b/crates/ui-egui/src/tests_progress.rs @@ -207,6 +207,12 @@ fn a_failed_filmstrip_thumbnail_is_announced_with_its_reason() { assert_eq!(spoken(true).as_deref(), Some("IMG_0001.jpg, This photo can't be opened")); } +#[test] +fn a_failed_filmstrip_thumbnail_without_a_file_name_has_no_leading_comma() { + assert_eq!(crate::panels::detail::film_spoken("", true), "This photo can't be opened"); + assert_eq!(crate::panels::detail::film_spoken("", false), ""); +} + #[test] fn the_sam_bar_keeps_its_name_and_height_while_the_size_is_unknown() { let (nodes, _, (known, unknown)) = frames(Vec::new(), |ui| { From 0198375edf0ffacb0938928b912d6c3f9e0d78b4 Mon Sep 17 00:00:00 2001 From: Nathaniel Ledford Date: Thu, 22 Oct 2026 13:23:53 -0400 Subject: [PATCH 10/11] LR-BEHAV-BG-TASKS: activity bars are named after their task, denoise percentage matches its announced value, Describe waits for a pending detail pass --- crates/ui-egui/src/panels/activity.rs | 9 +++++--- crates/ui-egui/src/panels/denoise.rs | 5 +++-- crates/ui-egui/src/panels/masking.rs | 23 +++++++++++++------- crates/ui-egui/src/tests_progress.rs | 31 +++++++++++++++++++++++++++ 4 files changed, 55 insertions(+), 13 deletions(-) diff --git a/crates/ui-egui/src/panels/activity.rs b/crates/ui-egui/src/panels/activity.rs index ed3c57cf8..7fa9570fe 100644 --- a/crates/ui-egui/src/panels/activity.rs +++ b/crates/ui-egui/src/panels/activity.rs @@ -146,8 +146,11 @@ fn row(ui: &mut egui::Ui, t: &Tokens, task: &TaskInfo, width: f32) -> bool { clicked } -/// The progress bar; while the amount of work isn't known, a segment sweeps across. -fn bar(ui: &mut egui::Ui, task: &TaskInfo, width: f32) { +/// The progress bar, named after its task for screen readers; while the amount of work isn't +/// known, a segment sweeps across. +pub(crate) fn bar(ui: &mut egui::Ui, task: &TaskInfo, width: f32) -> egui::Response { let fraction = (task.total > 0).then(|| (task.done as f64 / task.total as f64) as f32); - crate::progress::Progress::new(fraction).width(width).dimmed(task.cancelling).show(ui); + let response = crate::progress::Progress::new(fraction).width(width).dimmed(task.cancelling).show(ui); + crate::widgets::screen_reader_name(ui.ctx(), response.id, tr(&task.label)); + response } diff --git a/crates/ui-egui/src/panels/denoise.rs b/crates/ui-egui/src/panels/denoise.rs index 69e31dead..59a54364f 100644 --- a/crates/ui-egui/src/panels/denoise.rs +++ b/crates/ui-egui/src/panels/denoise.rs @@ -142,11 +142,12 @@ pub fn detail_toggle(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId, on } /// The bar under the Denoise slider while a photo's picture is made: at most 240 px wide, and -/// sweeping while the number of tiles is not known yet. +/// sweeping while the number of tiles is not known yet. The percentage is floored, as the bar's +/// accessible value is (and the SAM bar's text): 100 % only once done. pub(crate) fn detail_bar(ui: &mut egui::Ui, done: u64, total: u64) -> egui::Response { let fraction = crate::progress::fraction_of(done, total); let text = match fraction { - Some(f) => tr_format!("Making the denoised picture… {}%", (f * 100.0).round() as u32), + Some(f) => tr_format!("Making the denoised picture… {}%", (f * 100.0).floor() as u32), None => tr("Starting…").to_string(), }; crate::progress::Progress::new(fraction).label(text).width(ui.available_width().min(240.0)).widget("denoise:progress").show(ui) diff --git a/crates/ui-egui/src/panels/masking.rs b/crates/ui-egui/src/panels/masking.rs index 517452e06..5c1781e82 100644 --- a/crates/ui-egui/src/panels/masking.rs +++ b/crates/ui-egui/src/panels/masking.rs @@ -772,9 +772,10 @@ pub(crate) fn sam_download_bar(ui: &mut egui::Ui, done: u64, total: u64, width: crate::progress::Progress::new(fraction).label(label).width(width).widget("maskSamProgress").show(ui) } -/// Whether the segmenter has work (loading, analyzing, a request, a detail pass): nothing new starts then. -fn segmenter_working(analyzing: bool, busy: bool, detail_busy: bool) -> bool { - analyzing || busy || detail_busy +/// Whether the segmenter has work (loading, analyzing, a request, a detail pass running or waiting +/// to start): nothing new starts then. +fn segmenter_working(analyzing: bool, busy: bool, detail_busy: bool, detail_due: bool) -> bool { + analyzing || busy || detail_busy || detail_due } /// Enter in the Describe field is the Select button: it starts nothing while the segmenter works. @@ -800,7 +801,7 @@ fn describe_field(app: &mut LightcraftApp, ui: &mut egui::Ui, new_mask: bool) { // 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| { let seg = &app.session.segmenter; - let working = segmenter_working(seg.analyzing(), seg.busy(), seg.detail_busy()); + let working = segmenter_working(seg.analyzing(), seg.busy(), seg.detail_busy(), app.ui.detail_due.is_some()); if crate::button::Button::new("Select").small().busy(working).widget("button:maskDescribeGo").show(ui).clicked() { submit = true; } @@ -924,10 +925,16 @@ mod tests { #[test] fn a_detail_pass_counts_as_the_segmenter_working() { - assert!(segmenter_working(false, false, true)); - assert!(segmenter_working(true, false, false)); - assert!(segmenter_working(false, true, false)); - assert!(!segmenter_working(false, false, false)); + assert!(segmenter_working(false, false, true, false)); + assert!(segmenter_working(true, false, false, false)); + assert!(segmenter_working(false, true, false, false)); + assert!(!segmenter_working(false, false, false, false)); + } + + #[test] + fn a_pending_detail_pass_counts_as_the_segmenter_working() { + // a new prompt finishing before it starts would replace its target, and that mask never refines + assert!(segmenter_working(false, false, false, true)); } #[test] diff --git a/crates/ui-egui/src/tests_progress.rs b/crates/ui-egui/src/tests_progress.rs index 7d906bc81..41f779e0e 100644 --- a/crates/ui-egui/src/tests_progress.rs +++ b/crates/ui-egui/src/tests_progress.rs @@ -237,3 +237,34 @@ fn the_denoise_bars_keep_their_widths_in_a_wide_container() { assert_eq!((detail, settings, unknown), (240.0, 380.0, 240.0)); assert!(ids.iter().any(|(id, _)| id == "denoise:settingsProgress")); } + +#[test] +fn the_denoise_bar_label_and_its_accessible_value_agree() { + // 3 of 7 is 42.86 %: the number drawn on the bar is the one announced + let (nodes, _, _) = frames(Vec::new(), |ui| { + crate::panels::denoise::detail_bar(ui, 3, 7); + }); + let bar = nodes.iter().find(|n| n.role == "ProgressIndicator").unwrap_or_else(|| panic!("{nodes:?}")); + let shown: f64 = bar.label.rsplit(' ').next().unwrap().trim_end_matches('%').parse().unwrap(); + assert_eq!(bar.value, Some(shown), "{bar:?}"); +} + +#[test] +fn an_activity_bar_is_named_after_its_task() { + let task = lightcraft_engine::activity::TaskInfo { + id: 1, + kind: "export", + label: "Exporting".into(), + done: 3, + total: 25, + unit: lightcraft_engine::activity::Unit::Count, + detail: String::new(), + cancellable: true, + cancelling: false, + age_ms: 600, + }; + let (nodes, _, _) = frames(Vec::new(), |ui| crate::panels::activity::bar(ui, &task, 200.0).rect.width()); + let bar = nodes.iter().find(|n| n.role == "ProgressIndicator").unwrap_or_else(|| panic!("{nodes:?}")); + assert_eq!(bar.label, "Exporting"); + assert_eq!(bar.value, Some(12.0)); +} From 562655064009cc3c54fc55564c69799de2b9f4cb Mon Sep 17 00:00:00 2001 From: Nathaniel Ledford Date: Thu, 22 Oct 2026 13:46:24 -0400 Subject: [PATCH 11/11] LR-BEHAV-BG-TASKS: one whole-percent rule for the bar text and its announced value --- crates/ui-egui/src/panels/denoise.rs | 6 +++--- crates/ui-egui/src/panels/masking.rs | 2 +- crates/ui-egui/src/progress.rs | 11 ++++++++++- crates/ui-egui/src/tests_progress.rs | 28 +++++++++++++++++++++------- 4 files changed, 35 insertions(+), 12 deletions(-) diff --git a/crates/ui-egui/src/panels/denoise.rs b/crates/ui-egui/src/panels/denoise.rs index 59a54364f..bcd77176a 100644 --- a/crates/ui-egui/src/panels/denoise.rs +++ b/crates/ui-egui/src/panels/denoise.rs @@ -142,12 +142,12 @@ pub fn detail_toggle(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId, on } /// The bar under the Denoise slider while a photo's picture is made: at most 240 px wide, and -/// sweeping while the number of tiles is not known yet. The percentage is floored, as the bar's -/// accessible value is (and the SAM bar's text): 100 % only once done. +/// sweeping while the number of tiles is not known yet. The percentage is `progress::percent`, as the +/// bar's accessible value is: 100 % only once done. pub(crate) fn detail_bar(ui: &mut egui::Ui, done: u64, total: u64) -> egui::Response { let fraction = crate::progress::fraction_of(done, total); let text = match fraction { - Some(f) => tr_format!("Making the denoised picture… {}%", (f * 100.0).floor() as u32), + Some(f) => tr_format!("Making the denoised picture… {}%", crate::progress::percent(f)), None => tr("Starting…").to_string(), }; crate::progress::Progress::new(fraction).label(text).width(ui.available_width().min(240.0)).widget("denoise:progress").show(ui) diff --git a/crates/ui-egui/src/panels/masking.rs b/crates/ui-egui/src/panels/masking.rs index 5c1781e82..38d64b6c3 100644 --- a/crates/ui-egui/src/panels/masking.rs +++ b/crates/ui-egui/src/panels/masking.rs @@ -766,7 +766,7 @@ pub(crate) fn start_describe(app: &mut LightcraftApp, op: &str) { pub(crate) fn sam_download_bar(ui: &mut egui::Ui, done: u64, total: u64, width: f32) -> egui::Response { let fraction = crate::progress::fraction_of(done, total); let label = match fraction { - Some(f) => format!("SAM 3: {} %", (f64::from(f) * 100.0).floor()), + Some(f) => format!("SAM 3: {} %", crate::progress::percent(f)), None => "SAM 3".to_string(), }; crate::progress::Progress::new(fraction).label(label).width(width).widget("maskSamProgress").show(ui) diff --git a/crates/ui-egui/src/progress.rs b/crates/ui-egui/src/progress.rs index f80f81783..0f2dce012 100644 --- a/crates/ui-egui/src/progress.rs +++ b/crates/ui-egui/src/progress.rs @@ -45,6 +45,15 @@ pub fn fraction_of(done: u64, total: u64) -> Option { (total > 0).then(|| clamp_fraction(done as f32 / total as f32)) } +/// The whole percent a bar says for `fraction`: rounded down, so 100 comes only at exactly 1.0, but +/// with a little slack so a fraction that is exact in decimals (53 of 100, held as 0.52999997) +/// still says 53. A bar's text and its accessible value both come from here, so they agree. +pub fn percent(fraction: f32) -> u32 { + let f = clamp_fraction(fraction); + let whole = (f64::from(f) * 100.0 + 1e-3).floor() as u32; + if f < 1.0 { whole.min(99) } else { 100 } +} + /// A progress bar, built and then shown with [`Progress::show`]. #[derive(Clone, Debug)] pub struct Progress { @@ -103,7 +112,7 @@ impl Progress { Some(l) => egui::WidgetInfo::labeled(egui::WidgetType::ProgressIndicator, ui.is_enabled(), l), None => egui::WidgetInfo::new(egui::WidgetType::ProgressIndicator), }; - info.value = self.fraction.map(|f| f64::from(f * 100.0).floor()); + info.value = self.fraction.map(|f| f64::from(percent(f))); info }); if let Some(w) = &self.widget { diff --git a/crates/ui-egui/src/tests_progress.rs b/crates/ui-egui/src/tests_progress.rs index 41f779e0e..476b8a1a6 100644 --- a/crates/ui-egui/src/tests_progress.rs +++ b/crates/ui-egui/src/tests_progress.rs @@ -240,13 +240,27 @@ fn the_denoise_bars_keep_their_widths_in_a_wide_container() { #[test] fn the_denoise_bar_label_and_its_accessible_value_agree() { - // 3 of 7 is 42.86 %: the number drawn on the bar is the one announced - let (nodes, _, _) = frames(Vec::new(), |ui| { - crate::panels::denoise::detail_bar(ui, 3, 7); - }); - let bar = nodes.iter().find(|n| n.role == "ProgressIndicator").unwrap_or_else(|| panic!("{nodes:?}")); - let shown: f64 = bar.label.rsplit(' ').next().unwrap().trim_end_matches('%').parse().unwrap(); - assert_eq!(bar.value, Some(shown), "{bar:?}"); + // the number drawn on the bar is the one announced, and exact decimals stay exact + for (done, total, says) in + [(3, 7, 42.0), (53, 100, 53.0), (59, 100, 59.0), (29, 100, 29.0), (99, 100, 99.0), (100, 100, 100.0), (999, 1000, 99.0)] + { + let (nodes, _, _) = frames(Vec::new(), |ui| { + crate::panels::denoise::detail_bar(ui, done, total); + }); + let bar = nodes.iter().find(|n| n.role == "ProgressIndicator").unwrap_or_else(|| panic!("{nodes:?}")); + let shown: f64 = bar.label.rsplit(' ').next().unwrap().trim_end_matches('%').parse().unwrap(); + assert_eq!((shown, bar.value), (says, Some(says)), "{done}/{total}: {bar:?}"); + } +} + +#[test] +fn the_sam_bar_text_and_its_accessible_value_agree() { + for (done, total, says) in [(53, 100, 53.0), (59, 100, 59.0), (29, 100, 29.0), (999, 1000, 99.0), (100, 100, 100.0)] { + let (nodes, _, _) = frames(Vec::new(), |ui| crate::panels::masking::sam_download_bar(ui, done, total, 200.0).rect.width()); + let bar = nodes.iter().find(|n| n.role == "ProgressIndicator").unwrap_or_else(|| panic!("{nodes:?}")); + assert_eq!(bar.label, format!("SAM 3: {says} %"), "{done}/{total}"); + assert_eq!(bar.value, Some(says), "{done}/{total}"); + } } #[test]