From b2c1f2ddefd65528675f93528a17097bef7d000c Mon Sep 17 00:00:00 2001 From: Anand Pant Date: Fri, 30 Oct 2026 23:55:00 -0500 Subject: [PATCH] fix(class): set every class label clearly on its own line Multiplicities, roles and relationship names now only take a spot that is clearly nearer their own line than any other: every other line stands half again as far off and at least 8 units further. End labels try the end's first run, then the end's half of the line; names try every spot before settling for a merely free one. When a label still finds no such spot, the placements run again keeping room for labels (wider port gaps, boxes growing to hold them, and corridors standing clear of a side-by-side line's labels), and one takes over only if it draws no more crossings, detours or hugging runs and no more findings than the labels it clears. The class fuzz gains a label attribution invariant (96 misattributed labels across the 240-model full run on main, none now), the renderer test checks it on every class fixture, and a new stress fixture and story reproduce the observer pattern's opposing named dependencies. Fixes #585 --- packages/diagram/core/src/class-fixtures.ts | 73 ++++++- .../excalidraw/src/lib/class-fuzz.test.ts | 33 ++- .../diagram/renderer/src/class-diagram.ts | 203 +++++++++++++++--- packages/diagram/renderer/src/class-layout.ts | 61 +++++- .../renderer/src/diagram-types/class.test.ts | 34 +++ .../ui/src/diagram-types/class.stories.tsx | 7 +- 6 files changed, 367 insertions(+), 44 deletions(-) diff --git a/packages/diagram/core/src/class-fixtures.ts b/packages/diagram/core/src/class-fixtures.ts index 4e25cc12c..53b54378a 100644 --- a/packages/diagram/core/src/class-fixtures.ts +++ b/packages/diagram/core/src/class-fixtures.ts @@ -12,7 +12,8 @@ const build = (input: Record): ClassDiagram => * Class diagrams that stress the layout: a model of twenty classes, a * hierarchy wider than any row should run, one five levels deep with an * interface several rows above its implementer, a class related to itself - * twice, and two classes in one row with a third between them. + * twice, two classes in one row with a third between them, and two named + * dependencies running opposite ways between a pair of interfaces. */ export const classStressFixtures = { library: build({ @@ -293,4 +294,74 @@ export const classStressFixtures = { { id: "sale", kind: "realization", from: "Sale", to: "PricingStrategy" }, ], }), + // Sol's observer-pattern-class-v2 build (rv583): "attach / detach" stood between its own + // line and "update parameter"'s, 6 units from its own and 4 from the other. + opposingDependencies: build({ + id: "weather-observer", + title: "Weather station observer", + classes: [ + { + id: "Subject", + kind: "interface", + methods: [ + "+ attach(o: Observer): void", + "+ detach(o: Observer): void", + "+ notifyObservers(): void", + ], + }, + { id: "Observer", kind: "interface", methods: ["+ update(subject: Subject): void"] }, + { + id: "WeatherStation", + attributes: ["- temperature: double", "- humidity: double", "- observers: List"], + methods: [ + "+ attach(o: Observer): void", + "+ detach(o: Observer): void", + "+ notifyObservers(): void", + "+ setMeasurements(t: double, h: double): void", + "+ getTemperature(): double", + ], + }, + { + id: "PhoneDisplay", + attributes: ["- lastReading: double"], + methods: ["+ update(subject: Subject): void"], + }, + { id: "WindowDisplay", methods: ["+ update(subject: Subject): void"] }, + ], + relationships: [ + { id: "station-subject", kind: "realization", from: "WeatherStation", to: "Subject" }, + { id: "phone-observer", kind: "realization", from: "PhoneDisplay", to: "Observer" }, + { id: "window-observer", kind: "realization", from: "WindowDisplay", to: "Observer" }, + { + id: "observers", + kind: "association", + from: "WeatherStation", + to: "Observer", + toMultiplicity: "0..*", + toRole: "observers", + navigable: true, + }, + { + id: "attaches", + kind: "dependency", + from: "Subject", + to: "Observer", + label: "attach / detach", + }, + { + id: "updates", + kind: "dependency", + from: "Observer", + to: "Subject", + label: "update parameter", + }, + { + id: "reads", + kind: "dependency", + from: "PhoneDisplay", + to: "WeatherStation", + label: "casts to; getTemperature()", + }, + ], + }), } as const satisfies Record; diff --git a/packages/diagram/excalidraw/src/lib/class-fuzz.test.ts b/packages/diagram/excalidraw/src/lib/class-fuzz.test.ts index e0718d90a..4a72dd167 100644 --- a/packages/diagram/excalidraw/src/lib/class-fuzz.test.ts +++ b/packages/diagram/excalidraw/src/lib/class-fuzz.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it } from "vitest"; import { + type AxisAlignedSegment, candidateRouteQuality, type ClassDiagram, type ClassRelationshipKind, @@ -198,6 +199,12 @@ function routingCounts(scene: RenderedDiagramScene) { const PORT_CLEARANCE = 24; /** Parallel runs of two lines closer than this read as one line. */ const PARALLEL_CLEARANCE = 10; +/** + * A label reads as its own line's only when every other line stands half again + * as far off, and at least this much further. + */ +const ATTRIBUTION_RATIO = 1.5; +const ATTRIBUTION_MARGIN = 8; /** Everything the export validator, the canvas validator or the layout guards would flag. */ function defects(diagram: ClassDiagram): string[] { @@ -319,6 +326,29 @@ function defects(diagram: ClassDiagram): string[] { ) .map((other) => `${label.id} on ${other.id}`), ]); + // Every free label reads as its own line's: every other line stands clearly further off. + const routes = new Map(arrows.map((arrow) => [arrow.edgeId, segmentsFromPoints(arrow.points)])); + const gap = (label: (typeof labels)[number], run: AxisAlignedSegment) => { + const [x0, x1, y0, y1] = + run.orientation === "horizontal" + ? [run.min, run.max, run.staticCoordinate, run.staticCoordinate] + : [run.staticCoordinate, run.staticCoordinate, run.min, run.max]; + return Math.hypot( + Math.max(0, label.left - x1, x0 - label.right), + Math.max(0, label.top - y1, y0 - label.bottom), + ); + }; + const misattributed = labels.flatMap((label) => { + const owner = label.id.split(":")[1]!; + const own = Math.min(...routes.get(owner)!.map((run) => gap(label, run))); + const [other, nearest] = [...routes] + .filter(([id]) => id !== owner) + .map(([id, runs]) => [id, Math.min(...runs.map((run) => gap(label, run)))] as const) + .reduce((best, next) => (next[1] < best[1] ? next : best), ["", Infinity] as const); + return nearest < Math.max(ATTRIBUTION_RATIO * own, own + ATTRIBUTION_MARGIN) + ? [`${label.id} ${own.toFixed(0)} px from its line, ${nearest.toFixed(0)} px from ${other}`] + : []; + }); // No two lines run side by side closer than a reader can tell apart. const runs = arrows.flatMap((arrow) => segmentsFromPoints(arrow.points).map((segment) => ({ arrow: arrow.id, segment })), @@ -347,6 +377,7 @@ function defects(diagram: ClassDiagram): string[] { ...packed, ...unwrapped, ...crowded, + ...misattributed, ...hugging, ]; } @@ -356,7 +387,7 @@ function defects(diagram: ClassDiagram): string[] { * draws, in order. None may draw more crossings, nor more crossings and * findings together, and together they may draw no more of either. */ -const CLASS_ROUTING = "0/0 0/0 0/0 2/1 0/0 0/0 0/2 0/0 0/0 1/0 0/0 0/1 0/0 0/0 0/0 0/0"; +const CLASS_ROUTING = "0/0 0/0 0/0 0/2 0/0 0/0 0/2 0/0 0/0 1/0 0/0 0/1 0/0 0/0 0/0 0/0"; /** Models checked in CI; the full fuzz runs locally with CLASS_FUZZ=full (and any CLASS_FUZZ_SEED). */ const CI_MODELS = 16; diff --git a/packages/diagram/renderer/src/class-diagram.ts b/packages/diagram/renderer/src/class-diagram.ts index 165d713db..e5498f78e 100644 --- a/packages/diagram/renderer/src/class-diagram.ts +++ b/packages/diagram/renderer/src/class-diagram.ts @@ -55,6 +55,12 @@ const LABEL_HEIGHT = Math.ceil(LABEL_FONT_SIZE * 1.35); const LABEL_OFFSET = 10; /** Room kept between a label and every box, line and other label. */ const LABEL_CLEARANCE = 3; +/** + * A label reads as its own line's only when every other line stands half again + * as far off as its own, and at least this much further. + */ +const ATTRIBUTION_RATIO = 1.5; +const ATTRIBUTION_MARGIN = 8; /** Past this, a layout's export reads poorly; a candidate pays for any excess. */ const READABLE_ASPECT = 2.5; const NAME_FILL = "#efe6ec"; @@ -376,8 +382,19 @@ function detour(arrow: ArrowSceneElement): number { return Math.max(0, length / (direct + 2 * CLASS_END_RUN + 200) - 1); } -/** Route each distinct candidate placement and keep the one that reads best. */ -function bestScene(diagram: ClassDiagram, boxes: ReadonlyMap): RenderedDiagramScene { +/** A routed scene's class boxes, without the room it held for labels. */ +function classBoxes(scene: RenderedDiagramScene): NodeSceneElement[] { + return scene.elements.filter( + (element): element is NodeSceneElement => + element.type === "node" && !element.nodeId.startsWith(LABEL_ROOM_PREFIX), + ); +} + +/** Route each distinct candidate placement, keep the one that reads best, and place its labels. */ +function bestScene( + diagram: ClassDiagram, + boxes: ReadonlyMap, +): { readonly scene: RenderedDiagramScene; readonly labels: readonly TextSceneElement[] } { const links: ClassLayoutLink[] = diagram.relationships.map((relationship) => ({ id: relationship.id, from: relationship.from.trim(), @@ -432,7 +449,42 @@ function bestScene(diagram: ClassDiagram, boxes: ReadonlyMap): Rend best = routed; if (routed.cost === 0 && !long(best)) break; } - return best!.scene; + // While a label stands clearly its own line's nowhere, the placements run again + // keeping room for labels; one takes its place when it leaves fewer such labels, + // draws no more crossings, detours or hugging runs, reads no longer, and draws + // no more crossings and findings together than one more for each label it clears: + // a label read as another line's misleads a reader more than a jog does. + const labelled = (routed: Routed) => ({ + ...routed, + labels: placeLabels( + diagram, + routed.scene.elements.filter((e): e is ArrowSceneElement => e.type === "arrow"), + classBoxes(routed.scene), + ), + }); + let chosen = labelled(best!); + for (const option of options) { + if (chosen.labels.unclear === 0) break; + const placement = placeClasses(layoutBoxes, links, { ...option, labelRoom: true }); + const key = JSON.stringify([...placement.boxes, ...placement.anchors]); + if (tried.has(key)) continue; + tried.add(key); + const scene = routedScene(diagram, boxes, placement); + const counts = routedCounts(scene); + const routed = labelled({ cost: routedCost(scene, counts), counts, scene }); + const was = chosen.counts; + if ( + routed.labels.unclear < chosen.labels.unclear && + (!long(routed) || exportAspect(scene) <= exportAspect(chosen.scene)) && + counts.crossings <= was.crossings && + counts.crossings + counts.findings <= + was.crossings + was.findings + chosen.labels.unclear - routed.labels.unclear && + counts.detours <= was.detours && + counts.hugging <= was.hugging + ) + chosen = routed; + } + return { scene: chosen.scene, labels: chosen.labels.texts }; } /** A class's elements: its outline, its name band, its methods compartment and their texts. */ @@ -538,6 +590,17 @@ function rectTouchesSegment(rect: Rect, segment: AxisAlignedSegment, gap: number segment.min < rect.bottom + gap; } +function rectSegmentDistance(rect: Rect, segment: AxisAlignedSegment): number { + const [x0, x1, y0, y1] = + segment.orientation === "horizontal" + ? [segment.min, segment.max, segment.staticCoordinate, segment.staticCoordinate] + : [segment.staticCoordinate, segment.staticCoordinate, segment.min, segment.max]; + return Math.hypot( + Math.max(0, rect.left - x1, x0 - rect.right), + Math.max(0, rect.top - y1, y0 - rect.bottom), + ); +} + /** * Where a label may go along one run of a line, nearest its start first: on * either side of the run, `along` units out from the run's start. @@ -572,30 +635,78 @@ function* labelSpots( } } +/** A route from its start up to halfway along it. */ +function halfway(points: readonly { x: number; y: number }[]): { x: number; y: number }[] { + const lengths = points + .slice(1) + .map((point, index) => Math.hypot(point.x - points[index]!.x, point.y - points[index]!.y)); + let rest = lengths.reduce((sum, length) => sum + length, 0) / 2; + const half = [points[0]!]; + for (const [index, length] of lengths.entries()) { + const [from, to] = [points[index]!, points[index + 1]!]; + if (length >= rest) { + const t = length > 0 ? rest / length : 0; + half.push({ x: from.x + (to.x - from.x) * t, y: from.y + (to.y - from.y) * t }); + break; + } + half.push(to); + rest -= length; + } + return half; +} + /** * Place each multiplicity and role beside its end's first run, the two on * opposite sides of the line, and each relationship's name beside its - * longest run, every one clear of the boxes, the lines and the others. + * longest run, every one clear of the boxes, the lines and the others, and + * clearly nearer its own line than any other, so none reads as another's. */ function placeLabels( diagram: ClassDiagram, arrows: readonly ArrowSceneElement[], boxes: readonly NodeSceneElement[], -): TextSceneElement[] { +): { readonly texts: readonly TextSceneElement[]; readonly unclear: number } { const boxRects = boxes.map((box) => ({ left: box.x, top: box.y, right: box.x + box.width, bottom: box.y + box.height, })); - const segments = arrows.flatMap((arrow) => segmentsFromPoints(arrow.points)); + const owned = arrows.flatMap((arrow) => + segmentsFromPoints(arrow.points).map((segment) => ({ owner: arrow.edgeId, segment })), + ); + const segments = owned.map((run) => run.segment); const placed: Rect[] = []; const collisions = (rect: Rect) => boxRects.filter((box) => rectsOverlap(rect, box, LABEL_CLEARANCE)).length + segments.filter((segment) => rectTouchesSegment(rect, segment, LABEL_CLEARANCE)).length + placed.filter((other) => rectsOverlap(rect, other, LABEL_CLEARANCE)).length; const free = (rect: Rect) => collisions(rect) === 0; + const anywhere = () => free; + // Free, and the text as drawn within its room stands clearly nearer its own line. + const attributed = (owner: string, text: string) => (rect: Rect) => { + if (!free(rect)) return false; + const inset = { + x: (labelWidth(text) - estimateTextWidth(text, LABEL_FONT_SIZE)) / 2, + y: (text.split("\n").length * (LABEL_HEIGHT - LABEL_FONT_SIZE * 1.35)) / 2, + }; + const drawn = { + left: rect.left + inset.x, + top: rect.top + inset.y, + right: rect.right - inset.x, + bottom: rect.bottom - inset.y, + }; + let [own, other] = [Infinity, Infinity]; + for (const run of owned) { + const distance = rectSegmentDistance(drawn, run.segment); + if (run.owner === owner) own = Math.min(own, distance); + else other = Math.min(other, distance); + } + return other >= Math.max(ATTRIBUTION_RATIO * own, own + ATTRIBUTION_MARGIN); + }; const texts: TextSceneElement[] = []; + // Labels no spot left clearly their own line's. + let unclear = 0; const add = (id: string, text: string, rect: Rect, groupId?: string) => { placed.push(rect); texts.push({ @@ -610,7 +721,7 @@ function placeLabels( }); }; const byId = new Map(arrows.map((arrow) => [arrow.edgeId, arrow])); - // The first free spot along the route from one end, nearest that end first. + // The spots along the route from one end, nearest that end first. const spots = ( points: readonly { x: number; y: number }[], width: number, @@ -623,7 +734,6 @@ function placeLabels( .flatMap((point, index) => [ ...labelSpots(point, points[index + 1]!, width, height, sides, index === 0 ? start : 6), ]); - const along = (...spot: Parameters): Rect | undefined => spots(...spot).find(free); for (const relationship of diagram.relationships) { const arrow = byId.get(relationship.id); if (!arrow) continue; @@ -636,19 +746,16 @@ function placeLabels( relationship[end === "from" ? "fromMultiplicity" : "toMultiplicity"] && "multiplicity", relationship[end === "from" ? "fromRole" : "toRole"] && "role", ].filter((key): key is string => typeof key === "string"); - // Two labels take opposite sides, whichever way round fits; then each takes - // the nearest free spot on either side; then, with none free, its first spot. - const orders: (readonly (-1 | 1)[])[] = - labels.length === 2 - ? [ - [1, -1], - [-1, 1], - ] - : [[1], [-1]]; - const place = (side: (index: number) => readonly (-1 | 1)[]) => { + const place = ( + points: readonly { x: number; y: number }[], + side: (index: number) => readonly (-1 | 1)[], + accept: (text: string) => (rect: Rect) => boolean, + ) => { const rects: Rect[] = []; for (const [index, label] of labels.entries()) { - const rect = along(route, labelWidth(label), LABEL_HEIGHT, side(index), 4); + const rect = spots(points, labelWidth(label), LABEL_HEIGHT, side(index), 4).find( + accept(label), + ); if (!rect) break; rects.push(rect); placed.push(rect); @@ -656,9 +763,37 @@ function placeLabels( placed.splice(placed.length - rects.length, rects.length); return rects.length === labels.length ? rects : undefined; }; + // On the end's first run, clearly its own line's: two labels on opposite + // sides, whichever way round fits, else together on one side; one label on + // either side, nearest the end first. + const mine = (text: string) => attributed(relationship.id, text); + const sides: ((index: number) => readonly (-1 | 1)[])[] = + labels.length === 2 + ? [ + (index) => [index === 0 ? 1 : -1], + (index) => [index === 0 ? -1 : 1], + () => [1], + () => [-1], + ] + : [() => [1, -1]]; + // Failing that, each the nearest such spot on either side along the end's + // half of the line; then opposite sides, then each the nearest free spot on + // either side anywhere along the line; then, with none free, its first spot. + const orders: (readonly (-1 | 1)[])[] = + labels.length === 2 + ? [ + [1, -1], + [-1, 1], + ] + : [[1], [-1]]; + const clear = + sides.map((side) => place(route.slice(0, 2), side, mine)).find(Boolean) ?? + place(halfway(route), () => [1, -1], mine); + if (!clear) unclear += labels.length; const chosen = - orders.map((order) => place((index) => [order[index]!])).find(Boolean) ?? - place(() => [1, -1]) ?? + clear ?? + orders.map((order) => place(route, (index) => [order[index]!], anywhere)).find(Boolean) ?? + place(route, () => [1, -1], anywhere) ?? labels.map((label) => spots(route, labelWidth(label), LABEL_HEIGHT, [1, -1], 4)[0]!); for (const [index, rect] of chosen.entries()) add( @@ -670,7 +805,8 @@ function placeLabels( } } // Each name beside its longest run, else anywhere along its line past the end runs; - // a long name with no room on one line wraps onto up to five lines. + // a long name with no room on one line wraps onto up to five lines. A spot clearly + // its own line's comes first, then any free one. for (const relationship of diagram.relationships) { const arrow = byId.get(relationship.id); if (!arrow || !relationship.label) continue; @@ -696,9 +832,13 @@ function placeLabels( ...spots(arrow.points, width, height, [-1, 1], 0), ]; }; - const found = candidates - .map((text) => ({ text, rect: search(text).find(free) })) - .find((candidate) => candidate.rect !== undefined); + const first = (accept: (text: string) => (rect: Rect) => boolean) => + candidates + .map((text) => ({ text, rect: search(text).find(accept(text)) })) + .find((candidate) => candidate.rect !== undefined); + const clear = first((text) => attributed(relationship.id, text)); + if (!clear) unclear += 1; + const found = clear ?? first(anywhere); if (found) { add(`relationship:${relationship.id}:name`, found.text, found.rect!); continue; @@ -709,7 +849,7 @@ function placeLabels( .reduce((best, candidate) => (candidate.hits < best.hits ? candidate : best)); add(`relationship:${relationship.id}:name`, fallback.text, fallback.rect); } - return texts; + return { texts, unclear }; } /** Shift every element so nothing sits nearer the origin than the scene's padding. */ @@ -771,14 +911,8 @@ function padded(elements: readonly SceneElement[]): { */ export function renderClassDiagram(diagram: ClassDiagram): RenderedDiagramScene { const boxes = new Map(diagram.classes.map((node) => [node.id, boxOf(node)])); - const scene = bestScene(diagram, boxes); - const placed = new Map( - scene.elements.flatMap((element) => - element.type === "node" && !element.nodeId.startsWith(LABEL_ROOM_PREFIX) - ? [[element.nodeId, element] as const] - : [], - ), - ); + const { scene, labels } = bestScene(diagram, boxes); + const placed = new Map(classBoxes(scene).map((element) => [element.nodeId, element])); const drawn = diagram.classes.map((node) => boxElements(boxes.get(node.id)!, placed.get(node.id)!, diagram), ); @@ -797,7 +931,6 @@ export function renderClassDiagram(diagram: ClassDiagram): RenderedDiagramScene }, ]; }); - const labels = placeLabels(diagram, arrows, [...placed.values()]); const { elements, width, height } = padded([ ...drawn.flatMap((box) => [...box.nodes, ...box.texts]), ...arrows, diff --git a/packages/diagram/renderer/src/class-layout.ts b/packages/diagram/renderer/src/class-layout.ts index 6e1672452..2869e7637 100644 --- a/packages/diagram/renderer/src/class-layout.ts +++ b/packages/diagram/renderer/src/class-layout.ts @@ -50,6 +50,12 @@ export interface ClassPlacementOptions { * hierarchy can step a row down beside another instead of sharing its rows. */ readonly hierarchies?: boolean; + /** + * Keep the wider room that leaves each label clearly its own line's: between + * neighboring ports, every box growing to hold it on each of its sides, and + * between an end and a corridor passing beside it. + */ + readonly labelRoom?: boolean; } export interface ClassPlacement { @@ -84,6 +90,13 @@ const PORT_STEP = 32; */ const LABEL_ROOM = 14; const LABEL_ROW = 26; +/** + * The room that instead leaves a neighbor's line clearly further from a label + * than its own: past a label's width beside a vertical run, or its height + * beside a horizontal one. + */ +const CLEAR_LABEL_ROOM = 30; +const CLEAR_LABEL_ROW = 40; /** Height a channel adds above its tracks when a named line crosses it: a name on three lines. */ const NAME_ROOM = 54; /** Ports keep this far from a side's corners, past the router's own port padding. */ @@ -813,6 +826,8 @@ interface Spacing { readonly port: number; readonly track: number; readonly lane: number; + /** Keep the room that leaves each label clearly its own line's. */ + readonly clearLabels: boolean; } /** @@ -821,6 +836,8 @@ interface Spacing { * point their line heads for, so lines between rows run straight where they * can. A line that would still jog a little runs straight when one end can * move onto the other's line, and no port sits on a compartment's divider. + * Returns the length each box needs across and down to hold every side's + * ports with all their room. */ function placePorts( ends: readonly End[], @@ -829,7 +846,7 @@ function placePorts( labels: (end: End) => { readonly end: number; readonly name: number }, rowOf: (item: number) => number, spacing: Spacing, -): void { +): Map { const { port: portStep, lane: laneClearance } = spacing; const sides = new Map(); for (const end of ends) { @@ -843,10 +860,11 @@ function placePorts( // rows, its name wrapped onto two lines. A side too short shares out what it has. const roomOf = (end: End) => { const { end: label, name } = labels(end); - if (!horizontal(end.side)) return label > 0 ? LABEL_ROW : 0; + if (!horizontal(end.side)) + return label > 0 ? (spacing.clearLabels ? CLEAR_LABEL_ROW : LABEL_ROW) : 0; // A name on two lines runs a word wider than half of it. const widest = Math.max(label, end.loop || name === 0 ? 0 : Math.ceil(name / 2) + LABEL_ROOM); - return widest > 0 ? widest + LABEL_ROOM : 0; + return widest > 0 ? widest + (spacing.clearLabels ? CLEAR_LABEL_ROOM : LABEL_ROOM) : 0; }; const gapOf = (a: End, b: End) => { const [p, q] = [roomOf(a), roomOf(b)]; @@ -1033,6 +1051,22 @@ function placePorts( break; } } + const needs = new Map(); + for (const list of sides.values()) { + const ordered = list.toSorted((a, b) => a.at - b.at); + const need = + 2 * PORT_MARGIN + + ordered.slice(1).reduce((sum, end, index) => sum + gapOf(ordered[index]!, end), 0); + const { box: item, side } = ordered[0]!; + const known = needs.get(item) ?? { width: 0, height: 0 }; + needs.set( + item, + horizontal(side) + ? { ...known, width: Math.max(known.width, need) } + : { ...known, height: Math.max(known.height, need) }, + ); + } + return needs; } /** A line's sideways run in a channel between two rows: from its verticals above to those below. */ @@ -1196,8 +1230,9 @@ export function placeClasses( const [near, far] = p < q ? [from, to] : [to, from]; const [nearLabel, farLabel] = p < q ? [link.fromLabel, link.toLabel] : [link.toLabel, link.fromLabel]; - widen(near, between[0]!, Math.max(CLASS_END_RUN, nearLabel + 16)); - widen(between.at(-1)!, far, Math.max(CLASS_END_RUN, farLabel + 16)); + const past = options.labelRoom ? CLEAR_LABEL_ROOM : 16; + widen(near, between[0]!, Math.max(CLASS_END_RUN, nearLabel + past)); + widen(between.at(-1)!, far, Math.max(CLASS_END_RUN, farLabel + past)); } // A loop keeps room for its run and labels from the nearest box on each side, // past any corridor between them. @@ -1239,6 +1274,7 @@ export function placeClasses( port: Math.max(PORT_STEP, gap), track: Math.max(CLASS_TRACK_STEP, gap), lane: Math.max(LANE_CLEARANCE, gap), + clearLabels: options.labelRoom === true, }; const ends = endsOf(graph, layers, rows, center); if (attempt < MAX_GROWTH) { @@ -1265,7 +1301,20 @@ export function placeClasses( (item) => layers.row[item]!, spacing, ); - ports(); + const needs = ports(); + // With room kept for clear labels, a box too short for it grows and the layout runs again. + if (options.labelRoom && attempt < MAX_GROWTH) { + const grown = boxes.map((box, item) => { + const need = needs.get(item); + const [width, height] = [ + Math.ceil(Math.max(box.width, need?.width ?? 0)), + Math.ceil(Math.max(box.height, need?.height ?? 0)), + ]; + return width === box.width && height === box.height ? box : { ...box, width, height }; + }); + if (grown.some((box, item) => box !== boxes[item])) + return placeClasses(grown, links, options, attempt + 1); + } const left = (item: number) => center[item]! - size(item).width / 2; const portX = (end: End) => left(end.box) + end.at; const linkEnds = links.map((_, index) => diff --git a/packages/diagram/renderer/src/diagram-types/class.test.ts b/packages/diagram/renderer/src/diagram-types/class.test.ts index 977b9145c..0fb158991 100644 --- a/packages/diagram/renderer/src/diagram-types/class.test.ts +++ b/packages/diagram/renderer/src/diagram-types/class.test.ts @@ -303,6 +303,40 @@ describe("class diagram renderer", () => { if (role.type === "text") expect(Math.hypot(role.x - tip.x, role.y - tip.y)).toBeLessThan(80); }); + it("sets every label clearly nearer its own line than any other", () => { + // Each other line stands half again as far from the label as its own, and 8 units further. + const gap = (label: ReturnType[number], line: ArrowSceneElement) => + Math.min( + ...segmentsFromPoints(line.points).map((run) => { + const [low, high] = [run.min, run.max]; + const along = + run.orientation === "horizontal" + ? Math.max(0, label.left - high, low - label.right) + : Math.max(0, label.top - high, low - label.bottom); + const across = + run.orientation === "horizontal" + ? Math.max(0, label.top - run.staticCoordinate, run.staticCoordinate - label.bottom) + : Math.max(0, label.left - run.staticCoordinate, run.staticCoordinate - label.right); + return Math.hypot(along, across); + }), + ); + for (const diagram of all) { + const scene = renderDiagram(diagram); + for (const label of labelBoxes(scene)) { + const own = arrow(scene, label.id.split(":")[1]!); + const mine = gap(label, own); + const nearest = Math.min( + ...arrows(scene) + .filter((line) => line !== own) + .map((line) => gap(label, line)), + ); + expect(nearest, `${diagram.id} ${label.id}`).toBeGreaterThanOrEqual( + Math.max(1.5 * mine, mine + 8), + ); + } + } + }); + it("loops a relationship to its own class out and back on one side, nesting a second", () => { const scene = renderDiagram(classStressFixtures.selfReference); const employee = box(scene, "Employee"); diff --git a/packages/diagram/ui/src/diagram-types/class.stories.tsx b/packages/diagram/ui/src/diagram-types/class.stories.tsx index fcda5a52c..df0036f25 100644 --- a/packages/diagram/ui/src/diagram-types/class.stories.tsx +++ b/packages/diagram/ui/src/diagram-types/class.stories.tsx @@ -1,6 +1,6 @@ import type { Meta, StoryObj } from "@storybook/react-vite"; -import { classFixture } from "@sketchi/diagram-core"; +import { classFixture, classStressFixtures } from "@sketchi/diagram-core"; import { classScenarios } from "@sketchi/diagram-scenarios"; import { GenerationWorkspace } from "../components/generation-workspace"; @@ -38,6 +38,11 @@ export const ObserverPattern: Story = { args: { diagram: scenarioDiagram("observer-pattern-class-v2") }, }; +/** Two named dependencies running opposite ways between two interfaces, each name beside its own line. */ +export const OpposingDependencies: Story = { + args: { diagram: classStressFixtures.opposingDependencies }, +}; + /** Fifteen classes in rows, two hierarchies and the associations between them. */ export const LargeModel: Story = { args: { diagram: scenarioDiagram("library-system-class-v2") },