From 96b8c02e2f81fcca56c9ad3a42ba3176b812c85d Mon Sep 17 00:00:00 2001 From: Ivan Cheung Date: Tue, 23 Jun 2026 02:38:30 +0000 Subject: [PATCH] fix(sgcr): self-loop outside node + stop loop-reserve feedback; cross-axis card fill; label clearance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reported issues on sgcr-demos: - 06-compiler: the 'optimize' self-loop rendered INSIDE a ballooned (140x163) node. Root: #208's card-fills-cell made the measured re-pass read back the loop-reserve-inflated cell as content size, so each pass re-added LOOP_W (36→62→…→163), and the loop (routed in that reserve) ended up under the card. Fix: the DRAWN node rect now EXCLUDES the loop reserve () — the loop attaches on the face and bulges into the reserve BESIDE the card — which also makes the measured re-pass stable (no re-adding). Loop inner vertical now sits at the drawn far edge, not the port bound. - Card fills only the CROSS axis (width TB/BT, height LR/RL), not the along axis, so ports still land on the card edge while multi-line content (e.g. authz '+ challenge') can never be clipped by a forced height. - Edge labels now nudge along their node-free track to keep a 6px clearance from other edges/labels (the 'consent' label was 1.8px from a back-edge → 5.8px). Verified: stress fuzz 10,060 layouts zero violations (P1–P10, determinism, order-invariance); vitest 26/26; compiler loop 0/4 points inside node (compact 150x46). --- .../src/client/renderers/flow-nodes.tsx | 9 ++-- .../src/client/renderers/sgcr/layout.ts | 48 +++++++++++++++---- 2 files changed, 46 insertions(+), 11 deletions(-) diff --git a/packages/viewer/src/client/renderers/flow-nodes.tsx b/packages/viewer/src/client/renderers/flow-nodes.tsx index 73757df..3c0edf2 100644 --- a/packages/viewer/src/client/renderers/flow-nodes.tsx +++ b/packages/viewer/src/client/renderers/flow-nodes.tsx @@ -266,9 +266,12 @@ export function ChangeNode({ data }: NodeProps) { style={{ minWidth: 140, maxWidth: 280, - // Fill the node's reserved cell so edge ports (placed on the CELL face) land on the rendered - // card's edge — never in empty padding when the SGCR engine sized the cell taller than content. - height: "100%", + // Fill only the CROSS axis of the node's reserved cell (width for TB/BT, height for LR/RL) so + // edge ports — placed on the cell's cross face — land on the rendered card's edge even when the + // SGCR engine widened the cell for many ports. The ALONG axis stays content-sized (never forced) + // so multi-line content can't be clipped, and the self-loop reserve (excluded from the cell by + // the engine) stays beside the card, not under it. + ...((d._dir === "LR" || d._dir === "RL") ? { height: "100%" } : { width: "100%" }), boxSizing: "border-box", border: `1px solid ${s.border}`, background: s.bg, diff --git a/packages/viewer/src/client/renderers/sgcr/layout.ts b/packages/viewer/src/client/renderers/sgcr/layout.ts index 83edeb4..57997b3 100644 --- a/packages/viewer/src/client/renderers/sgcr/layout.ts +++ b/packages/viewer/src/client/renderers/sgcr/layout.ts @@ -61,7 +61,8 @@ interface Item { rank: number; order: number; isDummy: boolean; - crossSize: number; // reserved CELL cross extent (≥ drawn size); 0 for dummies + crossSize: number; // reserved CELL cross extent (≥ drawn size + loop reserve); 0 for dummies + loopReserve: number; // cross extent inside the cell reserved on the right for a self-loop (excluded from the DRAWN node rect, so the loop renders beside the card, not inside it) drawnCross: number; // the node's actual drawn cross size (label-driven); 0 for dummies alongSize: number; // node height (TB) / width (LR); 0 for dummies cc: number; // cross center (assigned in coordinate stage) @@ -223,7 +224,7 @@ function buildItemsAndChains( for (const n of nodes) { items.set(n.id, { id: n.id, rank: rank.get(n.id)!, order: 0, isDummy: false, - crossSize: crossSizeOf(n), drawnCross: crossSizeOf(n), alongSize: alongSizeOf(n), cc: 0, alongStart: 0, + crossSize: crossSizeOf(n), loopReserve: 0, drawnCross: crossSizeOf(n), alongSize: alongSizeOf(n), cc: 0, alongStart: 0, }); } const edgeRecs: EdgeRec[] = []; @@ -236,7 +237,7 @@ function buildItemsAndChains( const step = rt > rs ? 1 : -1; for (let r = rs + step; r !== rt; r += step) { const did = `__d_${e.id}_${r}`; - items.set(did, { id: did, rank: r, order: 0, isDummy: true, crossSize: 0, drawnCross: 0, alongSize: 0, cc: 0, alongStart: 0 }); + items.set(did, { id: did, rank: r, order: 0, isDummy: true, crossSize: 0, loopReserve: 0, drawnCross: 0, alongSize: 0, cc: 0, alongStart: 0 }); chain.push(did); } } @@ -360,6 +361,7 @@ function assignCoordinates( for (const it of items.values()) { if (it.isDummy) continue; const loopExtra = selfLoopCount.get(it.id) ? LOOP_W : 0; + it.loopReserve = loopExtra; const faceMax = Math.max(farWires.get(it.id)!.length, nearWires.get(it.id)!.length); const portSpan = faceMax > 0 ? (faceMax - 1) * step + 2 * o.portInset : 0; it.crossSize = Math.max(it.drawnCross, portSpan) + loopExtra; @@ -581,7 +583,9 @@ function assignCoordinates( for (const it of items.values()) { if (it.isDummy || !selfLoopCount.get(it.id)) continue; const cellRight = it.cc + it.crossSize / 2; - it.loopInner = (it.portHi ?? cellRight - LOOP_W) + 4; + // Inner vertical sits at the node's DRAWN far edge (cell minus the loop reserve) so the loop attaches + // on the face and bulges OUT into the reserve — never inside the rendered card. Outer near the cell edge. + it.loopInner = cellRight - it.loopReserve; it.loopOuter = cellRight - 4; } @@ -977,12 +981,13 @@ export function layoutSGCR(input: SGCRInput, opts: SGCROptions = {}): SGCRLayout const outNodes: PositionedNode[] = []; for (const n of nodes) { const it = items.get(n.id)!; - // The node is DRAWN at its full cell cross-size (≥ content) so every distributed port lands on the - // node face — high-degree nodes widen rather than letting an edge attach beside the box. - const crossDim = it.crossSize, alongDim = it.alongSize; + // DRAWN cross size = cell minus the self-loop reserve, so the card fills the port region (ports land + // on its edge) but the loop renders in the reserve BESIDE the card, not inside it. Excluding the + // reserve also keeps the measured re-pass stable (it never reads the reserve back as content size). + const crossDim = it.crossSize - it.loopReserve, alongDim = it.alongSize; const width = horizontal ? alongDim : crossDim; const height = horizontal ? crossDim : alongDim; - const corner = mapPoint({ along: it.alongStart, cross: it.cc - crossDim / 2 }, dir, totalAlong, totalCross); + const corner = mapPoint({ along: it.alongStart, cross: it.cc - it.crossSize / 2 }, dir, totalAlong, totalCross); // cell-left; the loop reserve sits on the +cross side, outside the drawn rect // For BT/RL the mapped corner is the far corner along the flipped axis; normalize to top-left. const x = dir === "RL" ? corner.x - width : corner.x; const y = dir === "BT" ? corner.y - height : corner.y; @@ -1010,7 +1015,34 @@ export function layoutSGCR(input: SGCRInput, opts: SGCROptions = {}): SGCRLayout if (o.routing === "octilinear") { diagonalize(outEdges, outNodes); + nudgeLabels(outEdges, width, height, horizontal); return { nodes: outNodes, edges: outEdges, width, height, direction: dir, routing: "octilinear" }; } + nudgeLabels(outEdges, width, height, horizontal); return { nodes: outNodes, edges: outEdges, width, height, direction: dir, routing: "orthogonal" }; } + +/** Keep each edge label a MIN clearance from OTHER edges and labels: a label rides a node-free track, + * so it can slide along the cross axis (x for TB/BT, y for LR/RL) into a clear spot without ever + * touching a node. Removes the "label crammed against a crossing edge" artifact. Deterministic. */ +function nudgeLabels(edges: RoutedEdge[], width: number, height: number, horizontal: boolean): void { + const MARGIN = 6; + const labels = edges.filter((e) => e.labelBox).map((e) => ({ id: e.id, b: e.labelBox! })).sort((a, b) => (a.id < b.id ? -1 : a.id > b.id ? 1 : 0)); + if (!labels.length) return; + const segs: { id: string; p: Pt; q: Pt }[] = []; + for (const e of edges) for (let i = 0; i + 1 < e.points.length; i++) segs.push({ id: e.id, p: e.points[i], q: e.points[i + 1] }); + const asNode = (b: { x: number; y: number; width: number; height: number }): PositionedNode => ({ id: "", x: b.x, y: b.y, width: b.width, height: b.height, rank: 0, order: 0 }); + const rectsClose = (a: { x: number; y: number; width: number; height: number }, b: { x: number; y: number; width: number; height: number }, m: number) => + a.x < b.x + b.width + m && a.x + a.width + m > b.x && a.y < b.y + b.height + m && a.y + a.height + m > b.y; + const clear = (box: { x: number; y: number; width: number; height: number }, ownerId: string) => + box.x >= 0 && box.y >= 0 && box.x + box.width <= width && box.y + box.height <= height && + !segs.some((s) => s.id !== ownerId && segHitsRect(s.p, s.q, asNode(box), MARGIN)) && + !labels.some((l) => l.id !== ownerId && rectsClose(l.b, box, MARGIN)); + for (const { id, b } of labels) { + if (clear(b, id)) continue; + const axis = horizontal ? "y" : "x"; // cross axis — the label's track is node-free along it + const orig = b[axis]; + let placed = false; + for (let d = 4; d <= 90 && !placed; d += 4) for (const s of [d, -d]) { const cand = { ...b, [axis]: orig + s }; if (clear(cand, id)) { b.x = cand.x; b.y = cand.y; placed = true; break; } } + } +}