From b5169e907c26fb09d6a649b9f2c39c068d85f191 Mon Sep 17 00:00:00 2001 From: Jason Baldwin <33934381+bitshiftza@users.noreply.github.com> Date: Thu, 15 Oct 2026 00:53:53 +0200 Subject: [PATCH] LR-META-XMP: radial masks keep their inside/outside from Lightroom (fixes #1815) crs:Flipped="true" is how Lightroom Classic writes an ordinary radial mask (effect inside); it was read as our invert, so every imported radial applied outside the circle. Established from Lightroom Classic 15.6's exports of synthetic DNGs: - MaskGroupBasedCorrections, Version 2: Flipped is the effect-inside flag; inverting is the component's MaskInverted. - MaskGroupBasedCorrections without Version: inside whatever Flipped says (Lightroom rewrites both values as Flipped=true, Version=2), and its Feather is halved, as Lightroom rescales it on that upgrade. - Legacy CircularGradientBasedCorrections Radial Filter: outside by default, Flipped=true (Invert Mask) is inside. One mapping serves XMP sidecars, DNG-embedded XMP, presets and .lrcat catalog import. LightCraft writes no crs: masks, so there is no writer to match. --- crates/engine/src/crs_masks.rs | 162 +++++++++++++++++++++++++++++++-- docs/parity-checklist.md | 2 +- docs/whats-new.md | 8 ++ docs/xmp-interop.md | 2 +- 4 files changed, 165 insertions(+), 9 deletions(-) diff --git a/crates/engine/src/crs_masks.rs b/crates/engine/src/crs_masks.rs index 483d1778b..382dbaa24 100644 --- a/crates/engine/src/crs_masks.rs +++ b/crates/engine/src/crs_masks.rs @@ -98,8 +98,46 @@ fn pt(x: f64, y: f64) -> Value { json!({"x": x, "y": y}) } -/// One mask component → our component JSON; `Err(kind)` names an unsupported one. -fn component(m: &XmpValue, aspect: f64) -> Result { +/// Whether a `Mask/CircularGradient` component puts its effect inside the ellipse, given the +/// container it came from (`crs:Flipped` means different things in different formats). +/// +/// What Lightroom Classic does, from its exports of synthetic DNGs whose masks we wrote (issue #1815): +/// - **`MaskGroupBasedCorrections`, `crs:Version` ≥ 2** (what Lightroom writes today): an ordinary +/// radial — effect inside — is written `Flipped="true"`, so `Flipped` is the "inside" flag. +/// Inverting the mask is a separate field, `crs:MaskInverted` (see [`component`]). `Flipped="false"` +/// is read as outside, the same sense as the legacy filter's; Lightroom never wrote one for us. +/// - **`MaskGroupBasedCorrections` without `crs:Version`**: Lightroom renders the effect inside +/// whatever `Flipped` says, and rewrites both `Flipped="false"` and `"true"` as `Flipped="true"` +/// with `Version="2"`. So `Flipped` is ignored here. +/// - **`CircularGradientBasedCorrections`** (the process-2012 Radial Filter): its default effect was +/// outside, and "Invert Mask" — effect inside — was recorded as `Flipped="true"`. The same "inside" +/// flag as version 2, with outside as the default when it is missing. +fn radial_inside(m: &XmpValue, container: &str) -> bool { + let flipped = flag(m, "crs:Flipped"); + if container == "crs:MaskGroupBasedCorrections" { + if num(m, "crs:Version").is_some_and(|v| v >= 2.0) { flipped.unwrap_or(true) } else { true } + } else { + flipped.unwrap_or(false) + } +} + +/// A `Mask/CircularGradient` component's feather on our 0..100 scale. +/// +/// Lightroom halves `crs:Feather` when it upgrades a `MaskGroupBasedCorrections` radial without +/// `crs:Version` to version 2 (50 → 25, 100 → 50) and renders both the same, so the version-less +/// value is on twice the scale. Version 2's value is used as it is: its 25 and 50 put the middle of +/// Lightroom's fall-off at about 0.87 and 0.76 of the radius, ours at about 0.86 and 0.73. (The +/// shape differs: Lightroom's fall-off straddles the ellipse edge, ours ends at it.) The legacy +/// Radial Filter's value is used as it is: no observation says otherwise. +fn radial_feather(m: &XmpValue, container: &str) -> f64 { + let f = num(m, "crs:Feather").unwrap_or(50.0); + let versionless_group = container == "crs:MaskGroupBasedCorrections" && !num(m, "crs:Version").is_some_and(|v| v >= 2.0); + (if versionless_group { f / 2.0 } else { f }).clamp(0.0, 100.0) +} + +/// One mask component of a correction in `container` → our component JSON; `Err(kind)` names an +/// unsupported one. +fn component(m: &XmpValue, container: &str, aspect: f64) -> Result { let what = text(m, "crs:What").unwrap_or(""); let op = match num(m, "crs:MaskBlendMode").unwrap_or(0.0) as i64 { 1 => "subtract", @@ -123,8 +161,9 @@ fn component(m: &XmpValue, aspect: f64) -> Result { "rx": ((r - l).abs() / 2.0 * sx).max(1e-3), "ry": ((b - t).abs() / 2.0 * sy).max(1e-3), "angle": g("Angle", 0.0), - "feather": g("Feather", 50.0).clamp(0.0, 100.0), - "invert": flag(m, "crs:Flipped").unwrap_or(false), + "feather": radial_feather(m, container), + // our radial's `invert` = effect outside; `crs:MaskInverted` stays the component's + "invert": !radial_inside(m, container), }) } "Mask/Paint" => { @@ -193,7 +232,7 @@ pub fn masks(values: &Values, aspect: f64) -> (Vec, Vec) { let comps = c.field("crs:CorrectionMasks").map(XmpValue::items).unwrap_or_default(); let mut parts = Vec::new(); for m in comps { - match component(m, aspect) { + match component(m, container, aspect) { Ok(v) => parts.push(v), Err(kind) => skipped.push(kind), } @@ -392,7 +431,7 @@ mod tests { - + @@ -433,7 +472,8 @@ mod tests { let MaskShape::Radial { center, rx, ry, angle, feather, invert } = &rad.components[0].shape else { panic!() }; assert!((center.x - 0.5).abs() < 1e-9 && (center.y - 0.5).abs() < 1e-9); assert!((rx - 0.3).abs() < 1e-9 && (ry - 0.2 / 1.5).abs() < 1e-9, "radii in long-edge units: {rx} {ry}"); - assert_eq!((*angle, *feather, *invert), (10.0, 40.0, true)); + assert_eq!((*angle, *feather, *invert), (10.0, 40.0, false), "Flipped=\"true\" is Lightroom's ordinary radial: effect inside"); + assert!(!rad.components[0].invert); let MaskShape::LuminanceRange { lo, hi, lo_feather, .. } = &rad.components[1].shape else { panic!() }; assert!(lo < hi && *lo_feather > 0.0); let brush = &s.masks[2]; @@ -491,6 +531,114 @@ mod tests { assert!((s["ry"].as_f64().unwrap() - 0.3).abs() < 1e-9, "square photo: y radius = its box half-height"); } + /// One radial correction in `container`, with `attrs` on its mask component (values written + /// for this test; the component attributes follow what Lightroom Classic writes). + fn radial_packet(container: &str, attrs: &str) -> String { + format!( + r#" + + + + + + + + +"# + ) + } + + fn radial_settings(container: &str, attrs: &str) -> DevelopSettings { + let d = lightcraft_meta::parse_xmp(&radial_packet(container, attrs)).unwrap(); + let (partial, unmapped) = crate::crs::to_partial_report(&d.properties, Some(&d.values), crate::crs::Target::Any, 1.5); + assert!(unmapped.is_empty(), "{unmapped:?}"); + let s = DevelopSettings::default().merged(&partial).unwrap(); + assert_eq!(s.masks.len(), 1, "{:?}", s.masks); + s + } + + /// (the radial's `invert`, its feather, the component's `invert`). + fn radial_of(s: &DevelopSettings) -> (bool, f64, bool) { + let c = &s.masks[0].components[0]; + let MaskShape::Radial { invert, feather, .. } = &c.shape else { panic!("{:?}", c.shape) }; + (*invert, *feather, c.invert) + } + + /// How much brighter the mask makes the centre and a corner of a flat grey frame (8-bit green). + fn lift_inside_outside(s: &DevelopSettings) -> (i32, i32) { + let img = lightcraft_raster::Image { width: 90, height: 60, data: vec![[0.18f32; 3]; 90 * 60] }; + let render = + |d: &DevelopSettings| lightcraft_pipeline::render(&img, &Default::default(), d, &lightcraft_pipeline::RenderRequest::fit(90, 60)).image; + let (with, without) = (render(s), render(&DevelopSettings { light: s.light, ..Default::default() })); + let g = |im: &lightcraft_raster::Rgba8, x: usize, y: usize| i32::from(im.data[y * im.width + x][1]); + (g(&with, 45, 30) - g(&without, 45, 30), g(&with, 3, 3) - g(&without, 3, 3)) + } + + #[test] + fn lightroom_radial_flipped_is_inside() { + // as Lightroom Classic writes an ordinary radial mask today (#1815) + let s = radial_settings("MaskGroupBasedCorrections", r#"crs:MaskInverted="false" crs:Feather="25" crs:Flipped="true" crs:Version="2""#); + assert_eq!(radial_of(&s), (false, 25.0, false)); + let (inside, outside) = lift_inside_outside(&s); + assert!(inside > 20 && outside == 0, "the effect is inside the circle: {inside} {outside}"); + // no MaskInverted at all reads the same + let s = radial_settings("MaskGroupBasedCorrections", r#"crs:Feather="25" crs:Flipped="true" crs:Version="2""#); + assert_eq!(radial_of(&s), (false, 25.0, false)); + } + + #[test] + fn lightroom_radial_mask_inverted_is_outside() { + let s = radial_settings("MaskGroupBasedCorrections", r#"crs:MaskInverted="true" crs:Feather="25" crs:Flipped="true" crs:Version="2""#); + assert_eq!(radial_of(&s), (false, 25.0, true), "the inversion is the component's"); + let (inside, outside) = lift_inside_outside(&s); + assert!(inside == 0 && outside > 20, "the effect is outside the circle: {inside} {outside}"); + // version 2 with Flipped="false": outside, the legacy sense of the flag + let s = radial_settings("MaskGroupBasedCorrections", r#"crs:MaskInverted="false" crs:Feather="25" crs:Flipped="false" crs:Version="2""#); + assert_eq!(radial_of(&s), (true, 25.0, false)); + } + + #[test] + fn versionless_group_radial_is_inside_with_half_the_feather() { + // Lightroom renders both inside and rewrites them Flipped="true", Feather halved, Version="2" + for flipped in ["true", "false"] { + let s = radial_settings("MaskGroupBasedCorrections", &format!(r#"crs:MaskInverted="false" crs:Feather="50" crs:Flipped="{flipped}""#)); + assert_eq!(radial_of(&s), (false, 25.0, false), "Flipped={flipped}"); + let (inside, outside) = lift_inside_outside(&s); + assert!(inside > 20 && outside == 0, "Flipped={flipped}: {inside} {outside}"); + } + } + + #[test] + fn legacy_radial_filter_is_outside_unless_flipped() { + // the process-2012 Radial Filter: effect outside by default, "Invert Mask" = Flipped="true" + let s = radial_settings("CircularGradientBasedCorrections", r#"crs:Feather="50" crs:Flipped="false""#); + assert_eq!(radial_of(&s), (true, 50.0, false)); + let (inside, outside) = lift_inside_outside(&s); + assert!(inside == 0 && outside > 20, "default: outside: {inside} {outside}"); + assert_eq!(radial_of(&radial_settings("CircularGradientBasedCorrections", r#"crs:Feather="50""#)), (true, 50.0, false)); + let s = radial_settings("CircularGradientBasedCorrections", r#"crs:Feather="50" crs:Flipped="true""#); + assert_eq!(radial_of(&s), (false, 50.0, false)); + let (inside, outside) = lift_inside_outside(&s); + assert!(inside > 20 && outside == 0, "inverted: inside: {inside} {outside}"); + } + + #[test] + fn lrtemplate_radial_filter_reads_flipped() { + // .lrtemplate presets and catalog develop settings are the same Lua tables + let t = |flipped: &str| { + format!( + r#"s = {{ title = "Rad", value = {{ settings = {{ CircularGradientBasedCorrections = {{ + {{ What = "Correction", CorrectionAmount = 1, LocalExposure2012 = 0.25, CorrectionMasks = {{ + {{ What = "Mask/CircularGradient", Top = 0.25, Left = 0.25, Bottom = 0.75, Right = 0.75, Angle = 0, Feather = 50, Flipped = {flipped}, MaskValue = 1 }} }} }} }} }} }} }}"# + ) + }; + for (flipped, outside) in [("true", false), ("false", true)] { + let v = crate::preset_import::read_presets("Rad.lrtemplate", t(flipped).as_bytes(), None).unwrap(); + let s = DevelopSettings::default().merged(&v[0].preset.settings).unwrap(); + assert_eq!(radial_of(&s).0, outside, "Flipped = {flipped}"); + } + } + /// Spot removal in the shape Lightroom Classic writes it (values written for this test): /// a healed circle with an explicit source, a cloned circle with an automatic source, a /// brushed removal, a circular-gradient spot, one without a mask, and the legacy list. diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index 7bb187811..62bcce12a 100644 --- a/docs/parity-checklist.md +++ b/docs/parity-checklist.md @@ -167,7 +167,7 @@ differences and missing bindings are in [ui-parity.md](ui-parity.md#shortcuts). | LR-META-COPYRIGHT-DEFAULT | Default copyright on import | P1 | ✅ | `cmd:library.preferences` (`import.copyright`, `import.creator`), `crates/ui-egui/src/panels/settings.rs` | Settings → Import → Metadata; fills only photos without their own | | LR-META-LOCATION | Location editing | P2 | 🟡 | `cmd:photo.setMeta` (`location`, `city`, `state`, `country`, `gps`), `cmd:metadata.reverseGeocode` | place fields and GPS (decimal or degrees / minutes / seconds) edited in Info, read/written as IPTC Core / Photoshop / EXIF XMP; Show on Map opens OpenStreetMap; Fill Location fills empty City / State / Country from GPS offline (GeoNames, downloaded on request: `cmd:metadata.downloadPlaceNames`); no embedded map | | LR-META-COPYPASTE | Copy / paste metadata | P2 | ✅ | `cmd:photo.copyMetadata`, `cmd:photo.pasteMetadata` | title, caption, alt text, extended description, copyright (notice, status, usage terms, info URL), creator, place fields, keywords; `fields` picks a subset | -| LR-META-XMP | XMP read/write | P0 | ✅ | `cmd:photo.saveMetadataToFile`, `cmd:photo.readMetadataFromFile`, `cmd:library.xmpPreferences`, `crates/engine/src/sidecar.rs`, `docs/xmp-interop.md` | sidecar wins for metadata; a sidecar capture time (`exif:DateTimeOriginal` / `photoshop:DateCreated` / `xmp:CreateDate`) fills in only when the file has none; saving merges into an existing sidecar (`crates/meta/src/xmp_merge.rs`): another app's `crs:`, `xmpMM:History` and unknown namespaces are kept byte for byte, an unreadable sidecar is backed up first; with stem naming, files sharing a stem (raw + JPEG) get separate sidecars (`.xmp` for all but the raw); a `crs:` custom white balance on a relative-WB raw is shifted from `crs:AsShotTemperature` rather than read as Kelvin, or kept As Shot and reported when the packet has no as-shot white (issue #510); heal and clone spots (`crs:RetouchAreas`, legacy `crs:RetouchInfo`) are read into our spots (`crates/engine/src/crs_masks.rs`, see `docs/xmp-interop.md` → Spot removal); Read Metadata from File never writes the sidecar back, even with auto-write on, so another app's later `crs:` edits are still read (issue #934) | +| LR-META-XMP | XMP read/write | P0 | ✅ | `cmd:photo.saveMetadataToFile`, `cmd:photo.readMetadataFromFile`, `cmd:library.xmpPreferences`, `crates/engine/src/sidecar.rs`, `docs/xmp-interop.md` | sidecar wins for metadata; a sidecar capture time (`exif:DateTimeOriginal` / `photoshop:DateCreated` / `xmp:CreateDate`) fills in only when the file has none; saving merges into an existing sidecar (`crates/meta/src/xmp_merge.rs`): another app's `crs:`, `xmpMM:History` and unknown namespaces are kept byte for byte, an unreadable sidecar is backed up first; with stem naming, files sharing a stem (raw + JPEG) get separate sidecars (`.xmp` for all but the raw); a `crs:` custom white balance on a relative-WB raw is shifted from `crs:AsShotTemperature` rather than read as Kelvin, or kept As Shot and reported when the packet has no as-shot white (issue #510); heal and clone spots (`crs:RetouchAreas`, legacy `crs:RetouchInfo`) are read into our spots (`crates/engine/src/crs_masks.rs`, see `docs/xmp-interop.md` → Spot removal); radial masks (`Mask/CircularGradient`) keep Lightroom's inside / outside: `crs:Flipped="true"` is the effect-inside flag of current masks and of the legacy Radial Filter, `crs:MaskInverted` the inversion, checked against Lightroom Classic's renders (issue #1815); Read Metadata from File never writes the sidecar back, even with auto-write on, so another app's later `crs:` edits are still read (issue #934) | | LR-META-EXIF-FULL | Full EXIF/IPTC [Classic] | P1 | ✅ | `cmd:photo.allMetadata`, `cmd:dialog.allMetadata`, `crates/meta/src/tags.rs`, `crates/meta/src/exif.rs`, `crates/meta/src/iptc.rs` | read and written on export; Info ▸ All Metadata…: every TIFF / EXIF / GPS / interop tag (named, common values spelled out) and the XMP fields, searchable; metadata presets | ## F. Edit panel — global adjustments (EDIT) diff --git a/docs/whats-new.md b/docs/whats-new.md index 1bf44c3f4..e15d65917 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -1,5 +1,13 @@ # What's new in LightCraft +## 2026-10-15 + +### Lightroom radial masks on the right side +- Radial masks brought over from Lightroom Classic (catalog import, XMP sidecars, edits inside DNGs and presets) now + apply their effect inside the circle, as Lightroom does, instead of outside it (issue #1815). A mask inverted in + Lightroom stays inverted, and older Radial Filters keep their Effect Outside / Invert Mask setting. Photos imported + before this keep the masks they got: Photo ▸ Read Metadata from File reads a sidecar's again. + ## 2026-10-14 ### Hard-edged Heal and Clone spots diff --git a/docs/xmp-interop.md b/docs/xmp-interop.md index ce235e2d3..662b2e752 100644 --- a/docs/xmp-interop.md +++ b/docs/xmp-interop.md @@ -182,7 +182,7 @@ mask: `CorrectionName` → name, `CorrectionAmount` → Amount, inactive correct | `LocalContrast2012`, `LocalHighlights2012`, `LocalShadows2012`, `LocalWhites2012`, `LocalBlacks2012`, `LocalClarity2012`, `LocalTexture`, `LocalDehaze`, `LocalTemperature`, `LocalTint`, `LocalSaturation`, `LocalHue`, `LocalSharpness`, `LocalLuminanceNoise`, `LocalMoire`, `LocalDefringe`, `LocalToningSaturation` | the matching `adjust.*` | stored −1..1, ×100 | | `LocalToningHue` | `adjust.color_hue` | degrees | | `Mask/Gradient` (`FullX/Y`, `ZeroX/Y`) | linear gradient | full effect at the Full point | -| `Mask/CircularGradient` (`Top/Left/Bottom/Right`, `Angle`, `Feather`, `Flipped`) | radial gradient | box fractions → long-edge radii (presets assume 3:2; sidecars use the photo's shape); `Flipped` → invert | +| `Mask/CircularGradient` (`Top/Left/Bottom/Right`, `Angle`, `Feather`, `Flipped`) | radial gradient | box fractions → long-edge radii (presets assume 3:2; sidecars use the photo's shape). `Flipped="true"` means effect inside: in `MaskGroupBasedCorrections` with `Version` 2 (what Lightroom writes for an ordinary radial) and in the legacy `CircularGradientBasedCorrections` Radial Filter, whose default (no `Flipped`) is outside. A `MaskGroupBasedCorrections` radial without `Version` is inside whatever `Flipped` says, and its `Feather` is halved (Lightroom rescales it that way when it upgrades one). Inverting is `MaskInverted` | | `Mask/Paint` (`Dabs` "d x y", `Radius`, `Flow`, `CenterWeight`, `MaskValue`) | brush | one brush component per correction; `MaskValue` ≤ 0 erases | | `Mask/Image` `MaskSubType` 1 / 2 | Subject / Sky | other AI selections are reported, not guessed | | `Mask/RangeMask` `Type` 2 / 3 (`LumRange`, `DepthRange`) | luminance / depth range | lightness converted to our range scale; colour ranges are reported |