From b26b449fe368bf331cda8c4ee21baf8105ea93ec Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Fri, 9 Oct 2026 13:34:25 -0400 Subject: [PATCH] Fixes from agent reports: hex colours everywhere, camelCase in/out, unique variable ids, strict batch, ui.key modifiers, export rounding, Fit images, inner shadows on all edges Co-Authored-By: Claude Opus 5.5 --- crates/color/src/lib.rs | 33 ++++++++++++++++++++++++++++++-- crates/color/src/tests.rs | 10 ++++++++++ crates/doc/src/props.rs | 1 + crates/engine/src/cmd/edit.rs | 7 ++++++- crates/engine/src/cmd/props.rs | 34 +++++++++++++++++++++++++++++---- crates/engine/src/cmd/styles.rs | 10 ++++++++-- crates/engine/src/inspect.rs | 33 +++++++++++++++++++++++++++++--- crates/render/src/draw.rs | 25 ++++++++++++++---------- crates/render/src/images.rs | 22 +++++++++++++++++++++ crates/render/src/lib.rs | 4 ++-- crates/render/src/paint.rs | 9 +++++++-- crates/render/src/tests.rs | 13 +++++++++++++ crates/ui-egui/src/control.rs | 4 ++-- 13 files changed, 177 insertions(+), 28 deletions(-) diff --git a/crates/color/src/lib.rs b/crates/color/src/lib.rs index 3e9b29b..a1620d7 100644 --- a/crates/color/src/lib.rs +++ b/crates/color/src/lib.rs @@ -12,8 +12,9 @@ mod hsx; pub use blend::BlendMode; pub use hsx::*; -/// A non-premultiplied sRGB colour with alpha, components in `0..=1`. -#[derive(Clone, Copy, Debug, PartialEq, Serialize, Deserialize)] +/// A non-premultiplied sRGB colour with alpha, components in `0..=1`. Deserializes from +/// `{r, g, b, a}`, `[r, g, b(, a)]`, or a CSS/hex string (`"#0D99FF"`, `"rgba(…)"`). +#[derive(Clone, Copy, Debug, PartialEq, Serialize)] pub struct Color { pub r: f32, pub g: f32, @@ -26,6 +27,34 @@ fn one() -> f32 { 1.0 } +#[derive(Deserialize)] +#[serde(untagged)] +enum ColorRepr { + Obj { + r: f32, + g: f32, + b: f32, + #[serde(default = "one")] + a: f32, + }, + Arr(Vec), + Text(String), +} + +impl<'de> Deserialize<'de> for Color { + fn deserialize>(d: D) -> std::result::Result { + match ColorRepr::deserialize(d)? { + ColorRepr::Obj { r, g, b, a } => Ok(Color::new(r, g, b, a)), + ColorRepr::Arr(v) => match v.as_slice() { + [r, g, b] => Ok(Color::new(*r, *g, *b, 1.0)), + [r, g, b, a] => Ok(Color::new(*r, *g, *b, *a)), + _ => Err(serde::de::Error::custom("colour arrays have 3 or 4 components")), + }, + ColorRepr::Text(t) => Color::parse_css(&t).ok_or_else(|| serde::de::Error::custom(format!("not a colour: {t}"))), + } + } +} + fn clamp01(v: f32) -> f32 { if v.is_finite() { v.clamp(0.0, 1.0) } else { 0.0 } } diff --git a/crates/color/src/tests.rs b/crates/color/src/tests.rs index 2f05db3..3f9e6b7 100644 --- a/crates/color/src/tests.rs +++ b/crates/color/src/tests.rs @@ -65,3 +65,13 @@ fn nan_inputs_are_sanitized() { let c = Color::new(f32::NAN, 2.0, -1.0, f32::INFINITY); assert_eq!(c.to_rgba8(), [0, 255, 0, 0]); } + +#[test] +fn colour_deserializes_from_many_forms() { + let a: Color = serde_json::from_str("\"#FF0000\"").unwrap(); + let b: Color = serde_json::from_str("{\"r\":1,\"g\":0,\"b\":0}").unwrap(); + let c: Color = serde_json::from_str("[1,0,0,1]").unwrap(); + assert_eq!(a, b); + assert_eq!(b, c); + assert!(serde_json::from_str::("\"nope\"").is_err()); +} diff --git a/crates/doc/src/props.rs b/crates/doc/src/props.rs index 59b47cd..590baea 100644 --- a/crates/doc/src/props.rs +++ b/crates/doc/src/props.rs @@ -106,6 +106,7 @@ pub struct Shadow { pub spread: f64, pub blend: BlendMode, /// Drop shadows only: show behind transparent areas of the layer. + #[serde(alias = "showBehind")] pub show_behind: bool, } diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index cc7d42c..a8b2a92 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -250,7 +250,12 @@ pub fn specs() -> Vec { let label = string(p, "label").unwrap_or_else(|| "Batch".into()); let cmds: Vec<(String, Value)> = match get(p, "commands") { Some(Value::Array(a)) => { - a.iter().filter_map(|c| Some((c.get("command")?.as_str()?.to_string(), c.get("params").cloned().unwrap_or(json!({}))))).collect() + let mut out = Vec::new(); + for (i, c) in a.iter().enumerate() { + let cmd = c.get("command").and_then(Value::as_str).ok_or_else(|| bad("command.batch", format!("step {i} has no `command`")))?; + out.push((cmd.to_string(), c.get("params").cloned().unwrap_or(json!({})))); + } + out } _ => return Err(bad("command.batch", "`commands` must be an array")), }; diff --git a/crates/engine/src/cmd/props.rs b/crates/engine/src/cmd/props.rs index 12dfbae..344eed6 100644 --- a/crates/engine/src/cmd/props.rs +++ b/crates/engine/src/cmd/props.rs @@ -19,6 +19,32 @@ fontSize, lineHeight (auto|px|\"150%\"), letterSpacing (px|\"2%\"), textAlign (l (widthAndHeight|height|none|truncate), textDecoration, textCase, paragraphSpacing, listStyle, pointCount, innerRadius, arcStart, arcSweep, \ arcInner, constrainProportions, description, interactions, overflow, pluginData {k: v}"; +/// camelCase object keys → snake_case (accept the inspect/camel forms for serde-backed values). +pub fn snake(v: &Value) -> Value { + match v { + Value::Object(m) => Value::Object( + m.iter() + .map(|(k, v)| { + let mut out = String::new(); + for c in k.chars() { + if c.is_ascii_uppercase() { + out.push('_'); + out.push(c.to_ascii_lowercase()); + } else { + out.push(c); + } + } + // Paint/effect type tags stay as they are. + let key = if matches!(k.as_str(), "type" | "kind") { k.clone() } else { out }; + (key, if matches!(k.as_str(), "type" | "kind") { v.clone() } else { snake(v) }) + }) + .collect(), + ), + Value::Array(a) => Value::Array(a.iter().map(snake).collect()), + other => other.clone(), + } +} + fn enum_of(v: &Value) -> Option { let s = v.as_str()?; // Accept camelCase, snake/upper-case and spaced variants. @@ -209,7 +235,7 @@ pub fn apply(cmd: &str, doc: &mut Document, id: NodeId, props: &Map doc.get_mut(id)?.corners.smoothing = f64_of(v).unwrap_or(0.0).clamp(0.0, 1.0), - "effects" => doc.get_mut(id)?.effects = serde_json::from_value(v.clone()).map_err(|e| bad(cmd, format!("effects: {e}")))?, + "effects" => doc.get_mut(id)?.effects = serde_json::from_value(snake(v)).map_err(|e| bad(cmd, format!("effects: {e}")))?, "clip" | "clipsContent" => doc.get_mut(id)?.clip = v.as_bool().ok_or_else(|| bad(cmd, "`clip` must be a boolean"))?, "isMask" => doc.get_mut(id)?.is_mask = v.as_bool().unwrap_or(false), "layout" | "layoutMode" => { @@ -229,7 +255,7 @@ pub fn apply(cmd: &str, doc: &mut Document, id: NodeId, props: &Map n.layout = Some(serde_json::from_value(v.clone()).map_err(|e| bad(cmd, format!("layout: {e}")))?), + Value::Object(_) => n.layout = Some(serde_json::from_value(snake(v)).map_err(|e| bad(cmd, format!("layout: {e}")))?), _ => return Err(bad(cmd, "layout: none|horizontal|vertical|grid|{...}")), } } @@ -293,9 +319,9 @@ pub fn apply(cmd: &str, doc: &mut Document, id: NodeId, props: &Map doc.get_mut(id)?.grids = serde_json::from_value(v.clone()).map_err(|e| bad(cmd, format!("grids: {e}")))?, + "grids" | "layoutGrids" => doc.get_mut(id)?.grids = serde_json::from_value(snake(v)).map_err(|e| bad(cmd, format!("grids: {e}")))?, "exports" | "exportSettings" => { - doc.get_mut(id)?.exports = serde_json::from_value(v.clone()).map_err(|e| bad(cmd, format!("exports: {e}")))? + doc.get_mut(id)?.exports = serde_json::from_value(snake(v)).map_err(|e| bad(cmd, format!("exports: {e}")))? } "interactions" => { doc.get_mut(id)?.interactions = serde_json::from_value(v.clone()).map_err(|e| bad(cmd, format!("interactions: {e}")))? diff --git a/crates/engine/src/cmd/styles.rs b/crates/engine/src/cmd/styles.rs index 4ea20c3..431231c 100644 --- a/crates/engine/src/cmd/styles.rs +++ b/crates/engine/src/cmd/styles.rs @@ -72,7 +72,7 @@ pub fn specs() -> Vec { spec!("variable.createCollection", "Create variable collection", "{name, modes? [names]}", true, |s, p| { let name = req_string("variable.createCollection", p, "name")?; let doc = s.doc_mut()?; - let id = format!("VariableCollectionId:{}", doc.variables.collections.len() + 1); + let id = unique_id("VariableCollectionId", doc.variables.collections.iter().map(|c| c.id.as_str())); let modes: Vec = match get(p, "modes") { Some(Value::Array(a)) => a.iter().filter_map(|m| m.as_str().map(str::to_string)).collect(), _ => vec!["Mode 1".into()], @@ -396,7 +396,7 @@ fn var_create(s: &mut Session, p: &Value) -> Result { for m in &coll.modes { values.entry(m.id.clone()).or_insert_with(|| default.clone()); } - let id = format!("VariableID:{}", doc.variables.variables.len() + 1); + let id = unique_id("VariableID", doc.variables.variables.iter().map(|v| v.id.as_str())); doc.variables.variables.push(Variable { id: id.clone(), name, @@ -455,3 +455,9 @@ pub fn apply_bindings(doc: &mut Document) { let _ = super::props::apply("variables", doc, id, &sets); } } + +/// `prefix:N` with N one more than the largest existing. +fn unique_id<'a>(prefix: &str, existing: impl Iterator) -> String { + let max = existing.filter_map(|e| e.strip_prefix(prefix)?.strip_prefix(':')?.parse::().ok()).max().unwrap_or(0); + format!("{prefix}:{}", max + 1) +} diff --git a/crates/engine/src/inspect.rs b/crates/engine/src/inspect.rs index bda376c..2891835 100644 --- a/crates/engine/src/inspect.rs +++ b/crates/engine/src/inspect.rs @@ -54,7 +54,7 @@ pub fn node_json(doc: &Document, id: NodeId, depth: usize) -> Value { } } if !n.effects.is_empty() { - o.insert("effects".into(), serde_json::to_value(&n.effects).unwrap_or(Value::Null)); + o.insert("effects".into(), camel(serde_json::to_value(&n.effects).unwrap_or(Value::Null))); } if n.corners.radii.iter().any(|r| *r > 0.0) { o.insert( @@ -69,7 +69,7 @@ pub fn node_json(doc: &Document, id: NodeId, depth: usize) -> Value { } } if let Some(l) = &n.layout { - o.insert("layout".into(), serde_json::to_value(l).unwrap_or(Value::Null)); + o.insert("layout".into(), camel(serde_json::to_value(l).unwrap_or(Value::Null))); } let parent_al = n.parent.and_then(|p| doc.node(p)).is_some_and(|p| p.layout.is_some()); if n.layout.is_some() || parent_al || matches!(n.kind, NodeKind::Text(_)) { @@ -88,7 +88,7 @@ pub fn node_json(doc: &Document, id: NodeId, depth: usize) -> Value { o.insert("isMask".into(), json!(true)); } if !n.grids.is_empty() { - o.insert("grids".into(), serde_json::to_value(&n.grids).unwrap_or(Value::Null)); + o.insert("grids".into(), camel(serde_json::to_value(&n.grids).unwrap_or(Value::Null))); } if !n.exports.is_empty() { o.insert("exports".into(), serde_json::to_value(&n.exports).unwrap_or(Value::Null)); @@ -217,3 +217,30 @@ pub fn document(s: &Session, depth: usize, all_pages: bool) -> Value { "history": {"undo": ds.history.undo.iter().rev().take(20).map(|h| h.label.clone()).collect::>(), "redo": ds.history.redo.len()}, }) } + +/// snake_case object keys → camelCase (inspect output matches the keys `node.set` accepts). +pub fn camel(v: Value) -> Value { + match v { + Value::Object(m) => Value::Object( + m.into_iter() + .map(|(k, v)| { + let mut out = String::new(); + let mut up = false; + for c in k.chars() { + if c == '_' { + up = true; + } else if up { + out.extend(c.to_uppercase()); + up = false; + } else { + out.push(c); + } + } + (out, camel(v)) + }) + .collect(), + ), + Value::Array(a) => Value::Array(a.into_iter().map(camel).collect()), + other => other, + } +} diff --git a/crates/render/src/draw.rs b/crates/render/src/draw.rs index faf8b36..0316c1c 100644 --- a/crates/render/src/draw.rs +++ b/crates/render/src/draw.rs @@ -367,23 +367,28 @@ pub fn draw_node(ctx: &Ctx, pix: &mut Pixmap, id: NodeId, parent_xf: Affine, dep let Some(sa) = shape_alpha() else { continue }; let mut sh = sa.clone(); fx::invert_alpha(&mut sh); - let mut shifted = Pixmap::new(lw.max(1), lh.max(1)); - let Some(shifted) = shifted.as_mut() else { continue }; - // Outside of the layer counts as "outside the shape" too. - shifted.fill(tiny_skia::Color::WHITE); + // Work in a padded buffer: everything beyond the layer is "outside the shape", so + // the blur pulls shadow in from every edge, not just the offset side. let (dx, dy) = (s.offset.0 * ctx.scale, s.offset.1 * ctx.scale); - let rect = tiny_skia::Rect::from_xywh(dx as f32, dy as f32, lw as f32, lh as f32); - if let Some(rect) = rect { + let pad = ((s.radius / 2.0 * ctx.scale * 3.0) + s.spread.abs() * ctx.scale + dx.abs().max(dy.abs())).ceil().clamp(0.0, 2048.0) as u32 + 2; + let Some(mut big) = Pixmap::new(lw.max(1) + pad * 2, lh.max(1) + pad * 2) else { continue }; + big.fill(tiny_skia::Color::WHITE); + let ox = pad as f32 + dx as f32; + let oy = pad as f32 + dy as f32; + if let Some(rect) = tiny_skia::Rect::from_xywh(ox, oy, lw as f32, lh as f32) { let clear = tiny_skia::Paint { shader: tiny_skia::Shader::SolidColor(tiny_skia::Color::TRANSPARENT), blend_mode: tiny_skia::BlendMode::Source, ..Default::default() }; - shifted.fill_rect(rect, &clear, Transform::identity(), None); + big.fill_rect(rect, &clear, Transform::identity(), None); } - shifted.draw_pixmap(dx.round() as i32, dy.round() as i32, sh.as_ref(), &PixmapPaint::default(), Transform::identity(), None); - fx::morph(shifted, (s.spread * ctx.scale).round() as i32); - fx::blur(shifted, s.radius / 2.0 * ctx.scale); + big.draw_pixmap(ox.round() as i32, oy.round() as i32, sh.as_ref(), &PixmapPaint::default(), Transform::identity(), None); + fx::morph(&mut big, (s.spread * ctx.scale).round() as i32); + fx::blur(&mut big, s.radius / 2.0 * ctx.scale); + let Some(mut cropped) = Pixmap::new(lw.max(1), lh.max(1)) else { continue }; + cropped.draw_pixmap(-(pad as i32), -(pad as i32), big.as_ref(), &PixmapPaint { blend_mode: tiny_skia::BlendMode::Source, ..Default::default() }, Transform::identity(), None); + let shifted = &mut cropped; fx::mask_by(shifted, &sa, false); fx::tint(shifted, [s.color.r, s.color.g, s.color.b, s.color.a]); inner_shadows.push(shifted.clone()); diff --git a/crates/render/src/images.rs b/crates/render/src/images.rs index f2c1dd8..f47f1e5 100644 --- a/crates/render/src/images.rs +++ b/crates/render/src/images.rs @@ -109,3 +109,25 @@ pub fn dimensions(bytes: &[u8]) -> Option<(u32, u32)> { let reader = image::ImageReader::new(std::io::Cursor::new(bytes)).with_guessed_format().ok()?; reader.into_dimensions().ok() } + +fn padded_cache() -> &'static Mutex>> { + static C: OnceLock>>> = OnceLock::new(); + C.get_or_init(|| Mutex::new(HashMap::new())) +} + +/// `pm` with a transparent 1-pixel border (cached). +pub fn padded(hash: &str, filters: &ImageFilters, pm: &Pixmap) -> Option> { + let key = (hash.to_string(), fkey(filters)); + if let Some(p) = padded_cache().lock().unwrap_or_else(std::sync::PoisonError::into_inner).get(&key) { + return Some(p.clone()); + } + let mut out = Pixmap::new(pm.width() + 2, pm.height() + 2)?; + out.draw_pixmap(1, 1, pm.as_ref(), &tiny_skia::PixmapPaint::default(), tiny_skia::Transform::identity(), None); + let out = Arc::new(out); + let mut c = padded_cache().lock().unwrap_or_else(std::sync::PoisonError::into_inner); + if c.len() > 256 { + c.clear(); + } + c.insert(key, out.clone()); + Some(out) +} diff --git a/crates/render/src/lib.rs b/crates/render/src/lib.rs index 95b7725..0aa858a 100644 --- a/crates/render/src/lib.rs +++ b/crates/render/src/lib.rs @@ -134,8 +134,8 @@ pub fn export_bounds(doc: &Document, id: NodeId) -> Option { pub fn render_node(doc: &Document, id: NodeId, scale: f64) -> Option { let b = export_bounds(doc, id)?; let scale = if scale.is_finite() && scale > 0.0 { scale.min(64.0) } else { 1.0 }; - let w = (b.width() * scale).ceil().max(1.0); - let h = (b.height() * scale).ceil().max(1.0); + let w = (b.width() * scale - 1e-6).ceil().max(1.0); + let h = (b.height() * scale - 1e-6).ceil().max(1.0); if w > 16_384.0 || h > 16_384.0 { return None; } diff --git a/crates/render/src/paint.rs b/crates/render/src/paint.rs index a12e88a..75bd391 100644 --- a/crates/render/src/paint.rs +++ b/crates/render/src/paint.rs @@ -123,8 +123,13 @@ pub fn prepare(paint: &Paint, w: f64, h: f64, doc: &Document) -> Option 245); +} diff --git a/crates/ui-egui/src/control.rs b/crates/ui-egui/src/control.rs index 770d973..dff817b 100644 --- a/crates/ui-egui/src/control.rs +++ b/crates/ui-egui/src/control.rs @@ -137,10 +137,10 @@ pub fn handle(app: &mut FigApp, ctx: &Context, method: &str, p: &Value) -> Resul return Ok(json!({"ok": true})); } match crate::shortcuts::parse(&spec) { - Some(_) => { + Some(want) => { let id = figcraft_engine::catalog::MENU .iter() - .find(|m| !m.shortcut.is_empty() && m.shortcut.eq_ignore_ascii_case(&spec)) + .find(|m| !m.shortcut.is_empty() && crate::shortcuts::parse(m.shortcut) == Some(want)) .map(|m| m.id); match id { Some(id) => {