Merge PR #1817

crates/engine/src/crs_masks.rs

@@ -110,8 +110,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<Value, String> {
+/// 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<Value, String> {
     let what = text(m, "crs:What").unwrap_or("");
     let op = match num(m, "crs:MaskBlendMode").unwrap_or(0.0) as i64 {
         1 => "subtract",
@@ -135,8 +173,9 @@ fn component(m: &XmpValue, aspect: f64) -> Result<Value, String> {
                 "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" => {
@@ -205,7 +244,7 @@ pub fn masks(values: &Values, aspect: f64) -> (Vec<Value>, Vec<String>) {
             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),
                 }
@@ -404,7 +443,7 @@ mod tests {
   </rdf:Description></rdf:li>
   <rdf:li><rdf:Description crs:What="Correction" crs:CorrectionAmount="0.5" crs:LocalContrast2012="0.2">
    <crs:CorrectionMasks><rdf:Seq>
-    <rdf:li crs:What="Mask/CircularGradient" crs:Top="0.3" crs:Left="0.2" crs:Bottom="0.7" crs:Right="0.8" crs:Angle="10" crs:Feather="40" crs:Flipped="true"/>
+    <rdf:li crs:What="Mask/CircularGradient" crs:Top="0.3" crs:Left="0.2" crs:Bottom="0.7" crs:Right="0.8" crs:Angle="10" crs:Feather="40" crs:Flipped="true" crs:Version="2"/>
     <rdf:li crs:What="Mask/RangeMask" crs:MaskBlendMode="2"><crs:CorrectionRangeMask crs:Type="2" crs:LumRange="0.4 0.5 0.8 0.9"/></rdf:li>
     <rdf:li crs:What="Mask/Image" crs:MaskSubType="9"/>
    </rdf:Seq></crs:CorrectionMasks>
@@ -445,7 +484,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];
@@ -503,6 +543,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#"<x:xmpmeta xmlns:x="adobe:ns:meta/"><rdf:RDF xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#">
+<rdf:Description rdf:about="" xmlns:crs="http://ns.adobe.com/camera-raw-settings/1.0/" crs:Exposure2012="0">
+ <crs:{container}><rdf:Seq>
+  <rdf:li><rdf:Description crs:What="Correction" crs:CorrectionAmount="1" crs:LocalExposure2012="0.25">
+   <crs:CorrectionMasks><rdf:Seq>
+    <rdf:li crs:What="Mask/CircularGradient" crs:MaskValue="1" crs:Top="0.25" crs:Left="0.25" crs:Bottom="0.75" crs:Right="0.75" crs:Angle="0" crs:Midpoint="50" crs:Roundness="0" {attrs}/>
+   </rdf:Seq></crs:CorrectionMasks>
+  </rdf:Description></rdf:li>
+ </rdf:Seq></crs:{container}>
+</rdf:Description></rdf:RDF></x:xmpmeta>"#
+        )
+    }
+
+    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.

docs/parity-checklist.md

@@ -171,7 +171,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 (`<file>.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 (`<file>.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)

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
 
 ### HDR merges of moving lights, and many brackets at once

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 |