← Commit history

Merge PR #165: Native silkscreen leader curves and borders with stable IDs and Undo (astra/native-silk-graphics -> master) merge

John Lauer ·9b24ed411a ·1mo ago ·parents 0fad23e, da12d65

Showing changes introduced by the merge (against its first parent).

6 files changed +440
docs/native-silk-graphics.mdadded+9
@@ -0,0 +1,9 @@+# Native silkscreen leaders and borders++`kicad_silk_graphics_batch` uses `filePath`, `expectedRevision`, and `create`, `update`, `remove` arrays. Each create has `kind`, `layer`, `points` and `stroke`; each full-definition update also has the stable `id`. Remove entries are IDs. Optional `dryRun` and `save` behave like native silkscreen text edits.++Kinds: `line` or `rect` with two points; `arc` with start/mid/end; `bezier` with start/control1/control2/end. Points are board-frame millimetres, including on B.SilkS. Only F.SilkS/B.SilkS, solid positive strokes, and unfilled shapes are supported. Locked items, footprint graphics, other layers, unknown properties, duplicate IDs, degenerate lines/rectangles and collinear arcs refuse.++These are manufactured silkscreen graphics. Ghost obstacle boxes remain dashboard-only. Native DRC compares the complete candidate including warnings before one native Undo transaction. UUIDs must be preserved in native replies. Deletion and live-ID checks run after commit. Any postCommit error means inspect state before retrying, not automatic replay. No shared runtime is replaced to test this.++Native acceptance before publication: create a line, border, arc and Bezier on both sides, inspect the board and native 3D viewer, verify native curves match the candidate DRC geometry, update one by UUID, and remove a mixed subset with one Undo restoring the originals. Confirm mask-overlap rejection and locked/non-silk refusal. Source tests alone are insufficient. Footprint fields, exact mask/body obstacles and automatic linked 3D refresh are separate capabilities.
rust/crates/kicad-bridge/src/verbs_routing.rs+13
@@ -17,6 +17,16 @@ use kicad_core::pcb::{self, NetRef}; use kicad_core::{dsn, freerouting, progress};  pub static VERBS: &[Verb] = &[+    Verb {+        name: "kicad_silk_graphics_batch",+        summary: "Create/update/remove native silkscreen lines, borders, arcs and Bezier leaders by stable ID.",+        mechanism: Mechanism::Ipc, risk: "write", timeout_sec: 130,+        input: "{filePath,expectedRevision,create:[{kind,layer,points,stroke}],update:[{id,kind,layer,points,stroke}],remove:[id],dryRun?,save?}",+        example: "kicad_silk_graphics_batch {filePath,expectedRevision,create:[{kind:line,layer:F.SilkS,points:[[1,2],[3,4]],stroke:0.15}]}",+        hint: "Coordinates are board-frame mm on either side. kind is line/rect (2 points), arc (start/mid/end), bezier (start/control1/control2/end). Full update definitions preserve UUID. Solid stroke, unfilled shapes; locked/non-silk IDs refuse. Native DRC includes warnings; one Undo step.",+        related: &["kicad_silk_text_batch", "kicad_text_bounds"],+        pitfalls: &["No footprint graphics or implicit text-box obstacles; these are manufactured silk, not dashboard ghost rectangles.","Inspect postCommit errors before replay; source tests do not establish native Windows acceptance."],+    },     Verb {         name: "kicad_text_bounds",         summary: "Measure candidate silkscreen text using native KiCad text metrics without editing.",@@ -200,6 +210,7 @@ pub static VERBS: &[Verb] = &[  pub fn dispatch(state: &mut State, command: &str, args: &Value) -> Option<Value> {     Some(match command {+        "kicad_silk_graphics_batch" => live(state,args,Live::SilkGraphicsBatch),         "kicad_silk_text_batch" => live(state,args,Live::SilkTextBatch),         "kicad_routing_state" => live(state, args, Live::State),         "kicad_text_bounds" => live(state,args,Live::TextBounds),@@ -228,6 +239,7 @@ pub fn dispatch(state: &mut State, command: &str, args: &Value) -> Option<Value>  #[derive(Clone, Copy)] enum Live {+    SilkGraphicsBatch,     SilkTextBatch,     State,     TextBounds,@@ -251,6 +263,7 @@ fn live(state: &mut State, args: &Value, which: Live) -> Value {         config_dir: info.primary().and_then(|p| detect::config_dir(&p.version)),     };     let result = match which {+        Live::SilkGraphicsBatch => ipc::silk_graphics_batch(&ctx,args),         Live::State => ipc::routing_state(&ctx, args),         Live::TextBounds => ipc::text_bounds(&ctx,args),         Live::SilkTextBatch => ipc::silk_text_batch(&ctx,args),
rust/crates/kicad-core/src/ipc.rs+3
@@ -41,6 +41,9 @@ pub const DRC_TIMEOUT: Duration = Duration::from_secs(90); /// Connectivity walks stop here (`board_too_large`). pub const MAX_WALK: usize = 100_000; +mod silk_graphics;+pub use silk_graphics::silk_graphics_batch;+ type RResult<T> = Result<T, RoutingError>;  /// What the verbs know that the operations need: where kicad-cli is (for the DRC
rust/crates/kicad-core/src/ipc/silk_graphics.rsadded+123
@@ -0,0 +1,123 @@+use super::*;+use crate::silk_graphics as graphics;++pub fn silk_graphics_batch(ctx: &Ctx, args: &Value) -> RResult<Value> {+    if args["expectedRevision"]+        .as_str()+        .filter(|s| !s.is_empty())+        .is_none()+    {+        return Err(RoutingError::new(+            "revision_required",+            "Read the board revision before editing silk graphics",+        ));+    }+    let s = connect(ctx, args)?;+    let (text, _, revision) = s.snapshot()?;+    check_revision(args, &revision)?;+    let plan = graphics::prepare(&text, args)?;+    let before = drc_snapshot(ctx.kicad_cli.as_deref(), &s.source_path(), &text)?;+    let after = drc_snapshot(ctx.kicad_cli.as_deref(), &s.source_path(), &plan.candidate)?;+    let all = |mut v: Value| {+        if let Some(rows) = v["violations"].as_array_mut() {+            for row in rows {+                row["severity"] = json!("error");+            }+        }+        v+    };+    let added = new_errors(&all(before), &all(after.clone()))?;+    if !added.is_empty() {+        return Err(RoutingError::new(+            "drc_rejected",+            "Silk graphics add native DRC findings; no live edit",+        )+        .with("violations", json!(added))+        .with("mutated", json!(false)));+    }+    if args["dryRun"] == true {+        return Ok(json!({"success":true,"mutated":false,"drc":after,"revision":revision}));+    }+    let encode = |g: &graphics::Graphic| prost_types::Any {+        type_url: graphics::TYPE_URL.into(),+        value: g.bytes(BoardLayerInfo::id_from_name(&g.layer).unwrap()),+    };+    let created: Vec<_> = plan.create.iter().map(|g| g.id.clone()).collect();+    let updated: Vec<_> = plan.update.iter().map(|g| g.id.clone()).collect();+    let verify = |rows: Vec<prost_types::Any>, wanted: &[String]| -> Result<(), IpcFailure> {+        let actual: HashSet<_> = rows.iter().filter_map(|a| graphics::id(&a.value)).collect();+        if actual != wanted.iter().cloned().collect() {+            return Err(IpcFailure::from(+                "Native graphics reply did not preserve requested UUIDs".to_string(),+            ));+        }+        Ok(())+    };+    check_revision(args, &s.revision()?)?;+    transaction(&s, "Adom silkscreen graphics", || {+        if !plan.remove.is_empty() {+            s.client.delete_items(plan.remove.clone())?;+        }+        if !plan.update.is_empty() {+            verify(+                s.client+                    .update_items(plan.update.iter().map(&encode).collect())?,+                &updated,+            )?;+        }+        if !plan.create.is_empty() {+            verify(+                s.client+                    .create_items(plan.create.iter().map(&encode).collect(), None)?,+                &created,+            )?;+        }+        Ok(())+    })?;+    let mut out = json!({"success":true,"committed":true,"mutated":true,"undoSteps":1,"createdIds":created,"updatedIds":updated,"removedIds":plan.remove,"drc":after,"saved":false,"source":"live-editor","_hint":"Inspect every postCommit error before retrying. Refresh the linked 3D viewer after edits. Coordinates are board-frame on either side, not mirrored screen coordinates."});+    match s+        .client+        .get_items_raw_by_type_codes(vec![PcbObjectTypeCode::new_shape().code])+    {+        Ok(rows) => {+            let live: HashSet<_> = rows.iter().filter_map(|a| graphics::id(&a.value)).collect();+            let remaining: Vec<_> = plan.remove.iter().filter(|id| live.contains(*id)).collect();+            let missing: Vec<_> = created+                .iter()+                .chain(&updated)+                .filter(|id| !live.contains(*id))+                .collect();+            out["deletionVerified"] = json!(remaining.is_empty());+            if !remaining.is_empty() || !missing.is_empty() {+                out["success"] = json!(false);+                out["postCommitReadbackError"] =+                    json!({"remainingRemovedIds":remaining,"missingLiveIds":missing});+            }+        }+        Err(e) => {+            out["success"] = json!(false);+            out["postCommitReadbackError"] = json!(e.to_string());+        }+    }+    match s.revision() {+        Ok(rev) => out["revision"] = json!(rev),+        Err(e) => {+            out["success"] = json!(false);+            out["postCommitRevisionError"] = json!(e.message);+        }+    }+    let ids = created.iter().chain(&updated).cloned().collect::<Vec<_>>();+    if !ids.is_empty() {+        match s.client.get_item_bounding_boxes(ids,false){Ok(rows)=>out["bounds"]=json!(rows.iter().map(|b|json!({"id":b.item_id,"boxMm":[b.x_nm as f64/1e6,b.y_nm as f64/1e6,(b.x_nm+b.width_nm) as f64/1e6,(b.y_nm+b.height_nm) as f64/1e6]})).collect::<Vec<_>>()),Err(e)=>{out["success"]=json!(false);out["postCommitBoundsError"]=json!(e.to_string());}}+    }+    if args["save"] == true {+        match s.client.save_document() {+            Ok(()) => out["saved"] = json!(true),+            Err(e) => {+                out["success"] = json!(false);+                out["postCommitSaveError"] = json!(e.to_string());+            }+        }+    }+    Ok(out)+}
rust/crates/kicad-core/src/lib.rs+2
@@ -32,3 +32,5 @@ pub mod freerouting; pub mod silk_text; pub mod zone_plan; ++pub mod silk_graphics;
rust/crates/kicad-core/src/silk_graphics.rsadded+290
@@ -0,0 +1,290 @@+//! Standalone silkscreen graphics, with stable UUIDs and matching DRC/IPC geometry.+use crate::pcb::RoutingError;+use crate::placement::{distance_bytes, parse_fields, vector2_bytes, wire_message, wire_varint};+use crate::schematic::{new_uuid, parse_node, Document, Node};+use serde_json::{json, Value};+use std::collections::HashSet;++pub const TYPE_URL: &str = "type.googleapis.com/kiapi.board.types.BoardGraphicShape";+fn fail(s: impl Into<String>) -> RoutingError {+    RoutingError::new("silk_graphics_refused", s).with("mutated", json!(false))+}+#[derive(Clone, Debug)]+pub struct Graphic {+    pub id: String,+    pub layer: String,+    pub kind: String,+    pub points: Vec<(f64, f64)>,+    pub stroke: f64,+}+impl Graphic {+    fn parse(v: &Value, id: String) -> Result<Self, RoutingError> {+        let o = v+            .as_object()+            .ok_or_else(|| fail("graphic must be an object"))?;+        if o.keys()+            .any(|k| !["id", "layer", "kind", "points", "stroke"].contains(&k.as_str()))+        {+            return Err(fail("unsupported graphic property"));+        }+        let layer = v["layer"]+            .as_str()+            .filter(|s| matches!(*s, "F.SilkS" | "B.SilkS"))+            .ok_or_else(|| fail("silkscreen layer required"))?;+        let kind = v["kind"].as_str().ok_or_else(|| fail("kind required"))?;+        let count = match kind {+            "line" | "rect" => 2,+            "arc" => 3,+            "bezier" => 4,+            _ => return Err(fail("kind must be line, rect, arc or bezier")),+        };+        let input = v["points"]+            .as_array()+            .filter(|a| a.len() == count)+            .ok_or_else(|| fail("wrong point count"))?;+        let mut points = Vec::new();+        for p in input {+            let a = p+                .as_array()+                .filter(|a| a.len() == 2)+                .ok_or_else(|| fail("point requires x,y"))?;+            let coord = |v: &Value| {+                v.as_f64()+                    .filter(|n| n.is_finite() && n.abs() <= 10000.)+                    .ok_or_else(|| fail("invalid coordinate"))+            };+            points.push((coord(&a[0])?, coord(&a[1])?));+        }+        let stroke = v["stroke"]+            .as_f64()+            .filter(|n| n.is_finite() && *n > 0. && *n <= 5.)+            .ok_or_else(|| fail("stroke must be positive mm, at most 5"))?;+        if points.first() == points.last() {+            return Err(fail("degenerate geometry"));+        }+        if kind == "rect" && (points[0].0 == points[1].0 || points[0].1 == points[1].1) {+            return Err(fail("degenerate rectangle"));+        }+        if kind == "arc" {+            let (a, b, c) = (points[0], points[1], points[2]);+            if ((b.0 - a.0) * (c.1 - a.1) - (b.1 - a.1) * (c.0 - a.0)).abs() < 1e-12 {+                return Err(fail("collinear arc"));+            }+        }+        Ok(Self {+            id,+            layer: layer.into(),+            kind: kind.into(),+            points,+            stroke,+        })+    }+    pub fn node(&self) -> Node {+        let point = |tag: &str, p: (f64, f64)| format!("({tag} {} {})", p.0, p.1);+        let geometry = match self.kind.as_str() {+            "bezier" => format!(+                "(pts {})",+                self.points+                    .iter()+                    .map(|p| point("xy", *p))+                    .collect::<Vec<_>>()+                    .join(" ")+            ),+            "arc" => format!(+                "{} {} {}",+                point("start", self.points[0]),+                point("mid", self.points[1]),+                point("end", self.points[2])+            ),+            _ => format!(+                "{} {}",+                point("start", self.points[0]),+                point("end", self.points[1])+            ),+        };+        let kind = if self.kind == "bezier" {+            "curve"+        } else {+            &self.kind+        };+        parse_node(&format!(+            "(gr_{kind} {geometry} (stroke (width {}) (type solid)) {} (layer {}) (uuid {}))",+            self.stroke,+            if self.kind == "rect" {+                "(fill none)"+            } else {+                ""+            },+            serde_json::to_string(&self.layer).unwrap(),+            serde_json::to_string(&self.id).unwrap()+        ))+        .unwrap()+    }+    pub fn bytes(&self, layer: i32) -> Vec<u8> {+        let mut stroke = Vec::new();+        wire_message(+            &mut stroke,+            1,+            &distance_bytes((self.stroke * 1e6).round() as i64),+        );+        wire_varint(&mut stroke, 2, 2);+        let mut fill = Vec::new();+        wire_varint(&mut fill, 1, 1);+        let mut attrs = Vec::new();+        wire_message(&mut attrs, 1, &stroke);+        wire_message(&mut attrs, 2, &fill);+        let mut geometry = Vec::new();+        for (i, p) in self.points.iter().enumerate() {+            wire_message(+                &mut geometry,+                (i + 1) as u32,+                &vector2_bytes((p.0 * 1e6).round() as i64, (p.1 * 1e6).round() as i64),+            );+        }+        let mut shape = Vec::new();+        wire_message(&mut shape, 3, &attrs);+        wire_message(+            &mut shape,+            match self.kind.as_str() {+                "line" => 4,+                "rect" => 5,+                "arc" => 6,+                _ => 9,+            },+            &geometry,+        );+        let mut id = Vec::new();+        wire_message(&mut id, 1, self.id.as_bytes());+        let mut out = Vec::new();+        wire_message(&mut out, 1, &shape);+        wire_varint(&mut out, 2, layer as u64);+        wire_message(&mut out, 4, &id);+        wire_varint(&mut out, 5, 1);+        out+    }+}+pub fn id(bytes: &[u8]) -> Option<String> {+    let fields = parse_fields(bytes).ok()?;+    let id = fields.iter().find(|f| f.number == 4)?;+    let nested = parse_fields(id.data).ok()?;+    String::from_utf8(nested.iter().find(|f| f.number == 1)?.data.to_vec()).ok()+}+pub struct Plan {+    pub candidate: String,+    pub create: Vec<Graphic>,+    pub update: Vec<Graphic>,+    pub remove: Vec<String>,+}+pub fn prepare(text: &str, args: &Value) -> Result<Plan, RoutingError> {+    let mut doc = Document::parse(text).map_err(fail)?;+    let mut touched = HashSet::new();+    let editable = |id: &str| -> Result<(), RoutingError> {+        let n = doc+            .root+            .children()+            .iter()+            .find(|n| n.child_value("uuid").as_deref() == Some(id))+            .ok_or_else(|| fail("unknown standalone graphic ID"))?;+        if !matches!(+            n.head(),+            Some("gr_line" | "gr_rect" | "gr_arc" | "gr_curve")+        ) || !matches!(+            n.child_value("layer").as_deref(),+            Some("F.SilkS" | "B.SilkS")+        ) || n+            .child("locked")+            .map(|c| c.arg(0).as_deref() != Some("no"))+            .unwrap_or(false)+        {+            return Err(fail(+                "ID is locked or is not a supported standalone silkscreen graphic",+            ));+        }+        Ok(())+    };+    let rows = |key: &str| -> Result<Vec<Value>, RoutingError> {+        match args.get(key) {+            None => Ok(vec![]),+            Some(v) => v+                .as_array()+                .filter(|a| a.len() <= 2000)+                .cloned()+                .ok_or_else(|| fail("batch must be an array of at most 2000 items")),+        }+    };+    let mut create = Vec::new();+    let mut update = Vec::new();+    let mut remove = Vec::new();+    for v in rows("create")? {+        if v.get("id").is_some() {+            return Err(fail("create IDs are bridge-assigned"));+        }+        create.push(Graphic::parse(&v, new_uuid())?);+    }+    for v in rows("update")? {+        let id = v["id"].as_str().ok_or_else(|| fail("update needs id"))?;+        editable(id)?;+        if !touched.insert(id.to_string()) {+            return Err(fail("duplicate ID"));+        }+        update.push(Graphic::parse(&v, id.into())?);+    }+    for v in rows("remove")? {+        let id = v.as_str().ok_or_else(|| fail("remove needs IDs"))?;+        editable(id)?;+        if !touched.insert(id.to_string()) {+            return Err(fail("duplicate ID"));+        }+        remove.push(id.into());+    }+    if create.is_empty() && update.is_empty() && remove.is_empty() {+        return Err(fail("empty batch"));+    }+    let children = doc.root.children_mut().unwrap();+    children.retain(|n| {+        !n.child_value("uuid")+            .map(|id| touched.contains(&id))+            .unwrap_or(false)+    });+    children.extend(create.iter().chain(&update).map(Graphic::node));+    Ok(Plan {+        candidate: doc.to_text(),+        create,+        update,+        remove,+    })+}+#[cfg(test)]+mod tests {+    use super::*;+    #[test]+    fn curves_and_borders_preserve_ids_and_coordinates() {+        for (kind, points) in [+            ("line", json!([[-2, 3], [4, 5]])),+            ("rect", json!([[-2, 3], [4, 5]])),+            ("arc", json!([[-2, 3], [0, 5], [4, 5]])),+            ("bezier", json!([[-2, 3], [0, 5], [3, 6], [4, 5]])),+        ] {+            let g = Graphic::parse(+                &json!({"kind":kind,"layer":"B.SilkS","points":points,"stroke":0.1}),+                "same-id".into(),+            )+            .unwrap();+            assert_eq!(id(&g.bytes(38)).as_deref(), Some("same-id"));+            assert_eq!(g.node().child_value("uuid").as_deref(), Some("same-id"));+            assert!(g.node().to_text().contains("-2 3"));+        }+    }+    #[test]+    fn refuses_foreign_locked_and_degenerate_shapes() {+        let text="(kicad_pcb (gr_line (uuid a) (layer F.Cu)) (gr_rect (uuid b) (layer F.SilkS) (locked yes)))";+        for id in ["a", "b", "unknown"] {+            assert!(prepare(text, &json!({"remove":[id]})).is_err());+        }+        assert!(Graphic::parse(+            &json!({"kind":"arc","layer":"F.SilkS","points":[[0,0],[1,1],[2,2]],"stroke":0.1}),+            "x".into()+        )+        .is_err());+    }+}