mirror of
https://github.com/storytold/lightcraft.git
synced 2026-10-28 12:08:10 -06:00
docs/whats-new.md: both entries kept, 2026-10-19 above 2026-10-15
This commit is contained in:
aiongg
+59
-11
@@ -186,6 +186,10 @@ pub struct Interaction {
|
||||
pub label: String,
|
||||
pub photo: PhotoId,
|
||||
pub original: Arc<DevelopSettings>,
|
||||
/// Whether an edit during the interaction changed the crop itself. If none did, the crop the
|
||||
/// photo shows was fitted by Constrain Crop, and the final commit starts again from the original
|
||||
/// crop, so Auto Sync carries the edit and not the active photo's fitted crop.
|
||||
pub crop_edited: bool,
|
||||
}
|
||||
|
||||
/// The most photos an edit may touch for the view to follow it photo by photo; a larger edit is worked out whole.
|
||||
@@ -881,22 +885,58 @@ impl Session {
|
||||
/// undo step; otherwise it is committed with a history entry.
|
||||
pub fn set_develop(&mut self, id: PhotoId, settings: DevelopSettings, label: &str) -> Result<()> {
|
||||
let now = (self.clock)();
|
||||
let settings = Arc::new(settings);
|
||||
if let Some(i) = &self.interaction
|
||||
// Auto Sync carries the edit as asked; each photo then fits its own crop (review of #1990)
|
||||
let requested = settings;
|
||||
let settings = Arc::new(self.fit_constrained_crop(id, requested.clone()));
|
||||
let crop_edited = self.develop_of(id).is_some_and(|d| d.crop.geometry != requested.crop.geometry);
|
||||
if let Some(i) = &mut self.interaction
|
||||
&& i.photo == id
|
||||
{
|
||||
i.crop_edited |= crop_edited;
|
||||
return self.apply_silent(Op::SetDevelop { id, settings, label: label.into(), edited: Some(now) });
|
||||
}
|
||||
let mut ops = vec![self.develop_op(id, (*settings).clone(), label).ok_or(lightcraft_catalog::CatalogError::NoPhoto(id))?];
|
||||
ops.extend(self.auto_sync_ops(id, &settings, label));
|
||||
ops.extend(self.auto_sync_ops(id, &requested, label));
|
||||
let op = if ops.len() == 1 { ops.remove(0) } else { Op::Batch { ops } };
|
||||
self.commit(label, op)
|
||||
}
|
||||
|
||||
/// With Auto Sync on, the ops that carry an edit of the active photo `id` (to `new`) over to the
|
||||
/// other selected photos: only the settings that changed; never spot removal or red eye (they
|
||||
/// belong to one photo's pixels), nor the rendering process (Sync doesn't carry it either), nor
|
||||
/// history / snapshot restores.
|
||||
/// "Constrain Crop" keeps the stored crop inside the warped photo, as Lightroom does (issue
|
||||
/// #1927): turning it on, or an edit that changes the warp (Geometry, Upright, lens corrections,
|
||||
/// their sections' eyes) while it is on, shrinks the crop about its centre until no blank corner
|
||||
/// is left, so the Crop tool shows what the render keeps. An edit of the crop itself is left as
|
||||
/// drawn (the render still constrains it).
|
||||
fn fit_constrained_crop(&self, id: PhotoId, mut d: DevelopSettings) -> DevelopSettings {
|
||||
if !d.geometry.constrain_crop {
|
||||
return d;
|
||||
}
|
||||
let Some(photo) = self.catalog.photo(id) else { return d };
|
||||
let before = &photo.develop;
|
||||
let turned_on = !before.geometry.constrain_crop;
|
||||
let warp_changed = before.geometry != d.geometry
|
||||
|| before.optics != d.optics
|
||||
|| before.orientation != d.orientation
|
||||
|| ["geometry", "optics"].iter().any(|s| before.section_enabled(s) != d.section_enabled(s));
|
||||
let crop_edited = before.crop.geometry != d.crop.geometry;
|
||||
if !(turned_on || (warp_changed && !crop_edited)) {
|
||||
return d;
|
||||
}
|
||||
let frame = lightcraft_pipeline::geometry::Frame::with_lens(
|
||||
photo.width.max(1) as usize,
|
||||
photo.height.max(1) as usize,
|
||||
&d,
|
||||
true,
|
||||
photo.embedded_lens.as_ref(),
|
||||
);
|
||||
d.crop.geometry.rect = frame.crop.rect;
|
||||
d
|
||||
}
|
||||
|
||||
/// With Auto Sync on, the ops that carry an edit of the active photo `id` (to `new`, as asked,
|
||||
/// before its crop is fitted) over to the other selected photos: only the settings that changed;
|
||||
/// never spot removal or red eye (they belong to one photo's pixels), nor the rendering process
|
||||
/// (Sync doesn't carry it either), nor history / snapshot restores. Constrain Crop then fits each
|
||||
/// photo's own crop: the active photo's fitted crop is never copied over another's.
|
||||
fn auto_sync_ops(&self, id: PhotoId, new: &DevelopSettings, label: &str) -> Vec<Op> {
|
||||
if !self.auto_sync
|
||||
|| self.active() != Some(id)
|
||||
@@ -922,7 +962,7 @@ impl Session {
|
||||
.filter(|x| **x != id)
|
||||
.filter_map(|x| self.develop_of(*x).map(|d| (*x, d)))
|
||||
.filter_map(|(x, d)| {
|
||||
let synced = lightcraft_develop::apply_partial(&d, &delta, 1.0);
|
||||
let synced = self.fit_constrained_crop(x, lightcraft_develop::apply_partial(&d, &delta, 1.0));
|
||||
(synced != *d).then(|| self.develop_op(x, synced, label)).flatten()
|
||||
})
|
||||
.collect()
|
||||
@@ -944,7 +984,7 @@ impl Session {
|
||||
}
|
||||
let id = self.active().ok_or_else(|| EngineError::Other("no active photo".into()))?;
|
||||
let original = self.develop_of(id).unwrap_or_default();
|
||||
self.interaction = Some(Interaction { label: label.into(), photo: id, original });
|
||||
self.interaction = Some(Interaction { label: label.into(), photo: id, original, crop_edited: false });
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -955,9 +995,15 @@ impl Session {
|
||||
if *cur == *i.original {
|
||||
return Ok(());
|
||||
}
|
||||
// Restore the original silently, then commit the final value as one step.
|
||||
// Restore the original silently, then commit the final value as one step. A crop nobody
|
||||
// edited goes back to the original's: Constrain Crop fits it again for the final warp, and
|
||||
// Auto Sync carries the edit, not the active photo's fitted crop (review of #1990).
|
||||
self.apply_silent(Op::SetDevelop { id: i.photo, settings: i.original.clone(), label: i.label.clone(), edited: None })?;
|
||||
self.set_develop(i.photo, (*cur).clone(), &i.label)
|
||||
let mut last = (*cur).clone();
|
||||
if !i.crop_edited {
|
||||
last.crop.geometry = i.original.crop.geometry;
|
||||
}
|
||||
self.set_develop(i.photo, last, &i.label)
|
||||
}
|
||||
|
||||
pub fn cancel_interaction(&mut self) -> Result<()> {
|
||||
@@ -1249,6 +1295,8 @@ mod tests_albums_local;
|
||||
#[cfg(test)]
|
||||
mod tests_color;
|
||||
#[cfg(test)]
|
||||
mod tests_constrain_crop;
|
||||
#[cfg(test)]
|
||||
mod tests_crop_angle;
|
||||
#[cfg(test)]
|
||||
mod tests_denoise;
|
||||
|
||||
@@ -0,0 +1,147 @@
|
||||
//! Constrain Crop changes the stored crop, as in Lightroom (issue #1927): with it on, the crop the
|
||||
//! Crop tool shows (the stored one) never takes in the blank corners a Geometry warp leaves, so
|
||||
//! ticking it, or moving a Geometry slider while it is ticked, visibly tightens the crop.
|
||||
|
||||
use lightcraft_geom::{CropGeometry, Rect};
|
||||
use lightcraft_pipeline::geometry::Frame;
|
||||
use serde_json::json;
|
||||
|
||||
use crate::Session;
|
||||
|
||||
fn crop(s: &Session) -> CropGeometry {
|
||||
s.develop_of(s.active().unwrap()).unwrap().crop.geometry
|
||||
}
|
||||
|
||||
/// Does the stored crop already fit the warped photo (the render's constraint would leave it alone)?
|
||||
fn fits(s: &Session) -> bool {
|
||||
let p = s.catalog.photo(s.active().unwrap()).unwrap();
|
||||
let mut d = (*p.develop).clone();
|
||||
d.geometry.constrain_crop = true;
|
||||
let f = Frame::with_lens(p.width as usize, p.height as usize, &d, true, p.embedded_lens.as_ref());
|
||||
let (a, b) = (f.crop.rect, p.develop.crop.geometry.rect);
|
||||
[a.x0 - b.x0, a.y0 - b.y0, a.x1 - b.x1, a.y1 - b.y1].iter().all(|d| d.abs() < 1e-9)
|
||||
}
|
||||
|
||||
fn smaller(a: Rect, b: Rect) -> bool {
|
||||
a.width() < b.width() - 1e-3 && a.height() < b.height() - 1e-3
|
||||
}
|
||||
|
||||
fn constrain(s: &mut Session, on: bool) {
|
||||
s.execute("develop.merge", &json!({"settings": {"geometry": {"constrain_crop": on}}, "label": "Constrain Crop"})).unwrap();
|
||||
}
|
||||
|
||||
fn vertical(s: &mut Session, v: f64) {
|
||||
s.execute("develop.set", &json!({"control": "geometry.vertical", "value": v})).unwrap();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_geometry_slider_tightens_the_crop_while_constrained() {
|
||||
let mut s = Session::with_demo();
|
||||
let full = crop(&s).rect;
|
||||
constrain(&mut s, true);
|
||||
assert_eq!(crop(&s).rect, full, "no warp yet: nothing to cut off");
|
||||
vertical(&mut s, -60.0);
|
||||
assert!(smaller(crop(&s).rect, full), "the stored crop shrank: {:?}", crop(&s).rect);
|
||||
assert!(fits(&s), "and holds no blank corner");
|
||||
s.execute("edit.undo", &json!({})).unwrap();
|
||||
assert_eq!(crop(&s).rect, full, "undo puts the crop back with the slider");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn ticking_constrain_after_the_warp_tightens_the_crop() {
|
||||
let mut s = Session::with_demo();
|
||||
let full = crop(&s).rect;
|
||||
vertical(&mut s, -60.0);
|
||||
assert_eq!(crop(&s).rect, full, "unconstrained: the crop keeps the blank corners");
|
||||
assert!(!fits(&s));
|
||||
constrain(&mut s, true);
|
||||
assert!(smaller(crop(&s).rect, full) && fits(&s), "{:?}", crop(&s).rect);
|
||||
// unticking keeps the tighter crop, as in Lightroom
|
||||
let tight = crop(&s).rect;
|
||||
constrain(&mut s, false);
|
||||
assert_eq!(crop(&s).rect, tight);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_crop_drawn_by_hand_is_kept_as_drawn() {
|
||||
let mut s = Session::with_demo();
|
||||
constrain(&mut s, true);
|
||||
vertical(&mut s, -60.0);
|
||||
s.execute("crop.set", &json!({"rect": [0.0, 0.0, 1.0, 1.0]})).unwrap();
|
||||
assert_eq!(crop(&s).rect, Rect::new(0.0, 0.0, 1.0, 1.0), "the crop edit itself is not refitted");
|
||||
}
|
||||
|
||||
/// Review of #1990: with Auto Sync, a Geometry edit on the active photo fits each selected photo's
|
||||
/// own crop; the active photo's fitted crop is never copied over another photo's (smaller) crop.
|
||||
#[test]
|
||||
fn auto_sync_fits_each_photos_own_crop() {
|
||||
let mut s = Session::with_demo();
|
||||
let ids: Vec<_> = s.catalog.photos().take(2).map(|p| p.id).collect();
|
||||
let (a, b) = (ids[0], ids[1]);
|
||||
// B: a small crop off-centre; both constrained
|
||||
s.selection = crate::Selection { ids: vec![b], active: Some(b) };
|
||||
s.execute("crop.set", &json!({"rect": [0.05, 0.1, 0.45, 0.5]})).unwrap();
|
||||
constrain(&mut s, true);
|
||||
s.selection = crate::Selection { ids: vec![a], active: Some(a) };
|
||||
constrain(&mut s, true);
|
||||
let b_before = s.develop_of(b).unwrap().crop.geometry.rect;
|
||||
// A active, both selected, Auto Sync on: a Geometry edit on A
|
||||
s.selection = crate::Selection { ids: vec![a, b], active: Some(a) };
|
||||
s.execute("develop.autoSync", &json!({"on": true})).unwrap();
|
||||
vertical(&mut s, -60.0);
|
||||
let (da, db) = (s.develop_of(a).unwrap(), s.develop_of(b).unwrap());
|
||||
assert_eq!(db.geometry.vertical, -60.0, "the edit reached B");
|
||||
let b_after = db.crop.geometry.rect;
|
||||
assert_ne!(b_after, da.crop.geometry.rect, "B keeps a crop of its own");
|
||||
let inside = |r: Rect, o: Rect| r.x0 >= o.x0 - 1e-9 && r.y0 >= o.y0 - 1e-9 && r.x1 <= o.x1 + 1e-9 && r.y1 <= o.y1 + 1e-9;
|
||||
assert!(inside(b_after, b_before), "B's crop only shrinks within itself: {b_before:?} -> {b_after:?}");
|
||||
s.selection = crate::Selection { ids: vec![b], active: Some(b) };
|
||||
assert!(fits(&s), "and fits B's warp");
|
||||
}
|
||||
|
||||
/// Review of #1990 (CodeRabbit): the same with a slider drag (an interaction), the path the
|
||||
/// window uses: the drag fits A's crop live, and its end must not carry that crop over to B.
|
||||
#[test]
|
||||
fn auto_sync_after_a_slider_drag_fits_each_photos_own_crop() {
|
||||
let mut s = Session::with_demo();
|
||||
let ids: Vec<_> = s.catalog.photos().take(2).map(|p| p.id).collect();
|
||||
let (a, b) = (ids[0], ids[1]);
|
||||
s.selection = crate::Selection { ids: vec![b], active: Some(b) };
|
||||
s.execute("crop.set", &json!({"rect": [0.05, 0.1, 0.45, 0.5]})).unwrap();
|
||||
constrain(&mut s, true);
|
||||
s.selection = crate::Selection { ids: vec![a], active: Some(a) };
|
||||
constrain(&mut s, true);
|
||||
let b_before = s.develop_of(b).unwrap().crop.geometry.rect;
|
||||
s.selection = crate::Selection { ids: vec![a, b], active: Some(a) };
|
||||
s.execute("develop.autoSync", &json!({"on": true})).unwrap();
|
||||
s.execute("develop.beginInteraction", &json!({"label": "Vertical"})).unwrap();
|
||||
for v in [-20.0, -40.0, -60.0] {
|
||||
vertical(&mut s, v);
|
||||
}
|
||||
s.execute("develop.endInteraction", &json!({})).unwrap();
|
||||
let (da, db) = (s.develop_of(a).unwrap(), s.develop_of(b).unwrap());
|
||||
assert_eq!((da.geometry.vertical, db.geometry.vertical), (-60.0, -60.0));
|
||||
let b_after = db.crop.geometry.rect;
|
||||
assert_ne!(b_after, da.crop.geometry.rect, "B keeps a crop of its own");
|
||||
let inside = |r: Rect, o: Rect| r.x0 >= o.x0 - 1e-9 && r.y0 >= o.y0 - 1e-9 && r.x1 <= o.x1 + 1e-9 && r.y1 <= o.y1 + 1e-9;
|
||||
assert!(inside(b_after, b_before), "{b_before:?} -> {b_after:?}");
|
||||
for photo in [a, b] {
|
||||
s.selection = crate::Selection { ids: vec![photo], active: Some(photo) };
|
||||
assert!(fits(&s), "each crop fits its photo's warp");
|
||||
}
|
||||
}
|
||||
|
||||
/// Review of #1990 (CodeRabbit): only the Geometry and Optics eyes change the warp; turning
|
||||
/// another section's eye off leaves a crop drawn by hand as it is.
|
||||
#[test]
|
||||
fn an_unrelated_section_eye_leaves_a_hand_drawn_crop() {
|
||||
let mut s = Session::with_demo();
|
||||
constrain(&mut s, true);
|
||||
vertical(&mut s, -60.0);
|
||||
s.execute("crop.set", &json!({"rect": [0.0, 0.0, 1.0, 1.0]})).unwrap();
|
||||
s.execute("develop.sectionEnabled", &json!({"section": "light", "enabled": false})).unwrap();
|
||||
assert_eq!(crop(&s).rect, Rect::new(0.0, 0.0, 1.0, 1.0), "the Light eye is no warp change");
|
||||
s.execute("develop.sectionEnabled", &json!({"section": "geometry", "enabled": false})).unwrap();
|
||||
s.execute("develop.sectionEnabled", &json!({"section": "geometry", "enabled": true})).unwrap();
|
||||
assert!(fits(&s), "the Geometry eye is one: the crop fits again");
|
||||
}
|
||||
@@ -220,7 +220,7 @@ differences and missing bindings are in [ui-parity.md](ui-parity.md#shortcuts).
|
||||
| LR-EDIT-OPTICS-MANUAL | Manual distortion / vignetting | P1 | ✅ | `ctl:optics.distortion`, `ctl:optics.vignetting`, `ctl:optics.vignettingMidpoint` | |
|
||||
| LR-EDIT-GEOM-UPRIGHT | Upright | P1 | ✅ | `cmd:geometry.upright`, `cmd:geometry.guides` | |
|
||||
| LR-EDIT-GEOM-MANUAL | Manual transform | P1 | ✅ | `ctl:geometry.*` | |
|
||||
| LR-EDIT-GEOM-CONSTRAIN | Constrain crop | P1 | ✅ | `crates/ui-egui/src/panels/right.rs` (checkbox → `cmd:develop.merge`) | |
|
||||
| LR-EDIT-GEOM-CONSTRAIN | Constrain crop | P1 | ✅ | `crates/ui-egui/src/panels/right.rs` (checkbox → `cmd:develop.merge`), `crates/engine/src/lib.rs` (`fit_constrained_crop`) | Ticking it, or changing Geometry, Upright or lens corrections while it is ticked, shrinks the stored crop about its centre until no blank corner is left, so the Crop tool shows it (it used to apply at render time only, outside the Crop tool: issue #1927; tests in `crates/engine/src/tests_constrain_crop.rs`). A crop drawn by hand is kept as drawn; unticking keeps the tighter crop |
|
||||
| LR-EDIT-GEOM-GRID | Grid while transforming | P2 | ✅ | `crates/ui-egui/src/panels/detail.rs` | a fine grid over the photo while a geometry slider is dragged |
|
||||
| LR-EDIT-LENSBLUR | Lens blur | P2 | ⬜ | | settings field reserved, not rendered |
|
||||
| LR-EDIT-CALIB | Calibration [Classic] | P1 | ✅ | `ctl:calibration.*` | shadows tint, red/green/blue primary hue and saturation; read/written in XMP |
|
||||
|
||||
@@ -6,6 +6,15 @@
|
||||
- With several locations removed from Local, "Show N hidden locations" now lists them, so each comes back on its own
|
||||
(Restore All is there too). The row used to bring every hidden location back at once (issue #2114).
|
||||
|
||||
## 2026-10-15
|
||||
|
||||
### Constrain Crop tightens the crop you see
|
||||
- Geometry ▸ Constrain Crop now changes the crop itself, as in Lightroom (issue #1927). Ticking it, or moving a
|
||||
Geometry slider, Upright or a lens correction while it is ticked, shrinks the crop about its centre until the blank
|
||||
corners left by the correction are gone, and the Crop tool shows the tighter crop. Before, it only applied when the
|
||||
photo was shown outside the Crop tool, so in the Crop panel, where the checkbox is, nothing seemed to change. A crop
|
||||
you draw by hand is kept as drawn, and unticking keeps the tighter crop.
|
||||
|
||||
## 2026-10-14
|
||||
|
||||
### Model downloads time out on a name that doesn't resolve
|
||||
|
||||
Reference in New Issue
Block a user