From f862beb08d6cebc2e496b214ca7f19817d8b7ac0 Mon Sep 17 00:00:00 2001 From: Joseph Cumines Date: Fri, 19 Jun 2026 09:54:27 +1000 Subject: [PATCH 1/4] Add heuristic overlap detection tool and CLI Implement the analyze overlaps CLI command, RPC endpoint, and ToolDef to enable heuristic detection of visual overlaps, parent overflows, and overlay patterns. Geometry computation now uses world-matrix visual bounds to handle nested ancestor rotations and clipping frames correctly. Update file-backed CLI commands to use Node fs/promises instead of Bun runtime file APIs. This is required for the published CLI to function when installed and executed in a standard Node environment. Add the package.json subpath export to the core package for metadata consumers. --- CHANGELOG.md | 5 + README.md | 1 + packages/cli/src/commands/analyze/index.ts | 4 +- packages/cli/src/commands/analyze/overlaps.ts | 264 +++++++++ packages/cli/src/commands/convert.ts | 3 +- packages/cli/src/commands/eval.ts | 4 +- packages/cli/src/commands/export.ts | 3 +- packages/cli/src/headless.ts | 4 +- packages/core/package.json | 1 + packages/core/src/geometry.ts | 99 +++- packages/core/src/rpc/analyze-commands.ts | 14 + packages/core/src/rpc/commands.ts | 4 +- packages/core/src/rpc/index.ts | 2 + packages/core/src/tools/analyze.ts | 9 + .../src/tools/analyze/overlaps/helpers.ts | 551 ++++++++++++++++++ .../core/src/tools/analyze/overlaps/index.ts | 394 +++++++++++++ .../core/src/tools/analyze/overlaps/params.ts | 47 ++ packages/core/src/tools/index.ts | 8 + packages/core/src/tools/registry-extended.ts | 2 + tests/engine/cli/eval.test.ts | 4 +- tests/engine/cli/overlaps.test.ts | 162 +++++ .../tools/analyze/overlaps/bounds.test.ts | 181 ++++++ .../engine/tools/analyze/overlaps/helpers.ts | 57 ++ .../tools/analyze/overlaps/index.test.ts | 420 +++++++++++++ .../tools/analyze/overlaps/page-scope.test.ts | 90 +++ tests/helpers/paths.ts | 10 + 26 files changed, 2334 insertions(+), 9 deletions(-) create mode 100644 packages/cli/src/commands/analyze/overlaps.ts create mode 100644 packages/core/src/tools/analyze/overlaps/helpers.ts create mode 100644 packages/core/src/tools/analyze/overlaps/index.ts create mode 100644 packages/core/src/tools/analyze/overlaps/params.ts create mode 100644 tests/engine/cli/overlaps.test.ts create mode 100644 tests/engine/tools/analyze/overlaps/bounds.test.ts create mode 100644 tests/engine/tools/analyze/overlaps/helpers.ts create mode 100644 tests/engine/tools/analyze/overlaps/index.test.ts create mode 100644 tests/engine/tools/analyze/overlaps/page-scope.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index eaeb0de22..4ab0b1f08 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ - Add JSX authoring support for components, component sets, and instances. - Add type-validated `bindVariable`/`unbindVariable` with event emission and indexed binding format (`fills/N/color` instead of `fills[N]`). - Add `unbind_variable` MCP tool for removing variable bindings. +- Add `openpencil analyze overlaps`, the `analyze_overlaps` RPC command, and the `analyze_overlaps` ToolDef for heuristic overlap detection. The command reports sibling overlaps, children overflowing non-clipping parents, and overlay/backdrop patterns, with filters for page/page ID, scope, category, severity, min area/ratio, node type, hidden/locked/absolute nodes, result limit, and `--json` output. +- Add overlap analysis exports for automation consumers, including `computeOverlaps`, `analyzeOverlaps`, overlap result types, and parameter parsers from core subpath exports. +- Add world-matrix visual bounds to overlap analysis, covering vector/stroke/text geometry, ancestor clipping, rotated clipping frames, and nested ancestor rotations. +- Add the `@open-pencil/core/package.json` subpath export for package metadata consumers. ### Fixes @@ -18,6 +22,7 @@ - Improve Figma boolean imports by preserving XOR operations as editable exclude nodes and falling back to imported fill geometry when boolean path reconstruction cannot produce a path. - Preserve rotated Figma transform origins for imported vector nodes. - Render complex text fills through vector glyph outlines so imported Figma text can use the normal fill pipeline for gradients, images, patterns, and other non-solid paints. +- Fix file-backed CLI commands (`convert`, `eval --output`, `export`) to use Node `fs/promises` instead of Bun runtime APIs, so the published CLI works when installed and run under Node. ## 0.13.2 — 2026-05-30 diff --git a/README.md b/README.md index f2c3b6fa1..a369dcdce 100644 --- a/README.md +++ b/README.md @@ -111,6 +111,7 @@ openpencil analyze colors design.fig openpencil analyze typography design.fig openpencil analyze spacing design.fig openpencil analyze clusters design.fig +openpencil analyze overlaps design.fig openpencil variables design.fig ``` diff --git a/packages/cli/src/commands/analyze/index.ts b/packages/cli/src/commands/analyze/index.ts index a4cb7818f..39b0543b4 100644 --- a/packages/cli/src/commands/analyze/index.ts +++ b/packages/cli/src/commands/analyze/index.ts @@ -2,6 +2,7 @@ import { defineCommand } from 'citty' import clusters from './clusters' import colors from './colors' +import overlaps from './overlaps' import spacing from './spacing' import typography from './typography' @@ -11,6 +12,7 @@ export default defineCommand({ colors, typography, spacing, - clusters + clusters, + overlaps } }) diff --git a/packages/cli/src/commands/analyze/overlaps.ts b/packages/cli/src/commands/analyze/overlaps.ts new file mode 100644 index 000000000..6f5ed9c00 --- /dev/null +++ b/packages/cli/src/commands/analyze/overlaps.ts @@ -0,0 +1,264 @@ +import { defineCommand } from 'citty' + +import type { AnalyzeOverlapsResult } from '@open-pencil/core/rpc' +import { + VALID_OVERLAP_CATEGORIES, + VALID_OVERLAP_SCOPES, + VALID_OVERLAP_SEVERITIES, + parseOverlapCategories, + parseOverlapScope, + parseOverlapSeverity +} from '@open-pencil/core/tools' + +import { bold, fail, fmtList, fmtSummary, kv } from '#cli/format' +import { loadRpcData } from '#cli/rpc-data' + +function validateScope(scope: string): string | undefined { + const normalized = scope.toLowerCase() + return (VALID_OVERLAP_SCOPES as readonly string[]).includes(normalized) + ? undefined + : `Invalid scope "${scope}". Must be one of: ${[...VALID_OVERLAP_SCOPES].join(', ')}` +} + +function validateSeverity(severity: string): string | undefined { + const normalized = severity.toLowerCase() + return (VALID_OVERLAP_SEVERITIES as readonly string[]).includes(normalized) + ? undefined + : `Invalid severity "${severity}". Must be one of: ${[...VALID_OVERLAP_SEVERITIES].join(', ')}` +} + +function validateMinRatio(minRatio: number | undefined): string | undefined { + if (minRatio === undefined) return undefined + if (Number.isNaN(minRatio) || minRatio < 0 || minRatio > 1) { + return '--min-ratio must be a number between 0.0 and 1.0' + } + return undefined +} + +function validateLimit(limit: string): string | undefined { + const value = Number(limit) + if (!Number.isFinite(value) || !Number.isInteger(value) || value <= 0) { + return '--limit must be a positive integer' + } + return undefined +} + +function validateMinArea(minArea: string): string | undefined { + const value = Number(minArea) + if (!Number.isFinite(value) || value < 0) { + return '--min-area must be a non-negative number' + } + return undefined +} + +function validateCategory(category: string): string | undefined { + const values = category + .split(',') + .map((c) => c.trim().toLowerCase()) + .filter((c) => c.length > 0) + if (values.length === 0) return undefined + const validCount = (parseOverlapCategories(category) ?? []).length + if (validCount === 0) { + return `Invalid categories: ${values.join(', ')}. Must be one or more of: ${[...VALID_OVERLAP_CATEGORIES].join(', ')}` + } + const invalid = values.filter((c) => !(VALID_OVERLAP_CATEGORIES as readonly string[]).includes(c)) + if (invalid.length > 0) { + return `Invalid categor${invalid.length === 1 ? 'y' : 'ies'}: ${invalid.join(', ')}. Must be one or more of: ${[...VALID_OVERLAP_CATEGORIES].join(', ')}` + } + return undefined +} + +function collectValidationError(args: { + scope: string + severity: string + category?: string + limit: string + 'min-area'?: string + 'min-ratio'?: string +}): string | undefined { + return ( + validateScope(args.scope) ?? + validateSeverity(args.severity) ?? + validateLimit(args.limit) ?? + (args.category ? validateCategory(args.category) : undefined) ?? + (args['min-area'] !== undefined ? validateMinArea(args['min-area']) : undefined) ?? + validateMinRatio(args['min-ratio'] ? Number(args['min-ratio']) : undefined) + ) +} + +function buildRpcArgs(args: { + scope: string + severity: string + category?: string + limit: string + 'min-area'?: string + 'min-ratio'?: string + 'include-hidden'?: boolean + 'include-locked'?: boolean + 'include-absolute'?: boolean + page?: string + 'page-id'?: string + type?: string +}): { rpcArgs: Record; error?: string } { + const error = collectValidationError(args) + if (error) return { rpcArgs: {}, error } + + const minAreaRaw = args['min-area'] + const minArea = minAreaRaw ? Number(minAreaRaw) : undefined + const minRatio = args['min-ratio'] ? Number(args['min-ratio']) : undefined + + const rpcArgs: Record = { + scope: parseOverlapScope(args.scope) ?? 'all', + severity: parseOverlapSeverity(args.severity) ?? 'info', + limit: Number(args.limit) + } + if (args.category) rpcArgs.category = args.category + if (minArea !== undefined) rpcArgs.min_area = minArea + if (minRatio !== undefined) rpcArgs.min_ratio = minRatio + if (args['include-hidden']) rpcArgs.include_hidden = true + if (args['include-locked']) rpcArgs.include_locked = true + if (args['include-absolute']) rpcArgs.include_absolute = true + if (args.page) rpcArgs.page = args.page + if (args['page-id']) rpcArgs.page_id = args['page-id'] + if (args.type) rpcArgs.type = args.type + + return { rpcArgs } +} + +export default defineCommand({ + meta: { description: 'Detect visual overlaps and layout overflows' }, + args: { + file: { + type: 'positional', + description: '.fig file path (omit to connect to running app)', + required: false + }, + scope: { + type: 'string', + description: + 'Which pairs to inspect: all, same-parent, cross-parent, top-level, inside-parent', + default: 'all' + }, + category: { + type: 'string', + description: 'Comma-separated categories: sibling-overlap, parent-overflow, overlay' + }, + severity: { + type: 'string', + description: 'Minimum severity to include: critical, major, minor, info', + default: 'info' + }, + 'min-area': { + type: 'string', + description: 'Minimum overlap area in square pixels' + }, + 'min-ratio': { + type: 'string', + description: 'Minimum overlap ratio relative to the smaller node, 0.0–1.0' + }, + 'include-hidden': { + type: 'boolean', + description: 'Include hidden nodes in the analysis' + }, + 'include-locked': { + type: 'boolean', + description: 'Include locked nodes in the analysis' + }, + 'include-absolute': { + type: 'boolean', + description: 'Include absolutely-positioned nodes in the analysis' + }, + page: { + type: 'string', + description: 'Limit analysis to nodes on the named page' + }, + 'page-id': { + type: 'string', + description: 'Limit analysis to nodes on the page with this stable ID' + }, + type: { + type: 'string', + description: 'Comma-separated node types to analyze, e.g. FRAME,TEXT' + }, + limit: { + type: 'string', + description: 'Maximum overlap findings to show', + default: '100' + }, + json: { type: 'boolean', description: 'Output as JSON' } + }, + async run({ args }) { + const { rpcArgs, error } = buildRpcArgs(args) + if (error) { + console.error(fail(error)) + process.exit(1) + } + + const data = await loadRpcData(args.file, 'analyze_overlaps', rpcArgs) + + if (args.json) { + console.log(JSON.stringify(data, null, 2)) + return + } + + if (data.summary.overlapCount === 0) { + console.log(kv('status', 'No overlaps found')) + console.log(fmtSummary({ 'analyzed nodes': data.summary.analyzedNodes })) + return + } + + const showing = data.overlaps.length + const total = data.summary.overlapCount + const header = + showing === total + ? ` Overlaps — ${total} found` + : ` Overlaps — ${total} found (${showing} shown)` + + console.log('') + console.log(bold(header)) + console.log('') + + console.log( + fmtList( + data.overlaps.map((overlap) => ({ + header: `[${overlap.severity}] ${overlap.category}`, + details: { + message: overlap.message, + area: `${overlap.area}px`, + ratio: `${(overlap.ratio * 100).toFixed(1)}%`, + a: `${overlap.nodeA.type} "${overlap.nodeA.name}" (${overlap.nodeA.id})`, + b: `${overlap.nodeB.type} "${overlap.nodeB.name}" (${overlap.nodeB.id})`, + suggestion: overlap.suggestion + } + })) + ) + ) + + console.log('') + console.log( + fmtSummary({ + 'analyzed nodes': data.summary.analyzedNodes, + 'total nodes': data.summary.totalNodes + }) + ) + console.log( + kv( + 'by category', + Object.entries(data.summary.byCategory) + .filter(([, count]) => count > 0) + .map(([k, count]) => `${k}: ${count}`) + .join(', ') || 'none' + ) + ) + console.log( + kv( + 'by severity', + Object.entries(data.summary.bySeverity) + .filter(([, count]) => count > 0) + .map(([k, count]) => `${k}: ${count}`) + .join(', ') || 'none' + ) + ) + console.log('') + } +}) diff --git a/packages/cli/src/commands/convert.ts b/packages/cli/src/commands/convert.ts index d52681e87..a87c4b12b 100644 --- a/packages/cli/src/commands/convert.ts +++ b/packages/cli/src/commands/convert.ts @@ -1,3 +1,4 @@ +import { writeFile } from 'node:fs/promises' import { basename, extname, resolve } from 'node:path' import { defineCommand } from 'citty' @@ -52,7 +53,7 @@ export default defineCommand({ const graph = await loadDocument(file) const result = await io.writeDocument(format, graph) const output = args.output ? resolve(args.output) : defaultOutput(file, format) - await Bun.write(output, result.data as Uint8Array) + await writeFile(output, result.data as Uint8Array) console.log(ok(`Converted ${file} → ${output}`)) } }) diff --git a/packages/cli/src/commands/eval.ts b/packages/cli/src/commands/eval.ts index e51ea99a3..32fa55311 100644 --- a/packages/cli/src/commands/eval.ts +++ b/packages/cli/src/commands/eval.ts @@ -1,3 +1,5 @@ +import { writeFile } from 'node:fs/promises' + import { defineCommand } from 'citty' import { FigmaAPI } from '@open-pencil/core/figma-api' @@ -96,7 +98,7 @@ export default defineCommand({ const io = new IORegistry(BUILTIN_IO_FORMATS) const outPath = args.output ? args.output : file const result = await io.writeDocument('fig', graph) - await Bun.write(outPath, result.data as Uint8Array) + await writeFile(outPath, result.data as Uint8Array) if (!args.quiet) { console.error(`Written to ${outPath}`) } diff --git a/packages/cli/src/commands/export.ts b/packages/cli/src/commands/export.ts index f3eb47ee2..7ed1a0c16 100644 --- a/packages/cli/src/commands/export.ts +++ b/packages/cli/src/commands/export.ts @@ -1,3 +1,4 @@ +import { writeFile } from 'node:fs/promises' import { basename, extname, resolve } from 'node:path' import { defineCommand } from 'citty' @@ -29,7 +30,7 @@ interface ExportArgs { } async function writeAndLog(path: string, content: string | Uint8Array) { - await Bun.write(path, content) + await writeFile(path, content) const size = typeof content === 'string' ? content.length : content.length console.log(ok(`Exported ${path} (${(size / 1024).toFixed(1)} KB)`)) } diff --git a/packages/cli/src/headless.ts b/packages/cli/src/headless.ts index e76217316..f6cae27c8 100644 --- a/packages/cli/src/headless.ts +++ b/packages/cli/src/headless.ts @@ -1,3 +1,5 @@ +import { readFile } from 'node:fs/promises' + import { BUILTIN_IO_FORMATS, IORegistry, initCanvasKit } from '@open-pencil/core/io' import { computeAllLayouts } from '@open-pencil/core/layout' import type { SceneGraph } from '@open-pencil/core/scene-graph' @@ -7,7 +9,7 @@ export { initCanvasKit } const io = new IORegistry(BUILTIN_IO_FORMATS) export async function loadDocument(filePath: string): Promise { - const bytes = new Uint8Array(await Bun.file(filePath).arrayBuffer()) + const bytes = new Uint8Array(await readFile(filePath)) const { graph } = await io.readDocument({ name: filePath, data: bytes }) computeAllLayouts(graph) return graph diff --git a/packages/core/package.json b/packages/core/package.json index 1a34dce99..bfdf83aae 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -8,6 +8,7 @@ }, "sideEffects": false, "exports": { + "./package.json": "./package.json", ".": { "types": "./dist/index.d.ts", "default": "./dist/index.js", diff --git a/packages/core/src/geometry.ts b/packages/core/src/geometry.ts index 4150c7a6a..d13e44f96 100644 --- a/packages/core/src/geometry.ts +++ b/packages/core/src/geometry.ts @@ -137,7 +137,7 @@ export function polygonVertices(node: { }) } -function strokeOverflow(strokes?: Stroke[]): number { +export function strokeOverflow(strokes?: Stroke[]): number { let overflow = 0 for (const stroke of strokes ?? []) { if (!stroke.visible) continue @@ -149,7 +149,7 @@ function strokeOverflow(strokes?: Stroke[]): number { return overflow } -function effectOverflow(effects?: Effect[]) { +export function effectOverflow(effects?: Effect[]) { let left = 0 let right = 0 let top = 0 @@ -398,3 +398,98 @@ export function computeDescendantVisualBounds( } return bounds } + +// ── Rotated-rectangle clipping ── + +function crossProduct(a: Vector, b: Vector, p: Vector): number { + return (b.x - a.x) * (p.y - a.y) - (b.y - a.y) * (p.x - a.x) +} + +function lineSegmentIntersect(p1: Vector, p2: Vector, p3: Vector, p4: Vector): Vector { + const dx1 = p2.x - p1.x + const dy1 = p2.y - p1.y + const dx2 = p4.x - p3.x + const dy2 = p4.y - p3.y + const denom = dx1 * dy2 - dy1 * dx2 + if (denom === 0) return p1 + const t = ((p3.x - p1.x) * dy2 - (p3.y - p1.y) * dx2) / denom + return { x: p1.x + t * dx1, y: p1.y + t * dy1 } +} + +function clipHalfPlane(polygon: Vector[], a: Vector, b: Vector, wantPositive: boolean): Vector[] { + const output: Vector[] = [] + const isInside = (p: Vector): boolean => { + const cross = crossProduct(a, b, p) + return wantPositive ? cross >= 0 : cross <= 0 + } + for (let i = 0; i < polygon.length; i++) { + const curr = polygon[i] + const prev = polygon[i === 0 ? polygon.length - 1 : i - 1] + const currInside = isInside(curr) + const prevInside = isInside(prev) + if (currInside) { + if (!prevInside) output.push(lineSegmentIntersect(prev, curr, a, b)) + output.push(curr) + } else if (prevInside) { + output.push(lineSegmentIntersect(prev, curr, a, b)) + } + } + return output +} + +/** + * Clip an axis-aligned VisualBounds rectangle against a convex polygon + * (e.g. the 4 canvas-space corners of a rotated clipping ancestor). + * + * Uses Sutherland–Hodgman polygon clipping with centroid-based interior + * detection, making it robust to either winding order of the clip polygon. + * Returns the AABB of the intersection, or null if the bounds are fully + * outside the clip polygon. + * + * For a non-rotated clip (axis-aligned corners) the result is identical + * to `intersectVisualBounds`. + */ +export function clipBoundsToPolygon( + bounds: VisualBounds, + clipCorners: Vector[] +): VisualBounds | null { + if (clipCorners.length < 3) return bounds + + let cx = 0 + let cy = 0 + for (const c of clipCorners) { + cx += c.x + cy += c.y + } + cx /= clipCorners.length + cy /= clipCorners.length + + let subject: Vector[] = [ + { x: bounds.minX, y: bounds.minY }, + { x: bounds.maxX, y: bounds.minY }, + { x: bounds.maxX, y: bounds.maxY }, + { x: bounds.minX, y: bounds.maxY } + ] + + for (let i = 0; i < clipCorners.length; i++) { + if (subject.length === 0) return null + const a = clipCorners[i] + const b = clipCorners[(i + 1) % clipCorners.length] + const centroidCross = crossProduct(a, b, { x: cx, y: cy }) + subject = clipHalfPlane(subject, a, b, centroidCross >= 0) + } + + if (subject.length === 0) return null + + let minX = Infinity + let minY = Infinity + let maxX = -Infinity + let maxY = -Infinity + for (const p of subject) { + if (p.x < minX) minX = p.x + if (p.y < minY) minY = p.y + if (p.x > maxX) maxX = p.x + if (p.y > maxY) maxY = p.y + } + return minX < maxX && minY < maxY ? { minX, minY, maxX, maxY } : null +} diff --git a/packages/core/src/rpc/analyze-commands.ts b/packages/core/src/rpc/analyze-commands.ts index 3c624b32f..1a9439860 100644 --- a/packages/core/src/rpc/analyze-commands.ts +++ b/packages/core/src/rpc/analyze-commands.ts @@ -3,10 +3,17 @@ import { orderBy, sortBy } from 'es-toolkit/array' import { colorToHex, colorDistance as colorDist } from '#core/color' import type { ColorUsageEntry } from '#core/color/analysis' import type { SceneGraph, SceneNode } from '#core/scene-graph' +import { + computeOverlaps, + type AnalyzeOverlapsArgs, + type AnalyzeOverlapsResult +} from '#core/tools/analyze/overlaps' import type { Color } from '#core/types' import type { RpcCommand } from './types' +export type { AnalyzeOverlapsArgs, AnalyzeOverlapsResult } from '#core/tools/analyze/overlaps' + // ── analyze colors ── export interface AnalyzeColorsArgs { @@ -297,3 +304,10 @@ export const analyzeClustersCommand: RpcCommand = { + name: 'analyze_overlaps', + execute: (graph, args) => computeOverlaps(graph, args) +} diff --git a/packages/core/src/rpc/commands.ts b/packages/core/src/rpc/commands.ts index 4a0336a66..fa9c1f741 100644 --- a/packages/core/src/rpc/commands.ts +++ b/packages/core/src/rpc/commands.ts @@ -3,6 +3,7 @@ import type { SceneGraph } from '#core/scene-graph' import { analyzeClustersCommand, analyzeColorsCommand, + analyzeOverlapsCommand, analyzeSpacingCommand, analyzeTypographyCommand } from './analyze-commands' @@ -33,7 +34,8 @@ export const ALL_RPC_COMMANDS = [ analyzeColorsCommand, analyzeTypographyCommand, analyzeSpacingCommand, - analyzeClustersCommand + analyzeClustersCommand, + analyzeOverlapsCommand ] as RpcCommand[] export function executeRpcCommand(graph: SceneGraph, name: string, args: unknown): unknown { diff --git a/packages/core/src/rpc/index.ts b/packages/core/src/rpc/index.ts index b3a239679..ba8d0e7f2 100644 --- a/packages/core/src/rpc/index.ts +++ b/packages/core/src/rpc/index.ts @@ -21,5 +21,7 @@ export type { SpacingValue, AnalyzeClustersArgs, AnalyzeClustersResult, + AnalyzeOverlapsArgs, + AnalyzeOverlapsResult, TypographyStyle } from './commands' diff --git a/packages/core/src/tools/analyze.ts b/packages/core/src/tools/analyze.ts index 5eab2bf34..0b2d13571 100644 --- a/packages/core/src/tools/analyze.ts +++ b/packages/core/src/tools/analyze.ts @@ -3,5 +3,14 @@ export { analyzeColors } from './analyze/colors' export { diffCreate, diffShow } from './analyze/diff' export { evalCode } from './analyze/eval' export { wrapEvalCode } from './analyze/eval/wrap' +export { analyzeOverlaps, computeOverlaps } from './analyze/overlaps' +export type { + AnalyzeOverlapsArgs, + AnalyzeOverlapsResult, + OverlapCategory, + OverlapItem, + OverlapNodeSummary, + OverlapSeverity +} from './analyze/overlaps' export { analyzeSpacing } from './analyze/spacing' export { analyzeTypography } from './analyze/typography' diff --git a/packages/core/src/tools/analyze/overlaps/helpers.ts b/packages/core/src/tools/analyze/overlaps/helpers.ts new file mode 100644 index 000000000..637ecf841 --- /dev/null +++ b/packages/core/src/tools/analyze/overlaps/helpers.ts @@ -0,0 +1,551 @@ +import { getWorldMatrix } from '#core/canvas/coordinate' +import Matrix from '#core/canvas/matrix' +import { + clipBoundsToPolygon, + effectOverflow, + geometryBlobBounds, + intersectVisualBounds, + strokeOverflow, + unionVisualBounds, + type VisualBounds +} from '#core/geometry' +import type { SceneGraph, SceneNode } from '#core/scene-graph' +import type { Rect, Vector } from '#core/types' + +import type { + AnalyzeOverlapsArgs, + OverlapCategory, + OverlapItem, + OverlapNodeSummary, + OverlapScope, + OverlapSeverity +} from './index' +const SEVERITY_RANK: Record = { + critical: 4, + major: 3, + minor: 2, + info: 1 +} + +export function parseNodeTypes(raw: string | undefined): Set | undefined { + if (!raw) return undefined + const types = raw + .split(',') + .map((v) => v.trim().toUpperCase()) + .filter((v) => v.length > 0) + return types.length > 0 ? new Set(types) : undefined +} + +export function visualBoundsArea(bounds: VisualBounds): number { + const width = bounds.maxX - bounds.minX + const height = bounds.maxY - bounds.minY + return width > 0 && height > 0 ? width * height : 0 +} + +export function boundsToRect(bounds: VisualBounds): Rect { + return { + x: bounds.minX, + y: bounds.minY, + width: bounds.maxX - bounds.minX, + height: bounds.maxY - bounds.minY + } +} + +export function toNodeSummary(node: SceneNode): OverlapNodeSummary { + return { + id: node.id, + name: node.name, + type: node.type, + parentId: node.parentId, + x: Math.round(node.x), + y: Math.round(node.y), + width: Math.round(node.width), + height: Math.round(node.height), + rotation: Math.round(node.rotation), + opacity: node.opacity, + visible: node.visible, + locked: node.locked + } +} + +export function isEffectivelyHidden(graph: SceneGraph, node: SceneNode): boolean { + let current: SceneNode | undefined = node + while (current) { + if (!current.visible) return true + current = current.parentId ? graph.getNode(current.parentId) : undefined + } + return false +} + +export function isEffectivelyLocked(graph: SceneGraph, node: SceneNode): boolean { + let current: SceneNode | undefined = node + while (current) { + if (current.locked) return true + current = current.parentId ? graph.getNode(current.parentId) : undefined + } + return false +} + +export function findPageId(graph: SceneGraph, node: SceneNode): string | null { + let current: SceneNode | undefined = node + while (current) { + if (current.type === 'CANVAS') return current.id + if (current.parentId === null) return null + current = graph.getNode(current.parentId) + } + return null +} + +export function findPageIdByName(graph: SceneGraph, name: string | undefined): string | undefined { + if (!name) return undefined + const page = graph.getPages().find((p) => p.name === name) + return page?.id +} + +export function pairRelationship( + nodeA: SceneNode, + nodeB: SceneNode, + graph: SceneGraph +): { + sameParent: boolean + topLevel: boolean + insideParent: boolean + ancestor: 'neither' | 'a-ancestor' | 'b-ancestor' +} { + const sameParent = nodeA.parentId === nodeB.parentId && nodeA.parentId !== null + const parentA = nodeA.parentId ? graph.getNode(nodeA.parentId) : undefined + const parentB = nodeB.parentId ? graph.getNode(nodeB.parentId) : undefined + const topLevel = parentA?.type === 'CANVAS' && parentB?.type === 'CANVAS' + const insideParent = sameParent && parentA?.type !== 'CANVAS' + let ancestor: 'neither' | 'a-ancestor' | 'b-ancestor' = 'neither' + if (nodeA.id !== nodeB.id) { + if (graph.isDescendant(nodeB.id, nodeA.id)) ancestor = 'a-ancestor' + else if (graph.isDescendant(nodeA.id, nodeB.id)) ancestor = 'b-ancestor' + } + return { sameParent, topLevel, insideParent, ancestor } +} + +export function matchesParentOverflowScope(scope: OverlapScope): boolean { + return scope === 'all' || scope === 'inside-parent' +} + +export function matchesScope( + rel: ReturnType, + scope: OverlapScope +): boolean { + switch (scope) { + case 'all': + return true + case 'same-parent': + return rel.sameParent + case 'cross-parent': + return !rel.sameParent + case 'top-level': + return rel.topLevel + case 'inside-parent': + return rel.insideParent + default: + return true + } +} + +export function scoredSeverity(severity: OverlapSeverity): number { + return SEVERITY_RANK[severity] +} + +function parentOverflowSeverity(outRatio: number): OverlapSeverity { + if (outRatio > 0.25) return 'critical' + if (outRatio > 0.05) return 'major' + return 'minor' +} + +function siblingOverlapSeverity(intersectionArea: number, smallerArea: number): OverlapSeverity { + if (smallerArea <= 0) return 'info' + const ratio = intersectionArea / smallerArea + if (ratio > 0.5) return 'major' + if (ratio > 0.08) return 'minor' + return 'info' +} + +function isCandidate( + node: SceneNode, + graph: SceneGraph, + options: { + includeHidden: boolean + includeLocked: boolean + includeAbsolute: boolean + pageId: string | undefined + } +): boolean { + if (node.type === 'CANVAS') return false + if (!options.includeHidden && isEffectivelyHidden(graph, node)) return false + if (!options.includeLocked && isEffectivelyLocked(graph, node)) return false + if (!options.includeAbsolute && node.layoutPositioning === 'ABSOLUTE') return false + if (options.pageId && findPageId(graph, node) !== options.pageId) return false + return true +} + +export function filterNodes( + graph: SceneGraph, + args: AnalyzeOverlapsArgs +): { candidates: SceneNode[]; totalNodes: number; analyzedNodes: number } { + const includeHidden = args.include_hidden === true + const includeLocked = args.include_locked === true + const includeAbsolute = args.include_absolute === true + const pageIdFilter = args.page_id?.trim() + const typeFilter = parseNodeTypes(args.type) + + const allNodes = [...graph.getAllNodes()] + const candidates: SceneNode[] = [] + let totalNodes = 0 + + for (const node of allNodes) { + if (node.type === 'CANVAS') continue + // Apply page scoping to totalNodes so summary counts reflect the filtered universe. + if (pageIdFilter && findPageId(graph, node) !== pageIdFilter) continue + totalNodes++ + if ( + !isCandidate(node, graph, { + includeHidden, + includeLocked, + includeAbsolute, + pageId: undefined + }) + ) + continue + if (typeFilter && !typeFilter.has(node.type)) continue + candidates.push(node) + } + + return { candidates, totalNodes, analyzedNodes: candidates.length } +} + +const EMPTY_BOUNDS: VisualBounds = { minX: 0, maxX: 0, minY: 0, maxY: 0 } + +/** + * Map a node's 4 local corners through the world matrix to canvas space. + * Unlike `getAbsolutePosition` + `rotatedBBox` (which only applies the node's + * own rotation), this correctly accounts for all ancestor transforms + * including nested rotations. + */ +function nodeWorldCorners(node: SceneNode, graph: SceneGraph): Vector[] { + const matrix = getWorldMatrix(node, graph) + const pts = Matrix.mapPoints(matrix, [ + 0, + 0, + node.width, + 0, + node.width, + node.height, + 0, + node.height + ]) + return [ + { x: pts[0], y: pts[1] }, + { x: pts[2], y: pts[3] }, + { x: pts[4], y: pts[5] }, + { x: pts[6], y: pts[7] } + ] +} + +function aabbFromCorners(corners: Vector[]): VisualBounds { + let minX = Infinity + let minY = Infinity + let maxX = -Infinity + let maxY = -Infinity + for (const c of corners) { + if (c.x < minX) minX = c.x + if (c.y < minY) minY = c.y + if (c.x > maxX) maxX = c.x + if (c.y > maxY) maxY = c.y + } + return { minX, minY, maxX, maxY } +} + +/** + * Compute a node's visual bounds in canvas space using the world matrix, + * correctly handling nested ancestor rotations. This replaces the previous + * `nodeVisualBounds` call which used `rotatedBBox` with the node's LOCAL + * rotation and the rotated origin — wrong when ancestors are rotated. + * + * Includes stroke overflow (uniform), effect overflow (directional), + * fill/stroke geometry (transformed through the world matrix), and + * text-decoration overflow (approximate: adds to canvas maxY). + */ +function computeNodeVisualBounds(node: SceneNode, graph: SceneGraph): VisualBounds { + const matrix = getWorldMatrix(node, graph) + const baseCorners = Matrix.mapPoints(matrix, [ + 0, + 0, + node.width, + 0, + node.width, + node.height, + 0, + node.height + ]) + let bounds = aabbFromCorners([ + { x: baseCorners[0], y: baseCorners[1] }, + { x: baseCorners[2], y: baseCorners[3] }, + { x: baseCorners[4], y: baseCorners[5] }, + { x: baseCorners[6], y: baseCorners[7] } + ]) + + const stroke = strokeOverflow(node.strokes) + if (stroke > 0) { + bounds.minX -= stroke + bounds.minY -= stroke + bounds.maxX += stroke + bounds.maxY += stroke + } + + const effects = effectOverflow(node.effects) + bounds.minX -= effects.left + bounds.minY -= effects.top + bounds.maxX += effects.right + bounds.maxY += effects.bottom + + const hasNonInsideStroke = node.strokes.some( + (stroke) => stroke.visible && stroke.align !== 'INSIDE' + ) + const localGeometry = geometryBlobBounds([ + ...node.fillGeometry, + ...(hasNonInsideStroke ? node.strokeGeometry : []) + ]) + if (localGeometry) { + const geomCorners = Matrix.mapPoints(matrix, [ + localGeometry.x, + localGeometry.y, + localGeometry.x + localGeometry.width, + localGeometry.y, + localGeometry.x + localGeometry.width, + localGeometry.y + localGeometry.height, + localGeometry.x, + localGeometry.y + localGeometry.height + ]) + const geomBounds = aabbFromCorners([ + { x: geomCorners[0], y: geomCorners[1] }, + { x: geomCorners[2], y: geomCorners[3] }, + { x: geomCorners[4], y: geomCorners[5] }, + { x: geomCorners[6], y: geomCorners[7] } + ]) + bounds = unionVisualBounds(bounds, geomBounds) ?? bounds + } + + if (node.type === 'TEXT' && node.textDecoration !== 'NONE') { + const fontSize = node.fontSize + const underlineOffset = node.textUnderlineOffset ?? fontSize * 0.18 + const thickness = node.textDecorationThickness ?? Math.max(1, fontSize / 16) + bounds.maxY += underlineOffset + thickness + fontSize * 0.35 + } + + return bounds +} + +/** + * Collect clipping ancestors as rotated polygons (4 canvas-space corners each). + * Unlike the previous axis-aligned approach, this correctly represents rotated + * clip frames by mapping their local corners through the world matrix. + */ +function collectClipChain(graph: SceneGraph, node: SceneNode): Vector[][] { + const clips: Vector[][] = [] + let currentId = node.parentId + while (currentId) { + const current = graph.getNode(currentId) + if (!current) break + if (current.type === 'CANVAS') break + if ( + current.clipsContent && + (current.type === 'FRAME' || current.type === 'COMPONENT' || current.type === 'INSTANCE') + ) { + clips.push(nodeWorldCorners(current, graph)) + } + currentId = current.parentId + } + return clips +} + +export function computeNodeBounds( + node: SceneNode, + graph: SceneGraph +): { bounds: VisualBounds; area: number } { + let bounds = computeNodeVisualBounds(node, graph) + const clips = collectClipChain(graph, node) + for (const clip of clips) { + const clipped = clipBoundsToPolygon(bounds, clip) + if (!clipped) { + return { bounds: EMPTY_BOUNDS, area: 0 } + } + bounds = clipped + } + return { bounds, area: visualBoundsArea(bounds) } +} + +function makeOverlapItem( + category: OverlapCategory, + severity: OverlapSeverity, + nodeA: SceneNode, + boundsA: VisualBounds, + nodeB: SceneNode, + boundsB: VisualBounds, + intersection: VisualBounds, + message: string, + suggestion: string, + areaField: 'intersection' | 'overflow' = 'intersection' +): OverlapItem { + const intersectionRect = boundsToRect(intersection) + const area = + areaField === 'intersection' + ? visualBoundsArea(intersection) + : visualBoundsArea(boundsA) - visualBoundsArea(intersection) + const ratio = + areaField === 'intersection' + ? area / Math.max(1, Math.min(visualBoundsArea(boundsA), visualBoundsArea(boundsB))) + : area / Math.max(1, visualBoundsArea(boundsA)) + + return { + category, + severity, + message, + suggestion, + area: Math.round(area), + ratio: Math.round(ratio * 1000) / 1000, + nodeA: toNodeSummary(nodeA), + nodeB: toNodeSummary(nodeB), + intersection: { + ...intersectionRect, + area: Math.round(visualBoundsArea(intersection)) + } + } +} + +export function buildParentOverflowResult( + child: SceneNode, + childBounds: VisualBounds, + parent: SceneNode, + parentBounds: VisualBounds +): OverlapItem | null { + if (parent.clipsContent) return null + const childArea = visualBoundsArea(childBounds) + if (childArea <= 0) return null + + const intersection = intersectVisualBounds(childBounds, parentBounds) + const outArea = intersection ? childArea - visualBoundsArea(intersection) : childArea + if (outArea <= 0) return null + + const outRatio = outArea / childArea + const severity = parentOverflowSeverity(outRatio) + const message = `${child.type === 'TEXT' ? 'Text' : `Node`} "${child.name}" extends ${Math.round(outArea)}px outside parent "${parent.name}"` + const suggestion = + child.type === 'TEXT' + ? 'Set the parent to clip content or constrain text sizing (textAutoResize, maxLines).' + : `Reposition inside "${parent.name}" or enable clip content on the parent.` + + return makeOverlapItem( + 'parent-overflow', + severity, + child, + childBounds, + parent, + parentBounds, + intersection ?? EMPTY_BOUNDS, + message, + suggestion, + 'overflow' + ) +} + +function isNodeAbove(ancestorGraph: SceneGraph, above: SceneNode, below: SceneNode): boolean { + if (above.parentId !== below.parentId || above.parentId === null) return false + const parent = ancestorGraph.getNode(above.parentId) + if (!parent) return false + const aboveIndex = parent.childIds.indexOf(above.id) + const belowIndex = parent.childIds.indexOf(below.id) + return aboveIndex > belowIndex +} + +function detectSiblingOverlay( + nodeA: SceneNode, + boundsA: VisualBounds, + nodeB: SceneNode, + boundsB: VisualBounds, + graph: SceneGraph +): OverlapCategory { + const areaA = visualBoundsArea(boundsA) + const areaB = visualBoundsArea(boundsB) + const smallerArea = Math.min(areaA, areaB) + const largerArea = Math.max(areaA, areaB) + const intersection = intersectVisualBounds(boundsA, boundsB) + if (!intersection || smallerArea <= 0) return 'sibling-overlap' + const overlapArea = visualBoundsArea(intersection) + const coversSmall = overlapArea / smallerArea > 0.85 + const sizeRatio = largerArea / Math.max(1, smallerArea) + if (sizeRatio < 5 || !coversSmall) return 'sibling-overlap' + + const larger = areaA >= areaB ? nodeA : nodeB + const smaller = areaA >= areaB ? nodeB : nodeA + return isNodeAbove(graph, larger, smaller) ? 'overlay' : 'sibling-overlap' +} + +export function buildSiblingOverlapResult( + nodeA: SceneNode, + boundsA: VisualBounds, + nodeB: SceneNode, + boundsB: VisualBounds, + graph: SceneGraph +): OverlapItem | null { + const intersection = intersectVisualBounds(boundsA, boundsB) + if (!intersection) return null + + const areaA = visualBoundsArea(boundsA) + const areaB = visualBoundsArea(boundsB) + const smallerArea = Math.min(areaA, areaB) + const category = detectSiblingOverlay(nodeA, boundsA, nodeB, boundsB, graph) + const intersectionArea = visualBoundsArea(intersection) + let severity: OverlapSeverity + if (category === 'overlay') { + severity = smallerArea > 0 && intersectionArea / smallerArea > 0.98 ? 'info' : 'minor' + } else { + severity = siblingOverlapSeverity(intersectionArea, smallerArea) + } + + const larger = areaA >= areaB ? nodeA : nodeB + const smaller = areaA >= areaB ? nodeB : nodeA + + const message = + category === 'overlay' + ? `"${larger.name}" appears to be an overlay covering "${smaller.name}"` + : `"${nodeA.name}" overlaps "${nodeB.name}"` + const suggestion = + category === 'overlay' + ? 'If intentional (modal/backdrop/dropdown), no action needed. Otherwise reposition or adjust z-order.' + : 'Review stacking and spacing — this overlap is likely unintended.' + + return makeOverlapItem( + category, + severity, + nodeA, + boundsA, + nodeB, + boundsB, + intersection, + message, + suggestion + ) +} + +export function passesThresholds( + item: OverlapItem, + minArea: number, + minRatio: number, + categoryFilter: OverlapCategory[] | undefined, + severityFilter: OverlapSeverity | undefined +): boolean { + if (item.area < minArea) return false + if (item.ratio < minRatio) return false + if (categoryFilter && !categoryFilter.includes(item.category)) return false + if (severityFilter && scoredSeverity(item.severity) < scoredSeverity(severityFilter)) return false + return true +} + +export type BoundsEntry = { node: SceneNode; bounds: VisualBounds; area: number } diff --git a/packages/core/src/tools/analyze/overlaps/index.ts b/packages/core/src/tools/analyze/overlaps/index.ts new file mode 100644 index 000000000..6aef894e3 --- /dev/null +++ b/packages/core/src/tools/analyze/overlaps/index.ts @@ -0,0 +1,394 @@ +import { orderBy } from 'es-toolkit/array' + +import type { FigmaAPI } from '#core/figma-api' +import type { SceneGraph, SceneNode } from '#core/scene-graph' +import { defineTool } from '#core/tools/schema' + +import { + buildParentOverflowResult, + buildSiblingOverlapResult, + computeNodeBounds, + filterNodes, + findPageId, + findPageIdByName, + matchesParentOverflowScope, + matchesScope, + pairRelationship, + passesThresholds, + scoredSeverity, + type BoundsEntry +} from './helpers' +import { + parseOverlapCategories as parseCategoryFilter, + parseOverlapScope as toScope, + parseOverlapSeverity as parseSeverity +} from './params' + +export type OverlapSeverity = 'critical' | 'major' | 'minor' | 'info' + +export type OverlapScope = 'all' | 'same-parent' | 'cross-parent' | 'top-level' | 'inside-parent' + +export type OverlapCategory = 'sibling-overlap' | 'parent-overflow' | 'overlay' + +export { findPageId, findPageIdByName } + +export interface OverlapNodeSummary { + id: string + name: string + type: string + parentId: string | null + x: number + y: number + width: number + height: number + rotation: number + opacity: number + visible: boolean + locked: boolean +} + +export interface OverlapIntersection { + x: number + y: number + width: number + height: number + area: number +} + +export interface OverlapItem { + category: OverlapCategory + severity: OverlapSeverity + message: string + suggestion: string + area: number + ratio: number + nodeA: OverlapNodeSummary + nodeB: OverlapNodeSummary + intersection: OverlapIntersection +} + +export interface AnalyzeOverlapsArgs { + scope?: OverlapScope + category?: string + severity?: OverlapSeverity + min_area?: number + min_ratio?: number + include_hidden?: boolean + include_locked?: boolean + include_absolute?: boolean + limit?: number + /** Page name fallback; used only when `page_id` is not supplied. */ + page?: string + /** Stable page ID; takes precedence over `page`. */ + page_id?: string + type?: string +} + +export interface AnalyzeOverlapsSummary { + totalNodes: number + analyzedNodes: number + overlapCount: number + byCategory: Record + bySeverity: Record +} + +export interface AnalyzeOverlapsResult { + overlaps: OverlapItem[] + summary: AnalyzeOverlapsSummary +} + +function buildBoundsCache( + candidates: SceneNode[], + graph: SceneGraph +): { boundsCache: Map; entries: BoundsEntry[] } { + const boundsCache = new Map() + const entries: BoundsEntry[] = [] + + for (const node of candidates) { + const cached = boundsCache.get(node.id) + if (cached) { + entries.push(cached) + continue + } + const computed = computeNodeBounds(node, graph) + if (computed.area <= 0) continue + const entry: BoundsEntry = { node, ...computed } + boundsCache.set(node.id, entry) + entries.push(entry) + } + + return { boundsCache, entries } +} + +function collectParentOverflows( + candidates: SceneNode[], + graph: SceneGraph, + boundsCache: Map, + scope: OverlapScope, + minArea: number, + minRatio: number, + categoryFilter: OverlapCategory[] | undefined, + severityFilter: OverlapSeverity | undefined +): OverlapItem[] { + const overlaps: OverlapItem[] = [] + + for (const child of candidates) { + if (!child.parentId) continue + const parent = graph.getNode(child.parentId) + if (!parent || parent.type === 'CANVAS') continue + const childEntry = boundsCache.get(child.id) + if (!childEntry || childEntry.area <= 0) continue + let parentEntry = boundsCache.get(parent.id) + if (!parentEntry) { + const computed = computeNodeBounds(parent, graph) + if (computed.area <= 0) continue + parentEntry = { node: parent, ...computed } + boundsCache.set(parent.id, parentEntry) + } + const item = buildParentOverflowResult(child, childEntry.bounds, parent, parentEntry.bounds) + if ( + item && + matchesParentOverflowScope(scope) && + passesThresholds(item, minArea, minRatio, categoryFilter, severityFilter) + ) { + overlaps.push(item) + } + } + + return overlaps +} + +function collectSiblingOverlaps( + entries: BoundsEntry[], + graph: SceneGraph, + scope: OverlapScope, + minArea: number, + minRatio: number, + categoryFilter: OverlapCategory[] | undefined, + severityFilter: OverlapSeverity | undefined +): OverlapItem[] { + const overlaps: OverlapItem[] = [] + entries.sort((a, b) => a.bounds.minX - b.bounds.minX) + + for (let i = 0; i < entries.length; i++) { + const entryA = entries[i] + if (entryA.area <= 0) continue + const maxX = entryA.bounds.maxX + for (let j = i + 1; j < entries.length; j++) { + const entryB = entries[j] + if (entryB.bounds.minX > maxX) break + if (entryB.bounds.maxY <= entryA.bounds.minY || entryB.bounds.minY >= entryA.bounds.maxY) { + continue + } + + const rel = pairRelationship(entryA.node, entryB.node, graph) + if (rel.ancestor !== 'neither') continue + if (!matchesScope(rel, scope)) continue + + const item = buildSiblingOverlapResult( + entryA.node, + entryA.bounds, + entryB.node, + entryB.bounds, + graph + ) + if (item && passesThresholds(item, minArea, minRatio, categoryFilter, severityFilter)) { + overlaps.push(item) + } + } + } + + return overlaps +} + +function emptyByCategory(): Record { + return { + 'sibling-overlap': 0, + 'parent-overflow': 0, + overlay: 0 + } +} + +function emptyBySeverity(): Record { + return { + critical: 0, + major: 0, + minor: 0, + info: 0 + } +} + +/** + * Compute overlap findings for a scoped subset of the graph. + * Page, scope, type, and visibility filtering is applied via `args`. + * + * Heuristics implemented: + * - `sibling-overlap`: two non-ancestor nodes visually intersect. + * - `parent-overflow`: a child protrudes from a non-clipping parent. + * - `overlay`: a large node covers a much smaller sibling (modal/backdrop pattern). + * + * Filters: + * - hidden and locked nodes are skipped unless explicitly included + * - absolutely-positioned nodes are skipped unless explicitly included + * - ancestor/descendant pairs are never emitted as pair overlaps + */ +export function computeOverlaps( + graph: SceneGraph, + args: AnalyzeOverlapsArgs = {} +): AnalyzeOverlapsResult { + const scope = toScope(args.scope) ?? 'all' + const categoryFilter = parseCategoryFilter(args.category) + const severityFilter = parseSeverity(args.severity) + const minArea = Math.max(0, Number.isFinite(Number(args.min_area)) ? Number(args.min_area) : 0) + const minRatio = Math.max( + 0, + Math.min(1, Number.isFinite(Number(args.min_ratio)) ? Number(args.min_ratio) : 0) + ) + + const explicitPageName = args.page?.trim() + const explicitPageId = args.page_id?.trim() + let resolvedPageId: string | undefined + if (explicitPageId) { + resolvedPageId = explicitPageId + } else if (explicitPageName) { + resolvedPageId = findPageIdByName(graph, explicitPageName) + } else { + resolvedPageId = graph.getPages()[0]?.id + } + + if (!resolvedPageId) { + return { + overlaps: [], + summary: { + totalNodes: 0, + analyzedNodes: 0, + overlapCount: 0, + byCategory: emptyByCategory(), + bySeverity: emptyBySeverity() + } + } + } + + const resolvedArgs: AnalyzeOverlapsArgs = { ...args, page_id: resolvedPageId } + + const { candidates, totalNodes, analyzedNodes } = filterNodes(graph, resolvedArgs) + const { boundsCache, entries } = buildBoundsCache(candidates, graph) + + const overlaps = [ + ...collectParentOverflows( + candidates, + graph, + boundsCache, + scope, + minArea, + minRatio, + categoryFilter, + severityFilter + ), + ...collectSiblingOverlaps( + entries, + graph, + scope, + minArea, + minRatio, + categoryFilter, + severityFilter + ) + ] + + const sorted = orderBy( + overlaps, + [(o) => scoredSeverity(o.severity), (o) => o.area], + ['desc', 'desc'] + ) + + const limit = Math.max(0, Number.isFinite(Number(args.limit)) ? Number(args.limit) : 100) + const trimmed = limit > 0 ? sorted.slice(0, limit) : sorted + + const byCategory = emptyByCategory() + const bySeverity = emptyBySeverity() + + for (const item of sorted) { + byCategory[item.category]++ + bySeverity[item.severity]++ + } + + return { + overlaps: trimmed, + summary: { + totalNodes, + analyzedNodes, + overlapCount: sorted.length, + byCategory, + bySeverity + } + } +} + +export const analyzeOverlaps = defineTool({ + name: 'analyze_overlaps', + description: + 'Detect visual overlaps and layout overflows across the current page. Useful for finding content that covers footers, text that bleeds outside frames, and accidental sibling overlaps.', + params: { + scope: { + type: 'string', + description: + 'Which pairs to inspect: all, same-parent, cross-parent, top-level, inside-parent (default: all)', + enum: ['all', 'same-parent', 'cross-parent', 'top-level', 'inside-parent'], + default: 'all' + }, + category: { + type: 'string', + description: + 'Comma-separated categories: sibling-overlap, parent-overflow, overlay (default: all)' + }, + severity: { + type: 'string', + description: 'Minimum severity to include: critical, major, minor, info (default: info)', + enum: ['critical', 'major', 'minor', 'info'], + default: 'info' + }, + min_area: { + type: 'number', + description: 'Minimum overlap area in square pixels (default: 0)' + }, + min_ratio: { + type: 'number', + description: 'Minimum overlap ratio relative to the smaller node, 0.0–1.0 (default: 0)' + }, + include_hidden: { + type: 'boolean', + description: 'Include hidden nodes in the analysis' + }, + include_locked: { + type: 'boolean', + description: 'Include locked nodes in the analysis' + }, + include_absolute: { + type: 'boolean', + description: 'Include absolutely-positioned nodes in the analysis' + }, + page: { + type: 'string', + description: 'Limit analysis to nodes on the named page' + }, + page_id: { + type: 'string', + description: + 'Limit analysis to nodes on the page with this stable ID (takes precedence over page)' + }, + type: { + type: 'string', + description: 'Comma-separated node types to analyze, e.g. FRAME,TEXT' + }, + limit: { + type: 'number', + description: 'Maximum overlap findings to return (default: 100)', + default: 100 + } + }, + execute: (figma: FigmaAPI, args) => { + const page_id = args.page_id ?? (args.page ? undefined : figma.currentPageId) + return computeOverlaps(figma.graph, { ...(args as AnalyzeOverlapsArgs), page_id }) + } +}) diff --git a/packages/core/src/tools/analyze/overlaps/params.ts b/packages/core/src/tools/analyze/overlaps/params.ts new file mode 100644 index 000000000..f1844c5c1 --- /dev/null +++ b/packages/core/src/tools/analyze/overlaps/params.ts @@ -0,0 +1,47 @@ +import type { OverlapCategory, OverlapScope, OverlapSeverity } from './index' + +export const VALID_OVERLAP_SCOPES: readonly OverlapScope[] = [ + 'all', + 'same-parent', + 'cross-parent', + 'top-level', + 'inside-parent' +] + +export const VALID_OVERLAP_CATEGORIES: readonly OverlapCategory[] = [ + 'sibling-overlap', + 'parent-overflow', + 'overlay' +] + +export const VALID_OVERLAP_SEVERITIES: readonly OverlapSeverity[] = [ + 'critical', + 'major', + 'minor', + 'info' +] + +export function parseOverlapScope(raw: string | undefined): OverlapScope | undefined { + if (!raw) return undefined + const normalized = raw.toLowerCase() + return VALID_OVERLAP_SCOPES.find((scope) => scope === normalized) +} + +export function parseOverlapCategories(raw: string | undefined): OverlapCategory[] | undefined { + if (!raw) return undefined + const values = raw + .split(',') + .map((v) => v.trim().toLowerCase()) + .filter((v) => v.length > 0) + if (values.length === 0) return undefined + const categories = values.filter((v): v is OverlapCategory => + VALID_OVERLAP_CATEGORIES.includes(v as OverlapCategory) + ) + return categories.length > 0 ? categories : undefined +} + +export function parseOverlapSeverity(raw: string | undefined): OverlapSeverity | undefined { + if (!raw) return undefined + const normalized = raw.toLowerCase() + return VALID_OVERLAP_SEVERITIES.find((severity) => severity === normalized) +} diff --git a/packages/core/src/tools/index.ts b/packages/core/src/tools/index.ts index 9d5d7808b..fe30d814e 100644 --- a/packages/core/src/tools/index.ts +++ b/packages/core/src/tools/index.ts @@ -6,5 +6,13 @@ export type { ToolDef, ParamDef, ParamType } from './schema' export { toolsToAI, buildDebugLog } from './ai-adapter' export type { ToolLogEntry, ToolDebugLog, AIAdapterOptions, StepBudget } from './ai-adapter' export { calcClusterConfidence, wrapEvalCode } from './analyze' +export { + VALID_OVERLAP_CATEGORIES, + VALID_OVERLAP_SCOPES, + VALID_OVERLAP_SEVERITIES, + parseOverlapCategories, + parseOverlapScope, + parseOverlapSeverity +} from './analyze/overlaps/params' export { setPexelsApiKey, setUnsplashAccessKey } from './stock-photo' export { importSvg } from './create' diff --git a/packages/core/src/tools/registry-extended.ts b/packages/core/src/tools/registry-extended.ts index fd0435bd8..4668cb9e5 100644 --- a/packages/core/src/tools/registry-extended.ts +++ b/packages/core/src/tools/registry-extended.ts @@ -1,6 +1,7 @@ import { analyzeClusters, analyzeColors, + analyzeOverlaps, analyzeSpacing, analyzeTypography, diffCreate, @@ -185,6 +186,7 @@ export const EXTENDED_TOOLS: ToolDef[] = [ analyzeTypography, analyzeSpacing, analyzeClusters, + analyzeOverlaps, diffCreate, diffShow, // Codegen diff --git a/tests/engine/cli/eval.test.ts b/tests/engine/cli/eval.test.ts index 95cc812da..e26104d9a 100644 --- a/tests/engine/cli/eval.test.ts +++ b/tests/engine/cli/eval.test.ts @@ -3,9 +3,11 @@ import { randomUUID } from 'node:crypto' import { tmpdir } from 'node:os' import { join } from 'node:path' -import { cliSourcePath, repoPath } from '#tests/helpers/paths' +import { cliSourcePath, repoPath, requireBuiltWorkspacePackages } from '#tests/helpers/paths' import { heavy } from '#tests/helpers/test-utils' +requireBuiltWorkspacePackages() + setDefaultTimeout(30_000) const CLI = cliSourcePath('index.ts') diff --git a/tests/engine/cli/overlaps.test.ts b/tests/engine/cli/overlaps.test.ts new file mode 100644 index 000000000..171d16a6e --- /dev/null +++ b/tests/engine/cli/overlaps.test.ts @@ -0,0 +1,162 @@ +import { expect, setDefaultTimeout, test } from 'bun:test' +import { mkdtempSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' + +import { SceneGraph } from '@open-pencil/core' +import { exportFigFile } from '@open-pencil/core/io/formats/fig' + +import { cliSourcePath, repoPath, requireBuiltWorkspacePackages } from '#tests/helpers/paths' +import { heavy } from '#tests/helpers/test-utils' + +requireBuiltWorkspacePackages() + +setDefaultTimeout(30_000) + +const CLI = cliSourcePath('index.ts') +const FIXTURE = repoPath('tests/fixtures/gold-preview.fig') + +async function makeTwoPageFixture(): Promise { + const graph = new SceneGraph() + const page1 = graph.getPages()[0].id + const page2 = graph.addPage('Page 2') + + graph.createNode('RECTANGLE', page1, { name: 'A', x: 0, y: 0, width: 100, height: 100 }) + graph.createNode('RECTANGLE', page1, { name: 'B', x: 50, y: 50, width: 100, height: 100 }) + graph.createNode('RECTANGLE', page2.id, { name: 'C', x: 0, y: 0, width: 100, height: 100 }) + graph.createNode('RECTANGLE', page2.id, { name: 'D', x: 50, y: 50, width: 100, height: 100 }) + + const data = await exportFigFile(graph) + const dir = mkdtempSync(join(tmpdir(), 'overlap-cli-')) + const file = join(dir, 'two-pages.fig') + writeFileSync(file, data) + return file +} + +async function run(args: string[]): Promise<{ stdout: string; stderr: string; exitCode: number }> { + const proc = Bun.spawn(['bun', CLI, ...args], { + stdout: 'pipe', + stderr: 'pipe' + }) + const [stdout, stderr] = await Promise.all([ + new Response(proc.stdout).text(), + new Response(proc.stderr).text() + ]) + const exitCode = await proc.exited + return { stdout: stdout.trim(), stderr: stderr.trim(), exitCode } +} + +heavy('analyze overlaps CLI', () => { + test('--json returns valid overlap data', async () => { + const { stdout, exitCode } = await run(['analyze', 'overlaps', FIXTURE, '--json']) + expect(exitCode).toBe(0) + const data = JSON.parse(stdout) + expect(data.overlaps).toBeArray() + expect(typeof data.summary.totalNodes).toBe('number') + expect(typeof data.summary.analyzedNodes).toBe('number') + expect(typeof data.summary.overlapCount).toBe('number') + }) + + test('human-readable mode prints a summary', async () => { + const { stdout, exitCode } = await run(['analyze', 'overlaps', FIXTURE]) + expect(exitCode).toBe(0) + expect(stdout.length).toBeGreaterThan(0) + expect(stdout).toContain('analyzed nodes') + }) + + test('invalid scope exits with error', async () => { + const { exitCode, stderr } = await run(['analyze', 'overlaps', FIXTURE, '--scope', 'invalid']) + expect(exitCode).not.toBe(0) + expect(stderr).toContain('Invalid scope') + }) + + test('invalid severity exits with error', async () => { + const { exitCode, stderr } = await run([ + 'analyze', + 'overlaps', + FIXTURE, + '--severity', + 'invalid' + ]) + expect(exitCode).not.toBe(0) + expect(stderr).toContain('Invalid severity') + }) + + test('invalid category exits with error', async () => { + const { exitCode, stderr } = await run(['analyze', 'overlaps', FIXTURE, '--category', 'bogus']) + expect(exitCode).not.toBe(0) + expect(stderr).toContain('Invalid categor') + }) + + test('invalid --min-area exits with error', async () => { + const { exitCode, stderr } = await run(['analyze', 'overlaps', FIXTURE, '--min-area', 'abc']) + expect(exitCode).not.toBe(0) + expect(stderr).toContain('--min-area') + }) + + test('invalid --limit exits with error', async () => { + const { exitCode, stderr } = await run(['analyze', 'overlaps', FIXTURE, '--limit', '0']) + expect(exitCode).not.toBe(0) + expect(stderr).toContain('--limit') + }) + + test('--category filters results', async () => { + const { stdout, exitCode } = await run([ + 'analyze', + 'overlaps', + FIXTURE, + '--json', + '--category', + 'parent-overflow' + ]) + expect(exitCode).toBe(0) + const data = JSON.parse(stdout) + expect(data.overlaps.every((o: { category: string }) => o.category === 'parent-overflow')).toBe( + true + ) + }) + + test('scope argument is case-insensitive', async () => { + const { stdout, exitCode } = await run([ + 'analyze', + 'overlaps', + FIXTURE, + '--json', + '--scope', + 'SAME-PARENT' + ]) + expect(exitCode).toBe(0) + const data = JSON.parse(stdout) + expect(data.overlaps).toBeArray() + expect(typeof data.summary.totalNodes).toBe('number') + }) + + test('default mode scopes analysis to the first page only', async () => { + const fixture = await makeTwoPageFixture() + const { stdout, exitCode } = await run(['analyze', 'overlaps', fixture, '--json']) + expect(exitCode).toBe(0) + const data = JSON.parse(stdout) + expect(data.summary.overlapCount).toBe(1) + expect(data.summary.totalNodes).toBe(2) + expect(data.overlaps[0].nodeA.name).toBe('A') + expect(data.overlaps[0].nodeB.name).toBe('B') + }) + + test('--page scopes analysis to the named page', async () => { + const fixture = await makeTwoPageFixture() + const { stdout, exitCode } = await run([ + 'analyze', + 'overlaps', + fixture, + '--json', + '--page', + 'Page 2' + ]) + expect(exitCode).toBe(0) + const data = JSON.parse(stdout) + expect(data.summary.overlapCount).toBe(1) + expect(data.summary.totalNodes).toBe(2) + expect(data.overlaps[0].nodeA.name).toBe('C') + expect(data.overlaps[0].nodeB.name).toBe('D') + }) +}) diff --git a/tests/engine/tools/analyze/overlaps/bounds.test.ts b/tests/engine/tools/analyze/overlaps/bounds.test.ts new file mode 100644 index 000000000..5430f6003 --- /dev/null +++ b/tests/engine/tools/analyze/overlaps/bounds.test.ts @@ -0,0 +1,181 @@ +import { describe, expect, test } from 'bun:test' + +import { SceneGraph, type Rect } from '@open-pencil/core' +import { computeOverlaps } from '@open-pencil/core/tools/analyze/overlaps' + +import { frame, pageId, rect } from './helpers' + +describe('analyze overlaps visible bounds', () => { + test('large background below a small badge is not reported as overlay', () => { + const graph = new SceneGraph() + const page = pageId(graph) + rect(graph, 'Backdrop', page, 0, 0, 100, 100) + rect(graph, 'Badge', page, 10, 10, 10, 10) + + const result = computeOverlaps(graph) + expect(result.overlaps.some((o) => o.category === 'overlay')).toBe(false) + expect(result.overlaps.some((o) => o.category === 'sibling-overlap')).toBe(true) + }) + + test('text decoration extends visual bounds below raw height', () => { + const graph = new SceneGraph() + const page = pageId(graph) + const t = graph.createNode('TEXT', page, { + name: 'Underlined', + x: 0, + y: 0, + width: 100, + height: 10, + text: 'Underlined', + fontSize: 14, + textDecoration: 'UNDERLINE' + }) + rect(graph, 'Below', page, 0, 11, 100, 10) + + const result = computeOverlaps(graph) + expect(result.summary.overlapCount).toBeGreaterThan(0) + expect(result.overlaps.some((o) => o.nodeA.id === t.id || o.nodeB.id === t.id)).toBe(true) + }) + + test('vector geometry expands bounds beyond width and height', () => { + const graph = new SceneGraph() + const page = pageId(graph) + + function rectGeometryBlob(local: Rect) { + const parts = [ + { cmd: 1, x: local.x, y: local.y }, + { cmd: 2, x: local.x + local.width, y: local.y }, + { cmd: 2, x: local.x + local.width, y: local.y + local.height }, + { cmd: 2, x: local.x, y: local.y + local.height }, + { cmd: 0 } + ] + let byteLength = 0 + for (const part of parts) { + byteLength += 1 + if (part.x !== undefined) byteLength += 8 + } + const bytes = new Uint8Array(byteLength) + const dv = new DataView(bytes.buffer, bytes.byteOffset, bytes.byteLength) + let offset = 0 + for (const part of parts) { + bytes[offset++] = part.cmd + if (part.x !== undefined) { + dv.setFloat32(offset, part.x, true) + dv.setFloat32(offset + 4, part.y as number, true) + offset += 8 + } + } + return bytes + } + + const v = graph.createNode('VECTOR', page, { + name: 'WideVector', + x: 0, + y: 0, + width: 10, + height: 10, + fillGeometry: [ + { + commandsBlob: rectGeometryBlob({ x: 0, y: 0, width: 200, height: 200 }), + windingRule: 'NONZERO' + } + ] + }) + rect(graph, 'Far', page, 150, 150, 50, 50) + + const result = computeOverlaps(graph) + expect(result.summary.overlapCount).toBeGreaterThan(0) + expect(result.overlaps.some((o) => o.nodeA.id === v.id || o.nodeB.id === v.id)).toBe(true) + }) + + test('ancestor clipping prevents false overlaps across the clip boundary', () => { + const graph = new SceneGraph() + const page = pageId(graph) + + const clipFrame = frame(graph, 'ClipFrame', page, 0, 0, 50, 50, true) + rect(graph, 'ClippedChild', clipFrame.id, 40, 0, 20, 20) + rect(graph, 'Outside', page, 50, 0, 20, 20) + + const clipped = computeOverlaps(graph, { category: 'sibling-overlap' }) + expect(clipped.summary.overlapCount).toBe(0) + + const noClipFrame = frame(graph, 'NoClipFrame', page, 200, 0, 50, 50, false) + rect(graph, 'UnclippedChild', noClipFrame.id, 40, 0, 20, 20) + rect(graph, 'Outside2', page, 250, 0, 20, 20) + + const unclipped = computeOverlaps(graph, { category: 'sibling-overlap' }) + expect(unclipped.summary.overlapCount).toBe(1) + }) + + test('rotated clipping frame prevents false sibling overlap', () => { + const graph = new SceneGraph() + const page = pageId(graph) + + // A 90°-rotated clipping frame. Its local space [0,100]x[0,100] maps to + // canvas [0,100]x[0,100] (a square rotated 90° around its center is the + // same square). But the OLD axis-aligned clip used the rotated origin + // (100,0) and produced a wrong clip rectangle [100,200]x[0,100]. + const clipFrame = frame(graph, 'RotatedClip', page, 0, 0, 100, 100, true) + graph.updateNode(clipFrame.id, { rotation: 90 }) + + // Child at local (10,10,20,20). After 90° rotation around the frame + // center (50,50), the child lands at canvas AABB [70,90]x[10,30]. + rect(graph, 'Child', clipFrame.id, 10, 10, 20, 20) + + // Outside node starting exactly at the clip frame's right boundary. + // With the BUGGY code the child's clipped bounds were [100,110]x[10,30] + // (wrong), producing a false overlap with [100,120]x[10,30]. + // With the fix the child's clipped bounds are [70,90]x[10,30] (correct), + // so there is no overlap. + rect(graph, 'Outside', page, 100, 10, 20, 20) + + const result = computeOverlaps(graph, { category: 'sibling-overlap' }) + expect(result.summary.overlapCount).toBe(0) + }) + + test('child of rotated non-clipping parent is detected at correct position', () => { + const graph = new SceneGraph() + const page = pageId(graph) + + // Non-clipping parent rotated 90°. A child at local (10,10,20,20) + // maps to canvas AABB [70,90]x[10,30] after rotation. A second node + // placed at canvas (72,12,10,10) should overlap — proving the bounds + // are computed from the world matrix, not from the rotated origin. + const parent = frame(graph, 'RotatedParent', page, 0, 0, 100, 100, false) + graph.updateNode(parent.id, { rotation: 90 }) + const child = rect(graph, 'Child', parent.id, 10, 10, 20, 20) + rect(graph, 'OverlapTarget', page, 72, 12, 10, 10) + + const result = computeOverlaps(graph, { category: 'sibling-overlap' }) + expect(result.summary.overlapCount).toBeGreaterThan(0) + expect(result.overlaps.some((o) => o.nodeA.id === child.id || o.nodeB.id === child.id)).toBe( + true + ) + }) + + test('rotated clipping frame still clips children that protrude', () => { + const graph = new SceneGraph() + const page = pageId(graph) + + // 90°-rotated clipping frame at (0,0,100,100). Its canvas-space clip is + // [0,100]x[0,100] (a square rotated 90° around its center is the same + // square). The OLD axis-aligned clip used the rotated origin (100,0) and + // produced a wrong clip rectangle [100,200]x[0,100]. + const clipFrame = frame(graph, 'RotatedClip90', page, 0, 0, 100, 100, true) + graph.updateNode(clipFrame.id, { rotation: 90 }) + + // Child at local (10,10,80,80) — a large child whose AABB (after 90° + // rotation) is [10,90]x[10,90], entirely inside the [0,100]x[0,100] clip. + rect(graph, 'Center', clipFrame.id, 10, 10, 80, 80) + + // Outside node at (110,10,20,20) — its AABB [110,130]x[10,30] does NOT + // overlap the child's clipped AABB [10,90]x[10,90] and does NOT overlap + // the clip frame's AABB [0,100]x[0,100]. With the BUGGY code, the child's + // clipped bounds were [100,170]x[10,90] (wrong), which WOULD overlap + // [110,130]x[10,30] — a false positive. + rect(graph, 'Outside', page, 110, 10, 20, 20) + + const result = computeOverlaps(graph, { category: 'sibling-overlap' }) + expect(result.summary.overlapCount).toBe(0) + }) +}) diff --git a/tests/engine/tools/analyze/overlaps/helpers.ts b/tests/engine/tools/analyze/overlaps/helpers.ts new file mode 100644 index 000000000..5d2002469 --- /dev/null +++ b/tests/engine/tools/analyze/overlaps/helpers.ts @@ -0,0 +1,57 @@ +import type { SceneGraph } from '@open-pencil/core' + +export function pageId(graph: SceneGraph) { + return graph.getPages()[0].id +} + +export function rect( + graph: SceneGraph, + name: string, + parentId: string, + x = 0, + y = 0, + w = 50, + h = 50 +) { + return graph.createNode('RECTANGLE', parentId, { name, x, y, width: w, height: h }) +} + +export function frame( + graph: SceneGraph, + name: string, + parentId: string, + x = 0, + y = 0, + w = 100, + h = 100, + clipsContent = false +) { + return graph.createNode('FRAME', parentId, { + name, + x, + y, + width: w, + height: h, + clipsContent + }) +} + +export function text( + graph: SceneGraph, + name: string, + parentId: string, + x = 0, + y = 0, + w = 100, + h = 20 +) { + return graph.createNode('TEXT', parentId, { + name, + x, + y, + width: w, + height: h, + text: name, + fontSize: 14 + }) +} diff --git a/tests/engine/tools/analyze/overlaps/index.test.ts b/tests/engine/tools/analyze/overlaps/index.test.ts new file mode 100644 index 000000000..14acfb329 --- /dev/null +++ b/tests/engine/tools/analyze/overlaps/index.test.ts @@ -0,0 +1,420 @@ +import { describe, expect, test } from 'bun:test' + +import { FigmaAPI, SceneGraph } from '@open-pencil/core' +import { + analyzeOverlaps, + computeOverlaps, + type OverlapScope +} from '@open-pencil/core/tools/analyze/overlaps' + +import { frame, pageId, rect, text } from './helpers' + +describe('analyze overlaps', () => { + test('detects overlapping sibling rectangles', () => { + const graph = new SceneGraph() + const page = pageId(graph) + const parent = frame(graph, 'Parent', page, 0, 0, 200, 200) + rect(graph, 'A', parent.id, 0, 0, 100, 100) + rect(graph, 'B', parent.id, 50, 50, 100, 100) + + const result = computeOverlaps(graph, { scope: 'same-parent' }) + expect(result.summary.overlapCount).toBeGreaterThan(0) + expect(result.overlaps.some((o) => o.category === 'sibling-overlap')).toBe(true) + }) + + test('ignores separated rectangles', () => { + const graph = new SceneGraph() + const page = pageId(graph) + rect(graph, 'A', page, 0, 0, 50, 50) + rect(graph, 'B', page, 200, 200, 50, 50) + + const result = computeOverlaps(graph) + expect(result.summary.overlapCount).toBe(0) + }) + + test('skips hidden and locked nodes by default', () => { + const graph = new SceneGraph() + const page = pageId(graph) + rect(graph, 'A', page, 0, 0, 100, 100) + const hidden = rect(graph, 'Hidden', page, 50, 50, 100, 100) + graph.updateNode(hidden.id, { visible: false }) + + const result = computeOverlaps(graph) + expect(result.summary.overlapCount).toBe(0) + }) + + test('includes hidden nodes when include_hidden is true', () => { + const graph = new SceneGraph() + const page = pageId(graph) + rect(graph, 'A', page, 0, 0, 100, 100) + const hidden = rect(graph, 'Hidden', page, 50, 50, 100, 100) + graph.updateNode(hidden.id, { visible: false }) + + const result = computeOverlaps(graph, { include_hidden: true }) + expect(result.summary.overlapCount).toBeGreaterThan(0) + }) + + test('reports text that overflows a non-clipping frame', () => { + const graph = new SceneGraph() + const page = pageId(graph) + const parent = frame(graph, 'Frame', page, 0, 0, 80, 80) + text(graph, 'Wide', parent.id, 0, 0, 200, 20) + + const result = computeOverlaps(graph) + expect(result.overlaps.some((o) => o.category === 'parent-overflow')).toBe(true) + }) + + test('reports a child that is fully outside its non-clipping parent', () => { + const graph = new SceneGraph() + const page = pageId(graph) + const parent = frame(graph, 'Frame', page, 0, 0, 100, 100) + rect(graph, 'Stray', parent.id, 200, 0, 50, 50) + + const result = computeOverlaps(graph) + expect(result.overlaps).toHaveLength(1) + const overflow = result.overlaps[0] + expect(overflow.category).toBe('parent-overflow') + expect(overflow.area).toBe(2500) + expect(overflow.ratio).toBe(1) + }) + + test('does not report overflow when parent clips content', () => { + const graph = new SceneGraph() + const page = pageId(graph) + const parent = frame(graph, 'Frame', page, 0, 0, 80, 80, true) + text(graph, 'Wide', parent.id, 0, 0, 200, 20) + + const result = computeOverlaps(graph) + expect(result.overlaps).toHaveLength(0) + }) + + test('detects rotated overlap', () => { + const graph = new SceneGraph() + const page = pageId(graph) + rect(graph, 'A', page, 0, 0, 100, 20) + const rotated = rect(graph, 'B', page, 0, 0, 100, 20) + graph.updateNode(rotated.id, { rotation: 45 }) + + const result = computeOverlaps(graph) + expect(result.summary.overlapCount).toBeGreaterThan(0) + }) + + test('detects a suspicious overlay covering a smaller sibling', () => { + const graph = new SceneGraph() + const page = pageId(graph) + // Create the smaller node first so the later (and therefore visually on-top) larger node + // genuinely covers it. + rect(graph, 'Badge', page, 10, 10, 10, 10) + rect(graph, 'Backdrop', page, 0, 0, 100, 100) + + const result = computeOverlaps(graph) + expect(result.overlaps.some((o) => o.category === 'overlay')).toBe(true) + const overlay = result.overlaps.find((o) => o.category === 'overlay') + expect(overlay?.message).toContain('"Backdrop" appears to be an overlay covering "Badge"') + }) + + test('filters by node type', () => { + const graph = new SceneGraph() + const page = pageId(graph) + rect(graph, 'A', page, 0, 0, 100, 100) + const t = text(graph, 'B', page, 50, 50, 100, 100) + + const result = computeOverlaps(graph, { type: 'TEXT' }) + expect(result.overlaps.every((o) => o.nodeA.id === t.id || o.nodeB.id === t.id)).toBe(true) + }) + + test('respects min_area threshold', () => { + const graph = new SceneGraph() + const page = pageId(graph) + rect(graph, 'A', page, 0, 0, 100, 100) + rect(graph, 'B', page, 99, 99, 2, 2) + + const result = computeOverlaps(graph, { min_area: 100 }) + expect(result.summary.overlapCount).toBe(0) + }) + + test('does not flag ancestor-descendant overlap', () => { + const graph = new SceneGraph() + const page = pageId(graph) + const parent = frame(graph, 'Frame', page, 0, 0, 200, 200) + rect(graph, 'Child', parent.id, 0, 0, 100, 100) + + const result = computeOverlaps(graph) + expect(result.summary.overlapCount).toBe(0) + }) + + test('scope=same-parent only reports siblings sharing a parent', () => { + const graph = new SceneGraph() + const page = pageId(graph) + const parentA = frame(graph, 'ParentA', page, 0, 0, 200, 200) + const parentB = frame(graph, 'ParentB', page, 300, 0, 200, 200) + rect(graph, 'A1', parentA.id, 0, 0, 100, 100) + rect(graph, 'A2', parentA.id, 50, 50, 100, 100) + rect(graph, 'B1', parentB.id, 0, 0, 100, 100) + rect(graph, 'B2', parentB.id, 50, 50, 100, 100) + + const result = computeOverlaps(graph, { scope: 'same-parent', category: 'sibling-overlap' }) + expect(result.summary.overlapCount).toBe(2) + expect( + result.overlaps.every( + (o) => o.nodeA.parentId === o.nodeB.parentId && o.nodeA.parentId !== page + ) + ).toBe(true) + }) + + test('scope=cross-parent only reports nodes from different parents', () => { + const graph = new SceneGraph() + const page = pageId(graph) + const parentA = frame(graph, 'ParentA', page, 0, 0, 200, 200) + const parentB = frame(graph, 'ParentB', page, 300, 0, 200, 200) + rect(graph, 'A', parentA.id, 150, 0, 110, 100) + rect(graph, 'B', parentB.id, -60, 0, 110, 100) + + const result = computeOverlaps(graph, { scope: 'cross-parent', category: 'sibling-overlap' }) + expect(result.summary.overlapCount).toBe(1) + expect(result.overlaps[0].nodeA.parentId).not.toBe(result.overlaps[0].nodeB.parentId) + }) + + test('scope=top-level only reports direct page children', () => { + const graph = new SceneGraph() + const page = pageId(graph) + rect(graph, 'TopA', page, 0, 0, 100, 100) + rect(graph, 'TopB', page, 50, 50, 100, 100) + const parent = frame(graph, 'Parent', page, 300, 300, 200, 200) + rect(graph, 'InnerA', parent.id, 0, 0, 100, 100) + rect(graph, 'InnerB', parent.id, 50, 50, 100, 100) + + const result = computeOverlaps(graph, { scope: 'top-level' }) + expect(result.summary.overlapCount).toBe(1) + expect(result.overlaps[0].nodeA.parentId).toBe(page) + expect(result.overlaps[0].nodeB.parentId).toBe(page) + }) + + test('scope=inside-parent only reports children inside a non-page parent', () => { + const graph = new SceneGraph() + const page = pageId(graph) + rect(graph, 'TopA', page, 0, 0, 100, 100) + rect(graph, 'TopB', page, 50, 50, 100, 100) + const parent = frame(graph, 'Parent', page, 300, 300, 200, 200) + rect(graph, 'InnerA', parent.id, 0, 0, 100, 100) + rect(graph, 'InnerB', parent.id, 50, 50, 100, 100) + + const result = computeOverlaps(graph, { scope: 'inside-parent' }) + expect(result.summary.overlapCount).toBe(1) + expect(result.overlaps[0].nodeA.parentId).toBe(parent.id) + expect(result.overlaps[0].nodeB.parentId).toBe(parent.id) + }) + + test('respects min_ratio threshold', () => { + const graph = new SceneGraph() + const page = pageId(graph) + const parent = frame(graph, 'Parent', page, 0, 0, 200, 200) + rect(graph, 'A', parent.id, 0, 0, 100, 100) + rect(graph, 'B', parent.id, 50, 50, 100, 100) + + const excluded = computeOverlaps(graph, { scope: 'same-parent', min_ratio: 0.5 }) + expect(excluded.overlaps).toHaveLength(0) + + const included = computeOverlaps(graph, { scope: 'same-parent', min_ratio: 0.1 }) + expect(included.overlaps.length).toBeGreaterThan(0) + }) + + test('excludes locked nodes by default', () => { + const graph = new SceneGraph() + const page = pageId(graph) + const locked = rect(graph, 'Locked', page, 0, 0, 100, 100) + graph.updateNode(locked.id, { locked: true }) + rect(graph, 'A', page, 50, 50, 100, 100) + + const result = computeOverlaps(graph) + expect( + result.overlaps.some((o) => o.nodeA.name === 'Locked' || o.nodeB.name === 'Locked') + ).toBe(false) + }) + + test('includes locked nodes when include_locked is true', () => { + const graph = new SceneGraph() + const page = pageId(graph) + const locked = rect(graph, 'Locked', page, 0, 0, 100, 100) + graph.updateNode(locked.id, { locked: true }) + rect(graph, 'A', page, 50, 50, 100, 100) + + const result = computeOverlaps(graph, { include_locked: true }) + expect( + result.overlaps.some((o) => o.nodeA.name === 'Locked' || o.nodeB.name === 'Locked') + ).toBe(true) + }) + + test('excludes absolutely-positioned nodes by default', () => { + const graph = new SceneGraph() + const page = pageId(graph) + const absolute = rect(graph, 'Absolute', page, 0, 0, 100, 100) + graph.updateNode(absolute.id, { layoutPositioning: 'ABSOLUTE' }) + rect(graph, 'A', page, 50, 50, 100, 100) + + const result = computeOverlaps(graph) + expect( + result.overlaps.some((o) => o.nodeA.name === 'Absolute' || o.nodeB.name === 'Absolute') + ).toBe(false) + }) + + test('includes absolutely-positioned nodes when include_absolute is true', () => { + const graph = new SceneGraph() + const page = pageId(graph) + const absolute = rect(graph, 'Absolute', page, 0, 0, 100, 100) + graph.updateNode(absolute.id, { layoutPositioning: 'ABSOLUTE' }) + rect(graph, 'A', page, 50, 50, 100, 100) + + const result = computeOverlaps(graph, { include_absolute: true }) + expect( + result.overlaps.some((o) => o.nodeA.name === 'Absolute' || o.nodeB.name === 'Absolute') + ).toBe(true) + }) + + test('filters results by page name', () => { + const graph = new SceneGraph() + const page1 = pageId(graph) + const page2 = graph.addPage('Page 2') + + const a1 = rect(graph, 'A1', page1, 0, 0, 100, 100) + const a2 = rect(graph, 'A2', page1, 50, 50, 100, 100) + const b1 = rect(graph, 'B1', page2.id, 0, 0, 100, 100) + const b2 = rect(graph, 'B2', page2.id, 50, 50, 100, 100) + + const page1Result = computeOverlaps(graph, { page: 'Page 1' }) + expect(page1Result.overlaps.length).toBeGreaterThan(0) + expect( + page1Result.overlaps.every( + (o) => + (o.nodeA.id === a1.id || o.nodeA.id === a2.id) && + (o.nodeB.id === a1.id || o.nodeB.id === a2.id) + ) + ).toBe(true) + + const page2Result = computeOverlaps(graph, { page: 'Page 2' }) + expect(page2Result.overlaps.length).toBeGreaterThan(0) + expect( + page2Result.overlaps.every( + (o) => + (o.nodeA.id === b1.id || o.nodeA.id === b2.id) && + (o.nodeB.id === b1.id || o.nodeB.id === b2.id) + ) + ).toBe(true) + + const missing = computeOverlaps(graph, { page: 'Missing' }) + expect(missing.overlaps).toHaveLength(0) + }) + + test('scope values are case-insensitive', () => { + const graph = new SceneGraph() + const page = pageId(graph) + rect(graph, 'A', page, 0, 0, 100, 100) + rect(graph, 'B', page, 50, 50, 100, 100) + + const result = computeOverlaps(graph, { scope: 'SAME-PARENT' as OverlapScope }) + expect(result.overlaps.length).toBeGreaterThan(0) + }) + + test('ToolDef defaults to current page and does not report overlaps on other pages', () => { + const graph = new SceneGraph() + const page2 = graph.addPage('Page 2') + + rect(graph, 'A', page2.id, 0, 0, 100, 100) + rect(graph, 'B', page2.id, 50, 50, 100, 100) + + const api = new FigmaAPI(graph) + const result = analyzeOverlaps.execute(api, {}) + expect(result.summary.overlapCount).toBe(0) + expect(result.overlaps).toHaveLength(0) + }) + + test('ToolDef respects explicit page argument', () => { + const graph = new SceneGraph() + const page2 = graph.addPage('Page 2') + + rect(graph, 'A', page2.id, 0, 0, 100, 100) + rect(graph, 'B', page2.id, 50, 50, 100, 100) + + const api = new FigmaAPI(graph) + const result = analyzeOverlaps.execute(api, { page: 'Page 2' }) + expect(result.summary.overlapCount).toBeGreaterThan(0) + }) + + test('scope=top-level excludes nested parent-overflow', () => { + const graph = new SceneGraph() + const page = pageId(graph) + const parent = frame(graph, 'Frame', page, 0, 0, 80, 80) + text(graph, 'Wide', parent.id, 0, 0, 200, 20) + + const all = computeOverlaps(graph) + expect(all.overlaps.some((o) => o.category === 'parent-overflow')).toBe(true) + + const topLevel = computeOverlaps(graph, { scope: 'top-level' }) + expect(topLevel.overlaps.some((o) => o.category === 'parent-overflow')).toBe(false) + }) + + test('scope=inside-parent includes nested parent-overflow', () => { + const graph = new SceneGraph() + const page = pageId(graph) + const parent = frame(graph, 'Frame', page, 0, 0, 80, 80) + text(graph, 'Wide', parent.id, 0, 0, 200, 20) + + const result = computeOverlaps(graph, { scope: 'inside-parent' }) + expect(result.overlaps.some((o) => o.category === 'parent-overflow')).toBe(true) + }) + + test('skips nodes inside a hidden ancestor by default', () => { + const graph = new SceneGraph() + const page = pageId(graph) + const hiddenParent = frame(graph, 'HiddenParent', page, 0, 0, 200, 200) + graph.updateNode(hiddenParent.id, { visible: false }) + rect(graph, 'A', hiddenParent.id, 0, 0, 100, 100) + rect(graph, 'B', hiddenParent.id, 50, 50, 100, 100) + + const result = computeOverlaps(graph) + expect(result.summary.overlapCount).toBe(0) + }) + + test('includes nodes inside a hidden ancestor when include_hidden is true', () => { + const graph = new SceneGraph() + const page = pageId(graph) + const hiddenParent = frame(graph, 'HiddenParent', page, 0, 0, 200, 200) + graph.updateNode(hiddenParent.id, { visible: false }) + rect(graph, 'A', hiddenParent.id, 0, 0, 100, 100) + rect(graph, 'B', hiddenParent.id, 50, 50, 100, 100) + + const result = computeOverlaps(graph, { include_hidden: true }) + expect(result.summary.overlapCount).toBeGreaterThan(0) + }) + + test('skips nodes inside a locked ancestor by default', () => { + const graph = new SceneGraph() + const page = pageId(graph) + const lockedParent = frame(graph, 'LockedParent', page, 0, 0, 200, 200) + graph.updateNode(lockedParent.id, { locked: true }) + rect(graph, 'A', lockedParent.id, 0, 0, 100, 100) + rect(graph, 'B', lockedParent.id, 50, 50, 100, 100) + + const result = computeOverlaps(graph) + expect(result.summary.overlapCount).toBe(0) + }) + + test('summary totals reflect full result set when a limit is applied', () => { + const graph = new SceneGraph() + const page = pageId(graph) + const parent = frame(graph, 'Frame', page, 0, 0, 200, 200) + + for (let i = 0; i < 4; i++) { + rect(graph, `R${i}`, parent.id, 0, 0, 100, 100) + } + + const result = computeOverlaps(graph, { scope: 'inside-parent', limit: 2 }) + expect(result.overlaps.length).toBe(2) + expect(result.summary.overlapCount).toBeGreaterThan(2) + + const totalByCategory = Object.values(result.summary.byCategory).reduce((a, b) => a + b, 0) + expect(totalByCategory).toBe(result.summary.overlapCount) + const totalBySeverity = Object.values(result.summary.bySeverity).reduce((a, b) => a + b, 0) + expect(totalBySeverity).toBe(result.summary.overlapCount) + }) +}) diff --git a/tests/engine/tools/analyze/overlaps/page-scope.test.ts b/tests/engine/tools/analyze/overlaps/page-scope.test.ts new file mode 100644 index 000000000..6e0b13b88 --- /dev/null +++ b/tests/engine/tools/analyze/overlaps/page-scope.test.ts @@ -0,0 +1,90 @@ +import { describe, expect, test } from 'bun:test' + +import { FigmaAPI, SceneGraph } from '@open-pencil/core' +import { + analyzeOverlaps, + computeOverlaps, + findPageId +} from '@open-pencil/core/tools/analyze/overlaps' + +import { pageId, rect } from './helpers' + +describe('analyze overlaps page scope', () => { + test('default scope limits analysis to a single page', () => { + const graph = new SceneGraph() + const page1 = pageId(graph) + const page2 = graph.addPage('Page 2') + + rect(graph, 'A', page1, 0, 0, 100, 100) + rect(graph, 'B', page1, 50, 50, 100, 100) + rect(graph, 'C', page2.id, 0, 0, 100, 100) + rect(graph, 'D', page2.id, 50, 50, 100, 100) + + const result = computeOverlaps(graph) + expect(result.summary.overlapCount).toBe(1) + expect(result.overlaps).toHaveLength(1) + expect( + result.overlaps.every((o) => { + const nodeA = graph.getNode(o.nodeA.id) + const nodeB = graph.getNode(o.nodeB.id) + if (!nodeA || !nodeB) return false + return findPageId(graph, nodeA) === page1 && findPageId(graph, nodeB) === page1 + }) + ).toBe(true) + }) + + test('missing page name returns an empty result', () => { + const graph = new SceneGraph() + const page1 = pageId(graph) + const page2 = graph.addPage('Page 2') + + rect(graph, 'A', page1, 0, 0, 100, 100) + rect(graph, 'B', page1, 50, 50, 100, 100) + rect(graph, 'C', page2.id, 0, 0, 100, 100) + rect(graph, 'D', page2.id, 50, 50, 100, 100) + + const result = computeOverlaps(graph, { page: 'Missing' }) + expect(result.summary.totalNodes).toBe(0) + expect(result.summary.analyzedNodes).toBe(0) + expect(result.summary.overlapCount).toBe(0) + expect(result.overlaps).toHaveLength(0) + }) + + test('ToolDef currentPage default uses page ID so duplicate page names do not leak', () => { + const graph = new SceneGraph() + const page1 = pageId(graph) + const page2 = graph.addPage('Page 1') + + rect(graph, 'A', page2.id, 0, 0, 100, 100) + rect(graph, 'B', page2.id, 50, 50, 100, 100) + + const api = new FigmaAPI(graph) + expect(api.currentPageId).toBe(page1) + + const defaultResult = analyzeOverlaps.execute(api, {}) + expect(defaultResult.summary.overlapCount).toBe(0) + expect(defaultResult.overlaps).toHaveLength(0) + + api.currentPage = api.wrapNode(page2.id) + const currentResult = analyzeOverlaps.execute(api, {}) + expect(currentResult.summary.overlapCount).toBeGreaterThan(0) + }) + + test('page_id argument disambiguates duplicate page names', () => { + const graph = new SceneGraph() + const page1 = pageId(graph) + const page2 = graph.addPage('Page 1') + + rect(graph, 'A', page2.id, 0, 0, 100, 100) + rect(graph, 'B', page2.id, 50, 50, 100, 100) + + const byId = computeOverlaps(graph, { page_id: page2.id }) + expect(byId.summary.overlapCount).toBe(1) + + const byName = computeOverlaps(graph, { page: 'Page 1' }) + expect(byName.summary.overlapCount).toBe(0) + + const byFirstPage = computeOverlaps(graph, { page_id: page1 }) + expect(byFirstPage.summary.overlapCount).toBe(0) + }) +}) diff --git a/tests/helpers/paths.ts b/tests/helpers/paths.ts index cd33e27af..3c6d868b7 100644 --- a/tests/helpers/paths.ts +++ b/tests/helpers/paths.ts @@ -1,3 +1,4 @@ +import { existsSync } from 'node:fs' import { join } from 'node:path' const repoRoot = join(import.meta.dir, '..', '..') @@ -21,3 +22,12 @@ export function testPath(...segments: string[]): string { export function publicPath(...segments: string[]): string { return repoPath('public', ...segments) } + +export function requireBuiltWorkspacePackages(): void { + const coreDist = repoPath('packages/core/dist/index.js') + if (!existsSync(coreDist)) { + throw new Error( + 'CLI integration tests require built workspace packages. Run `bun run check` or `bun run --filter @open-pencil/core build` first.' + ) + } +} From fa5c072cad65e64b7c403f60d7eb644591c432d0 Mon Sep 17 00:00:00 2001 From: Joseph Cumines Date: Fri, 19 Jun 2026 11:05:25 +1000 Subject: [PATCH 2/4] test: extend timeout for slow gold-preview fixture tests Port the timeout fixes from PR #351 that resolved the unit-test CI timeouts. Add HEAVY_TEST_TIMEOUT_MS (30s) in tests/helpers/test-utils.ts and apply it to the slow gold-preview.fig fixture tests/hooks: - clipboard roundtrip - group reclassification - glyph blob preservation - auto-layout text measurement - render/canvas/cache --- CHANGELOG.md | 1 + .../fixtures/gold-preview-roundtrip.test.ts | 22 +-- .../io/fig/import/group-reclassify.test.ts | 19 ++- .../io/fig/roundtrip/glyph-blob.test.ts | 43 +++--- .../auto-layout/text/measurement.test.ts | 144 ++++++++++-------- tests/engine/render/canvas/cache.test.ts | 5 +- tests/helpers/test-utils.ts | 4 + 7 files changed, 138 insertions(+), 100 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4ab0b1f08..bab5146e3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ ### Fixes +- Increase per-test timeout for slow `gold-preview.fig` fixture tests (`clipboard roundtrip`, `group reclassification`, `glyph blob preservation`, `auto-layout text measurement`, and `render/canvas/cache`) so they no longer flake on slower CI runners. - Fix clone operations (duplicate, instance creation, clipboard copy) sharing mutable references with the original — editing fills, strokes, variable bindings, overrides, or vector networks on one no longer corrupts the other. - Fix instance overrides shallow-copied on clone — override values containing objects are now deep-copied. - Fix stale variable bindings not cleaned up when fills/strokes arrays shrink — any indexed sub-path is now handled, not just `/color`. diff --git a/tests/engine/clipboard/fixtures/gold-preview-roundtrip.test.ts b/tests/engine/clipboard/fixtures/gold-preview-roundtrip.test.ts index 6e539eb68..7e4268858 100644 --- a/tests/engine/clipboard/fixtures/gold-preview-roundtrip.test.ts +++ b/tests/engine/clipboard/fixtures/gold-preview-roundtrip.test.ts @@ -14,6 +14,7 @@ import { } from '@open-pencil/core' import { expectDefined } from '#tests/helpers/assert' +import { HEAVY_TEST_TIMEOUT_MS } from '#tests/helpers/test-utils' describe('gold-preview.fig clipboard roundtrip', () => { let graph: SceneGraph @@ -29,15 +30,18 @@ describe('gold-preview.fig clipboard roundtrip', () => { return res } - beforeAll(async () => { - await initCodec() - const buf = readFileSync('tests/fixtures/gold-preview.fig') - const file = new File([buf], 'gold-preview.fig') - graph = await readFigFile(file) - const page = graph.getPages()[0] - pageId = page.id - topLevelNodes = graph.getChildren(pageId) - }) + beforeAll( + async () => { + await initCodec() + const buf = readFileSync('tests/fixtures/gold-preview.fig') + const file = new File([buf], 'gold-preview.fig') + graph = await readFigFile(file) + const page = graph.getPages()[0] + pageId = page.id + topLevelNodes = graph.getChildren(pageId) + }, + { timeout: HEAVY_TEST_TIMEOUT_MS } + ) it('OpenPencil format: zero property differences', () => { const html = buildOpenPencilClipboardHTML(topLevelNodes, graph) diff --git a/tests/engine/io/fig/import/group-reclassify.test.ts b/tests/engine/io/fig/import/group-reclassify.test.ts index 584d58696..1ad6f3927 100644 --- a/tests/engine/io/fig/import/group-reclassify.test.ts +++ b/tests/engine/io/fig/import/group-reclassify.test.ts @@ -5,6 +5,7 @@ import { nodeChangeToProps } from '#core/kiwi/fig/node-change/convert' import { parseFixture } from '#tests/helpers/fig-fixtures' import { collectAllNodes } from '#tests/helpers/fig-traversal' +import { HEAVY_TEST_TIMEOUT_MS } from '#tests/helpers/test-utils' describe('Figma group reclassification on import', () => { test('FRAME with resizeToFit imports as GROUP', () => { @@ -56,11 +57,15 @@ describe('Figma group reclassification on import', () => { expect(props.nodeType).toBe('FRAME') }) - test('gold-preview.fig fixture imports its groups as GROUP nodes', async () => { - const graph = await parseFixture('gold-preview.fig') - const groups = collectAllNodes(graph).filter((n) => n.type === 'GROUP') - // gold-preview.fig contains real Figma groups (FRAME + resizeToFit) that must - // import as GROUP, not FRAME. - expect(groups.length).toBeGreaterThan(0) - }) + test( + 'gold-preview.fig fixture imports its groups as GROUP nodes', + { timeout: HEAVY_TEST_TIMEOUT_MS }, + async () => { + const graph = await parseFixture('gold-preview.fig') + const groups = collectAllNodes(graph).filter((n) => n.type === 'GROUP') + // gold-preview.fig contains real Figma groups (FRAME + resizeToFit) that must + // import as GROUP, not FRAME. + expect(groups.length).toBeGreaterThan(0) + } + ) }) diff --git a/tests/engine/io/fig/roundtrip/glyph-blob.test.ts b/tests/engine/io/fig/roundtrip/glyph-blob.test.ts index a26cffac1..b5c56b655 100644 --- a/tests/engine/io/fig/roundtrip/glyph-blob.test.ts +++ b/tests/engine/io/fig/roundtrip/glyph-blob.test.ts @@ -7,6 +7,8 @@ import { fontManager } from '@open-pencil/core/text' import { parseFigBuffer } from '#core/kiwi/fig/parse/core' +import { HEAVY_TEST_TIMEOUT_MS } from '#tests/helpers/test-utils' + const FIXTURES = resolve(import.meta.dir, '../../../../fixtures') const INTER_ASSETS = resolve(import.meta.dir, '../../../../../packages/core/assets') @@ -44,27 +46,34 @@ function loadInterFonts() { } describe('roundtrip: text glyph blobs', () => { - beforeAll(async () => { - await initCodec() - }) + beforeAll( + async () => { + await initCodec() + }, + { timeout: HEAVY_TEST_TIMEOUT_MS } + ) - test('preserves imported Figma glyph blobs for fallback rendering', async () => { - const fixtureBytes = new Uint8Array(readFileSync(resolve(FIXTURES, 'gold-preview.fig'))) - const input = countGlyphBlobs(fixtureBytes) + test( + 'preserves imported Figma glyph blobs for fallback rendering', + { timeout: HEAVY_TEST_TIMEOUT_MS }, + async () => { + const fixtureBytes = new Uint8Array(readFileSync(resolve(FIXTURES, 'gold-preview.fig'))) + const input = countGlyphBlobs(fixtureBytes) - const graph = await parseFigFile( - fixtureBytes.buffer.slice( - fixtureBytes.byteOffset, - fixtureBytes.byteOffset + fixtureBytes.byteLength + const graph = await parseFigFile( + fixtureBytes.buffer.slice( + fixtureBytes.byteOffset, + fixtureBytes.byteOffset + fixtureBytes.byteLength + ) ) - ) - const exported = await exportFigFile(graph) - const output = countGlyphBlobs(exported) + const exported = await exportFigFile(graph) + const output = countGlyphBlobs(exported) - expect(input.glyphsWithBlob).toBeGreaterThan(0) - expect(output.glyphsWithBlob).toBe(input.glyphsWithBlob) - expect(output.uniqueGlyphBlobs).toBeLessThanOrEqual(input.uniqueGlyphBlobs) - }) + expect(input.glyphsWithBlob).toBeGreaterThan(0) + expect(output.glyphsWithBlob).toBe(input.glyphsWithBlob) + expect(output.uniqueGlyphBlobs).toBeLessThanOrEqual(input.uniqueGlyphBlobs) + } + ) test('deduplicates generated glyph blobs across repeated text', async () => { loadInterFonts() diff --git a/tests/engine/layout/auto-layout/text/measurement.test.ts b/tests/engine/layout/auto-layout/text/measurement.test.ts index ccfebc086..8b0d2deca 100644 --- a/tests/engine/layout/auto-layout/text/measurement.test.ts +++ b/tests/engine/layout/auto-layout/text/measurement.test.ts @@ -6,6 +6,7 @@ import { createEditorStore } from '@/app/editor/session' import { getNodeOrThrow } from '#tests/helpers/assert' import { autoFrame, loadFixtureGraph, pageId, rect } from '#tests/helpers/layout' +import { HEAVY_TEST_TIMEOUT_MS } from '#tests/helpers/test-utils' describe('text measurement', () => { test('derived text layout preserves imported auto-layout text bounds during measurement', () => { @@ -85,80 +86,91 @@ describe('text measurement', () => { expect(graph.getNode(tabs.id)?.height).toBe(62) }) - test('opening imported fig keeps stored text bounds before CanvasKit measurement', async () => { - const graph = await loadFixtureGraph('gold-preview.fig') - const store = createEditorStore(graph) - const title = [...store.graph.getAllNodes()].find( - (node) => node.type === 'TEXT' && node.text === "World's largest" - ) - const subtitle = [...store.graph.getAllNodes()].find( - (node) => - node.type === 'TEXT' && node.text === 'Preline UI Figma - crafted with Tailwind CSS styles' - ) - const description = [...store.graph.getAllNodes()].find( - (node) => - node.type === 'TEXT' && - node.text.startsWith('Preline UI Figma is the largest free design system for Figma') - ) + test( + 'opening imported fig keeps stored text bounds before CanvasKit measurement', + { timeout: HEAVY_TEST_TIMEOUT_MS }, + async () => { + const graph = await loadFixtureGraph('gold-preview.fig') + const store = createEditorStore(graph) + const title = [...store.graph.getAllNodes()].find( + (node) => node.type === 'TEXT' && node.text === "World's largest" + ) + const subtitle = [...store.graph.getAllNodes()].find( + (node) => + node.type === 'TEXT' && + node.text === 'Preline UI Figma - crafted with Tailwind CSS styles' + ) + const description = [...store.graph.getAllNodes()].find( + (node) => + node.type === 'TEXT' && + node.text.startsWith('Preline UI Figma is the largest free design system for Figma') + ) - if (!title || !subtitle || !description) { - throw new Error('Expected imported text nodes in gold-preview.fig') + if (!title || !subtitle || !description) { + throw new Error('Expected imported text nodes in gold-preview.fig') + } + + expect(title.width).toBe(444) + expect(title.height).toBe(73) + expect(subtitle.width).toBe(439) + expect(subtitle.height).toBe(22) + expect(description.width).toBe(878) + expect(description.height).toBe(60) + + await store.switchPage(store.graph.getPages()[0].id) + + expect(store.graph.getNode(title.id)?.width).toBe(444) + expect(store.graph.getNode(title.id)?.height).toBe(73) + expect(store.graph.getNode(subtitle.id)?.width).toBe(439) + expect(store.graph.getNode(subtitle.id)?.height).toBe(22) + expect(store.graph.getNode(description.id)?.width).toBe(878) + expect(store.graph.getNode(description.id)?.height).toBe(60) } + ) - expect(title.width).toBe(444) - expect(title.height).toBe(73) - expect(subtitle.width).toBe(439) - expect(subtitle.height).toBe(22) - expect(description.width).toBe(878) - expect(description.height).toBe(60) + test( + 'imported nested instance layout keeps hidden sibling offsets stable', + { timeout: HEAVY_TEST_TIMEOUT_MS }, + async () => { + const graph = await loadFixtureGraph('gold-preview.fig') + const previewRoot = graph.getChildren(graph.getPages()[0].id)[0] + const wysiwygEditor = graph + .getChildren(previewRoot.id) + .find((node) => node.name === '_WYSIWYG-editor') + const toolbarVariant = wysiwygEditor + ? graph + .getChildren(wysiwygEditor.id) + .find((node) => node.name === '_on-text-WYSIWYG-toolbar') + : undefined + const toolbarRow = toolbarVariant + ? graph.getChildren(toolbarVariant.id).find((node) => node.name === 'Toolbar') + : undefined + const hiddenInput = toolbarRow + ? graph.getChildren(toolbarRow.id).find((node) => node.name === 'Input') + : undefined + const visibleToolbar = toolbarRow + ? graph + .getChildren(toolbarRow.id) + .find((node) => node.name === 'Toolbar' && node.id !== toolbarRow.id) + : undefined - await store.switchPage(store.graph.getPages()[0].id) + if (!toolbarRow || !hiddenInput || !visibleToolbar) { + throw new Error('Expected imported WYSIWYG toolbar nodes in gold-preview.fig') + } - expect(store.graph.getNode(title.id)?.width).toBe(444) - expect(store.graph.getNode(title.id)?.height).toBe(73) - expect(store.graph.getNode(subtitle.id)?.width).toBe(439) - expect(store.graph.getNode(subtitle.id)?.height).toBe(22) - expect(store.graph.getNode(description.id)?.width).toBe(878) - expect(store.graph.getNode(description.id)?.height).toBe(60) - }) + expect(hiddenInput.visible).toBe(false) + // x=8 is the Figma-exported fixture value (verified via fixture inspection 2026-05-03). + // The old assertion of x=298 was incorrect — it was a post-layout computed value from + // an older layout implementation. Post-layout, left-aligned HUG content retains x=8. + expect(visibleToolbar.x).toBe(8) - test('imported nested instance layout keeps hidden sibling offsets stable', async () => { - const graph = await loadFixtureGraph('gold-preview.fig') - const previewRoot = graph.getChildren(graph.getPages()[0].id)[0] - const wysiwygEditor = graph - .getChildren(previewRoot.id) - .find((node) => node.name === '_WYSIWYG-editor') - const toolbarVariant = wysiwygEditor - ? graph.getChildren(wysiwygEditor.id).find((node) => node.name === '_on-text-WYSIWYG-toolbar') - : undefined - const toolbarRow = toolbarVariant - ? graph.getChildren(toolbarVariant.id).find((node) => node.name === 'Toolbar') - : undefined - const hiddenInput = toolbarRow - ? graph.getChildren(toolbarRow.id).find((node) => node.name === 'Input') - : undefined - const visibleToolbar = toolbarRow - ? graph - .getChildren(toolbarRow.id) - .find((node) => node.name === 'Toolbar' && node.id !== toolbarRow.id) - : undefined + setTextMeasurer(null) + computeAllLayouts(graph, graph.getPages()[0].id) - if (!toolbarRow || !hiddenInput || !visibleToolbar) { - throw new Error('Expected imported WYSIWYG toolbar nodes in gold-preview.fig') + expect(graph.getNode(visibleToolbar.id)?.x).toBe(8) + expect(graph.getNode(visibleToolbar.id)?.y).toBe(8) } - - expect(hiddenInput.visible).toBe(false) - // x=8 is the Figma-exported fixture value (verified via fixture inspection 2026-05-03). - // The old assertion of x=298 was incorrect — it was a post-layout computed value from - // an older layout implementation. Post-layout, left-aligned HUG content retains x=8. - expect(visibleToolbar.x).toBe(8) - - setTextMeasurer(null) - computeAllLayouts(graph, graph.getPages()[0].id) - - expect(graph.getNode(visibleToolbar.id)?.x).toBe(8) - expect(graph.getNode(visibleToolbar.id)?.y).toBe(8) - }) + ) test('WIDTH_AND_HEIGHT text uses measured width in centered layout', () => { const graph = new SceneGraph() diff --git a/tests/engine/render/canvas/cache.test.ts b/tests/engine/render/canvas/cache.test.ts index 4622fde9d..c7c76cb01 100644 --- a/tests/engine/render/canvas/cache.test.ts +++ b/tests/engine/render/canvas/cache.test.ts @@ -1,4 +1,4 @@ -import { beforeAll, describe, expect, test } from 'bun:test' +import { beforeAll, describe, expect, setDefaultTimeout, test } from 'bun:test' import { readFileSync } from 'node:fs' import { @@ -15,6 +15,9 @@ import { initCanvasKit } from '#cli/headless' import { expectDefined } from '#tests/helpers/assert' import { repoPath } from '#tests/helpers/paths' +import { HEAVY_TEST_TIMEOUT_MS } from '#tests/helpers/test-utils' + +setDefaultTimeout(HEAVY_TEST_TIMEOUT_MS) let graph: SceneGraph let movingNodeId: string diff --git a/tests/helpers/test-utils.ts b/tests/helpers/test-utils.ts index 7e6ff1f89..5c1e57754 100644 --- a/tests/helpers/test-utils.ts +++ b/tests/helpers/test-utils.ts @@ -8,3 +8,7 @@ export const runsHeavyTests = process.env.BUN_HEAVY_TESTS : true export const heavy = describe.if(runsHeavyTests) + +// Default per-test timeout in CI is often too short for fixture I/O tests that parse/export +// real .fig files. Use this timeout for tests that are bounded by fixture parsing speed. +export const HEAVY_TEST_TIMEOUT_MS = 30_000 From ad22513bc7baae76a1ad17ff01d5999636359b3d Mon Sep 17 00:00:00 2001 From: Joseph Cumines Date: Fri, 19 Jun 2026 11:38:38 +1000 Subject: [PATCH 3/4] fix(tools): address CodeRabbit review on analyze_overlaps - Export OverlapScope, AnalyzeOverlapsSummary, and OverlapIntersection from the analyze barrel so consumers don't deep-import. - Expand the local rectangle by stroke overflow before transforming through the world matrix, so stroked rotated nodes get their true rotated footprint instead of an underestimated canvas-space expansion. - Add clipPolygon to geometry and preserve the clipped polygon through the full clip chain in computeNodeBounds, so a later outer clip can't reintroduce corners an inner clip already removed. - Cap returned overlaps to an empty list when limit is 0 or negative; summary totals still reflect the complete result set. - Trim whitespace from scope and severity inputs so " major " resolves. - Add geometry and overlap tests covering each fix. --- CHANGELOG.md | 5 + packages/core/src/geometry.ts | 64 +++++++---- packages/core/src/tools/analyze.ts | 3 + .../src/tools/analyze/overlaps/helpers.ts | 54 +++++---- .../core/src/tools/analyze/overlaps/index.ts | 4 +- .../core/src/tools/analyze/overlaps/params.ts | 6 +- tests/engine/geometry/clip-polygon.test.ts | 107 ++++++++++++++++++ .../tools/analyze/overlaps/bounds.test.ts | 56 +++++++++ .../tools/analyze/overlaps/index.test.ts | 45 ++++++++ 9 files changed, 298 insertions(+), 46 deletions(-) create mode 100644 tests/engine/geometry/clip-polygon.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index bab5146e3..635532f5e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,11 @@ - Preserve rotated Figma transform origins for imported vector nodes. - Render complex text fills through vector glyph outlines so imported Figma text can use the normal fill pipeline for gradients, images, patterns, and other non-solid paints. - Fix file-backed CLI commands (`convert`, `eval --output`, `export`) to use Node `fs/promises` instead of Bun runtime APIs, so the published CLI works when installed and run under Node. +- Fix `analyze_overlaps` stroke-overflow bounds for rotated nodes by expanding the local rectangle before transforming through the world matrix, so stroked rotated nodes are measured with their true rotated footprint. +- Fix `analyze_overlaps` clipping across multiple rotated ancestors by preserving the clipped polygon through the full clip chain instead of collapsing to an AABB between clips, which could reintroduce corners removed by an inner clip. +- Fix `analyze_overlaps` `limit` of `0` (or a negative value) so it caps the returned overlaps to an empty list instead of returning the full set; summary totals still reflect the complete result. +- Trim whitespace from `analyze_overlaps` `scope` and `severity` inputs so values like `" major "` resolve instead of falling back to defaults. +- Export `OverlapScope`, `AnalyzeOverlapsSummary`, and `OverlapIntersection` from the `@open-pencil/core/tools/analyze` barrel so consumers do not need to deep-import the overlaps module. ## 0.13.2 — 2026-05-30 diff --git a/packages/core/src/geometry.ts b/packages/core/src/geometry.ts index d13e44f96..7355d90a0 100644 --- a/packages/core/src/geometry.ts +++ b/packages/core/src/geometry.ts @@ -438,22 +438,20 @@ function clipHalfPlane(polygon: Vector[], a: Vector, b: Vector, wantPositive: bo } /** - * Clip an axis-aligned VisualBounds rectangle against a convex polygon - * (e.g. the 4 canvas-space corners of a rotated clipping ancestor). + * Clip a subject polygon against a convex polygon (e.g. the 4 canvas-space + * corners of a rotated clipping ancestor). * * Uses Sutherland–Hodgman polygon clipping with centroid-based interior * detection, making it robust to either winding order of the clip polygon. - * Returns the AABB of the intersection, or null if the bounds are fully - * outside the clip polygon. + * Returns the clipped polygon, or null if the subject is fully outside the + * clip polygon. When `clipCorners` has fewer than 3 points the subject is + * returned unchanged (no clipping). * - * For a non-rotated clip (axis-aligned corners) the result is identical - * to `intersectVisualBounds`. + * Preserving the polygon (rather than collapsing to an AABB) lets callers + * chain multiple clips without reintroducing corners removed by an inner clip. */ -export function clipBoundsToPolygon( - bounds: VisualBounds, - clipCorners: Vector[] -): VisualBounds | null { - if (clipCorners.length < 3) return bounds +export function clipPolygon(subject: Vector[], clipCorners: Vector[]): Vector[] | null { + if (clipCorners.length < 3) return subject let cx = 0 let cy = 0 @@ -464,28 +462,50 @@ export function clipBoundsToPolygon( cx /= clipCorners.length cy /= clipCorners.length - let subject: Vector[] = [ + let polygon: Vector[] = subject + + for (let i = 0; i < clipCorners.length; i++) { + if (polygon.length === 0) return null + const a = clipCorners[i] + const b = clipCorners[(i + 1) % clipCorners.length] + const centroidCross = crossProduct(a, b, { x: cx, y: cy }) + polygon = clipHalfPlane(polygon, a, b, centroidCross >= 0) + } + + return polygon.length === 0 ? null : polygon +} + +/** + * Clip an axis-aligned VisualBounds rectangle against a convex polygon + * (e.g. the 4 canvas-space corners of a rotated clipping ancestor). + * + * Returns the AABB of the intersection, or null if the bounds are fully + * outside the clip polygon. Delegates to {@link clipPolygon}. + * + * For a non-rotated clip (axis-aligned corners) the result is identical + * to `intersectVisualBounds`. + */ +export function clipBoundsToPolygon( + bounds: VisualBounds, + clipCorners: Vector[] +): VisualBounds | null { + if (clipCorners.length < 3) return bounds + + const subject: Vector[] = [ { x: bounds.minX, y: bounds.minY }, { x: bounds.maxX, y: bounds.minY }, { x: bounds.maxX, y: bounds.maxY }, { x: bounds.minX, y: bounds.maxY } ] - for (let i = 0; i < clipCorners.length; i++) { - if (subject.length === 0) return null - const a = clipCorners[i] - const b = clipCorners[(i + 1) % clipCorners.length] - const centroidCross = crossProduct(a, b, { x: cx, y: cy }) - subject = clipHalfPlane(subject, a, b, centroidCross >= 0) - } - - if (subject.length === 0) return null + const polygon = clipPolygon(subject, clipCorners) + if (!polygon) return null let minX = Infinity let minY = Infinity let maxX = -Infinity let maxY = -Infinity - for (const p of subject) { + for (const p of polygon) { if (p.x < minX) minX = p.x if (p.y < minY) minY = p.y if (p.x > maxX) maxX = p.x diff --git a/packages/core/src/tools/analyze.ts b/packages/core/src/tools/analyze.ts index 0b2d13571..f1d414f8d 100644 --- a/packages/core/src/tools/analyze.ts +++ b/packages/core/src/tools/analyze.ts @@ -7,9 +7,12 @@ export { analyzeOverlaps, computeOverlaps } from './analyze/overlaps' export type { AnalyzeOverlapsArgs, AnalyzeOverlapsResult, + AnalyzeOverlapsSummary, OverlapCategory, + OverlapIntersection, OverlapItem, OverlapNodeSummary, + OverlapScope, OverlapSeverity } from './analyze/overlaps' export { analyzeSpacing } from './analyze/spacing' diff --git a/packages/core/src/tools/analyze/overlaps/helpers.ts b/packages/core/src/tools/analyze/overlaps/helpers.ts index 637ecf841..717349714 100644 --- a/packages/core/src/tools/analyze/overlaps/helpers.ts +++ b/packages/core/src/tools/analyze/overlaps/helpers.ts @@ -1,7 +1,7 @@ import { getWorldMatrix } from '#core/canvas/coordinate' import Matrix from '#core/canvas/matrix' import { - clipBoundsToPolygon, + clipPolygon, effectOverflow, geometryBlobBounds, intersectVisualBounds, @@ -274,15 +274,20 @@ function aabbFromCorners(corners: Vector[]): VisualBounds { */ function computeNodeVisualBounds(node: SceneNode, graph: SceneGraph): VisualBounds { const matrix = getWorldMatrix(node, graph) + // Expand the local rectangle by the stroke overflow *before* transforming + // through the world matrix. Expanding the already-rotated AABB by `stroke` + // underestimates the stroked bounds for rotated nodes (the true stroked AABB + // grows by `stroke * (|cos θ| + |sin θ|)`, not `stroke`). + const stroke = strokeOverflow(node.strokes) const baseCorners = Matrix.mapPoints(matrix, [ - 0, - 0, - node.width, - 0, - node.width, - node.height, - 0, - node.height + -stroke, + -stroke, + node.width + stroke, + -stroke, + node.width + stroke, + node.height + stroke, + -stroke, + node.height + stroke ]) let bounds = aabbFromCorners([ { x: baseCorners[0], y: baseCorners[1] }, @@ -291,14 +296,8 @@ function computeNodeVisualBounds(node: SceneNode, graph: SceneGraph): VisualBoun { x: baseCorners[6], y: baseCorners[7] } ]) - const stroke = strokeOverflow(node.strokes) - if (stroke > 0) { - bounds.minX -= stroke - bounds.minY -= stroke - bounds.maxX += stroke - bounds.maxY += stroke - } - + // Effects (drop shadow, blur) radiate in screen space, so expanding the + // canvas-space AABB by the directional overflow is correct regardless of rotation. const effects = effectOverflow(node.effects) bounds.minX -= effects.left bounds.minY -= effects.top @@ -369,15 +368,28 @@ export function computeNodeBounds( node: SceneNode, graph: SceneGraph ): { bounds: VisualBounds; area: number } { - let bounds = computeNodeVisualBounds(node, graph) + const visual = computeNodeVisualBounds(node, graph) const clips = collectClipChain(graph, node) + if (clips.length === 0) { + return { bounds: visual, area: visualBoundsArea(visual) } + } + // Seed the subject polygon from the visual AABB, then clip against each + // ancestor preserving the polygon. Collapsing to an AABB between clips would + // reintroduce corners already removed by an inner clip, so hidden regions + // could look visible again to a later outer clip. + let polygon: Vector[] | null = [ + { x: visual.minX, y: visual.minY }, + { x: visual.maxX, y: visual.minY }, + { x: visual.maxX, y: visual.maxY }, + { x: visual.minX, y: visual.maxY } + ] for (const clip of clips) { - const clipped = clipBoundsToPolygon(bounds, clip) - if (!clipped) { + polygon = clipPolygon(polygon, clip) + if (!polygon) { return { bounds: EMPTY_BOUNDS, area: 0 } } - bounds = clipped } + const bounds = aabbFromCorners(polygon) return { bounds, area: visualBoundsArea(bounds) } } diff --git a/packages/core/src/tools/analyze/overlaps/index.ts b/packages/core/src/tools/analyze/overlaps/index.ts index 6aef894e3..3316c3e2e 100644 --- a/packages/core/src/tools/analyze/overlaps/index.ts +++ b/packages/core/src/tools/analyze/overlaps/index.ts @@ -303,7 +303,9 @@ export function computeOverlaps( ) const limit = Math.max(0, Number.isFinite(Number(args.limit)) ? Number(args.limit) : 100) - const trimmed = limit > 0 ? sorted.slice(0, limit) : sorted + // A limit of 0 (or a negative value clamped to 0) caps the returned overlaps + // to an empty list. The summary totals still reflect the full `sorted` set. + const trimmed = sorted.slice(0, limit) const byCategory = emptyByCategory() const bySeverity = emptyBySeverity() diff --git a/packages/core/src/tools/analyze/overlaps/params.ts b/packages/core/src/tools/analyze/overlaps/params.ts index f1844c5c1..ba5048703 100644 --- a/packages/core/src/tools/analyze/overlaps/params.ts +++ b/packages/core/src/tools/analyze/overlaps/params.ts @@ -23,7 +23,8 @@ export const VALID_OVERLAP_SEVERITIES: readonly OverlapSeverity[] = [ export function parseOverlapScope(raw: string | undefined): OverlapScope | undefined { if (!raw) return undefined - const normalized = raw.toLowerCase() + const normalized = raw.trim().toLowerCase() + if (!normalized) return undefined return VALID_OVERLAP_SCOPES.find((scope) => scope === normalized) } @@ -42,6 +43,7 @@ export function parseOverlapCategories(raw: string | undefined): OverlapCategory export function parseOverlapSeverity(raw: string | undefined): OverlapSeverity | undefined { if (!raw) return undefined - const normalized = raw.toLowerCase() + const normalized = raw.trim().toLowerCase() + if (!normalized) return undefined return VALID_OVERLAP_SEVERITIES.find((severity) => severity === normalized) } diff --git a/tests/engine/geometry/clip-polygon.test.ts b/tests/engine/geometry/clip-polygon.test.ts new file mode 100644 index 000000000..b9edfde83 --- /dev/null +++ b/tests/engine/geometry/clip-polygon.test.ts @@ -0,0 +1,107 @@ +import { describe, expect, test } from 'bun:test' + +import type { Vector } from '@open-pencil/core' + +import { clipBoundsToPolygon, clipPolygon } from '#core/geometry' + +describe('clipPolygon', () => { + test('returns the subject unchanged when the clip has fewer than 3 corners', () => { + const subject: Vector[] = [ + { x: 0, y: 0 }, + { x: 10, y: 0 }, + { x: 10, y: 10 }, + { x: 0, y: 10 } + ] + expect(clipPolygon(subject, [])).toBe(subject) + expect( + clipPolygon(subject, [ + { x: 0, y: 0 }, + { x: 1, y: 1 } + ]) + ).toBe(subject) + }) + + test('returns null when the subject is fully outside the clip', () => { + const subject: Vector[] = [ + { x: 100, y: 100 }, + { x: 110, y: 100 }, + { x: 110, y: 110 }, + { x: 100, y: 110 } + ] + const clip: Vector[] = [ + { x: 0, y: 0 }, + { x: 10, y: 0 }, + { x: 10, y: 10 }, + { x: 0, y: 10 } + ] + expect(clipPolygon(subject, clip)).toBeNull() + }) + + test('preserves the clipped polygon across a chain so a later clip excludes corners the inner clip removed', () => { + // Subject: the full [0,100]² square. + const subject: Vector[] = [ + { x: 0, y: 0 }, + { x: 100, y: 0 }, + { x: 100, y: 100 }, + { x: 0, y: 100 } + ] + + // Inner clip D1: the diamond inscribed in the square (corners at the edge + // midpoints). It removes the four square corners, including (0,0). + const d1: Vector[] = [ + { x: 50, y: 0 }, + { x: 100, y: 50 }, + { x: 50, y: 100 }, + { x: 0, y: 50 } + ] + + // Outer clip D2: a small square [0,30]² covering the (0,0) corner that D1 + // already removed. If the chain collapsed D1's result to its AABB ([0,100]², + // which re-adds the corner) before applying D2, the corner would survive. + // Preserving the polygon keeps it excluded: the only surviving region is the + // triangle where the diamond's x+y=50 edge meets [0,30]², i.e. AABB + // [20,30]×[20,30]. + const d2: Vector[] = [ + { x: 0, y: 0 }, + { x: 30, y: 0 }, + { x: 30, y: 30 }, + { x: 0, y: 30 } + ] + + const afterD1 = clipPolygon(subject, d1) + expect(afterD1).not.toBeNull() + const clipped = afterD1 ? clipPolygon(afterD1, d2) : null + expect(clipped).not.toBeNull() + expect(clipped?.length).toBeGreaterThanOrEqual(3) + + let minX = Infinity + let minY = Infinity + let maxX = -Infinity + let maxY = -Infinity + for (const p of clipped ?? []) { + if (p.x < minX) minX = p.x + if (p.y < minY) minY = p.y + if (p.x > maxX) maxX = p.x + if (p.y > maxY) maxY = p.y + } + + // The corner (0,0) must stay excluded: the result does not reach below 20. + expect(minX).toBeGreaterThanOrEqual(20) + expect(minY).toBeGreaterThanOrEqual(20) + expect(maxX).toBeLessThanOrEqual(30) + expect(maxY).toBeLessThanOrEqual(30) + }) + + test('clipBoundsToPolygon agrees with clipPolygon for a single axis-aligned clip', () => { + const bounds = { minX: 0, minY: 0, maxX: 100, maxY: 100 } + const clip: Vector[] = [ + { x: 25, y: 25 }, + { x: 75, y: 25 }, + { x: 75, y: 75 }, + { x: 25, y: 75 } + ] + + const aabb = clipBoundsToPolygon(bounds, clip) + expect(aabb).toEqual({ minX: 25, minY: 25, maxX: 75, maxY: 75 }) + }) +}) diff --git a/tests/engine/tools/analyze/overlaps/bounds.test.ts b/tests/engine/tools/analyze/overlaps/bounds.test.ts index 5430f6003..c08cc9381 100644 --- a/tests/engine/tools/analyze/overlaps/bounds.test.ts +++ b/tests/engine/tools/analyze/overlaps/bounds.test.ts @@ -178,4 +178,60 @@ describe('analyze overlaps visible bounds', () => { const result = computeOverlaps(graph, { category: 'sibling-overlap' }) expect(result.summary.overlapCount).toBe(0) }) + + test('a stroked rotated node expands bounds along the rotated axes', () => { + const graph = new SceneGraph() + const page = pageId(graph) + + // 10x10 rectangle at (45,45) rotated 45° around its center (50,50). Its + // unstroked canvas AABB is [42.93, 57.07]² (half-diagonal ≈ 7.07). + // An OUTSIDE stroke of weight 20 expands the LOCAL box by 20 on each side; + // after 45° rotation the canvas-space expansion is 20*(|cos45|+|sin45|) ≈ + // 28.28 per side, so the stroked AABB is [14.64, 85.36]². The OLD code + // expanded the already-rotated AABB by 20 → only [22.93, 77.07]². + const stroked = graph.createNode('RECTANGLE', page, { + name: 'RotatedStroked', + x: 45, + y: 45, + width: 10, + height: 10, + rotation: 45, + strokes: [ + { + color: { r: 0, g: 0, b: 0, a: 1 }, + weight: 20, + opacity: 1, + visible: true, + align: 'OUTSIDE' + } + ] + }) + + // Target at canvas (18, 48, 2, 2): inside the rotation-aware stroked AABB + // (18 > 14.64) but outside both the unstroked AABB (18 < 42.93) and the + // OLD naive expansion (18 < 22.93). Only the rotation-aware stroke reaches it. + rect(graph, 'GapTarget', page, 18, 48, 2, 2) + + const result = computeOverlaps(graph, { category: 'sibling-overlap' }) + expect(result.summary.overlapCount).toBeGreaterThan(0) + expect( + result.overlaps.some((o) => o.nodeA.id === stroked.id || o.nodeB.id === stroked.id) + ).toBe(true) + + // Without the stroke, the same target must NOT overlap — proving the reach + // into the gap is caused by the stroked bounds, not the raw node box. + const plain = new SceneGraph() + const plainPage = pageId(plain) + graph.createNode('RECTANGLE', plainPage, { + name: 'RotatedPlain', + x: 45, + y: 45, + width: 10, + height: 10, + rotation: 45 + }) + rect(plain, 'GapTarget', plainPage, 18, 48, 2, 2) + const plainResult = computeOverlaps(plain, { category: 'sibling-overlap' }) + expect(plainResult.summary.overlapCount).toBe(0) + }) }) diff --git a/tests/engine/tools/analyze/overlaps/index.test.ts b/tests/engine/tools/analyze/overlaps/index.test.ts index 14acfb329..825de49fa 100644 --- a/tests/engine/tools/analyze/overlaps/index.test.ts +++ b/tests/engine/tools/analyze/overlaps/index.test.ts @@ -315,6 +315,23 @@ describe('analyze overlaps', () => { expect(result.overlaps.length).toBeGreaterThan(0) }) + test('scope and severity inputs are trimmed of surrounding whitespace', () => { + const graph = new SceneGraph() + const page = pageId(graph) + rect(graph, 'A', page, 0, 0, 100, 100) + rect(graph, 'B', page, 50, 50, 100, 100) + + const scoped = computeOverlaps(graph, { scope: ' same-parent ' as OverlapScope }) + expect(scoped.overlaps.length).toBeGreaterThan(0) + + const severityCapped = computeOverlaps(graph, { + scope: ' same-parent ' as OverlapScope, + severity: ' minor ' as OverlapSeverity + }) + // Both overlaps are minor sibling-overlaps, so trimming severity keeps them. + expect(severityCapped.overlaps.length).toBe(scoped.overlaps.length) + }) + test('ToolDef defaults to current page and does not report overlaps on other pages', () => { const graph = new SceneGraph() const page2 = graph.addPage('Page 2') @@ -417,4 +434,32 @@ describe('analyze overlaps', () => { const totalBySeverity = Object.values(result.summary.bySeverity).reduce((a, b) => a + b, 0) expect(totalBySeverity).toBe(result.summary.overlapCount) }) + + test('limit of zero returns no overlaps while the summary still reports the full set', () => { + const graph = new SceneGraph() + const page = pageId(graph) + const parent = frame(graph, 'Frame', page, 0, 0, 200, 200) + + for (let i = 0; i < 4; i++) { + rect(graph, `R${i}`, parent.id, 0, 0, 100, 100) + } + + const result = computeOverlaps(graph, { scope: 'inside-parent', limit: 0 }) + expect(result.overlaps).toHaveLength(0) + expect(result.summary.overlapCount).toBeGreaterThan(0) + }) + + test('a negative limit is clamped to zero and returns no overlaps', () => { + const graph = new SceneGraph() + const page = pageId(graph) + const parent = frame(graph, 'Frame', page, 0, 0, 200, 200) + + for (let i = 0; i < 4; i++) { + rect(graph, `R${i}`, parent.id, 0, 0, 100, 100) + } + + const result = computeOverlaps(graph, { scope: 'inside-parent', limit: -3 }) + expect(result.overlaps).toHaveLength(0) + expect(result.summary.overlapCount).toBeGreaterThan(0) + }) }) From 99e4153ec592923b6470e3f3352a355c42f3aaed Mon Sep 17 00:00:00 2001 From: Joseph Cumines Date: Fri, 19 Jun 2026 11:47:37 +1000 Subject: [PATCH 4/4] test(tools): create control-case node on the plain graph CodeRabbit found the RotatedPlain control node was created on instead of , so the unstroked control assertion was not actually validating the same setup without a stroke. --- tests/engine/tools/analyze/overlaps/bounds.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/engine/tools/analyze/overlaps/bounds.test.ts b/tests/engine/tools/analyze/overlaps/bounds.test.ts index c08cc9381..5a753513a 100644 --- a/tests/engine/tools/analyze/overlaps/bounds.test.ts +++ b/tests/engine/tools/analyze/overlaps/bounds.test.ts @@ -222,7 +222,7 @@ describe('analyze overlaps visible bounds', () => { // into the gap is caused by the stroked bounds, not the raw node box. const plain = new SceneGraph() const plainPage = pageId(plain) - graph.createNode('RECTANGLE', plainPage, { + plain.createNode('RECTANGLE', plainPage, { name: 'RotatedPlain', x: 45, y: 45,