From db15d8d583f890df469e4d95e2f133e2a6e55635 Mon Sep 17 00:00:00 2001 From: rcoenen <753704+rcoenen@users.noreply.github.com> Date: Sat, 1 Aug 2026 13:17:42 -0400 Subject: [PATCH] fix(tools): create vectors from SVG paths (#440) - Accept SVG path data with tight curve-aware node bounds - Validate complete VectorNetwork topology while preserving JSON input - Reject empty and malformed paths before creating blank layers Co-authored-by: Danila Poyarkov --- CHANGELOG.md | 1 + packages/core/src/tools/create/vector.ts | 76 ++++++++++-- packages/scene-graph/src/parse-path.ts | 4 +- packages/scene-graph/src/vector-network.ts | 134 ++++++++++++++++----- tests/engine/io/svg/path-parse.test.ts | 4 + tests/engine/tools/create.test.ts | 88 ++++++++++++++ tests/engine/vector/validate.test.ts | 21 +++- 7 files changed, 286 insertions(+), 42 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 976222769..7bd888aba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ ### Fixed +- Let AI and MCP tools create arbitrary vectors from SVG path data without leaving blank layers after invalid input. (#440) - Show stroke colors and weights in AI visual descriptions. (#447) - Stop warning AI agents that supported inline SVG attributes were ignored. (#445) - Help AI agents discover every shape supported by `create_shape`. (#448) diff --git a/packages/core/src/tools/create/vector.ts b/packages/core/src/tools/create/vector.ts index 143dc45e7..b5614d5ea 100644 --- a/packages/core/src/tools/create/vector.ts +++ b/packages/core/src/tools/create/vector.ts @@ -1,40 +1,90 @@ import { safeDestr } from 'destr' -import { normalizeVectorNetwork, validateVectorNetwork } from '@open-pencil/scene-graph' +import { + normalizeVectorNetwork, + transformVectorNetwork, + validateVectorNetwork +} from '@open-pencil/scene-graph' import type { VectorNetwork } from '@open-pencil/scene-graph' +import { parseSVGPath } from '@open-pencil/scene-graph/parse-path' import { parseColor } from '#core/color' import { defineTool, nodeSummary } from '#core/tools/schema' +import { computeAccurateBounds } from '#core/vector/curve-math' + +interface ParsedVectorPath { + network: VectorNetwork + size: { width: number; height: number } | null +} + +type VectorPathResult = ParsedVectorPath | { error: string } + +function parseSVGVectorPath(path: string): VectorPathResult { + const network = parseSVGPath(path) + if (network.segments.length === 0) { + return { error: 'SVG path data must contain at least one drawable segment' } + } + + const bounds = computeAccurateBounds(network) + return { + network: transformVectorNetwork([1, 0, -bounds.x, 0, 1, -bounds.y, 0, 0, 1], network), + size: { width: bounds.width, height: bounds.height } + } +} + +function parseVectorNetworkJSON(path: string): VectorPathResult { + let parsed: unknown + try { + parsed = safeDestr(path) + } catch { + return { error: 'Invalid VectorNetwork JSON' } + } + + const errors = validateVectorNetwork(parsed) + if (errors.length > 0) return { error: `Invalid VectorNetwork: ${errors.join('; ')}` } + + return { network: normalizeVectorNetwork(parsed as VectorNetwork), size: null } +} + +function parseVectorPath(path: string): VectorPathResult { + const trimmed = path.trim() + if (/^[Mm]/.test(trimmed)) return parseSVGVectorPath(trimmed) + return parseVectorNetworkJSON(trimmed) +} export const createVector = defineTool({ name: 'create_vector', mutates: true, - description: 'Create a vector node with optional path data.', + description: 'Create a vector node from SVG path data or a VectorNetwork.', params: { x: { type: 'number', description: 'X position', required: true }, y: { type: 'number', description: 'Y position', required: true }, name: { type: 'string', description: 'Node name' }, - path: { type: 'string', description: 'VectorNetwork JSON' }, + path: { + type: 'string', + description: + 'SVG path data (preferred, e.g. "M0 0 L100 0 L50 80 Z") or VectorNetwork JSON, e.g. {"vertices":[{"x":0,"y":0},{"x":10,"y":0}],"segments":[{"start":0,"end":1}],"regions":[]}' + }, fill: { type: 'color', description: 'Fill color (hex)' }, stroke: { type: 'color', description: 'Stroke color (hex)' }, stroke_weight: { type: 'number', description: 'Stroke weight' }, parent_id: { type: 'string', description: 'Parent node ID' } }, execute: (figma, args) => { + let parsedPath: ParsedVectorPath | null = null + if (args.path !== undefined) { + const parsed = parseVectorPath(args.path) + if ('error' in parsed) return parsed + parsedPath = parsed + } + const node = figma.createVector() node.x = args.x node.y = args.y if (args.name) node.name = args.name - if (args.path) { - let parsed: VectorNetwork - try { - parsed = safeDestr(args.path) - } catch { - return { error: 'Invalid JSON in path parameter' } - } - const errors = validateVectorNetwork(parsed) - if (errors.length > 0) return { error: `Invalid VectorNetwork: ${errors.join('; ')}` } - figma.graph.updateNode(node.id, { vectorNetwork: normalizeVectorNetwork(parsed) }) + if (parsedPath) { + figma.graph.updateNode(node.id, { vectorNetwork: parsedPath.network }) + if (parsedPath.size) node.resize(parsedPath.size.width, parsedPath.size.height) } if (args.fill) { node.fills = [{ type: 'SOLID', color: parseColor(args.fill), opacity: 1, visible: true }] diff --git a/packages/scene-graph/src/parse-path.ts b/packages/scene-graph/src/parse-path.ts index 60fa27884..87f3fb8f3 100644 --- a/packages/scene-graph/src/parse-path.ts +++ b/packages/scene-graph/src/parse-path.ts @@ -76,7 +76,9 @@ export function parseSVGPath(d: string, windingRule: WindingRule = 'NONZERO'): V addSegment(x1, y1, x2, y2, cp1x, cp1y, cp2x, cp2y) } - const normalized = svgpath(d).abs().unshort().unarc() + const parsed = svgpath(d) + if ('err' in parsed && parsed.err) return { vertices, segments, regions: [] } + const normalized = parsed.abs().unshort().unarc() normalized.iterate((seg) => { const cmd = seg[0] diff --git a/packages/scene-graph/src/vector-network.ts b/packages/scene-graph/src/vector-network.ts index 96b6f75b4..5924160fc 100644 --- a/packages/scene-graph/src/vector-network.ts +++ b/packages/scene-graph/src/vector-network.ts @@ -96,38 +96,118 @@ export function cloneVectorNetwork(vn: VectorNetwork): VectorNetwork { * Validate a VectorNetwork structure, returning an array of error messages. * Empty array means the network is valid. */ -export function validateVectorNetwork(vn: VectorNetwork): string[] { +export function validateVectorNetwork(value: unknown): string[] { + if (!isRecord(value)) return ['network must be an object'] + if (!Array.isArray(value.vertices)) return ['vertices must be an array'] + if (!Array.isArray(value.segments)) return ['segments must be an array'] + const errors: string[] = [] - if (!Array.isArray(vn.vertices)) { - errors.push('vertices must be an array') - return errors - } - if (!Array.isArray(vn.segments)) { - errors.push('segments must be an array') - return errors - } - if (!Array.isArray(vn.regions)) errors.push('regions must be an array') - const vertexCount = vn.vertices.length - for (let i = 0; i < vn.vertices.length; i++) { - const v = vn.vertices[i] - if (typeof v.x !== 'number' || typeof v.y !== 'number') { - errors.push(`vertex[${i}]: x and y must be numbers`) - } - } - for (let i = 0; i < vn.segments.length; i++) { - const s = vn.segments[i] - if (typeof s.start !== 'number' || typeof s.end !== 'number') { - errors.push(`segment[${i}]: start and end must be numbers`) - } else { - if (s.start < 0 || s.start >= vertexCount) - errors.push(`segment[${i}]: start index ${s.start} out of range`) - if (s.end < 0 || s.end >= vertexCount) - errors.push(`segment[${i}]: end index ${s.end} out of range`) - } + validateVertices(value.vertices, errors) + validateSegments(value.segments, value.vertices.length, errors) + if (Array.isArray(value.regions)) { + validateRegions(value.regions, value.segments.length, errors) + } else { + errors.push('regions must be an array') } return errors } +function validateVertices(vertices: unknown[], errors: string[]): void { + for (let index = 0; index < vertices.length; index++) { + if (!isFiniteVector(vertices[index])) { + errors.push(`vertex[${index}]: x and y must be finite numbers`) + } + } +} + +function validateSegments(segments: unknown[], vertexCount: number, errors: string[]): void { + for (let index = 0; index < segments.length; index++) { + const segment = segments[index] + if (!isRecord(segment)) { + errors.push(`segment[${index}] must be an object`) + continue + } + if (!isInteger(segment.start) || !isInteger(segment.end)) { + errors.push(`segment[${index}]: start and end must be integers`) + continue + } + if (segment.start < 0 || segment.start >= vertexCount) { + errors.push(`segment[${index}]: start index ${segment.start} out of range`) + } + if (segment.end < 0 || segment.end >= vertexCount) { + errors.push(`segment[${index}]: end index ${segment.end} out of range`) + } + validateSegmentTangents(segment, index, errors) + } +} + +function validateSegmentTangents( + segment: Record, + index: number, + errors: string[] +): void { + for (const tangentKey of ['tangentStart', 'tangentEnd'] as const) { + const tangent = segment[tangentKey] + if (tangent !== undefined && !isFiniteVector(tangent)) { + errors.push(`segment[${index}]: ${tangentKey} must contain finite x and y numbers`) + } + } +} + +function validateRegions(regions: unknown[], segmentCount: number, errors: string[]): void { + for (let regionIndex = 0; regionIndex < regions.length; regionIndex++) { + const region = regions[regionIndex] + if (!isRecord(region) || !Array.isArray(region.loops)) { + errors.push(`region[${regionIndex}]: loops must be an array`) + continue + } + if (region.windingRule !== 'NONZERO' && region.windingRule !== 'EVENODD') { + errors.push(`region[${regionIndex}]: windingRule must be NONZERO or EVENODD`) + } + validateRegionLoops(region.loops, regionIndex, segmentCount, errors) + } +} + +function validateRegionLoops( + loops: unknown[], + regionIndex: number, + segmentCount: number, + errors: string[] +): void { + for (let loopIndex = 0; loopIndex < loops.length; loopIndex++) { + const loop = loops[loopIndex] + if (!Array.isArray(loop)) { + errors.push(`region[${regionIndex}].loop[${loopIndex}] must be an array`) + continue + } + for (const segmentIndex of loop) { + if (!isInteger(segmentIndex) || segmentIndex < 0 || segmentIndex >= segmentCount) { + errors.push( + `region[${regionIndex}].loop[${loopIndex}]: segment index ${String(segmentIndex)} out of range` + ) + } + } + } +} + +function isFiniteVector(value: unknown): value is Vector { + return ( + isRecord(value) && + typeof value.x === 'number' && + Number.isFinite(value.x) && + typeof value.y === 'number' && + Number.isFinite(value.y) + ) +} + +function isInteger(value: unknown): value is number { + return typeof value === 'number' && Number.isInteger(value) +} + +function isRecord(value: unknown): value is Record { + return typeof value === 'object' && value !== null +} + /** * Ensure every segment has tangentStart/tangentEnd. * Missing tangents default to {x:0, y:0} (straight line segments). diff --git a/tests/engine/io/svg/path-parse.test.ts b/tests/engine/io/svg/path-parse.test.ts index 0e5a1dae1..6ae296f20 100644 --- a/tests/engine/io/svg/path-parse.test.ts +++ b/tests/engine/io/svg/path-parse.test.ts @@ -98,6 +98,10 @@ describe('parseSVGPath', () => { expect(vn.segments.length).toBeGreaterThan(5) }) + test('malformed input does not preserve partial geometry', () => { + expect(parseSVGPath('M0 0 L')).toEqual({ vertices: [], segments: [], regions: [] }) + }) + test('round-trip: parse → export produces valid SVG paths', () => { const d = 'M0 0 L100 0 L100 100 L0 100 Z' const vn = parseSVGPath(d) diff --git a/tests/engine/tools/create.test.ts b/tests/engine/tools/create.test.ts index e1c80a087..0f115ceca 100644 --- a/tests/engine/tools/create.test.ts +++ b/tests/engine/tools/create.test.ts @@ -116,3 +116,91 @@ describe('render', () => { expect(result.textAlignHorizontal).toBe('CENTER') }) }) + +describe('create_vector', () => { + test('creates tightly bounded curved geometry from SVG path data', () => { + const { figma, graph } = setupToolTest() + const result = getTool('create_vector').execute(figma, { + x: 100, + y: 200, + name: 'Curved outline', + path: 'M10 20 C20 20 30 40 40 40 L10 40 Z' + }) as ToolResult + const node = expectDefined( + graph.getNode(expectDefined(result.id, 'created vector id')), + 'created vector' + ) + const network = expectDefined(node.vectorNetwork, 'SVG vector network') + + expect({ x: node.x, y: node.y, width: node.width, height: node.height }).toEqual({ + x: 100, + y: 200, + width: 30, + height: 20 + }) + expect(network.vertices).toEqual([ + { x: 0, y: 0 }, + { x: 30, y: 20 }, + { x: 0, y: 20 } + ]) + expect(network.segments[0]?.tangentStart).toEqual({ x: 10, y: 0 }) + expect(network.segments[0]?.tangentEnd).toEqual({ x: -10, y: 0 }) + expect(network.regions).toEqual([{ windingRule: 'NONZERO', loops: [[0, 1, 2]] }]) + }) + + test('preserves complete VectorNetwork JSON topology', () => { + const { figma, graph } = setupToolTest() + const result = getTool('create_vector').execute(figma, { + x: 0, + y: 0, + path: JSON.stringify({ + vertices: [ + { x: 0, y: 0 }, + { x: 10, y: 0 }, + { x: 0, y: 10 } + ], + segments: [ + { start: 0, end: 1, tangentStart: { x: 1, y: 0 }, tangentEnd: { x: -1, y: 0 } }, + { start: 1, end: 2 }, + { start: 2, end: 0 } + ], + regions: [{ windingRule: 'EVENODD', loops: [[0, 1, 2]] }] + }) + }) as ToolResult + const node = expectDefined( + graph.getNode(expectDefined(result.id, 'created vector id')), + 'created vector' + ) + + expect(node.vectorNetwork).toEqual({ + vertices: [ + { x: 0, y: 0 }, + { x: 10, y: 0 }, + { x: 0, y: 10 } + ], + segments: [ + { + start: 0, + end: 1, + tangentStart: { x: 1, y: 0 }, + tangentEnd: { x: -1, y: 0 } + }, + { start: 1, end: 2, tangentStart: { x: 0, y: 0 }, tangentEnd: { x: 0, y: 0 } }, + { start: 2, end: 0, tangentStart: { x: 0, y: 0 }, tangentEnd: { x: 0, y: 0 } } + ], + regions: [{ windingRule: 'EVENODD', loops: [[0, 1, 2]] }] + }) + }) + + test('rejects invalid supplied paths before creating a node', () => { + const { figma, graph } = setupToolTest() + const tool = getTool('create_vector') + const before = graph.nodes.size + + for (const path of ['', 'not a path', 'null', 'M0 0 L']) { + const result = tool.execute(figma, { x: 0, y: 0, name: 'Ghost', path }) as ToolResult + expect(result.error).toBeDefined() + expect(graph.nodes.size).toBe(before) + } + }) +}) diff --git a/tests/engine/vector/validate.test.ts b/tests/engine/vector/validate.test.ts index 1de7e9e0f..c443ade57 100644 --- a/tests/engine/vector/validate.test.ts +++ b/tests/engine/vector/validate.test.ts @@ -51,6 +51,25 @@ describe('validateVectorNetwork', () => { regions: [] } as VectorNetwork const errors = validateVectorNetwork(network) - expect(errors[0]).toContain('x and y must be numbers') + expect(errors[0]).toContain('x and y must be finite numbers') + }) + + test('rejects non-object input without throwing', () => { + expect(validateVectorNetwork(null)).toEqual(['network must be an object']) + expect(validateVectorNetwork('not a network')).toEqual(['network must be an object']) + }) + + test('rejects invalid region topology', () => { + const errors = validateVectorNetwork({ + vertices: [ + { x: 0, y: 0 }, + { x: 10, y: 0 } + ], + segments: [{ start: 0, end: 1 }], + regions: [{ windingRule: 'INVALID', loops: [[1]] }] + }) + + expect(errors).toContain('region[0]: windingRule must be NONZERO or EVENODD') + expect(errors).toContain('region[0].loop[0]: segment index 1 out of range') }) })