diff --git a/crates/ui-egui/src/headless.rs b/crates/ui-egui/src/headless.rs index 7bb0cba033..0915c4a2c0 100644 --- a/crates/ui-egui/src/headless.rs +++ b/crates/ui-egui/src/headless.rs @@ -1853,6 +1853,67 @@ mod tests { assert!(h.app.hover_preview.is_none(), "the applied profile is shown, not the last hovered one"); } + /// The preset applied to a photo is still marked (and its Amount slider back) after visiting + /// another photo, which shows none (issue #2392); an edit to the photo drops the mark. + #[test] + fn applied_preset_is_marked_again_on_returning_to_the_photo() { + let mut h = demo([1400.0, 900.0]); + let t = Duration::from_secs(10); + h.request("ui.set", json!({"view": "detail"}), t); + h.request("engine.execute", json!({"command": "panel.presets"}), t); + h.settle(SETTLE); + let a = h.app.session.active().unwrap(); + let b = h.app.session.visible_cloned().into_iter().find(|id| *id != a).unwrap(); + let has = |h: &Headless, w: &str| h.app.widgets.iter().any(|(id, _)| id == w); + let marked = |h: &Headless| { + let r = h.app.widgets.iter().find(|(id, _)| id == "presetApplied").map(|(_, r)| *r)?; + h.app.widgets.iter().find(|(id, wr)| id.starts_with("preset:") && *wr == r).map(|(id, _)| id.clone()) + }; + let r = h.request("ui.clickWidget", json!({"id": "preset:lc.warm-glow"}), t); + assert_eq!(r["ok"], true, "{r}"); + h.settle(SETTLE); + assert_eq!(marked(&h).as_deref(), Some("preset:lc.warm-glow")); + assert!(has(&h, "slider:presetAmount")); + for (photo, want) in [(b, None), (a, Some("preset:lc.warm-glow"))] { + h.request("engine.execute", json!({"command": "library.select", "params": {"ids": [photo.0]}}), t); + h.settle(SETTLE); + h.step(); + assert_eq!(marked(&h).as_deref(), want, "on photo {photo:?}"); + assert_eq!(has(&h, "slider:presetAmount"), want.is_some(), "the Amount slider follows the mark"); + } + h.request("engine.execute", json!({"command": "develop.set", "params": {"control": "light.exposure", "value": 0.5}}), t); + h.settle(SETTLE); + h.step(); + assert_eq!(marked(&h), None, "an edit after the preset drops the mark"); + } + + /// A preset applied while the active photo lies outside the selection changes only the + /// selection, so the active photo isn't marked as having it. + #[test] + fn applied_preset_is_not_marked_on_an_active_photo_outside_the_selection() { + let mut h = demo([1400.0, 900.0]); + let t = Duration::from_secs(10); + h.request("ui.set", json!({"view": "detail"}), t); + h.request("engine.execute", json!({"command": "panel.presets"}), t); + h.settle(SETTLE); + let a = h.app.session.active().unwrap(); + let b = h.app.session.visible_cloned().into_iter().find(|id| *id != a).unwrap(); + h.app.session.selection.ids = vec![b]; + h.app.session.selection.active = Some(a); + let before = h.app.session.develop_of(a).unwrap(); + let r = h.request("ui.clickWidget", json!({"id": "preset:lc.warm-glow"}), t); + assert_eq!(r["ok"], true, "{r}"); + h.settle(SETTLE); + h.step(); + assert_eq!(h.app.session.active(), Some(a)); + assert!(*h.app.session.develop_of(a).unwrap() == *before, "the active photo wasn't changed"); + assert!(!h.app.widgets.iter().any(|(id, _)| id == "presetApplied"), "nor marked"); + h.request("engine.execute", json!({"command": "library.select", "params": {"ids": [b.0]}}), t); + h.settle(SETTLE); + h.step(); + assert!(h.app.widgets.iter().any(|(id, _)| id == "presetApplied"), "the selected photo is"); + } + /// Presets column: resting on a preset previews it in the loupe without a history entry; /// thumbnails are variant renders; Create Preset includes only the checked groups. #[test] diff --git a/crates/ui-egui/src/panels/presets.rs b/crates/ui-egui/src/panels/presets.rs index 9f2f788069..a8cf214241 100644 --- a/crates/ui-egui/src/panels/presets.rs +++ b/crates/ui-egui/src/panels/presets.rs @@ -35,15 +35,50 @@ impl AppliedPreset { } fn is_current(&self, session: &lightcraft_engine::Session) -> bool { - self.selected == session.selection.ids - && self.active == session.active() - && self.commits == session.commits() + self.selected == session.selection.ids && self.active == session.active() && !self.is_stale(session) + } + + /// A later edit, undo or redo (on any photo) owns the undo step: the slider may never replace it. + fn is_stale(&self, session: &lightcraft_engine::Session) -> bool { + !(self.commits == session.commits() && session.interaction.is_none() && session.redo.is_empty() && session .undo .last() - .is_some_and(|entry| session.presets.iter().any(|preset| preset.id == self.id && entry.label == format!("Preset: {}", preset.name))) + .is_some_and(|entry| session.presets.iter().any(|preset| preset.id == self.id && entry.label == format!("Preset: {}", preset.name)))) + } +} + +/// The preset each photo was given, with its settings right after: the panel marks it while the +/// photo still has exactly those settings, so it is still marked on coming back to the photo +/// (issue #2392) and drops once the photo is edited further. +#[derive(Clone, Default)] +struct AppliedByPhoto(std::collections::HashMap)>); + +impl AppliedByPhoto { + const ID: &'static str = "preset-applied-by-photo"; + + /// Remember `preset` on the photos `preset.apply` just changed (the selection, else the active + /// photo; an active photo outside the selection wasn't changed and isn't marked). + fn remember(ctx: &egui::Context, session: &lightcraft_engine::Session, preset: &str) { + let id = egui::Id::new(Self::ID); + let mut map: Self = ctx.data(|d| d.get_temp(id)).unwrap_or_default(); + for photo in session.selected() { + if let Some(d) = session.develop_of(photo) { + map.0.insert(photo, (preset.to_string(), d)); + } + } + ctx.data_mut(|d| d.insert_temp(id, map)); + } + + /// The preset to mark for `photo`: the one it was given, while its settings are unchanged since. + fn of(ctx: &egui::Context, session: &lightcraft_engine::Session, photo: Option) -> Option { + let photo = photo?; + let map: Self = ctx.data(|d| d.get_temp(egui::Id::new(Self::ID)))?; + let (preset, then) = map.0.get(&photo)?; + let now = session.develop_of(photo)?; + (Arc::ptr_eq(then, &now) || **then == *now).then(|| preset.clone()) } } @@ -106,6 +141,7 @@ pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui) { { // Only replace the most recent preset; never undo intervening edits. if app.run("preset.apply", json!({"id": &last.id, "amount": v})).is_ok() { + AppliedByPhoto::remember(ui.ctx(), &app.session, &last.id); ui.data_mut(|d| d.insert_temp(amt_id, AppliedPreset::new(&app.session, last.id, v))); } else { let _ = app.run("edit.redo", json!({})); @@ -113,10 +149,13 @@ pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui) { } } divider(ui); - } else { + } else if last.is_some_and(|last| last.is_stale(&app.session)) { + // kept across a switch to another photo and back (the slider returns with it); gone once + // a later edit, undo or redo owns the step ui.data_mut(|d| d.remove::(amt_id)); } let active = app.session.active(); + let marked = AppliedByPhoto::of(ui.ctx(), &app.session, active); let current = active.and_then(|id| app.session.develop_of(id)).map(|d| (*d).clone()); let thumbs = app.ui.preset_thumbs && active.is_some(); let row_h = if thumbs { 54.0 } else { 26.0 }; @@ -167,8 +206,9 @@ pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui) { let (r, resp) = ui.allocate_exact_size(vec2(ui.available_width(), row_h), Sense::click()); crate::widgets::announce(&resp, name.clone()); register(ui.ctx(), format!("preset:{pid}"), r); - let is_last = last.as_ref().is_some_and(|l| l.id == pid); + let is_last = marked.as_deref() == Some(pid.as_str()); if is_last { + register(ui.ctx(), "presetApplied", r); ui.painter().rect_filled(r, 0.0, t.tool_active); } else if resp.hovered() { ui.painter().rect_filled(r, 0.0, t.hover.gamma_multiply(0.7)); @@ -200,6 +240,7 @@ pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui) { } } if resp.clicked() && app.run("preset.apply", json!({"id": pid, "amount": 100})).is_ok() { + AppliedByPhoto::remember(ui.ctx(), &app.session, &pid); ui.data_mut(|d| d.insert_temp(amt_id, AppliedPreset::new(&app.session, pid.clone(), 100.0))); ui.data_mut(|d| d.remove::<(lightcraft_catalog::PhotoId, Preset, String)>(egui::Id::new("last-preset-hover"))); app.toast(ui.ctx(), crate::i18n::tr_format!("Preset: {name}", name = name)); diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index be2b312966..3f7627a111 100644 --- a/docs/parity-checklist.md +++ b/docs/parity-checklist.md @@ -1,6 +1,6 @@ # Lightroom parity checklist -> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-20 · **Change:** minor (LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict) · **Target:** Adobe Lightroom Classic 15.5 / Lightroom 9.6 +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-20 · **Change:** minor (LR-PRE-PANEL: the applied preset stays marked per photo) · **Target:** Adobe Lightroom Classic 15.5 / Lightroom 9.6 The measured, row-by-row checklist: one row per Lightroom feature, menu item and shortcut, what we have and what is missing. It answers *does the feature exist?* (feature breadth). Whether it is good @@ -319,7 +319,7 @@ differences and missing bindings are in [ui-parity.md](ui-parity.md#shortcuts). | Id | Feature | Tier | Status | Evidence | Missing / notes | |---|---|---|---|---|---| -| LR-PRE-PANEL | Presets panel | P0 | ✅ | `cmd:panel.presets`, `cmd:preset.apply`, `crates/ui-egui/src/panels/presets.rs` | grouped list with amount; resting on a preset previews it in the loupe (no history entry); optional live thumbnails (⋯ → Show Thumbnails) | +| LR-PRE-PANEL | Presets panel | P0 | ✅ | `cmd:panel.presets`, `cmd:preset.apply`, `crates/ui-egui/src/panels/presets.rs` | grouped list with amount; the preset applied to a photo stays marked on returning to it until the photo is edited further (issue #2392); resting on a preset previews it in the loupe (no history entry); optional live thumbnails (⋯ → Show Thumbnails) | | LR-PRE-CREATE | Create preset | P0 | ✅ | `cmd:dialog.createPreset`, `cmd:preset.create` (`groups`) | name, group and a checklist of settings groups (crop, masks, remove, red eye off by default; All / None) | | LR-PRE-MANAGE | Manage presets | P1 | ✅ | `cmd:preset.delete`, `cmd:preset.favorite`, `cmd:preset.import`, `cmd:preset.export`, `cmd:preset.rename`, `cmd:preset.update`, `cmd:preset.move` | rename, update with current settings, move to a group (existing or new); no hiding of groups | | LR-PRE-AMOUNT | Preset amount | P1 | ✅ | `cmd:preset.apply` (`amount` 0–200) | | @@ -797,6 +797,7 @@ Latin stays upright. Advanced Japanese composition remains open. Regression cove | Date | Change | Summary | |---|---|---| +| 2026-10-20 | minor | LR-PRE-PANEL: the preset applied to a photo is marked again (with its Amount slider) on returning to the photo, until the photo is edited further (#2392); status unchanged (✅) | | 2026-10-20 | patch | LR-VIEW-ZOOM: the 20 ms rule clears a slow-drag verdict only on a drag timed at full size; a drag that drafts small leaves it unchanged, as the code does; status unchanged | | 2026-10-19 | patch | LR-VIEW-ZOOM: the slow slider-drag verdict is described as per photo and control, as the code keys it; status unchanged | | 2026-10-15 | minor | LR-VIEW-ZOOM: a slow slider-drag verdict needs two drags over 20 ms and expires after 30 s (doubling to 240 s); hidden-window frame times are ignored; the slow set forgets its oldest entry, not all (issue #330); status unchanged |