Merge PR #3385: Never crash: a mask id at the u32 limit doesn't overflow the next mask's id

# Conflicts:
#	docs/whats-new.md

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<u32> = 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);
+    }
 }

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::<crate::Mask>(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<u32> = 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);
+    }
 }