aleleba/lightcraft
1
0
Fork 0
mirror of https://github.com/storytold/lightcraft.git synced 2026-11-01 14:07:34 -06:00
Code Issues Packages Projects Releases Wiki Activity

Masks and spots: an id or index that isn't one is an error, not the selected one

Browse Source 
mask_id read `id` with `as_u64().map(|v| v as u32)`, so text fell back to
the selected mask (mask.delete {"id": "1"} deleted it) and an id past
u32 wrapped onto another mask (4294967297 as u32 is 1). spot_index and
spot.select did the same with `index`, and a colour range's `component`
fell back to the first colour range part. They now refuse such values,
as the photo ids do; a missing or null value still means the selection.
This commit is contained in:
Bruno-Alumn
2026-10-21 01:39:29 +00:00
parent 42c7cc03a4
commit 622988d7f7
2 changed files with 65 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/engine/src/cmd/masks.rs
+26 -7
View File
@@ -199,8 +199,27 @@ fn connected_strokes(from: Point, last: &BrushStroke, to: BrushStroke) -> Vec<Br
out
}
/// The mask `id` names: `None` when it is missing or null. An `id` that isn't a mask id (text, a
/// negative or fractional number, or one past `u32`, which `as u32` would have wrapped onto another
/// mask) is an error, never the selected mask.
fn opt_mask_id(p: &Value, c: &str) -> Result<Option<u32>> {
match p.get("id") {
None | Some(Value::Null) => Ok(None),
Some(v) => v.as_u64().and_then(|n| u32::try_from(n).ok()).map(Some).ok_or_else(|| bad(c, "`id` must be a mask id")),
}
}
fn mask_id(p: &Value, active: Option<u32>, c: &str) -> Result<u32> {
p.get("id").and_then(Value::as_u64).map(|v| v as u32).or(active).ok_or_else(|| bad(c, "no mask selected (give `id`)"))
opt_mask_id(p, c)?.or(active).ok_or_else(|| bad(c, "no mask selected (give `id`)"))
}
/// A whole-number index parameter `key`: `None` when it is missing or null, else an error unless it
/// is one (an index that isn't must not fall back to the selected item).
fn opt_index(p: &Value, key: &str, c: &str) -> Result<Option<usize>> {
match p.get(key) {
None | Some(Value::Null) => Ok(None),
Some(v) => v.as_u64().and_then(|n| usize::try_from(n).ok()).map(Some).ok_or_else(|| bad(c, format!("`{key}` must be a whole number"))),
}
}
fn find(masks: &mut [Mask], id: u32, c: &str) -> Result<usize> {
@@ -307,7 +326,7 @@ pub fn specs() -> Vec<CommandSpec> {
let connect = bool_or(p, "connect", false);
let mut connected = false;
let next = s.active().and_then(|id| s.develop_of(id)).map(|d| d.next_mask_id()).unwrap_or(1);
let want = p.get("id").and_then(Value::as_u64).map(|v| v as u32).or(s.active_mask);
let want = opt_mask_id(p, "mask.brushStroke")?.or(s.active_mask);
let adjust = super::mask_effects::new_mask_adjust(s, "brush");
let mut out = masks_edit(s, "mask.brushStroke", "Brush", |masks, active| {
let i = match want.and_then(|m| masks.iter().position(|x| x.id == m)) {
@@ -391,7 +410,7 @@ pub fn specs() -> Vec<CommandSpec> {
let lab = lightcraft_pipeline::color_range_sample(&src, &info, &d, &req, lightcraft_geom::Point::new(x, y))
.ok_or_else(|| bad(c, "the point is outside the photo"))?;
let mid = mask_id(p, s.active_mask, c)?;
let comp = p.get("component").and_then(Value::as_u64).map(|v| v as usize);
let comp = opt_index(p, "component", c)?;
let mut out = Vec::new();
masks_edit(s, c, "Color Range", |masks, _| {
let i = find(masks, mid, c)?;
@@ -604,7 +623,7 @@ pub fn specs() -> Vec<CommandSpec> {
}
),
cmd!("mask.select", "Select Mask", [], None, "{id|null}", has_active, |s, p| {
s.active_mask = p.get("id").and_then(Value::as_u64).map(|v| v as u32);
s.active_mask = opt_mask_id(p, "mask.select")?;
ok()
}),
cmd!("mask.delete", "Delete Mask", [], None, "{id?}", has_active, |s, p| {
@@ -879,8 +898,8 @@ pub fn specs() -> Vec<CommandSpec> {
),
cmd!("spot.select", "Select Spot", [], None, "{index|null}", has_active, |s, p| {
let n = spots_len(s);
s.active_spot = match p.get("index").and_then(Value::as_u64) {
Some(i) if (i as usize) < n => Some(i as usize),
s.active_spot = match opt_index(p, "index", "spot.select")? {
Some(i) if i < n => Some(i),
Some(_) => return Err(bad("spot.select", "no such spot")),
None => None,
};
@@ -1040,7 +1059,7 @@ fn spots_len(s: &Session) -> usize {
/// `index` from the params, else the selected spot.
fn spot_index(s: &Session, p: &Value, c: &str) -> Result<usize> {
let i = p.get("index").and_then(Value::as_u64).map(|v| v as usize).or(s.active_spot).ok_or_else(|| bad(c, "no spot selected (give `index`)"))?;
let i = opt_index(p, "index", c)?.or(s.active_spot).ok_or_else(|| bad(c, "no spot selected (give `index`)"))?;
if i >= spots_len(s) {
return Err(bad(c, "no such spot"));
}
crates/engine/src/tests_ids_params.rs
+39
View File
@@ -104,3 +104,42 @@ fn single_photo_commands_refuse_an_id_that_is_not_a_photo_id() {
assert_eq!(source(&s), before);
assert_eq!(s.catalog.photo(active).unwrap().meta.regions.len(), 1);
}
/// Mask ids and spot indexes the same way: `mask.delete {"id": "1"}` deleted the selected mask, and an
/// id past `u32` wrapped (`4294967297 as u32` is 1) onto another mask.
#[test]
fn mask_and_spot_commands_refuse_ids_that_are_not_ids() {
let (mut s, _, _) = session();
s.execute("mask.add", &json!({"kind": "linear"})).unwrap();
s.execute("mask.add", &json!({"kind": "radial"})).unwrap();
s.execute("spot.add", &json!({"mode": "heal", "points": [[0.4, 0.5]], "size": 0.02, "source": [0.1, 0.0]})).unwrap();
let state = |s: &Session| {
let d = s.develop_of(s.active().unwrap()).unwrap();
(d.masks.iter().map(|m| (m.id, m.name.clone(), m.visible)).collect::<Vec<_>>(), d.spots.len())
};
let before = state(&s);
assert_eq!((before.0.len(), before.1), (2, 1));
let first = before.0[0].0;
let commits = s.commits();
for bad in [json!(u64::from(u32::MAX) + 1 + u64::from(first)), json!("1"), json!(-1), json!(1.5), json!([first])] {
for (cmd, params) in [
("mask.delete", json!({})),
("mask.rename", json!({"name": "renamed"})),
("mask.visible", json!({"visible": false})),
("mask.select", json!({})),
] {
let mut params = params;
params["id"] = bad.clone();
assert!(s.execute(cmd, &params).is_err(), "{cmd} {bad}");
}
for cmd in ["spot.delete", "spot.select"] {
assert!(s.execute(cmd, &json!({"index": bad})).is_err(), "{cmd} {bad}");
}
}
assert_eq!(s.commits(), commits, "nothing was changed");
assert_eq!(state(&s), before);
// well-formed ids and a null still work
s.execute("mask.delete", &json!({"id": first})).unwrap();
s.execute("spot.select", &json!({"index": null})).unwrap();
assert_eq!(state(&s).0.len(), 1);
}