mirror of
https://github.com/storytold/lightcraft.git
synced 2026-11-01 05:57:09 -06:00
Merge PR #3379: LR-MASK-COLORRANGE: a colour-range mask keeps at most 64 samples however it is loaded (#2556)
# Conflicts: # docs/parity-checklist.md # docs/whats-new.md
This commit is contained in:
@@ -271,6 +271,54 @@ mod tests {
|
||||
assert_eq!(s.grading.blending, 50.0);
|
||||
}
|
||||
|
||||
/// Settings with one colour-range mask of `n` distinct samples (built in code, so not capped).
|
||||
fn color_range_settings(n: usize) -> DevelopSettings {
|
||||
let samples = (0..n).map(|i| [i as f64 / 1000.0, 0.1, -0.1]).collect();
|
||||
let mut s = DevelopSettings::default();
|
||||
s.masks.push(Mask {
|
||||
id: 1,
|
||||
components: vec![MaskComponent { name: None, op: MaskOp::Add, invert: false, shape: MaskShape::ColorRange { samples, refine: 50.0 } }],
|
||||
..Default::default()
|
||||
});
|
||||
s
|
||||
}
|
||||
|
||||
fn color_range_samples(s: &DevelopSettings) -> &[[f64; 3]] {
|
||||
match s.masks.first().and_then(|m| m.components.first()).map(|c| &c.shape) {
|
||||
Some(MaskShape::ColorRange { samples, .. }) => samples,
|
||||
other => panic!("expected a colour range, got {other:?}"),
|
||||
}
|
||||
}
|
||||
|
||||
/// Issue #2556: `mask.add`/`mask.update` capped a colour range's samples, but a sidecar `lc:settings`, a
|
||||
/// preset, `develop.merge`, `render --settings` or a catalog / history load took any number (render cost is
|
||||
/// pixels x samples). Every load now keeps the first [`MAX_COLOR_RANGE_SAMPLES`], in order.
|
||||
#[test]
|
||||
fn color_range_samples_are_capped_however_settings_load() {
|
||||
let big = color_range_settings(1000);
|
||||
let want = &color_range_samples(&big)[..MAX_COLOR_RANGE_SAMPLES];
|
||||
let v = big.to_json();
|
||||
let loaded = DevelopSettings::from_json(&v).unwrap();
|
||||
assert_eq!(color_range_samples(&loaded).len(), MAX_COLOR_RANGE_SAMPLES);
|
||||
assert_eq!(color_range_samples(&loaded), want);
|
||||
let from_str: DevelopSettings = serde_json::from_str(&v.to_string()).unwrap();
|
||||
assert_eq!(color_range_samples(&from_str), want);
|
||||
// `develop.merge`, `render --settings` and presets merge a partial
|
||||
let merged = DevelopSettings::default().merged(&json!({"masks": v["masks"]})).unwrap();
|
||||
assert_eq!(color_range_samples(&merged), want);
|
||||
let preset =
|
||||
Preset { id: "p".into(), name: "P".into(), group: "G".into(), settings: json!({"masks": v["masks"]}), favorite: false, builtin: false };
|
||||
assert_eq!(color_range_samples(&preset.apply(&DevelopSettings::default(), 1.0)), want);
|
||||
// a capped list round-trips, and a short one is unchanged
|
||||
assert_eq!(DevelopSettings::from_json(&loaded.to_json()).unwrap(), loaded);
|
||||
let few = color_range_settings(2);
|
||||
assert_eq!(DevelopSettings::from_json(&few.to_json()).unwrap(), few);
|
||||
// a list that isn't samples is still refused
|
||||
let mut bad = v.clone();
|
||||
bad["masks"][0]["components"][0]["shape"]["samples"] = json!([[0.1, 0.2]]);
|
||||
assert!(DevelopSettings::from_json(&bad).is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn merge_partial() {
|
||||
let s = DevelopSettings::default();
|
||||
|
||||
@@ -921,6 +921,35 @@ impl Default for BrushStroke {
|
||||
}
|
||||
}
|
||||
|
||||
/// Most samples a colour-range mask keeps. It compares every pixel against every sample, so render and export cost
|
||||
/// is pixels x samples; the interactive sampler stops at 5, and a sidecar, preset or `mask.add` with thousands could
|
||||
/// stall a render or lose the GPU device (issue #2556).
|
||||
pub const MAX_COLOR_RANGE_SAMPLES: usize = 64;
|
||||
|
||||
/// Deserialize a colour range's samples, keeping the first [`MAX_COLOR_RANGE_SAMPLES`] (the rest are skipped without
|
||||
/// being stored), however the settings arrive: sidecar, catalog, history, preset or merge.
|
||||
fn de_color_range_samples<'de, D: serde::Deserializer<'de>>(d: D) -> Result<Vec<[f64; 3]>, D::Error> {
|
||||
struct Capped;
|
||||
impl<'de> serde::de::Visitor<'de> for Capped {
|
||||
type Value = Vec<[f64; 3]>;
|
||||
fn expecting(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
|
||||
f.write_str("a list of OkLab colours")
|
||||
}
|
||||
fn visit_seq<A: serde::de::SeqAccess<'de>>(self, mut seq: A) -> Result<Vec<[f64; 3]>, A::Error> {
|
||||
let mut out = Vec::new();
|
||||
while out.len() < MAX_COLOR_RANGE_SAMPLES {
|
||||
match seq.next_element::<[f64; 3]>()? {
|
||||
Some(s) => out.push(s),
|
||||
None => return Ok(out),
|
||||
}
|
||||
}
|
||||
while seq.next_element::<serde::de::IgnoredAny>()?.is_some() {}
|
||||
Ok(out)
|
||||
}
|
||||
}
|
||||
d.deserialize_seq(Capped)
|
||||
}
|
||||
|
||||
#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
|
||||
#[serde(tag = "kind", rename_all = "camelCase")]
|
||||
pub enum MaskShape {
|
||||
@@ -941,8 +970,9 @@ pub enum MaskShape {
|
||||
feather: f64,
|
||||
invert: bool,
|
||||
},
|
||||
/// Colour range: sampled OkLab colours with a refine amount 0..100.
|
||||
/// Colour range: sampled OkLab colours (at most [`MAX_COLOR_RANGE_SAMPLES`]) with a refine amount 0..100.
|
||||
ColorRange {
|
||||
#[serde(deserialize_with = "de_color_range_samples")]
|
||||
samples: Vec<[f64; 3]>,
|
||||
refine: f64,
|
||||
},
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
//! Masking and Remove (spot) commands on the active photo.
|
||||
|
||||
use lightcraft_develop::{BrushStroke, LocalAdjustments, Mask, MaskComponent, MaskOp, MaskShape, RedEye, Spot, SpotMode};
|
||||
use lightcraft_develop::{BrushStroke, LocalAdjustments, MAX_COLOR_RANGE_SAMPLES, Mask, MaskComponent, MaskOp, MaskShape, RedEye, Spot, SpotMode};
|
||||
use lightcraft_geom::Point;
|
||||
use serde_json::{Value, json};
|
||||
|
||||
@@ -8,11 +8,6 @@ use super::{CommandSpec, bad, bool_or, cmd, f64_or, has_active, ok, point, str_p
|
||||
use crate::segment::{Click, MAX_CLICKS};
|
||||
use crate::{Result, Session};
|
||||
|
||||
/// A colour-range mask compares every pixel against every sample, so render and export cost is
|
||||
/// pixels x samples. The interactive sampler stops at 5; automation (`mask.add`/`mask.update`)
|
||||
/// passed its `samples` array straight through, so a few hundred samples on a large render hung.
|
||||
const MAX_COLOR_RANGE_SAMPLES: usize = 64;
|
||||
|
||||
/// Caps a colour-range shape's sample list at [`MAX_COLOR_RANGE_SAMPLES`] (render cost is pixels x samples).
|
||||
fn cap_color_range_samples(shape: &mut MaskShape) {
|
||||
if let MaskShape::ColorRange { samples, .. } = shape {
|
||||
|
||||
@@ -980,6 +980,23 @@ mod tests {
|
||||
assert_eq!(sc.keywords, Some(vec!["Places|Lisbon".to_string()]));
|
||||
}
|
||||
|
||||
/// Issue #2556: a sidecar's `lc:settings` colour-range mask keeps at most `MAX_COLOR_RANGE_SAMPLES` samples.
|
||||
#[test]
|
||||
fn sidecar_color_range_samples_are_capped() {
|
||||
use lightcraft_develop::{MAX_COLOR_RANGE_SAMPLES, Mask, MaskComponent, MaskOp, MaskShape};
|
||||
let samples: Vec<[f64; 3]> = (0..1000).map(|i| [i as f64 / 1000.0, 0.1, -0.1]).collect();
|
||||
let mut d = DevelopSettings::default();
|
||||
let shape = MaskShape::ColorRange { samples: samples.clone(), refine: 50.0 };
|
||||
d.masks.push(Mask { id: 1, components: vec![MaskComponent { name: None, op: MaskOp::Add, invert: false, shape }], ..Default::default() });
|
||||
let xmp = lightcraft_meta::write_xmp(&lightcraft_meta::Metadata::default(), Some(&d.to_json().to_string()));
|
||||
let sc = super::parse_sidecar(&xmp, crate::crs::Target::Rendered).unwrap();
|
||||
let Some(DevelopPatch::Full(s)) = sc.develop else { panic!("expected full settings") };
|
||||
let Some(MaskShape::ColorRange { samples: got, .. }) = s.masks.first().and_then(|m| m.components.first()).map(|c| &c.shape) else {
|
||||
panic!("expected a colour range")
|
||||
};
|
||||
assert_eq!(got.as_slice(), &samples[..MAX_COLOR_RANGE_SAMPLES]);
|
||||
}
|
||||
|
||||
use super::*;
|
||||
use lightcraft_catalog::Meta;
|
||||
|
||||
|
||||
@@ -1017,8 +1017,10 @@ fn masks(cx: &mut Cx<'_>, lin: &Buf, prep: &Prep, plan: &Plan<'_>, host: &mut Ho
|
||||
}
|
||||
MaskShape::ColorRange { samples, refine } => {
|
||||
let tol = lightcraft_pipeline::masks::color_range_tolerance(*refine);
|
||||
// at most the CPU's samples (`MAX_COLOR_RANGE_SAMPLES`): cost is pixels x samples
|
||||
let samples = samples.iter().take(lightcraft_develop::MAX_COLOR_RANGE_SAMPLES);
|
||||
p.extend([tol.to_bits(), ev.exp2().to_bits(), samples.len() as u32]);
|
||||
aux.extend(samples.iter().flat_map(|s| s.map(|v| v as f32)));
|
||||
aux.extend(samples.flat_map(|s| s.map(|v| v as f32)));
|
||||
Some(3)
|
||||
}
|
||||
MaskShape::Brush { strokes } => {
|
||||
|
||||
@@ -251,7 +251,9 @@ pub fn shape_alpha(shape: &MaskShape, frame: &Frame, w: usize, h: usize, img: &R
|
||||
}
|
||||
MaskShape::ColorRange { samples, refine } => {
|
||||
let tol = color_range_tolerance(*refine);
|
||||
let samples: Vec<[f32; 3]> = samples.iter().map(|s| [s[0] as f32, s[1] as f32, s[2] as f32]).collect();
|
||||
// (a settings value built in code can skip the deserializer's cap: cost is pixels x samples)
|
||||
let samples: Vec<[f32; 3]> =
|
||||
samples.iter().take(lightcraft_develop::MAX_COLOR_RANGE_SAMPLES).map(|s| [s[0] as f32, s[1] as f32, s[2] as f32]).collect();
|
||||
for (v, c) in out.data.iter_mut().zip(&img.data) {
|
||||
let lab = lightcraft_color::perceptual::oklab_from_2020(tonemap_for_select(c.map(|v| v * gain)));
|
||||
let d = samples
|
||||
|
||||
@@ -299,7 +299,7 @@ differences and missing bindings are in [ui-parity.md](ui-parity.md#shortcuts).
|
||||
| LR-MASK-BRUSH | Brush mask | P0 | 🟡 | `cmd:tool.brush`, `cmd:mask.brushStroke` (`autoMask`), `crates/pipeline/src/masks.rs`, `crates/ui-egui/src/brush_live.rs` | size/feather/flow/density/erase; Create New Mask starts a separate brush in paint mode; the stroke shows while the button is held with the committed stroke's dab spacing, feather and flow, so fast strokes have no gaps (#517); with the mask overlay hidden (O / the eye, or after a mask slider hid it) or a colour overlay at opacity 0 it is not tinted while painting either, so dodging and burning is judged on the photo, and its effect shows once the stroke is committed (#2549); coverage builds up by passes, not hold time (holding the brush still adds nothing, live or committed, #518); oversized path segments skip intermediate dabs instead of allocating a million samples; Auto Mask: dabs weighted by similarity to the colour under the dab centre, refined by a guided filter on luminance (CPU + GPU); ⇧-click draws a straight stroke from where the last stroke ended (`connect`), tapering size, feather and flow from that stroke's values when they changed; no A/B brushes; pen pressure (Brush ▸ Pen Pressure, on by default; `pressure` per point in `mask.brushStroke`) scales each dab's size (a tenth to all of it) and flow, live and committed alike, where winit reports a pen force (Windows; not macOS / Linux tablets yet); no tilt |
|
||||
| LR-MASK-LINEAR | Linear gradient | P0 | ✅ | `cmd:tool.linear`, `cmd:mask.update` | |
|
||||
| LR-MASK-RADIAL | Radial gradient | P0 | ✅ | `cmd:tool.radial`, `cmd:mask.update`, `crates/ui-egui/src/panels/detail.rs` | drag the selected ellipse's body or centre pin to move; edge grips resize, outer grip rotates; rotated and compound radial components, hidden pins, outside hit rejection and one-step undo tested |
|
||||
| LR-MASK-COLORRANGE | Colour range | P1 | ✅ | `cmd:mask.add` (`colorRange`), `cmd:mask.sampleColor`, `cmd:mask.update`, `crates/pipeline/src/lib.rs` (`color_range_sample`) | Color tile → click the photo to sample (⇧-click adds, up to 5; samples taken in the space the mask compares in, respecting section eyes and the requested HDR/SDR rendition), Pick button, Refine slider |
|
||||
| LR-MASK-COLORRANGE | Colour range | P1 | ✅ | `cmd:mask.add` (`colorRange`), `cmd:mask.sampleColor`, `cmd:mask.update`, `crates/pipeline/src/lib.rs` (`color_range_sample`) | Color tile → click the photo to sample (⇧-click adds, up to 5; samples taken in the space the mask compares in, respecting section eyes and the requested HDR/SDR rendition), Pick button, Refine slider; at most 64 samples (`MAX_COLOR_RANGE_SAMPLES`, render cost is pixels × samples) however the mask arrives: `mask.add` / `mask.update`, a sidecar's `lc:settings`, a preset, `develop.merge`, `render --settings` or the catalog and history keep the first 64 (issue #2556) |
|
||||
| LR-MASK-LUMRANGE | Luminance range | P1 | ✅ | `cmd:mask.add` (`luminanceRange`), `cmd:mask.update`, `crates/ui-egui/src/panels/masking.rs` (`range_controls`) | range bar with two handles (the pressed endpoint stays selected through fast/crossing drags; one undo step per drag), Smoothness, Show Luminance Map (B&W photo with the range tinted) |
|
||||
| LR-MASK-DEPTHRANGE | Depth range | P2 | ⬜ | | shape exists, needs depth data |
|
||||
| LR-MASK-COMBINE | Add / subtract / intersect | P0 | ✅ | `cmd:mask.addComponent`, `cmd:tool.brush` | Add/Subtract > Brush creates a component with the chosen operation; Erase removes paint within that brush component |
|
||||
@@ -964,3 +964,4 @@ Latin stays upright. Advanced Japanese composition remains open. Regression cove
|
||||
| 2026-10-27 | patch | MENU-EDIT-SELECTNONE, MENU-EDIT-PASTESELECTED: Select Only Active Photo and Paste Settings from Previous were not in the Edit menu's layout and ended up after Settings…, near the end of the menu; they now sit with the other Select and Paste rows. On macOS the Edit menu no longer shows two separators in a row where Settings… moved to the app menu; status unchanged (✅) |
|
||||
| 2026-10-27 | patch | MENU-FILE-CLOSE, KEY-CLOSEWINDOW: the notes say that Close Window and the red close button of the main window quit the app, after the check Quit makes; kept on purpose; a user page says so; status unchanged (✅) |
|
||||
| 2026-10-29 | minor | LR-MASK-OBJECTS: an Object / Describe selection's stored grid is enlarged along the photo's edges (fast guided filter of its logits by log luminance, a 1.5-cell window), so a raised Exposure no longer draws an outline around the subject (#3272); existing AI masks are 1–2 px tighter at edges; status unchanged (🟡) |
|
||||
| 2026-10-30 | patch | LR-MASK-COLORRANGE: a colour-range mask keeps its first 64 samples however it is loaded (sidecar `lc:settings`, presets, `develop.merge`, `render --settings`, catalog and history), not only through `mask.add` / `mask.update`, so a document with thousands can't stall a render or lose the GPU device (issue #2556); status unchanged (✅) |
|
||||
|
||||
@@ -1,5 +1,14 @@
|
||||
# What's new in LightCraft
|
||||
|
||||
## 2026-10-30
|
||||
|
||||
### A colour-range mask keeps at most 64 samples, however it arrives
|
||||
- A colour-range mask compares every pixel with every colour it has sampled, so a mask with thousands of samples
|
||||
could stall a render or export, or lose the GPU for the rest of the session. `mask.add` and `mask.update` already
|
||||
kept the first 64; a sidecar's `lc:settings`, a preset, `develop.merge`, `render --settings` and the catalog and
|
||||
edit history now keep the first 64 too, in order, and the extra ones are skipped as they are read. Masks made in
|
||||
the app (up to 5 samples) are unchanged.
|
||||
|
||||
## 2026-10-29
|
||||
|
||||
### Object and Describe masks show their pins
|
||||
@@ -21,6 +30,7 @@
|
||||
- Fielded search tokens now take inclusive comparisons: `rating:>=3`, `iso:<=400`, `f:>=2.8`, `focal:<=50`. Before,
|
||||
the `=` was read as part of the number, so such a search quietly showed no photos at all.
|
||||
|
||||
|
||||
## 2026-10-24
|
||||
|
||||
### Relative-date filters and smart albums follow the clock
|
||||
|
||||
Reference in New Issue
Block a user