← All Pull Requests

Native silkscreen leader curves and borders with stable IDs and Undo #15

Merged opened by John Lauer 2026-09-18
Merges astra/native-silk-graphics → master

AI Flow needs its silk leader lines and contact borders to land through native KiCad edits, not private offline rewriting. This adds kicad_silk_graphics_batch for standalone line, rectangle, three-point arc and cubic Bezier create/update/remove. Both silk layers, explicit board-frame mm, bridge-assigned create UUIDs and preserved update UUIDs, solid strokes and unfilled shapes. Locked, foreign, unsupported or degenerate input refuses.

Full candidate native DRC includes warnings before one Undo transaction. Native creation/update replies must preserve requested IDs; post-commit readback verifies deleted IDs absent and created/updated IDs present, and returns native bounds/revision. Post-commit errors explicitly prevent successful certification. This is manufactured silk, not ghost calculation boxes.

Base 0fad23e1d0794f30e7e6c30ef8d09c03d0bb7b9a, current 1.0.24. Rebased the focused patch after PR #14 shipped; its refill acknowledgement and owner's catalog/skill changes are preserved. Real branch PR. Production check and 258 tests pass (25 bridge + 233 core). No existing module or verb deleted. New modules rustfmt-formatted. Protobuf geometry fields match the pinned kicad-ipc-rs 0.5.1 schema and use the bridge's existing wire helpers.

Native Windows acceptance is NOT yet performed. docs/native-silk-graphics.md specifies both-face curve/border geometry, IDs, mixed updates/removals, one Undo, mask-warning refusal and 3D inspection. Please run on the owning development bridge before publication, especially BoardGraphicShape Create/Update UUID preservation and curve control-point fidelity. No shared runtime replaced. Partial #104: footprint fields, mask/body obstacles and automatic linked refresh remain separate.

Diff Skip to comments (1)

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

Comments

John Lauer 2026-09-18

Merged on the wiki (9b24ed4), shipped as KiCad Bridge 1.0.25 (insiders), accepted on ConfRoomROG (KiCad 10.0.5) against the checklist in docs/native-silk-graphics.md. Everything passes.

check result
refusals: a zone id, layer F.Cu, a collinear arc, a zero-width rect each silk_graphics_refused with the exact reason, nothing mutated
a line across the D1/R1 area drc_rejected naming silk over copper and the two overlaps; a Bezier over bottom copper likewise
create line + rect on F.SilkS, arc + Bezier on B.SilkS in one call committed:true, undoSteps:1, four ids kept, returned revision equals a fresh read. KiCad's own bounds match the requested geometry: line 107.92..112.08 x 60.42..60.58 (the 4 mm line plus half the 0.15 stroke), arc 115.92..118.08 x 60.42..61.58, Bezier inside its control hull
update the rect by UUID to 108..111 updatedIds carries the same id, bounds 107.92..111.08
remove line + arc in one call removedIds both, deletionVerified:true
one Undo both are back (a dry-run remove of the pair is accepted again)

Merge note: the diff text the wiki served for this PR was malformed at line 508 (inside the new silk_graphics.rs), so I took all six files byte-for-byte from the astra/native-silk-graphics branch clone; against 1.0.24 that is 18 added lines in the three existing files plus the three new ones, zero removals. Nothing to change on your side; noting it in case the same shows on the page's diff view.

Closing.

Log in to comment.