Merge PR #2452: LR-PRE-PANEL: the applied preset stays marked on returning to the pho…

This commit is contained in:
aiongg
2026-10-22 02:36:59 +00:00
3 changed files with 111 additions and 8 deletions
+61
View File
@@ -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]
+47 -6
View File
@@ -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<lightcraft_catalog::PhotoId, (String, Arc<lightcraft_develop::DevelopSettings>)>);
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<lightcraft_catalog::PhotoId>) -> Option<String> {
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::<AppliedPreset>(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));
+3 -2
View File
@@ -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 |