diff --git a/crates/develop/src/lib.rs b/crates/develop/src/lib.rs index ea7ed1bd0..9c5afa696 100644 --- a/crates/develop/src/lib.rs +++ b/crates/develop/src/lib.rs @@ -161,9 +161,20 @@ impl DevelopSettings { } } - /// Next free mask id. + /// Next free mask id: one past the highest, or, when that is `u32::MAX` (ids come from files, + /// pastes and agents), the lowest one no mask uses. pub fn next_mask_id(&self) -> u32 { - self.masks.iter().map(|m| m.id).max().map_or(1, |m| m + 1) + match self.masks.iter().map(|m| m.id).max() { + None => 1, + // fewer masks than ids: one of the first `masks.len() + 1` is free + Some(max) => max + .checked_add(1) + .or_else(|| { + let used: std::collections::HashSet = self.masks.iter().map(|m| m.id).collect(); + (1..=u32::MAX).find(|id| !used.contains(id)) + }) + .unwrap_or(0), + } } } @@ -411,4 +422,16 @@ mod tests { s.reset_section(Section::Detail); assert_eq!(s.enhance.denoise_on, None); } + + #[test] + fn next_mask_id_after_the_highest_id_is_unused() { + // ids come from files, pastes and agents: one at the limit must not overflow (a panic in + // debug builds) or wrap to an id another mask has + let mut s = DevelopSettings::default(); + assert_eq!(s.next_mask_id(), 1); + s.masks = vec![Mask { id: u32::MAX, ..Default::default() }, Mask { id: 1, ..Default::default() }]; + let next = s.next_mask_id(); + assert!(!s.masks.iter().any(|m| m.id == next), "{next}"); + assert_eq!(next, 2); + } } diff --git a/crates/develop/src/presets.rs b/crates/develop/src/presets.rs index b3edc801c..d5a658f51 100644 --- a/crates/develop/src/presets.rs +++ b/crates/develop/src/presets.rs @@ -220,14 +220,12 @@ impl Preset { o.remove("masks"); } let mut out = apply_partial(s, &rest, amount); - let mut next = out.masks.iter().map(|m| m.id).max().unwrap_or(0); for m in masks { let Ok(mut m) = serde_json::from_value::(m.clone()) else { continue }; if out.masks.iter().any(|q| q.name == m.name && q.components == m.components) { continue; } - next += 1; - m.id = next; + m.id = out.next_mask_id(); m.adjust.amount = (m.adjust.amount * amount).clamp(0.0, 200.0); out.masks.push(m); } @@ -293,4 +291,23 @@ mod tests { deep_merge(&mut a, &json!({"x": {"b": 3}, "y": [2, 3]})); assert_eq!(a, json!({"x": {"a": 1, "b": 3}, "y": [2, 3]})); } + + #[test] + fn preset_masks_get_unused_ids_after_a_mask_id_at_the_limit() { + let s = DevelopSettings { masks: vec![crate::Mask { id: u32::MAX, name: "Theirs".into(), ..Default::default() }], ..Default::default() }; + let p = Preset { + id: "p".into(), + name: "P".into(), + group: "User".into(), + settings: json!({"masks": [{"id": 7, "name": "A"}, {"id": 7, "name": "B"}]}), + favorite: false, + builtin: false, + }; + let out = p.apply(&s, 1.0); + let mut ids: Vec = out.masks.iter().map(|m| m.id).collect(); + assert_eq!(ids.len(), 3); + ids.sort_unstable(); + ids.dedup(); + assert_eq!(ids.len(), 3, "{:?}", out.masks); + } } diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index ae03859d2..db77c3cba 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -412,6 +412,17 @@ fn connected_brush_strokes_start_where_the_last_one_ended() { assert_eq!(strokes(&s).len(), 2); } +#[test] +fn a_mask_id_at_the_limit_does_not_overflow_the_next_masks() { + let mut s = demo(); + s.execute("develop.merge", &json!({"settings": {"masks": [{"id": u32::MAX, "name": "Pasted"}]}})).unwrap(); + s.execute("mask.add", &json!({"kind": "radial"})).unwrap(); + s.execute("mask.duplicate", &json!({})).unwrap(); + let ids: Vec = active_dev(&s).masks.iter().map(|m| m.id).collect(); + assert_eq!(ids.len(), 3); + assert!(ids.iter().enumerate().all(|(i, id)| !ids[..i].contains(id)), "distinct ids: {ids:?}"); +} + #[test] fn brush_strokes_keep_pen_pressure_and_refuse_bad_lists() { let mut s = demo(); diff --git a/docs/whats-new.md b/docs/whats-new.md index 7d43b2ec8..b494eb8cf 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -15,6 +15,11 @@ place, AI masks failed with "unsupported dtype C64". LightCraft now says that the file is Meta's original checkpoint and which file it needs instead. +### Reliability +- A photo with a mask id at the 32-bit limit (from a damaged or hand-edited sidecar, a paste or an agent) no longer + breaks the masks added after it: new masks, duplicates and the masks of a preset each get an id no other mask has. + Before, the next id wrapped around to 0, so a second new mask shared the first one's id (debug builds crashed). + ## 2026-10-29 ### Object and Describe masks show their pins