diff --git a/docs/layout.md b/docs/layout.md index d863b4c9..9d10a09c 100644 --- a/docs/layout.md +++ b/docs/layout.md @@ -20,7 +20,13 @@ Every write updates the diagram. `--layout ` says how; `--relayout` is `-- | `full` | the engine redraws everything; colours and DI ids are kept, positions are not | So a drawing you formatted by hand, here or in a modeler, stays as it is. A format command that moves -a shape or reroutes a flow makes the drawing hand-made, so your formatting persists. If an +a shape or reroutes a flow makes the drawing hand-made, so your formatting persists. + +Room is made only where it is needed: what follows the new element moves right, with what it would +run into on the way, and by no more than the room that is missing. Another row of the diagram keeps +its place, so a task added to a rejection path leaves the main path above it alone. A new flow that +would enter an element left of its source (not a loop return) moves that element and what follows it +right of the source. If an incremental update fails, `auto` falls back to a full redraw (`W_LAYOUT_INCREMENTAL_FAILED`).
diff --git a/plugins/bpmn-cli/skills/bpmn/SKILL.md b/plugins/bpmn-cli/skills/bpmn/SKILL.md index c9bd78a8..92dfcf26 100644 --- a/plugins/bpmn-cli/skills/bpmn/SKILL.md +++ b/plugins/bpmn-cli/skills/bpmn/SKILL.md @@ -237,9 +237,9 @@ Details, every op key and worked batches: [reference/ops.md](reference/ops.md). - `auto` (default): a new file or a drawing exactly as the engine would draw it is redrawn in full; anything hand-made or formatted is **kept** - new - elements are placed locally, room is made like the modeler's space tool, - only affected flows are rerouted (and a line that does not reach its ends, - `detachedEdge`, is routed again). + elements are placed locally, room is made for what follows them (other rows + stay), only affected flows are rerouted (and a line that does not reach its + ends, `detachedEdge`, is routed again). - `--layout incremental`: always keep (fails with `E_LAYOUT_INCREMENTAL` instead of redrawing) - use it when keeping the drawing is a requirement. - `--relayout` (= `--layout full`, `bpmn layout `): redraw everything; diff --git a/src/adapters/in/report/AgentGuide.ts b/src/adapters/in/report/AgentGuide.ts index 1f5f77eb..f9571cd2 100644 --- a/src/adapters/in/report/AgentGuide.ts +++ b/src/adapters/in/report/AgentGuide.ts @@ -266,11 +266,13 @@ export class AgentGuide { [ ' auto (default) no diagram yet, or the drawing is exactly what the engine would draw (nobody changed it):', ' redraw in full. Otherwise keep it: new elements are placed next to their neighbours, room is', - ' made like the modeler\'s space tool, only affected flows are rerouted, removed elements\' DI pruned.', + ' made for what follows them (and what that runs into; other rows stay), only affected flows are', + ' rerouted, removed elements\' DI pruned.', ' A line that does not reach an end of its own (left at an old place by an older --no-layout', ' write or another tool) is routed again and listed as rerouted.', ' A batch places its new nodes like the same ops sent one by one (exception paths after the', - ' normal flow); a `connect` that would enter a merge backwards moves the merge after its sources.', + ' normal flow); a flow that would enter a node backwards (no loop return) moves it and what', + ' follows it right of its source (a merge: after all its sources).', ' A new branch / boundary path whose rows below cross a line opens the row right under its anchor', ' (the line moves down), or goes above it (a boundary event the CLI placed then on the top border;', ' one drawn by hand keeps its border; the path of an event on the top border runs above its host);', diff --git a/src/domain/diagram/di/PlaneQuery.ts b/src/domain/diagram/di/PlaneQuery.ts index 616c0dc3..a7f1f6f3 100644 --- a/src/domain/diagram/di/PlaneQuery.ts +++ b/src/domain/diagram/di/PlaneQuery.ts @@ -51,14 +51,15 @@ export class PlaneQuery { return [...plane.shapes.values()].filter((shape) => shape.hostId === hostId); } - /** Gap and row spacing of a plane, measured on its sequence flows. */ - static spacingOf(plane: Plane): Spacing { + /** Gap and row spacing of a plane, measured on its sequence flows (between the ends `endsOf` names; none: skipped). */ + static spacingOf(plane: Plane, endsOf: (edge: DEdge) => Pick | undefined = (edge) => edge): Spacing { const gaps: number[] = []; const rows: number[] = []; for (const edge of plane.edges.values()) { - if (edge.kind !== 'sequenceFlow' || !edge.sourceId || !edge.targetId) continue; - const source = plane.shapes.get(edge.sourceId); - const target = plane.shapes.get(edge.targetId); + const ends = edge.kind === 'sequenceFlow' ? endsOf(edge) : undefined; + if (!ends?.sourceId || !ends.targetId) continue; + const source = plane.shapes.get(ends.sourceId); + const target = plane.shapes.get(ends.targetId); if (!source || !target || source.kind === 'boundary' || source.container || target.container) continue; const dy = Math.abs(cy(source.bounds) - cy(target.bounds)); if (dy <= 5) { diff --git a/src/domain/diagram/layout/incremental/CrossingRepair.ts b/src/domain/diagram/layout/incremental/CrossingRepair.ts index b8d03cfc..f2bfc0c9 100644 --- a/src/domain/diagram/layout/incremental/CrossingRepair.ts +++ b/src/domain/diagram/layout/incremental/CrossingRepair.ts @@ -3,7 +3,8 @@ import { LayoutDebug } from '../LayoutDebug.js'; import { LayoutAnalysis } from '../../di/LayoutAnalysis.js'; import { TrialGuard, type TrialPolicy } from '../../di/TrialGuard.js'; import type { DEdge, Plane, Point } from '../../di/types.js'; -import { copyBox, polylinesCross, samePoints } from '../../utils/geom.js'; +import { copyBox, polylinesCross, samePoints, toSegments } from '../../utils/geom.js'; +import { collinear } from '../../utils/harness.js'; import { raw } from '../../utils/raw.js'; import { isHardProblem, problemKey, type LayoutMetrics } from '../../utils/problems.js'; import { PlaneWriter } from '../../di/PlaneWriter.js'; @@ -14,10 +15,14 @@ const REPAIR_TRIALS = 8; const CHAIN_RETRIES = 2; +/** px a rerouted hand-drawn line may run along more of the others, the length of a parallel run */ +const RUN_SLACK = 20; + /** * Removes crossings between connections drawn in this run and existing ones: first by rerouting, * then by placing the new chain at its next row candidate. A trial stays only when it lowers the - * score and adds no hard problem and no crossing among the hand-drawn lines. + * score and adds no hard problem and no crossing among the hand-drawn lines. A hand-drawn line is + * not laid on top of the others to save a crossing: it would read as their continuation. */ export class CrossingRepair { constructor(private readonly run: IncrementalLayout) {} @@ -80,7 +85,10 @@ export class CrossingRepair { return undefined; } - /** A route that still crosses is skipped without measuring, as the guard would reject it anyway. */ + /** + * A route that still crosses is skipped without measuring, as the guard would reject it anyway; so + * is one that lays a hand-drawn line along more of the others. + */ async repair(rerouted: string[], start: Checkpoint, redraw: () => Promise): Promise { if (!this.newCrossings().length) return rerouted; let trials = 0; @@ -113,8 +121,9 @@ export class CrossingRepair { const pts = drawing.route(target, { lines: drawing.routingLines(target.id) }); if (!pts || pts.length < 2 || samePoints(pts, target.points)) continue; const other = target === freshEdge ? keptEdge : freshEdge; - if (polylinesCross(pts, other.points) || (target === keptEdge && crossesNewDrawnLine(plane, target, pts))) { - if (LayoutDebug.on()) LayoutDebug.log(`[repair] ${target.id} routed again (it crossed: ${freshEdge.id} x ${keptEdge.id}): still crossing, not measured`); + const crossing = polylinesCross(pts, other.points) || (target === keptEdge && crossesNewDrawnLine(plane, target, pts)); + if (crossing || (target === keptEdge && runsAlongDrawnLines(plane, target, pts))) { + if (LayoutDebug.on()) LayoutDebug.log(`[repair] ${target.id} routed again (it crossed: ${freshEdge.id} x ${keptEdge.id}): ${crossing ? 'still crossing' : 'along drawn lines'}, not measured`); continue; } trials++; @@ -137,6 +146,19 @@ export class CrossingRepair { }; const crossesNewDrawnLine = (plane: Plane, target: DEdge, pts: readonly Point[]): boolean => [...plane.edges.values()].some((other) => other !== target && !this.run.newEdges.has(other.id) && other.points.length >= 2 && polylinesCross(pts, other.points) && !polylinesCross(target.points, other.points)); + /** collinear with more of the drawn lines that share no end with the edge than its points are, measured like a parallel run */ + const runsAlongDrawnLines = (plane: Plane, target: DEdge, pts: Point[]): boolean => { + const along = (points: Point[]): number => { + const segs = toSegments(points); + let len = 0; + for (const other of plane.edges.values()) { + if (other === target || this.run.newEdges.has(other.id) || [other.sourceId, other.targetId].some((end) => end === target.sourceId || end === target.targetId)) continue; + for (const sb of toSegments(other.points)) for (const sa of segs) len += Math.max(0, collinear(sa, sb)); + } + return len; + }; + return along(pts) > along(target.points) + RUN_SLACK; + }; reroutes(rerouted); const tried = new Map(); while (trials < REPAIR_TRIALS && retries < CHAIN_RETRIES && !this.run.fromEngine.size && this.run.planes.length === start.planes.length) { diff --git a/src/domain/diagram/layout/incremental/Drawing.ts b/src/domain/diagram/layout/incremental/Drawing.ts index 76193b86..61c4497e 100644 --- a/src/domain/diagram/layout/incremental/Drawing.ts +++ b/src/domain/diagram/layout/incremental/Drawing.ts @@ -9,7 +9,7 @@ import type { DrawingSnapshot } from './DrawingSnapshot.js'; import type { TrialGuard } from '../../di/TrialGuard.js'; import type { Box, DEdge, DShape, Plane, Point, Side } from '../../di/types.js'; import type { Semantics } from '../../di/Semantics.js'; -import { segmentHits, translateEdge } from '../../utils/geom.js'; +import { pathHits, translateEdge } from '../../utils/geom.js'; import { hasExternalLabel, labelSideOf, type LabelSide } from '../../utils/labels.js'; import { raw } from '../../utils/raw.js'; @@ -24,12 +24,14 @@ export class Drawing { private readonly separation: Separation; private readonly frames: FrameEditor; + /** `fresh`: the nodes placed in this run (EdgeRouter) */ constructor( readonly plane: Plane, readonly touched: Set = new Set(), + fresh?: ReadonlySet, ) { this.space = new SpaceTool(plane); - this.routes = new EdgeRouter(plane); + this.routes = new EdgeRouter(plane, fresh); this.labels = new LabelPlacer(plane); this.separation = new Separation(plane, this.space); this.frames = new FrameEditor(plane, this.space, touched); @@ -103,6 +105,14 @@ export class Drawing { return this.routes.detachedEnds(edge); } + grazes(edge: DEdge, ids: Iterable): boolean { + return this.routes.grazes(edge, ids); + } + + touchesFresh(edge: DEdge): boolean { + return this.routes.touchesFresh(edge); + } + reroute(edge: DEdge, opts: { lines?: Point[][]; sourceSides?: Side[]; targetSides?: Side[] } = {}): boolean { const { lines, ...sides } = opts; const pts = this.routes.routeEdge(edge, { lines: lines ?? this.routes.routingLines(edge.id), ...sides }); @@ -137,10 +147,8 @@ export class Drawing { } clearLabels(fresh: readonly DEdge[]): void { - const segs = fresh.flatMap((edge) => edge.points.slice(1).map((end, index) => [edge.points[index]!, end] as const)); - if (!segs.length) return; for (const shape of this.plane.shapes.values()) { - if (!shape.label || !hasExternalLabel(shape) || !segs.some(([start, end]) => segmentHits(start, end, shape.label!, 1))) continue; + if (!shape.label || !hasExternalLabel(shape) || !fresh.some((edge) => pathHits(edge.points, shape.label!, 1))) continue; const host = shape.hostId ? this.plane.shapes.get(shape.hostId)?.bounds : undefined; const side = labelSideOf(shape, host); this.labels.placeShapeLabel(shape, side === 'inside' ? undefined : side); diff --git a/src/domain/diagram/layout/incremental/EdgeRouter.ts b/src/domain/diagram/layout/incremental/EdgeRouter.ts index a5bd5d39..ebf19d34 100644 --- a/src/domain/diagram/layout/incremental/EdgeRouter.ts +++ b/src/domain/diagram/layout/incremental/EdgeRouter.ts @@ -1,5 +1,5 @@ import { attachedSide, defaultSides, gatewaySide } from '../../utils/sides.js'; -import { crossings } from '../../utils/routing.js'; +import { BEND, crossings } from '../../utils/routing.js'; import { OrthogonalRouter, type RouteRequest } from './OrthogonalRouter.js'; import type { Side, DEdge, DShape, Plane } from '../../di/types.js'; import { borderDistance, borderPoint, containsPoint, cx, cy, degenerate, isOrthogonal, pathHits, roundPoints, type Box, type Point } from '../../utils/geom.js'; @@ -27,9 +27,22 @@ export interface EdgeBefore { /** The `detachedEdge` metric uses the same gap. */ export const DETACHED_GAP = 10; -/** Routes and repairs single connections inside a kept drawing. */ +/** An association passing this close to a node placed in this run or its label reads as touching it. */ +const GRAZE_GAP = 3; + +/** A dock route that cuts a shape loses to any that cuts none, one more crossing to any length. */ +const DOCK_CUT = 1e9; +const DOCK_CROSSING = 1e6; + +/** + * Routes and repairs single connections inside a kept drawing; `fresh`: the nodes placed in this + * run, which associations keep clear of. + */ export class EdgeRouter { - constructor(private readonly plane: Plane) {} + constructor( + private readonly plane: Plane, + private readonly fresh: ReadonlySet = new Set(), + ) {} private ancestors(shape: DShape): Set { const out = new Set(); @@ -142,10 +155,12 @@ export class EdgeRouter { } /** - * The nearest point of the flow a straight line from the shape reaches (`clear`), - * else the middle of the nearest segment, entered across it (`sides`). + * The nearest point of the flow a straight line of `edge` from the shape reaches (no `points`), + * else the route into the middle of a segment, entered across it, that cuts the fewest shapes, + * then crosses the fewest lines, then is the shortest (150 per bend); ties keep the segment + * nearer the shape. */ - private flowDock(flow: DEdge, from: Box, obstacles: readonly Box[]): { at: Box; clear: boolean; sides?: Side[] } | undefined { + private flowDock(edge: DEdge, flow: DEdge, from: Box, obstacles: Box[], lines: Point[][]): { at: Box; points?: Point[] } | undefined { const center = { x: cx(from), y: cy(from) }; const cands: Point[] = []; for (let index = 0; index + 1 < flow.points.length; index++) { @@ -158,18 +173,25 @@ export class EdgeRouter { const dist = (point: Point): number => Math.hypot(point.x - center.x, point.y - center.y); const box = (point: Point): Box => ({ x: Math.round(point.x), y: Math.round(point.y), width: 0, height: 0 }); const sorted = cands.sort((first, second) => dist(first) - dist(second) || first.x - second.x || first.y - second.y); - for (const candidate of sorted) if (!obstacles.some((obstacle) => pathHits(this.straight(from, box(candidate)), obstacle))) return { at: box(candidate), clear: true }; - let best: { at: Box; clear: boolean; sides?: Side[] } | undefined; - let bestD = Infinity; + for (const candidate of sorted) { + const line = this.straight(from, box(candidate)); + if (!obstacles.some((obstacle) => pathHits(line, obstacle)) && !this.touchesFresh(edge, line)) return { at: box(candidate) }; + } + let best: { at: Box; points: Point[] } | undefined; + let least = Infinity; for (let index = 0; index + 1 < flow.points.length; index++) { const start = flow.points[index]!; const end = flow.points[index + 1]!; - if (Math.abs(start.x - end.x) + Math.abs(start.y - end.y) < 20 && flow.points.length > 2) continue; - const distance = dist(this.nearestOn([start, end], center)!); - if (distance >= bestD) continue; - bestD = distance; - const horizontal = Math.abs(start.x - end.x) >= Math.abs(start.y - end.y); - best = { at: box({ x: (start.x + end.x) / 2, y: (start.y + end.y) / 2 }), clear: false, sides: horizontal ? ['top', 'bottom'] : ['left', 'right'] }; + const dx = Math.abs(start.x - end.x); + const dy = Math.abs(start.y - end.y); + if (dx + dy < 20 && flow.points.length > 2) continue; + const at = box({ x: (start.x + end.x) / 2, y: (start.y + end.y) / 2 }); + const points = roundPoints(OrthogonalRouter.route({ source: from, target: at, obstacles, lines, targetSides: dx >= dy ? ['top', 'bottom'] : ['left', 'right'] })); + let cost = DOCK_CUT * obstacles.filter((obstacle) => pathHits(points, obstacle)).length + DOCK_CROSSING * crossings(points, lines) + BEND * (points.length - 2) + dist(this.nearestOn([start, end], center)!) / DOCK_CROSSING; + for (let step = 1; step < points.length; step++) cost += Math.abs(points[step]!.x - points[step - 1]!.x) + Math.abs(points[step]!.y - points[step - 1]!.y); + if (cost >= least) continue; + best = { at, points }; + least = cost; } return best; } @@ -220,17 +242,14 @@ export class EdgeRouter { const flow = !source ? this.plane.edges.get(edge.sourceId ?? '') : !target ? this.plane.edges.get(edge.targetId ?? '') : undefined; if (shape && flow && flow.points.length >= 2) { const obstacles = this.obstaclesFor(edge, shape); - const dock = this.flowDock(flow, shape.bounds, obstacles); - if (dock?.clear) return source ? this.straight(source.bounds, dock.at) : this.straight(dock.at, shape.bounds); - if (dock) { - const pts = roundPoints(OrthogonalRouter.route({ source: shape.bounds, target: dock.at, obstacles, lines: opts.lines, ...(dock.sides ? { targetSides: dock.sides } : {}) })); - return source ? pts : pts.reverse(); - } + const dock = this.flowDock(edge, flow, shape.bounds, obstacles, opts.lines); + if (dock?.points) return source ? dock.points : dock.points.reverse(); + if (dock) return source ? this.straight(source.bounds, dock.at) : this.straight(dock.at, shape.bounds); } const line = this.straight(sb, tb); if (!source || !target) return line; const obstacles = this.obstaclesFor(edge, source, target); - if (!obstacles.some((obstacle) => pathHits(line, obstacle))) return line; + if (!obstacles.some((obstacle) => pathHits(line, obstacle)) && !this.touchesFresh(edge, line)) return line; return roundPoints(OrthogonalRouter.route({ source: source.bounds, target: target.bounds, obstacles, lines: opts.lines })); } if (!source || !target) return undefined; @@ -280,6 +299,34 @@ export class EdgeRouter { return out; } + /** + * Whether the line runs along the border of one of `ids` or touches it within `margin` (reads as + * docking on it); with `labels` their labels count too. Its own ends, their boundary events and + * hosts do not count. + */ + grazes(edge: DEdge, ids: Iterable, { points = edge.points, margin = 1, labels = false }: { points?: readonly Point[]; margin?: number; labels?: boolean } = {}): boolean { + if (points.length < 2) return false; + const own = new Set(); + for (const end of [edge.sourceId, edge.targetId]) { + const shape = end ? this.plane.shapes.get(end) : undefined; + if (!shape) continue; + own.add(shape.id); + if (shape.hostId) own.add(shape.hostId); + for (const boundary of PlaneQuery.boundariesOf(this.plane, shape.id)) own.add(boundary.id); + } + for (const id of ids) { + const shape = this.plane.shapes.get(id); + if (!shape || own.has(id) || !PlaneQuery.isLeaf(shape)) continue; + if (pathHits(points, shape.bounds, -margin) || (labels && shape.label && pathHits(points, shape.label, -margin))) return true; + } + return false; + } + + /** Whether an association's line touches a node placed in this run or its label. */ + touchesFresh(edge: DEdge, points: readonly Point[] = edge.points): boolean { + return (edge.kind === 'association' || edge.kind === 'dataAssociation') && this.grazes(edge, this.fresh, { points, margin: GRAZE_GAP, labels: true }); + } + private endGaps(edge: DEdge): { sourceGap: number; targetGap: number } { const gapOf = (id: string | undefined, point: Point | undefined): number => { const shape = id ? this.plane.shapes.get(id) : undefined; diff --git a/src/domain/diagram/layout/incremental/IncrementalLayout.ts b/src/domain/diagram/layout/incremental/IncrementalLayout.ts index 16f2e5fc..8114f98b 100644 --- a/src/domain/diagram/layout/incremental/IncrementalLayout.ts +++ b/src/domain/diagram/layout/incremental/IncrementalLayout.ts @@ -173,8 +173,10 @@ export class IncrementalLayout implements PlacementHooks { await this.placeContainers(wrapped); this.diff.laneChanges(); this.joinPuller.pullJoins(); + this.joinPuller.pushTargets(); const start = this.checkpoint(); const rerouted = await new CrossingRepair(this).repair(await this.draw(), start, () => this.draw()); + for (const id of this.routing.routeGrazed()) if (!rerouted.includes(id)) rerouted.push(id); for (const id of new Set(rerouted)) { const ends = detached.get(id); if (ends) this.notes.push(`${id} routed again: it did not reach ${ends.join(' and ')}`); @@ -317,9 +319,12 @@ export class IncrementalLayout implements PlacementHooks { return (this.engineRun ??= EngineDrawing.of(this.doc, this.opts)); } - /** The drawing of a plane in this run: what its frame edits move counts as moved. */ + /** + * The drawing of a plane in this run: what its frame edits move counts as moved, associations + * keep clear of the nodes placed. + */ drawing(plane: Plane): Drawing { - return new Drawing(plane, this.place.moved); + return new Drawing(plane, this.place.moved, this.place.placed); } /** The id an element had before the ops (its DI object survives a `set id=` rename). */ diff --git a/src/domain/diagram/layout/incremental/IncrementalRouting.ts b/src/domain/diagram/layout/incremental/IncrementalRouting.ts index b01406e4..e21e4d79 100644 --- a/src/domain/diagram/layout/incremental/IncrementalRouting.ts +++ b/src/domain/diagram/layout/incremental/IncrementalRouting.ts @@ -2,7 +2,6 @@ import type { Snapshot } from './Snapshot.js'; import type { IncrementalLayout } from './IncrementalLayout.js'; import { LayoutDebug } from '../LayoutDebug.js'; import { is, type El } from '../../../document/utils/moddle.js'; -import { segmentHits } from '../../utils/geom.js'; import { hasExternalLabel } from '../../utils/labels.js'; import { idOf, raw, rawList } from '../../utils/raw.js'; import { DiagramReader } from '../../di/DiagramReader.js'; @@ -38,8 +37,9 @@ export class IncrementalRouting { * the placement broke (not message flows) are routed now, and the spacing is measured again, so * the next placement sees what the next single command would. Step 8 routes such a flow again * only when it broke since. - * What the placement broke (the space tool, a new shape on a line) is routed again now; a flow - * whose end changes later (into a node still to be placed) is routed then. + * What the placement broke (the space tool, a new shape on a line, an association a new shape + * touches) is routed again now; a flow whose end changes later (into a node still to be placed) + * is routed then. */ routeNow(plane: Plane, id: string): void { const shape = plane.shapes.get(id); @@ -64,17 +64,18 @@ export class IncrementalRouting { this.routeKept(plane, edge); } const detached = !PlaneQuery.verticalBand(plane); + const drawing = this.run.drawing(plane); for (const edge of plane.edges.values()) { if (edge.kind === 'messageFlow' || edge.points.length < 2 || edge.sourceId === id || edge.targetId === id || !edge.sourceId || !edge.targetId) continue; const ends = DiagramReader.edgeEnds(edge.el); if (ends.sourceId !== edge.sourceId || ends.targetId !== edge.targetId || !plane.shapes.has(edge.sourceId) || !plane.shapes.has(edge.targetId)) continue; const was = this.run.routedNow.get(edge.id) ?? this.edgesBefore.get(edge); - if (!was || (!this.run.drawing(plane).brokenEdge(edge, was, { detached }) && !(this.run.place.continued?.has(id) && this.grazes(plane, edge, [id])))) continue; + if (!was || (!drawing.brokenEdge(edge, was, { detached }) && !(this.run.place.continued?.has(id) && drawing.grazes(edge, [id])) && !drawing.touchesFresh(edge))) continue; if (LayoutDebug.on()) LayoutDebug.log(`[route] ${edge.id}: broken when ${id} was placed`); this.routeKept(plane, edge); } - if (!shape.label && hasExternalLabel(shape) && raw(el, 'name')) this.run.drawing(plane).placeShapeLabel(shape); - this.run.place.spacing.set(plane, PlaneQuery.spacingOf(plane)); + if (!shape.label && hasExternalLabel(shape) && raw(el, 'name')) drawing.placeShapeLabel(shape); + this.run.place.spacing.delete(plane); } /** Routes a connection during placement (routeNow) and records its state then. */ @@ -86,9 +87,10 @@ export class IncrementalRouting { /** * Creates DEdges for connections without DI and collects every edge to (re)route. - * A line not reaching its ends is routed again, except on vertical pools / lanes; one routed when - * its second end was placed is kept unless it broke since. Ends are compared by their ids now, so - * renaming an end (`set id=`) changes nothing. + * A line not reaching its ends is routed again, except on vertical pools / lanes, and so is an + * association a new node or its label touches; one routed when its second end was placed is kept + * unless it broke since. Ends are compared by their ids now, so renaming an end (`set id=`) + * changes nothing. */ collectEdges(before: Map): void { const edges = this.edgeIndex(this.run.planes); @@ -108,14 +110,16 @@ export class IncrementalRouting { this.run.newEdges.add(id); } } + const continued = this.run.place.continued ?? []; for (const plane of this.run.planes) { const detached = !PlaneQuery.verticalBand(plane); + const drawing = this.run.drawing(plane); for (const edge of plane.edges.values()) { if (this.run.fromEngine.has(edge.id)) continue; const now = this.run.routedNow.get(edge.id); if (now) { const ends = DiagramReader.edgeEnds(edge.el); - if (ends.sourceId === edge.sourceId && ends.targetId === edge.targetId && edge.points.length >= 2 && !this.run.drawing(plane).brokenEdge(edge, now, { detached }) && !this.grazes(plane, edge, this.run.place.continued)) continue; + if (ends.sourceId === edge.sourceId && ends.targetId === edge.targetId && edge.points.length >= 2 && !drawing.brokenEdge(edge, now, { detached }) && !drawing.grazes(edge, continued) && !drawing.touchesFresh(edge)) continue; this.run.reroute.add(edge.id); if (LayoutDebug.on()) LayoutDebug.log(`[route] ${edge.id}: broken after it was routed`); continue; @@ -133,11 +137,13 @@ export class IncrementalRouting { ? 'ends changed' : (edge.sourceId && this.run.place.placed.has(edge.sourceId)) || (edge.targetId && this.run.place.placed.has(edge.targetId)) ? 'end placed' - : this.run.drawing(plane).brokenEdge(edge, before.get(edge), { detached }) + : drawing.brokenEdge(edge, before.get(edge), { detached }) ? 'broken' - : this.grazes(plane, edge, this.run.place.continued) + : drawing.grazes(edge, continued) ? 'runs along a new shape' - : undefined; + : drawing.touchesFresh(edge) + ? 'touches a new shape' + : undefined; if (why) { this.run.reroute.add(edge.id); if (LayoutDebug.on()) LayoutDebug.log(`[route] ${edge.id}: ${why}`); @@ -147,26 +153,26 @@ export class IncrementalRouting { } /** - * Whether a connection runs along the border of one of `ids` or touches it (reads as docking on - * it): checked for nodes continuing a chain. Its own ends, their boundary events and hosts do - * not count. + * Once the labels are settled (the last node's is placed after the routing, a repair route may + * run over one): a kept association that a node placed in this run or its label touches is + * routed again, once, and the labels its line then runs over step aside (one placed before a + * repair route ran over it too). A route that still touches one keeps the drawn line, whose + * labels step aside all the same. Returns the ones routed again. */ - private grazes(plane: Plane, edge: DEdge, ids: Iterable | undefined): boolean { - if (!ids || edge.points.length < 2) return false; - const own = new Set(); - for (const end of [edge.sourceId, edge.targetId]) { - const shape = end ? plane.shapes.get(end) : undefined; - if (!shape) continue; - own.add(shape.id); - if (shape.hostId) own.add(shape.hostId); - for (const boundary of PlaneQuery.boundariesOf(plane, shape.id)) own.add(boundary.id); - } - for (const id of ids) { - const shape = plane.shapes.get(id); - if (!shape || own.has(id) || !PlaneQuery.isLeaf(shape)) continue; - for (let index = 0; index + 1 < edge.points.length; index++) if (segmentHits(edge.points[index]!, edge.points[index + 1]!, shape.bounds, -1)) return true; - } - return false; + routeGrazed(): string[] { + return this.run.planes.flatMap((plane) => { + const drawing = this.run.drawing(plane); + const touched = [...plane.edges.values()].filter((edge) => !this.run.newEdges.has(edge.id) && drawing.touchesFresh(edge)); + const routed = touched.filter((edge) => { + const drawn = { points: edge.points, label: edge.label }; + if (!drawing.reroute(edge)) return false; + if (!drawing.touchesFresh(edge)) return true; + Object.assign(edge, drawn); + return false; + }); + drawing.clearLabels(touched); + return routed.map((edge) => edge.id); + }); } /** diff --git a/src/domain/diagram/layout/incremental/JoinPuller.ts b/src/domain/diagram/layout/incremental/JoinPuller.ts index 430c3fea..a45fe038 100644 --- a/src/domain/diagram/layout/incremental/JoinPuller.ts +++ b/src/domain/diagram/layout/incremental/JoinPuller.ts @@ -10,7 +10,9 @@ import type { DShape } from '../../di/types.js'; /** * A join drawn before the ops that a new flow would enter backwards (no loop return) is placed * again right of its rightmost predecessor, before its one successor: a `connect` into a merge an - * earlier command spliced draws what the batch adding both draws. Its DI is reused. + * earlier command spliced draws what the batch adding both draws. Its DI is reused. Any other + * drawn node such a flow from a drawn node enters backwards moves right of its source with what + * follows it, as when the batch places the source (NodePlacer pushTargets). */ export class JoinPuller { constructor( @@ -18,9 +20,26 @@ export class JoinPuller { private readonly snap: Snapshot, ) {} + /** Sequence flows new in this run or with a new target. */ + private fresh(): El[] { + const before = new Set([...this.snap.flowEnds.keys()].map((id) => this.snap.currentId(id))); + return [...this.run.sem.byId.values()].filter((el) => is(el, 'bpmn:SequenceFlow') && !!idOf(el) && (!before.has(idOf(el)!) || this.run.place.oldTarget.has(idOf(el)!))); + } + + /** Runs after pullJoins: the joins placed again are no target any more. */ + pushTargets(): void { + for (const el of this.fresh()) { + const { sourceId, targetId } = DiagramReader.edgeEnds(el); + const plane = this.run.planes.find((candidate) => !!targetId && candidate.shapes.has(targetId)); + const target = targetId ? plane?.shapes.get(targetId) : undefined; + const source = sourceId ? plane?.shapes.get(sourceId) : undefined; + if (!plane || !target || !source || this.run.relocated.has(target.id) || this.run.relocated.has(source.id)) continue; + if (target.poolId === source.poolId && target.parentId === source.parentId) this.run.place.pushBehind(plane, source, target); + } + } + /** Only a gateway with 2+ incoming flows and one outgoing, all in its pool / sub-process. */ pullJoins(): void { - const before = new Set([...this.snap.flowEnds.keys()].map((id) => this.snap.currentId(id))); let next: Map | undefined; const reaches = (from: string, to: string): boolean => { if (!next) { @@ -43,9 +62,8 @@ export class JoinPuller { return false; }; const seqFlows = (el: El | undefined, prop: 'incoming' | 'outgoing'): El[] => rawList(el, prop).filter((flow) => is(flow, 'bpmn:SequenceFlow')); - for (const el of this.run.sem.byId.values()) { - const fid = idOf(el); - if (!fid || !is(el, 'bpmn:SequenceFlow') || (before.has(fid) && !this.run.place.oldTarget.has(fid))) continue; + for (const el of this.fresh()) { + const fid = idOf(el)!; const { sourceId, targetId } = DiagramReader.edgeEnds(el); const plane = this.run.planes.find((candidate) => !!targetId && candidate.shapes.has(targetId)); const join = targetId ? plane?.shapes.get(targetId) : undefined; @@ -58,7 +76,8 @@ export class JoinPuller { if (ins.length < 2 || !successor || reaches(join.id, source.id)) continue; const preds = ins.map((flow) => plane.shapes.get(idOf(raw(flow, 'sourceRef')) ?? '')); const sameFrame = (shape: DShape | undefined): boolean => !!shape && shape.poolId === join.poolId && shape.parentId === join.parentId; - if (!preds.every(sameFrame) || !sameFrame(successor) || successor.bounds.x <= Math.max(...preds.map((pred) => right(pred!.bounds)))) continue; + /** S left of a predecessor is pushed right of the join again, when the join led forward into it */ + if (!preds.every(sameFrame) || !sameFrame(successor) || (successor.bounds.x <= Math.max(...preds.map((pred) => right(pred!.bounds))) && successor.bounds.x < right(join.bounds))) continue; plane.shapes.delete(join.id); this.run.place.reuse.set(join.id, { shape: join, boundaries: [], from: plane }); (this.run.place.joins ??= new Map()).set(join.id, successor.id); diff --git a/src/domain/diagram/layout/incremental/LabelPlacer.ts b/src/domain/diagram/layout/incremental/LabelPlacer.ts index 9e73ec66..61a604ec 100644 --- a/src/domain/diagram/layout/incremental/LabelPlacer.ts +++ b/src/domain/diagram/layout/incremental/LabelPlacer.ts @@ -1,6 +1,7 @@ import { labelSize } from '../../utils/text.js'; +import { glyph } from '../../utils/harness.js'; import { is } from '../../../document/utils/moddle.js'; -import { bottom, cx, cy, inside, overlaps, right, segmentHits, type Box } from '../../utils/geom.js'; +import { bottom, cx, cy, inside, overlaps, polylinesCross, right, segmentHits, toSegments, type Box } from '../../utils/geom.js'; import { Frames } from '../../di/Frames.js'; import { raw } from '../../utils/raw.js'; import type { DEdge, DShape, Plane } from '../../di/types.js'; @@ -11,12 +12,15 @@ interface Clutter { shapes: Box[]; labels: Box[]; segments: Array<[{ x: number; y: number }, { x: number; y: number }]>; + /** of lines crossing a connection of the owner, which the crossing repair may route again */ + crossing: Clutter['segments']; } /** * Places the external labels of shapes and connections into a kept drawing. * A label must keep 2 px off shapes, labels and sub-process titles and 1 px off lines and frame - * borders, and should stay inside its home frame (the one checkLabelFrames expects). + * borders, and should stay inside its home frame (the one checkLabelFrames expects); a spot that + * keeps its drawn text off lines too comes first. */ export class LabelPlacer { constructor(private readonly plane: Plane) {} @@ -30,6 +34,7 @@ export class LabelPlacer { const shapes: Box[] = []; const labels: Box[] = []; const segments: Clutter['segments'] = []; + const crossing: Clutter['segments'] = []; for (const shape of this.plane.shapes.values()) { if (!shape.container && shape.kind !== 'group' && !ignoreShapes.has(shape.id)) shapes.push(shape.bounds); if (shape.label && shape.id !== ownerId) labels.push(shape.label); @@ -44,31 +49,51 @@ export class LabelPlacer { segments.push([tl, tr], [tr, br], [br, bl], [bl, tl]); } } + const own = [...this.plane.edges.values()].filter((edge) => edge.sourceId === ownerId || edge.targetId === ownerId); for (const edge of this.plane.edges.values()) { if (edge.label && edge.id !== ownerId) labels.push(edge.label); - for (let index = 0; index + 1 < edge.points.length; index++) segments.push([edge.points[index]!, edge.points[index + 1]!]); + const into = !own.includes(edge) && own.some((mine) => polylinesCross(mine.points, edge.points)) ? crossing : segments; + into.push(...toSegments(edge.points)); } - return { shapes, labels, segments }; + return { shapes, labels, segments, crossing }; } - private collides(candidate: Box, clutter: Clutter): boolean { - return clutter.shapes.some((shape) => overlaps(shape, candidate, 2)) || clutter.labels.some((label) => overlaps(label, candidate, 2)) || clutter.segments.some(([start, end]) => segmentHits(start, end, candidate, 1)); + /** + * With `name` the drawn text keeps off shapes and lines too: bpmn-js draws a short name on one + * line up to 90 px wide, centred on the box, so it can reach a line beside the narrower wrapped + * box. Not off a line crossing the owner's connections: routed again, it would take the side the + * text dodged to. + */ + private collides(candidate: Box, clutter: Clutter, name?: string): boolean { + const onLine = ([start, end]: Clutter['segments'][number]): boolean => segmentHits(start, end, candidate, 1); + if (clutter.shapes.some((shape) => overlaps(shape, candidate, 2)) || clutter.labels.some((label) => overlaps(label, candidate, 2)) || clutter.segments.some(onLine) || clutter.crossing.some(onLine)) return true; + if (!name) return false; + const text = glyph(candidate, name); + return clutter.shapes.some((shape) => overlaps(shape, text)) || clutter.segments.some(([start, end]) => segmentHits(start, end, text, 0)); } /** - * Per list: a free spot in the frame's label area, else in the home frame; - * only then any free spot of all lists (a boundary event's label hangs outside its host). + * Per list: a free spot in the frame's label area, else in the home frame; only then any free + * spot of all lists (a boundary event's label hangs outside its host). Each first with its text + * clear, then with the box alone. */ private firstFree(shape: DShape, clutter: Clutter, ...lists: Array): Box | undefined { const frame = shape.kind === 'boundary' ? undefined : Frames.of(this.plane, shape); const home = frame ? this.homeOf(shape) : undefined; const area = frame ? Frames.labelArea(frame) : undefined; - for (const cands of lists) { - const free = cands.filter((candidate) => !this.collides(candidate, clutter)); - const win = (area ? free.find((candidate) => inside(candidate, area)) : undefined) ?? free.find((candidate) => !home || inside(candidate, home.bounds)); + const tiers = [this.nameOf(shape.el), undefined]; + for (const name of tiers) { + for (const cands of lists) { + const free = cands.filter((candidate) => !this.collides(candidate, clutter, name)); + const win = (area ? free.find((candidate) => inside(candidate, area)) : undefined) ?? free.find((candidate) => !home || inside(candidate, home.bounds)); + if (win) return win; + } + } + for (const name of tiers) { + const win = lists.flat().find((candidate) => !this.collides(candidate, clutter, name)); if (win) return win; } - return lists.flat().find((candidate) => !this.collides(candidate, clutter)); + return undefined; } /** The sub-process a shape lives in, else its pool; a flow's only when both ends share it. */ @@ -79,14 +104,20 @@ export class LabelPlacer { return source && source === of(owner.targetId ? this.plane.shapes.get(owner.targetId) : undefined) ? source : undefined; } - /** Centred, then shifted off a line leaving the shape on that side, at three steps out. */ + /** + * Centred, then shifted off a line leaving the shape on that side, at three steps out; above and + * below also as far as a wider one-line text needs. + */ private sideSpots(shape: DShape, side: LabelSide, size: { width: number; height: number }, host?: DShape): Box[] { + const name = this.nameOf(shape.el); + const over = name ? Math.max(0, glyph(shape.bounds, name).width - size.width) / 2 : 0; const cands: Box[] = []; for (let step = 0; step < 3; step++) { const base = labelAt(shape, size, side, host, step); cands.push(base); - if (side === 'below' || side === 'above') cands.push({ ...base, x: Math.round(cx(shape.bounds) + 6) }, { ...base, x: Math.round(cx(shape.bounds) - 6 - size.width) }); - else cands.push({ ...base, y: Math.round(cy(shape.bounds) + 6) }, { ...base, y: Math.round(cy(shape.bounds) - 6 - size.height) }); + if (side === 'below' || side === 'above') { + for (const shift of over ? [0, over] : [0]) cands.push({ ...base, x: Math.round(cx(shape.bounds) + 6 + shift) }, { ...base, x: Math.round(cx(shape.bounds) - 6 - size.width - shift) }); + } else cands.push({ ...base, y: Math.round(cy(shape.bounds) + 6) }, { ...base, y: Math.round(cy(shape.bounds) - 6 - size.height) }); } return cands; } @@ -195,7 +226,7 @@ export class LabelPlacer { const start = pts[0]!; cands.push({ x: Math.round(start.x + 6), y: Math.round(start.y - size.height - 4), ...size }); } - edge.label = cands.find((candidate) => !this.collides(candidate, clutter)) ?? cands[0]!; + edge.label = cands.find((candidate) => !this.collides(candidate, clutter, name)) ?? cands.find((candidate) => !this.collides(candidate, clutter)) ?? cands[0]!; return edge.label; } diff --git a/src/domain/diagram/layout/incremental/LabelRefresh.ts b/src/domain/diagram/layout/incremental/LabelRefresh.ts index 4d01116a..455fd580 100644 --- a/src/domain/diagram/layout/incremental/LabelRefresh.ts +++ b/src/domain/diagram/layout/incremental/LabelRefresh.ts @@ -1,6 +1,6 @@ import type { Snapshot } from './Snapshot.js'; import type { IncrementalLayout } from './IncrementalLayout.js'; -import { overlaps, segmentHits } from '../../utils/geom.js'; +import { overlaps, pathHits } from '../../utils/geom.js'; import { hasExternalLabel } from '../../utils/labels.js'; import { raw } from '../../utils/raw.js'; import type { DEdge, DShape } from '../../di/types.js'; @@ -21,7 +21,7 @@ export class LabelRefresh { */ labels(routed: ReadonlySet): void { for (const plane of this.run.planes) { - const fresh = [...plane.edges.values()].filter((edge) => routed.has(edge.id)).flatMap((edge) => edge.points.slice(1).map((end, index) => [edge.points[index]!, end] as const)); + const fresh = [...plane.edges.values()].filter((edge) => routed.has(edge.id)); const grown = [...this.run.grown].map((id) => plane.shapes.get(id)).filter((shape): shape is DShape => !!shape); for (const shape of plane.shapes.values()) { if (this.run.fromEngine.has(shape.id)) continue; @@ -29,7 +29,7 @@ export class LabelRefresh { this.run.drawing(plane).placeShapeLabel(shape); continue; } - if (shape.label && hasExternalLabel(shape) && fresh.some(([start, end]) => segmentHits(start, end, shape.label!, 1))) { + if (shape.label && hasExternalLabel(shape) && fresh.some((edge) => pathHits(edge.points, shape.label!, 1))) { this.run.drawing(plane).placeShapeLabel(shape); this.run.notes.push(`label of ${shape.id} moved off a new line`); continue; diff --git a/src/domain/diagram/layout/incremental/NeighbourPlacer.ts b/src/domain/diagram/layout/incremental/NeighbourPlacer.ts index 6af36f64..1d6f0e78 100644 --- a/src/domain/diagram/layout/incremental/NeighbourPlacer.ts +++ b/src/domain/diagram/layout/incremental/NeighbourPlacer.ts @@ -13,7 +13,7 @@ import type { DShape, Plane } from '../../di/types.js'; import { attachedSide } from '../../utils/sides.js'; import { labelSize } from '../../utils/text.js'; import type { NodePlacer, Probe } from './NodePlacer.js'; -import type { PlacementObstacles } from './PlacementObstacles.js'; +import type { PlacementObstacles, PushRequest } from './PlacementObstacles.js'; import { TOP_ROW, type Cand, type RowChooser } from './RowChooser.js'; import type { BoundaryPlacer } from './BoundaryPlacer.js'; @@ -111,6 +111,43 @@ export class NeighbourPlacer { return false; } + /** + * Makes room right of the line. Inside an expanded sub-process the frame grows (frame mode); + * elsewhere only the successor, what follows it and what that would run into moves. The shapes + * in `fixed` stay where they are, also beyond the line. + */ + private push(plane: Plane, anchor: DShape, scope: Pick, req: PushRequest): void { + const band = this.obstacles.spaceBand(plane, scope, anchor); + const push = band.frame ? undefined : this.obstacles.pushSet(plane, req); + const keep = req.fixed?.size ? { keep: req.fixed } : {}; + /** inside the frame, so it grows instead of moving off its content */ + const line = band.within && band.frame ? Math.max(req.line, band.within.x + 1) : req.line; + this.placer.recordSpace(this.placer.drawing(plane).makeSpace({ axis: 'x', line, delta: req.delta, anchors: [anchor.id, scope.parentId], ...band, ...(push ? { push } : {}), ...keep })); + } + + /** + * A flow from `source` that enters `target` backwards (no loop return): the target and what + * follows it move right of the source, one gap away; what leads to the source stays. False when + * nothing moved. + */ + pushBehind(plane: Plane, source: DShape, target: DShape): boolean { + if (cx(target.bounds) >= source.bounds.x || this.placer.leadsTo(target.id, source.id)) return false; + const fixed = new Set(); + const queue = [source.id]; + for (let cur = queue.shift(); cur && fixed.size <= 500; cur = queue.shift()) { + if (fixed.has(cur)) continue; + fixed.add(cur); + PlaneQuery.boundariesOf(plane, cur).forEach((boundary) => fixed.add(boundary.id)); + const host = idOf(raw(this.placer.sem.byId.get(cur), 'attachedToRef')); + queue.push(...this.placer.sem.predecessors(cur), ...(host ? [host] : [])); + } + if (fixed.has(target.id)) return false; + const delta = Math.ceil(right(source.bounds) + this.placer.spacingWithout(plane, [source.id, target.id]).gap - target.bounds.x); + if (LayoutDebug.on()) LayoutDebug.log(`[place] ${target.id}: entered backwards from ${source.id}, pushed ${delta} px`); + this.push(plane, source, target, { line: target.bounds.x - 0.5, delta, successor: target, fixed }); + return true; + } + /** * How far the space tool at `line` has to push for `box` when part of the room is free already: * what is in the way must end right of it, `S` one gap further. Undefined when something in the @@ -218,7 +255,14 @@ export class NeighbourPlacer { if (!this.obstacles.isFree(box, blockers) || !succRoom) { const full = size.width + gap + Math.max(sp.gap, sGap) - sp.gap; const delta = Math.min(full, this.localShift(plane, box, maxRight + 1, blockers, successor, Math.max(sp.gap, sGap)) ?? full); - this.placer.recordSpace(this.placer.drawing(plane).makeSpace({ axis: 'x', line: maxRight + 1, delta, anchors: [main.id, probe.parentId], ...this.obstacles.spaceBand(plane, probe, main) })); + /** + * S starts before the line (a merge placed right of its rightmost predecessor, S right of + * another one): S and what follows it move right of the node; a loop return stays + */ + const behind = !!successor && successor.bounds.x < maxRight + 1 && preds.some((pred) => successor.bounds.x >= right(pred.bounds)) && !preds.some((pred) => this.placer.leadsTo(successor.id, pred.id)); + const fixed = behind ? new Set(preds.flatMap((pred) => [pred.id, ...PlaneQuery.boundariesOf(plane, pred.id).map((boundary) => boundary.id)])) : undefined; + if (behind) this.push(plane, main, probe, { id, box, line: successor.bounds.x - 0.5, delta: Math.max(delta, Math.ceil(right(box) + Math.max(sp.gap, sGap) - successor.bounds.x)), successor, fixed }); + else this.push(plane, main, probe, { id, box, line: maxRight + 1, delta, successor }); } if (LayoutDebug.on()) LayoutDebug.log(`[place] ${id} after ${main.id}: ${successor ? `splice before ${successor.id}` : 'continuation'} at x=${left} c=${center}`); this.placer.placeAt(plane, id, left, center, size); @@ -254,7 +298,8 @@ export class NeighbourPlacer { } } } - this.placer.recordSpace(this.placer.drawing(plane).makeSpace({ axis: 'x', line: succ.bounds.x - 0.5, delta: size.width + sp.gap, anchors: [succ.id, probe.parentId], ...this.obstacles.spaceBand(plane, probe, succ) })); + const at = { x: succ.bounds.x - 0.5, y: cy(succ.bounds) - size.height / 2, ...size }; + this.push(plane, succ, probe, { id, box: at, line: at.x, delta: size.width + sp.gap, successor: succ }); const center = this.rows.intoOwnLane(plane, probe, succ.bounds.x - sp.gap - size.width, cy(succ.bounds), size, [], succ); this.placer.placeAt(plane, id, succ.bounds.x - sp.gap - size.width, center, size); } diff --git a/src/domain/diagram/layout/incremental/NodePlacer.ts b/src/domain/diagram/layout/incremental/NodePlacer.ts index 1b8aef22..edb70275 100644 --- a/src/domain/diagram/layout/incremental/NodePlacer.ts +++ b/src/domain/diagram/layout/incremental/NodePlacer.ts @@ -78,6 +78,8 @@ export class NodePlacer { skip?: Map; /** nodes continuing a chain on their anchor's row: a line touching them is rerouted */ continued?: Set; + /** the node placeFlowNodes places now */ + private current?: string; private readonly drawings = new Map(); private readonly rows: RowChooser; private readonly boundaries: BoundaryPlacer; @@ -150,12 +152,26 @@ export class NodePlacer { spacingFor(plane: Plane): Spacing { let spacing = this.spacing.get(plane); if (!spacing) { - spacing = PlaneQuery.spacingOf(plane); + spacing = PlaneQuery.spacingOf(plane, (edge) => this.drawnEnds(plane, edge)); this.spacing.set(plane, spacing); } return spacing; } + /** The spacing without the flows of `ids` (they may not be routed yet in a batch). */ + spacingWithout(plane: Plane, ids: readonly string[]): Spacing { + return PlaneQuery.spacingOf(plane, (edge) => (ids.includes(edge.sourceId ?? '') || ids.includes(edge.targetId ?? '') ? undefined : this.drawnEnds(plane, edge))); + } + + /** + * The ends of a flow as the single command placing the current node sees them: an end the ops + * moved to a node not drawn yet counts at its old place, unless it is the current node. + */ + private drawnEnds(plane: Plane, edge: DEdge): Pick { + const end = (now: string | undefined, old: string | undefined): string | undefined => (!now || plane.shapes.has(now) || now === this.current ? now : old); + return { sourceId: end(edge.sourceId, this.oldSource.get(edge.id)), targetId: end(edge.targetId, this.oldTarget.get(edge.id)) }; + } + /** * Size of a new shape: the median of its kind on the plane, else the engine default. * A relocated node keeps its size. Events and boundary events count as one kind, so do tasks and @@ -298,7 +314,7 @@ export class NodePlacer { } /** Whether `to` follows `from` along sequence flows (at most 500 nodes visited). */ - private leadsTo(from: string, to: string): boolean { + leadsTo(from: string, to: string): boolean { const seen = new Set([from]); const queue = [from]; while (queue.length && seen.size <= 500) { @@ -326,15 +342,37 @@ export class NodePlacer { const successor = outs.length === 1 ? plane.shapes.get(outs[0]!) : undefined; if (preds.length < 2 || !successor || !preds.every((pred): pred is DShape => !!pred)) continue; const sameFrame = (shape: DShape): boolean => shape.poolId === join.poolId && shape.parentId === join.parentId; - if (!preds.every(sameFrame) || !sameFrame(successor) || successor.bounds.x <= Math.max(...preds.map((pred) => right(pred.bounds)))) continue; + /** S left of a predecessor is pushed right of the join again, when the join led forward into it */ + if (!preds.every(sameFrame) || !sameFrame(successor) || (successor.bounds.x <= Math.max(...preds.map((pred) => right(pred.bounds))) && successor.bounds.x < right(join.bounds))) continue; if (!preds.some((pred) => cx(join.bounds) < pred.bounds.x && !this.leadsTo(jid, pred.id))) continue; plane.shapes.delete(jid); (this.joins ??= new Map()).set(jid, successor.id); + this.current = jid; + this.spacing.delete(plane); if (LayoutDebug.on()) LayoutDebug.log(`[join] ${jid}: a flow from ${preds.map((pred) => pred.id).join(', ')} would enter it backwards: placed again before ${successor.id}`); this.neighbours.placeAfter(plane, jid, preds, pending); } } + /** + * Drawn targets the node `id` just placed would enter backwards move right of it (pushBehind); + * the node it was spliced before keeps the direction of the flow it was spliced into. + */ + private pushTargets(plane: Plane, id: string, pending: ReadonlySet): void { + const source = plane.shapes.get(id); + if (!source) return; + for (const tid of this.sem.successors(id)) { + const target = plane.shapes.get(tid); + if (!target || pending.has(tid) || this.spliced?.get(id) === tid || this.joins?.get(id) === tid) continue; + if (target.poolId === source.poolId && target.parentId === source.parentId) this.pushBehind(plane, source, target); + } + } + + /** A flow from `source` that enters `target` backwards moves the target and what follows it right of the source. */ + pushBehind(plane: Plane, source: DShape, target: DShape): boolean { + return this.neighbours.pushBehind(plane, source, target); + } + /** * Gives every flow node in `ids` a shape, in op order as far as their neighbours allow: the * normal flow, boundary events, their exception paths, nodes before a placed successor, the @@ -395,14 +433,18 @@ export class NodePlacer { break; } pending.delete(chosen.id); + this.current = chosen.id; + this.spacing.delete(chosen.plane); if (chosen.mode === 'boundary') { if (!this.boundaries.placeBoundary(chosen.plane, chosen.id)) this.neighbours.placeUnconnected(chosen.plane, chosen.id); } else if (chosen.mode === 'after' || chosen.mode === 'exception') this.neighbours.placeAfter(chosen.plane, chosen.id, chosen.preds!, pending); else if (chosen.mode === 'before') this.neighbours.placeBefore(chosen.plane, chosen.id, chosen.succ!); else if (!is(this.sem.byId.get(chosen.id)!, 'bpmn:BoundaryEvent') || !this.boundaries.placeBoundary(chosen.plane, chosen.id)) this.neighbours.placeUnconnected(chosen.plane, chosen.id); this.pullPlacedJoins(chosen.plane, chosen.id, pending); + this.pushTargets(chosen.plane, chosen.id, pending); last = { plane: chosen.plane, id: chosen.id }; } + delete this.current; return skipped; } diff --git a/src/domain/diagram/layout/incremental/PlacementObstacles.ts b/src/domain/diagram/layout/incremental/PlacementObstacles.ts index e0e4d0ae..3cf55d91 100644 --- a/src/domain/diagram/layout/incremental/PlacementObstacles.ts +++ b/src/domain/diagram/layout/incremental/PlacementObstacles.ts @@ -1,10 +1,26 @@ -import { bottom, cx, cy, overlaps, right, type Box } from '../../utils/geom.js'; +import { bottom, containsPoint, cx, cy, overlaps, right, segmentHits, type Box } from '../../utils/geom.js'; import { Frames } from '../../di/Frames.js'; import { PlaneQuery } from '../../di/PlaneQuery.js'; import type { DShape, Plane } from '../../di/types.js'; import type { NodePlacer } from './NodePlacer.js'; +const CLEAR = 20; +const LABEL_CLEAR = 4; + +/** A push of the space tool to the right, for a new node `id` at `box` or for a flow's target. */ +export interface PushRequest { + line: number; + delta: number; + /** moves with what follows it */ + successor: DShape | undefined; + /** the new node and where it goes: what lies in the way moves too */ + id?: string; + box?: Box; + /** stays where it is, also beyond the line */ + fixed?: ReadonlySet; +} + /** What a new shape in a frame has to keep clear of, and where the space tool may push for it. */ export class PlacementObstacles { constructor(private readonly placer: NodePlacer) {} @@ -69,6 +85,98 @@ export class PlacementObstacles { return group ? { within: { ...group.bounds } } : {}; } + /** + * What a push at `line` by `delta` moves: `successor` and what follows it beyond the line (along + * the drawn flows, so a batch pushes like the same ops sent one by one, and the paths of + * boundary events) with what hangs on them (boundary events, content, associated artifacts, the + * groups around them), and in turn what a pushed shape or the new node `id` at `box` would come + * close to or put on a line, with what follows that. The connections listed bend with the push. + * The rest beyond the line keeps its place: another row never moves for nothing, nor does a + * shape in `fixed`. + */ + pushSet(plane: Plane, req: PushRequest): Set { + const { line, delta, box, fixed } = req; + const out = new Set(); + const next = new Map(); + const links = new Map(); + const add = (map: Map, from: string, to: string): void => void (map.get(from) ?? map.set(from, []).get(from)!).push(to); + for (const edge of plane.edges.values()) { + if (!edge.sourceId || !edge.targetId) continue; + if (edge.kind === 'sequenceFlow') add(next, edge.sourceId, edge.targetId); + if (edge.kind === 'association' || edge.kind === 'dataAssociation') { + add(links, edge.sourceId, edge.targetId); + add(links, edge.targetId, edge.sourceId); + } + } + const movable = (shape: DShape): boolean => + !fixed?.has(shape.id) && !shape.hostId && !shape.parentId && shape.kind !== 'participant' && shape.kind !== 'lane' && shape.kind !== 'group' && (shape.bounds.x >= line || (shape.container && cx(shape.bounds) >= line)); + const queue: DShape[] = []; + const take = (shape: DShape | undefined): void => { + if (!shape || out.has(shape.id) || !movable(shape)) return; + out.add(shape.id); + queue.push(shape); + }; + const drain = (): void => { + for (let shape = queue.shift(); shape; shape = queue.shift()) { + const dragged = [shape, ...PlaneQuery.boundariesOf(plane, shape.id), ...(shape.container ? PlaneQuery.contentOf(plane, shape.id).shapes : [])]; + dragged.forEach((part) => out.add(part.id)); + for (const part of dragged) { + for (const target of next.get(part.id) ?? []) take(plane.shapes.get(target)); + for (const other of links.get(part.id) ?? []) { + const artifact = plane.shapes.get(other); + if (artifact?.kind === 'annotation' || artifact?.kind === 'data') take(artifact); + } + } + } + }; + take(req.successor); + drain(); + + /** the room a pushed box sweeps (the new node's box: itself) */ + const swept = (before: Box): Box => (before === box ? box : { ...before, width: before.width + delta }); + /** + * newly close on the way of the push (what it jumps over counts too); a shape that was close + * before counts when the push closes in on it + */ + const closer = (before: Box, other: Box, margin: number): boolean => { + if (!overlaps(swept(before), other, margin)) return false; + return before === box || !overlaps(before, other, margin) || other.x >= right(before) - 1 || (overlaps(swept(before), other) && !overlaps(before, other)); + }; + for (let grew = true, guard = 0; grew && guard <= plane.shapes.size + plane.edges.size; guard++) { + grew = false; + const pushed = [...out].map((pushedId) => plane.shapes.get(pushedId)).filter((shape): shape is DShape => !!shape); + const solids = [...(box ? [box] : []), ...pushed.map((shape) => shape.bounds)]; + const labels = pushed.flatMap((shape) => (shape.label ? [shape.label] : [])); + const hits = (other: Box, margin: number): boolean => solids.some((before) => closer(before, other, margin)) || labels.some((before) => closer(before, other, LABEL_CLEAR)); + for (const group of plane.shapes.values()) { + if (group.kind !== 'group' || out.has(group.id) || !pushed.some((shape) => PlaneQuery.isLeaf(shape) && containsPoint(group.bounds, { x: cx(shape.bounds), y: cy(shape.bounds) }))) continue; + out.add(group.id); + grew = true; + if (group.bounds.x < line) continue; + for (const member of plane.shapes.values()) if (containsPoint(group.bounds, { x: cx(member.bounds), y: cy(member.bounds) })) take(member); + } + for (const shape of plane.shapes.values()) { + if (out.has(shape.id) || !movable(shape)) continue; + const own = [shape, ...PlaneQuery.boundariesOf(plane, shape.id)].flatMap((part) => (part.label ? [part.label] : [])); + if (!hits(this.placer.drawing(plane).unitBox(shape), CLEAR) && !own.some((label) => hits(label, LABEL_CLEAR))) continue; + take(shape); + grew = true; + } + for (const edge of plane.edges.values()) { + if (out.has(edge.id) || [edge.sourceId, edge.targetId].some((end) => !end || end === req.id || out.has(end))) continue; + const bendsInBox = !!box && edge.points.slice(1, -1).some((point) => overlaps({ ...point, width: 0, height: 0 }, box, 10)); + const crossed = edge.points.some((point, index) => index > 0 && solids.some((before) => segmentHits(edge.points[index - 1]!, point, swept(before)) && !(before !== box && segmentHits(edge.points[index - 1]!, point, before)))); + if (!bendsInBox && !crossed && !(edge.label && hits(edge.label, LABEL_CLEAR))) continue; + out.add(edge.id); + take(plane.shapes.get(edge.sourceId!)); + take(plane.shapes.get(edge.targetId!)); + grew = true; + } + drain(); + } + return out; + } + /** The centre of `b` lies inside `frame` (the plane: always). */ private centreIn(box: Box, frame: DShape | undefined): boolean { if (!frame) return true; diff --git a/src/domain/diagram/layout/incremental/RowChooser.ts b/src/domain/diagram/layout/incremental/RowChooser.ts index 86c08069..03105226 100644 --- a/src/domain/diagram/layout/incremental/RowChooser.ts +++ b/src/domain/diagram/layout/incremental/RowChooser.ts @@ -34,6 +34,8 @@ export interface Cand { const CROSSING = 300; /** an existing line through the room the new nodes take: it is routed around them */ const LINE_HIT = 400; +/** a trial route through a label, per segment that hits it */ +const LABEL_HIT = 100; /** an opened row: base, per shape it moves (capped: one crossing always costs more) */ const OPEN_ROW = 100; const OPEN_PER_SHAPE = 3; @@ -104,7 +106,7 @@ export class RowChooser { } } const labels = [...plane.shapes.values(), ...plane.edges.values()].map((owner) => owner.label).filter((label): label is Box => !!label); - for (const label of labels) for (let index = 0; index + 1 < pts.length; index++) if (segmentHits(pts[index]!, pts[index + 1]!, this.shiftedBox(label, shift), 0)) cost += 60; + for (const label of labels) for (let index = 0; index + 1 < pts.length; index++) if (segmentHits(pts[index]!, pts[index + 1]!, this.shiftedBox(label, shift), 0)) cost += LABEL_HIT; return cost; } diff --git a/src/domain/diagram/layout/incremental/SpaceTool.ts b/src/domain/diagram/layout/incremental/SpaceTool.ts index 3ae78651..d30ad646 100644 --- a/src/domain/diagram/layout/incremental/SpaceTool.ts +++ b/src/domain/diagram/layout/incremental/SpaceTool.ts @@ -18,6 +18,12 @@ export interface SpaceRequest { keep?: ReadonlySet; /** growing only: crossed sub-processes holding none of these move or stay whole, never grow */ anchors?: ReadonlyArray; + /** + * Beyond the line only these shapes move (boundary events with their host, content with its + * container) and only connections touching a changed shape or listed here bend; groups the line + * crosses grow only when listed. Pools and lanes keep the usual rules. + */ + push?: ReadonlySet; } export interface SpaceResult { @@ -49,6 +55,7 @@ interface Pass { keep: ReadonlySet; before: Map; held: Set | undefined; + push: ReadonlySet | undefined; scope?: FrameScope; } @@ -163,6 +170,17 @@ export class SpaceTool { return id; } + /** + * Whether one of `lanes` holds `shape` (or its host): by its frame, else (artifacts, groups, + * nodes no lane lists) by where its centre was drawn. + */ + private inLanes(lanes: DShape[], shape: DShape, before: ReadonlyMap): boolean { + const owner = shape.hostId ? this.plane.shapes.get(shape.hostId) ?? shape : shape; + if (Frames.of(this.plane, owner)) return lanes.some((lane) => Frames.within(this.plane, owner, lane)); + const box = before.get(owner.id) ?? owner.bounds; + return lanes.some((lane) => containsPoint(before.get(lane.id)!, { x: cx(box), y: cy(box) })); + } + /** A shrunk container keeps `pad` beyond its members and never ends further out than before. */ private capShrink(container: DShape, axis: Axis, before: Box, pad = 10): void { let need = -Infinity; @@ -177,6 +195,50 @@ export class SpaceTool { else container.bounds.height = limit - container.bounds.y; } + /** + * With a push set a crossed pool or lane grows only as far as its content now reaches, keeping + * the margin it had; a lane that ended with its pool ends with it again. Without a pool the lanes + * that ended with a grown top lane grow like a pool, also holding the groups drawn in them. + */ + private capGrowth(pass: Pass, res: SpaceResult): void { + const { axis, delta, before } = pass; + const setFar = (container: DShape, to: number): void => { + if (axis === 'x') container.bounds.width = to - container.bounds.x; + else container.bounds.height = to - container.bounds.y; + if (Math.abs(to - far(before.get(container.id)!, axis)) < 0.5) res.resized.delete(container.id); + else res.resized.add(container.id); + }; + const cap = (frames: DShape[], holds: (shape: DShape) => boolean): void => { + let farBefore = -Infinity; + let farNow = -Infinity; + for (const shape of this.plane.shapes.values()) { + if (shape.kind === 'participant' || shape.kind === 'lane' || !holds(shape)) continue; + const shift = res.moved.has(shape.id) ? delta : 0; + farBefore = Math.max(farBefore, far(before.get(shape.id)!, axis), shape.label ? far(shape.label, axis) - shift : -Infinity); + farNow = Math.max(farNow, far(shape.bounds, axis), shape.label ? far(shape.label, axis) : -Infinity); + } + const old = Math.max(...frames.map((frame) => far(before.get(frame.id)!, axis))); + if (Number.isFinite(farNow)) for (const frame of frames) setFar(frame, old + Math.min(delta, Math.max(0, farNow + Math.max(0, old - farBefore) - old))); + }; + const containers = [...res.resized].map((id) => this.plane.shapes.get(id)!).filter((shape) => shape.kind === 'participant' || shape.kind === 'lane'); + for (const pool of containers.filter((shape) => shape.kind === 'participant')) cap([pool], (shape) => shape.kind !== 'group' && Frames.within(this.plane, shape, pool)); + const free = [...this.plane.shapes.values()].filter((shape) => shape.kind === 'lane' && !shape.poolId); + /** a lane beyond the line moved: the lanes do not end where the line crosses them */ + const tops = free.some((lane) => res.moved.has(lane.id)) ? [] : free.filter((lane) => !lane.laneId && res.resized.has(lane.id)); + const done = new Set(); + for (const top of tops) { + if (done.has(top)) continue; + const end = far(before.get(top.id)!, axis); + const set = free.filter((lane) => Math.abs(far(before.get(lane.id)!, axis) - end) <= 2); + set.forEach((lane) => done.add(lane)); + cap(set, (shape) => this.inLanes(set, shape, before)); + } + for (const lane of containers.filter((shape) => shape.kind === 'lane')) { + const pool = lane.poolId ? this.plane.shapes.get(lane.poolId) : undefined; + if (pool && Math.abs(far(before.get(lane.id)!, axis) - far(before.get(pool.id)!, axis)) <= 2) setFar(lane, far(pool.bounds, axis)); + } + } + private holders(anchors: ReadonlyArray): Set { const out = new Set(); for (const id of anchors) { @@ -223,6 +285,11 @@ export class SpaceTool { return out; } + /** Without a push set everything beyond the line may move. */ + private pushed(pass: Pass, shape: DShape): boolean { + return !pass.push || shape.kind === 'participant' || shape.kind === 'lane' || pass.push.has(shape.id); + } + private rigid(pass: Pass, candidate: DShape): boolean { if (!pass.held || candidate.kind !== 'subProcess' || !candidate.container || pass.held.has(candidate.id) || pass.keep.has(candidate.id)) return false; const box = pass.before.get(candidate.id)!; @@ -295,11 +362,11 @@ export class SpaceTool { verdict = decide(block, depth + 1) === 'move' ? 'move' : 'stay'; } else if (this.rigid(pass, shape)) { const box = before.get(shape.id)!; - verdict = (axis === 'x' ? cx(box) : cy(box)) >= line ? 'move' : 'stay'; + verdict = (axis === 'x' ? cx(box) : cy(box)) >= line && this.pushed(pass, shape) ? 'move' : 'stay'; } else { const ref = before.get(shape.id)!; - if (near(ref, axis) >= line) verdict = overlapsBand(ref, band, axis) ? 'move' : 'stay'; - else if ((shape.container || shape.kind === 'group') && far(ref, axis) > line && overlapsBand(ref, band, axis)) { + if (near(ref, axis) >= line) verdict = overlapsBand(ref, band, axis) && this.pushed(pass, shape) ? 'move' : 'stay'; + else if ((shape.container || (shape.kind === 'group' && this.pushed(pass, shape))) && far(ref, axis) > line && overlapsBand(ref, band, axis)) { verdict = delta < 0 && beyondBand(ref, band, axis) ? 'stay' : 'resize'; } } @@ -332,6 +399,7 @@ export class SpaceTool { const container = plane.shapes.get(id)!; if (container.container) this.capShrink(container, axis, before.get(id)!); } + if (pass.push && delta > 0) this.capGrowth(pass, res); if (axis === 'x') this.alignPoolsAndLanes(before, res); const blockOf = new Map(); @@ -353,6 +421,7 @@ export class SpaceTool { if (edge.sourceId && edge.targetId && keep.has(edge.sourceId) && keep.has(edge.targetId)) continue; if (pass.scope && !changed(edge.sourceId) && !changed(edge.targetId) && outside(edge.sourceId) && outside(edge.targetId)) continue; const pointCount = edge.points.length; + const bends = !pass.push || pass.push.has(edge.id) || changed(edge.sourceId) || changed(edge.targetId); const block = edge.sourceId && edge.targetId ? blockOf.get(edge.sourceId) : undefined; const inBlock = !!block && blockOf.get(edge.targetId!) === block; const seg = edge.label && pointCount >= 2 ? this.nearestSegment(edge.points, edge.label) : -1; @@ -366,7 +435,7 @@ export class SpaceTool { else if (end && res.moved.has(end.id)) shift = true; else if (end && res.resized.has(end.id)) shift = beyond; else if (end) shift = false; - else shift = beyond && pointInBand(pt, band, axis); + else shift = beyond && bends && pointInBand(pt, band, axis); shifted.push(shift); if (shift) { if (axis === 'x') pt.x += delta; @@ -376,7 +445,7 @@ export class SpaceTool { if (!edge.label) continue; const startShifted = seg >= 0 ? shifted[seg] : undefined; const endShifted = seg >= 0 ? shifted[seg + 1] : undefined; - const follow = startShifted !== undefined && startShifted === endShifted ? startShifted : near(edge.label, axis) >= line && (seg >= 0 || centreInBand(edge.label, band, axis)); + const follow = startShifted !== undefined && startShifted === endShifted ? startShifted : bends && near(edge.label, axis) >= line && (seg >= 0 || centreInBand(edge.label, band, axis)); if (follow) shiftBox(edge.label, axis, delta); } return res; @@ -472,7 +541,7 @@ export class SpaceTool { for (const shape of this.plane.shapes.values()) before.set(shape.id, { ...shape.bounds }); const keep = req.keep ?? new Set(); const held = req.anchors && delta > 0 ? this.holders(req.anchors) : undefined; - const base = { plane: this.plane, axis, line, delta, keep, before, held }; + const base = { plane: this.plane, axis, line, delta, keep, before, held, push: req.push }; const frame = req.frame ? this.plane.shapes.get(req.frame) : undefined; const fb = frame && before.get(frame.id)!; if (frame && fb && frame.container && delta > 0 && near(fb, axis) < line && far(fb, axis) > line) return this.spaceInFrame(base, frame); diff --git a/test/domain/diagram/layout/incremental/association-graze.test.ts b/test/domain/diagram/layout/incremental/association-graze.test.ts new file mode 100644 index 00000000..7af22a62 --- /dev/null +++ b/test/domain/diagram/layout/incremental/association-graze.test.ts @@ -0,0 +1,65 @@ +/** + * A kept association that a node placed in this run or its label touches is routed again, and not + * along the same straight line (src/domain/diagram/layout/incremental/IncrementalRouting.ts, + * EdgeRouter.ts `touchesFresh`): + * - a diagonal passing 2 px beside a new task leaves it; the annotation and the data object stay + * - a line 1.5 px above a new gateway's label leaves it (the gateway is placed in a batch) + * - one passing 6 px beside the new task is kept + */ +import { describe, expect, it } from 'vitest'; +import { Doc } from '../../../../../src/domain/document/Doc.js'; +import { pathHits, type Box } from '../../../../../src/domain/diagram/utils/geom.js'; +import { mutateDoc } from '../../../../../src/index.js'; +import { DI_NS, edgeDi, edges, shapeDi, shapes } from '../../../../helpers/drawing.js'; + + +/** S -> A on a row; the note N's association runs to the data object D along `line` */ +const drawing = (note: Box, data: Box, line: Array<[number, number]>): string => ` + + + +note + +${shapeDi('S', 100, 100, 36, 36)}${shapeDi('A', 196, 78, 100, 80)}${shapeDi('N', note.x, note.y, note.width, note.height)}${shapeDi('D', data.x, data.y, data.width, data.height)} +${edgeDi('F1', [[136, 118], [196, 118]])}${edgeDi('AS', line)} +`; + +const run = async (xml: string, ops: object[]) => { + const result = await mutateDoc(await Doc.fromXml(xml), ops, { dryRun: true, layout: 'incremental' }); + const after = await Doc.fromXml(result.xml); + return { result, before: shapes(await Doc.fromXml(xml)), after: shapes(after), line: edges(after).get('AS')!.points }; +}; + +describe('a kept association a new node touches is routed again', () => { + it('a diagonal 2 px beside a new task goes around it; the annotation and the data object stay', async () => { + const xml = drawing({ x: 517, y: 20, width: 120, height: 30 }, { x: 300, y: 280, width: 36, height: 50 }, [[563, 50], [336, 286]]); + const { result, before, after, line } = await run(xml, [{ op: 'add', kind: 'task', id: 'T', name: 'T', after: 'A' }]); + expect(after.get('T')!.bounds).toMatchObject({ x: 356, y: 78 }); + expect(result.layout.rerouted).toContain('AS'); + expect(pathHits(line, after.get('T')!.bounds, -3)).toBe(false); + for (const id of ['N', 'D']) expect(after.get(id)!.bounds, id).toEqual(before.get(id)!.bounds); + }); + + it("a line 1.5 px above a new gateway's label leaves it", async () => { + const xml = drawing({ x: 620, y: 40, width: 100, height: 30 }, { x: 150, y: 30, width: 36, height: 50 }, [[620, 50], [186, 62]]); + const { result, before, after, line } = await run(xml, [ + { op: 'add', kind: 'exclusiveGateway', id: 'G', name: 'Order approved?', after: 'A' }, + { op: 'add', kind: 'task', id: 'Y', name: 'Y', after: 'G' }, + { op: 'add', kind: 'task', id: 'R', name: 'R', after: 'G', flowName: 'no' }, + ]); + const label = after.get('G')!.label!; + expect(label.y + label.height).toBeLessThan(after.get('G')!.bounds.y); + expect(result.layout.rerouted).toContain('AS'); + expect(pathHits(line, label, -3)).toBe(false); + for (const id of ['N', 'D']) expect(after.get(id)!.bounds, id).toEqual(before.get(id)!.bounds); + expect(result.layout.metrics!.added).toEqual([]); + }); + + it('one 6 px beside the new task is kept', async () => { + const points: Array<[number, number]> = [[571, 50], [336, 287]]; + const xml = drawing({ x: 526, y: 20, width: 120, height: 30 }, { x: 300, y: 280, width: 36, height: 50 }, points); + const { result, line } = await run(xml, [{ op: 'add', kind: 'task', id: 'T', name: 'T', after: 'A' }]); + expect(result.layout.rerouted).not.toContain('AS'); + expect(line.map((point) => [point.x, point.y])).toEqual(points); + }); +}); diff --git a/test/domain/diagram/layout/incremental/flow-dock.test.ts b/test/domain/diagram/layout/incremental/flow-dock.test.ts new file mode 100644 index 00000000..abafb000 --- /dev/null +++ b/test/domain/diagram/layout/incremental/flow-dock.test.ts @@ -0,0 +1,69 @@ +/** + * An association docked on a flow, when no straight line reaches the flow, is routed to the middle + * of every segment and keeps the route crossing the fewest lines, not the route to the segment + * nearest the annotation (src/domain/diagram/layout/incremental/EdgeRouter.ts `flowDock`): + * - a note on a loop under the row: the loop's near segment lies behind two flows going down, the + * association docks on another segment of the loop across one line + * - the public scenario: 'Max 3 iterations' on the loop of Approved? no longer crosses both new + * branches of the added gateway and timer + */ +import { readFileSync } from 'node:fs'; +import { join } from 'node:path'; +import { describe, expect, it } from 'vitest'; +import { applyToXml } from '../../../../../src/api.js'; +import { Doc } from '../../../../../src/domain/document/Doc.js'; +import { crossings } from '../../../../../src/domain/diagram/utils/routing.js'; +import type { Point } from '../../../../../src/domain/diagram/utils/geom.js'; +import { mutateDoc } from '../../../../../src/index.js'; +import { DI_NS, edgeDi, edges, shapeDi } from '../../../../helpers/drawing.js'; + +const ROOT = join(import.meta.dirname, '..', '..', '..', '..', '..'); + +/** S -> A -> G -> B -> H -> E on a row, the loop L from G back into A below it, B and H branch down to D and C; the note N top right */ +const XML = ` + + + + +note + +${shapeDi('S', 100, 182, 36, 36)}${shapeDi('A', 200, 160, 100, 80)}${shapeDi('G', 350, 175, 50, 50)}${shapeDi('B', 420, 160, 100, 80)}${shapeDi('H', 600, 175, 50, 50)}${shapeDi('E', 700, 182, 36, 36)} +${shapeDi('C', 700, 305, 100, 80)}${shapeDi('D', 550, 440, 100, 80)}${shapeDi('N', 900, 50, 100, 30)} +${edgeDi('F1', [[136, 200], [200, 200]])}${edgeDi('F2', [[300, 200], [350, 200]])}${edgeDi('F3', [[400, 200], [420, 200]])}${edgeDi('F4', [[520, 200], [600, 200]])}${edgeDi('F5', [[650, 200], [700, 200]])} +${edgeDi('F6', [[625, 225], [625, 345], [700, 345]])}${edgeDi('F7', [[470, 240], [470, 480], [550, 480]])}${edgeDi('L', [[375, 225], [375, 300], [250, 300], [250, 240]])} +`; + +const onLine = (point: Point, line: readonly Point[]): boolean => + line.some((start, index) => { + const end = line[index + 1]; + return !!end && Math.min(start.x, end.x) <= point.x && point.x <= Math.max(start.x, end.x) && Math.min(start.y, end.y) <= point.y && point.y <= Math.max(start.y, end.y); + }); + +describe('an association docks on the segment of a flow its route crosses the fewest lines to', () => { + it('a note on a loop behind two branches docks across one line, on the loop', async () => { + const result = await mutateDoc(await Doc.fromXml(XML), [{ op: 'connect', source: 'N', target: 'L', id: 'AS' }], { dryRun: true, layout: 'incremental' }); + const drawn = edges(await Doc.fromXml(result.xml)); + const line = drawn.get('AS')!.points; + const others = [...drawn.values()].filter((edge) => edge.id !== 'AS').map((edge) => edge.points); + // the loop's segment nearest the note lies behind F6 and F7 + expect(crossings(line, others)).toBeLessThanOrEqual(1); + expect(onLine(line[line.length - 1]!, drawn.get('L')!.points)).toBe(true); + }); + + it("'Max 3 iterations' on the loop crosses at most one of the new branches (public scenario)", async () => { + const xml = readFileSync(join(ROOT, 'tools', 'scenarios', 'artifacts__16-annotations-events-loop.bpmn'), 'utf8'); + const ops = [ + { op: 'add', kind: 'userTask', id: 'Activity_BenchE8', name: 'Bench approval', flow: 'Flow_4' }, + { op: 'add', kind: 'exclusiveGateway', id: 'Gateway_BenchE8', name: 'Bench approved?', after: 'Activity_BenchE8', before: 'Activity_Publish' }, + { op: 'add', kind: 'sendTask', id: 'Activity_BenchE8No', name: 'Bench rejection', after: 'Gateway_BenchE8', flowName: 'no' }, + { op: 'add', kind: 'endEvent', id: 'Event_BenchE8NoEnd', name: 'Bench rejected', after: 'Activity_BenchE8No' }, + { op: 'add', kind: 'boundaryEvent:timer', id: 'Event_BenchE9Timer', name: 'Bench 3 days', on: 'Activity_BenchE8', timer: 'P3D', nonInterrupting: true }, + { op: 'add', kind: 'sendTask', id: 'Activity_BenchE9Remind', name: 'Bench reminder', after: 'Event_BenchE9Timer' }, + { op: 'add', kind: 'endEvent', id: 'Event_BenchE9Reminded', name: 'Bench reminded', after: 'Activity_BenchE9Remind' }, + ]; + const applied = await applyToXml(xml, ops, { force: true }); + expect(applied.result.layout?.mode).toBe('incremental'); + const crossed = (applied.result.layout?.metrics?.added ?? []).filter((problem) => problem.kind === 'crossings' && problem.ids.includes('Association_3')); + expect(crossed.length).toBeLessThanOrEqual(1); + }); +}); diff --git a/test/domain/diagram/layout/incremental/label-graze.test.ts b/test/domain/diagram/layout/incremental/label-graze.test.ts new file mode 100644 index 00000000..80f30fce --- /dev/null +++ b/test/domain/diagram/layout/incremental/label-graze.test.ts @@ -0,0 +1,66 @@ +/** + * Once the labels are settled, a kept association that a node placed in this run or its label + * touches is routed again, and the new labels its line still runs over step aside + * (src/domain/diagram/layout/incremental/IncrementalRouting.ts `routeGrazed`): the last node's + * label is placed after the routing, and a crossing repair route may run over one. + * - a single `add exclusiveGateway` whose label lands 1.4 px below a kept diagonal association: + * the association leaves it, as when the gateway is placed in a batch + * - a repair route of a data association runs over the label of a new end event (public + * scenario): the label steps aside + */ +import { readFileSync } from 'node:fs'; +import { join } from 'node:path'; +import { describe, expect, it } from 'vitest'; +import { applyToXml } from '../../../../../src/api.js'; +import { Doc } from '../../../../../src/domain/document/Doc.js'; +import { pathHits } from '../../../../../src/domain/diagram/utils/geom.js'; +import { mutateDoc } from '../../../../../src/index.js'; +import { DI_NS, edges, shapes } from '../../../../helpers/drawing.js'; + +const ROOT = join(import.meta.dirname, '..', '..', '..', '..', '..'); + +/** S -> A on a row; the note N's association runs diagonally to the data object D above A */ +const XML = ` + + + +note + + + + + + + +`; + +describe('a kept association a new label touches is routed again once the labels are settled', () => { + it("a single gateway's label below a diagonal association: the association leaves it", async () => { + const result = await mutateDoc(await Doc.fromXml(XML), [{ op: 'add', kind: 'exclusiveGateway', id: 'G', name: 'Order approved?', after: 'A' }], { dryRun: true, layout: 'incremental' }); + const after = await Doc.fromXml(result.xml); + const before = shapes(await Doc.fromXml(XML)); + const gateway = shapes(after).get('G')!; + expect(result.layout.rerouted).toContain('AS'); + expect(pathHits(edges(after).get('AS')!.points, gateway.label!, -3)).toBe(false); + for (const id of ['N', 'D']) expect(shapes(after).get(id)!.bounds, id).toEqual(before.get(id)!.bounds); + expect(result.layout.metrics!.added).toEqual([]); + }); + + it('a repair route over the label of a new end event: the label steps aside (public scenario)', async () => { + let xml = readFileSync(join(ROOT, 'tools', 'scenarios', 'artifacts__04b-data-crossscope.bpmn'), 'utf8'); + xml = (await applyToXml(xml, [{ op: 'remove', ids: ['Activity_Collect'] }], { force: true })).xml; + xml = (await applyToXml(xml, [{ op: 'add', kind: 'userTask', id: 'Activity_BenchE1', name: 'Bench inserted task', after: 'Activity_Inform' }], { force: true })).xml; + const applied = await applyToXml( + xml, + [ + { op: 'add', kind: 'boundaryEvent:timer', id: 'Event_BenchE4', name: 'Bench timeout', on: 'Activity_Score', timer: 'PT1H' }, + { op: 'add', kind: 'endEvent', id: 'Event_BenchE4End', name: 'Bench timed out', after: 'Event_BenchE4' }, + ], + { force: true }, + ); + const after = await Doc.fromXml(applied.xml); + expect(applied.result.layout?.rerouted).toContain('DataInputAssociation_1'); + expect(pathHits(edges(after).get('DataInputAssociation_1')!.points, shapes(after).get('Event_BenchE4End')!.label!, -1)).toBe(false); + expect(applied.result.layout?.metrics?.added.filter((problem) => problem.kind === 'labelOnLine')).toEqual([]); + }); +}); diff --git a/test/domain/diagram/layout/incremental/label-text.test.ts b/test/domain/diagram/layout/incremental/label-text.test.ts new file mode 100644 index 00000000..20017a02 --- /dev/null +++ b/test/domain/diagram/layout/incremental/label-text.test.ts @@ -0,0 +1,94 @@ +/** + * A label spot must keep the text bpmn-js draws off lines, not only the wrapped label box + * (src/domain/diagram/layout/incremental/LabelPlacer.ts): a short name is drawn on one line up to + * 90 px wide, centred on the box, so it reaches a line a few px beside the narrower box. + * - a spot clear for box and text first, also shifted as far as the text needs; a line 1 px + * inside the box (allowed for the box) is on the text + * - only when no spot keeps the text clear, the first one clear for the box + * - a line crossing the owner's own connection does not push the text aside: the crossing repair + * routes it again, and then along the side the label dodged to + * - flow labels alike + */ +import { describe, expect, it } from 'vitest'; +import type { DEdge, DShape, Plane } from '../../../../../src/domain/diagram/di/types.js'; +import { Doc } from '../../../../../src/domain/document/Doc.js'; +import { Drawing } from '../../../../../src/domain/diagram/layout/incremental/Drawing.js'; +import { glyph } from '../../../../../src/domain/diagram/utils/harness.js'; +import { segmentHits, type Box } from '../../../../../src/domain/diagram/utils/geom.js'; + +const els = Doc.create({ processId: 'P' }); +const TYPES: Partial> = { task: 'bpmn:Task', gateway: 'bpmn:ExclusiveGateway', event: 'bpmn:EndEvent' }; + +const sh = (id: string, kind: DShape['kind'], [left, top, width, height]: [number, number, number, number], name?: string): DShape => ({ + id, + el: els.moddle.create(TYPES[kind]!, { id, ...(name ? { name } : {}) }), + kind, + container: false, + bounds: { x: left, y: top, width, height }, +}); +const ed = (id: string, pts: Array<[number, number]>, name?: string, ends?: [string, string]): DEdge => ({ + id, + el: els.moddle.create('bpmn:SequenceFlow', { id, ...(name ? { name } : {}) }), + kind: 'sequenceFlow', + ...(ends ? { sourceId: ends[0], targetId: ends[1] } : {}), + points: pts.map(([left, top]) => ({ x: left, y: top })), +}); +const plane = (shapes: DShape[], edges: DEdge[]): Plane => ({ + id: 'p', + rootId: 'P', + root: els.moddle.create('bpmn:Process', { id: 'P' }), + shapes: new Map(shapes.map((shape) => [shape.id, shape])), + edges: new Map(edges.map((edge) => [edge.id, edge])), + dropped: [], +}); +/** The lines that run over the text bpmn-js draws for `name` in `box` (what labelOnLine counts). */ +const linesOverText = (testPlane: Plane, box: Box, name: string): string[] => + [...testPlane.edges.values()].filter((edge) => edge.points.slice(1).some((end, index) => segmentHits(edge.points[index]!, end, glyph(box, name), 0))).map((edge) => edge.id); + +describe('label spots keep the drawn text off lines', () => { + it('puts a gateway label beside its own branch going up, as far as the one-line text needs', () => { + // A -> G -> B on the row, "no" leaves G's top vertex up to N; a task right below G + const testPlane = plane( + [sh('A', 'task', [150, 160, 100, 80]), sh('G', 'gateway', [300, 175, 50, 50], 'Bench approved?'), sh('B', 'task', [420, 160, 100, 80]), sh('N', 'task', [420, 20, 100, 80]), sh('D', 'task', [255, 240, 120, 80])], + [ed('F1', [[250, 200], [300, 200]]), ed('F2', [[350, 200], [420, 200]]), ed('No', [[325, 175], [325, 60], [420, 60]], 'no')], + ); + const box = new Drawing(testPlane).placeShapeLabel(testPlane.shapes.get('G')!)!; + // the wrapped box would be clear 6 px right of the line already; the 90 px text is not + expect(box).toEqual({ x: 339, y: 141, width: 75, height: 28 }); + expect(linesOverText(testPlane, box, 'Bench approved?')).toEqual([]); + }); + + it('takes a spot clear for the box when no spot keeps the text clear', () => { + // lines 6 px left and right of the centred box below the event reach the text on every spot + const testPlane = plane([sh('E', 'event', [300, 200, 36, 36], 'Order rejected')], [ed('L', [[278, 0], [278, 500]]), ed('R', [[358, 0], [358, 500]])]); + const box = new Drawing(testPlane).placeShapeLabel(testPlane.shapes.get('E')!)!; + expect(box).toEqual({ x: 284, y: 242, width: 68, height: 28 }); + expect(linesOverText(testPlane, box, 'Order rejected')).toEqual(['L', 'R']); + }); + + it('puts a flow label where a crossing line misses its text', () => { + // a line crosses F 4 px before the first spot near the start + const testPlane = plane([], [ed('F', [[200, 200], [500, 200]], 'Order rejected'), ed('V', [[204, 56], [204, 400]])]); + const box = new Drawing(testPlane).placeEdgeLabel(testPlane.edges.get('F')!)!; + expect(box).toEqual({ x: 316, y: 168, width: 68, height: 28 }); + expect(linesOverText(testPlane, box, 'Order rejected')).toEqual([]); + }); + + it('skips a flow label spot whose text a line touches 1 px inside the box', () => { + // H runs 1 px inside the bottom of the first spot above F + const testPlane = plane([], [ed('F', [[200, 200], [500, 200]], 're-check'), ed('H', [[150, 195], [260, 195]])]); + const box = new Drawing(testPlane).placeEdgeLabel(testPlane.edges.get('F')!)!; + expect(box).toEqual({ x: 208, y: 204, width: 68, height: 14 }); + expect(linesOverText(testPlane, box, 're-check')).toEqual([]); + }); + + it('keeps an event label below when the line its text would touch crosses the incoming flow', () => { + // V runs 4 px left of the box below the end event E, through its text; In comes from the left + const lines = (top: number): DEdge[] => [ed('In', [[200, 218], [300, 218]], undefined, ['X', 'E']), ed('V', [[280, top], [280, 400]], undefined, ['Y', 'Z'])]; + const crossed = plane([sh('E', 'event', [300, 200, 36, 36], 'Order rejected')], lines(100)); + expect(new Drawing(crossed).placeShapeLabel(crossed.shapes.get('E')!)).toEqual({ x: 284, y: 242, width: 68, height: 28 }); + // starting below In, V stays where it is: the label goes above + const kept = plane([sh('E', 'event', [300, 200, 36, 36], 'Order rejected')], lines(230)); + expect(new Drawing(kept).placeShapeLabel(kept.shapes.get('E')!)).toEqual({ x: 284, y: 166, width: 68, height: 28 }); + }); +}); diff --git a/test/domain/diagram/layout/incremental/lane-growth.test.ts b/test/domain/diagram/layout/incremental/lane-growth.test.ts new file mode 100644 index 00000000..e7a82ca3 --- /dev/null +++ b/test/domain/diagram/layout/incremental/lane-growth.test.ts @@ -0,0 +1,84 @@ +/** + * Lanes without a pool grow like a pool when a splice pushes (SpaceTool.ts capGrowth): only as far + * as the content drawn in them now reaches, keeping the margin they had; nested lanes follow their + * top lane, and content no lane lists (and a group) counts where it is drawn. + */ +import { describe, expect, it } from 'vitest'; +import { Doc } from '../../../../../src/domain/document/Doc.js'; +import { mutateDoc } from '../../../../../src/index.js'; +import { DI_NS, edgeDi, sequenceFlow, shapeDi, shapes } from '../../../../helpers/drawing.js'; + +const sh = (id: string, left: number, top: number, width: number, height: number): string => shapeDi(id, left, top, width, height, id.startsWith('L') ? ' isHorizontal="true"' : ''); +const refs = (ids: string[]): string => ids.map((id) => `${id}`).join(''); + +/** + * S -> A -> G; "yes": B -> C -> E on the main row in lane LMain, "no": R -> RE below in lane LRej + * (nested: in its child lane LInner); `listed` false leaves LMain without flow node refs, `group` + * draws a group around R and RE up to the lanes' end + */ +const laned = ({ nested = false, listed = true, group = false } = {}): string => { + const main = listed ? refs(['S', 'A', 'G', 'B', 'C', 'E']) : ''; + const rejection = nested ? `${refs(['R', 'RE'])}` : refs(['R', 'RE']); + return `\n +${main}${rejection} + + + ${group ? '' : ''} + ${sequenceFlow('F1', 'S', 'A')}${sequenceFlow('F2', 'A', 'G')}${sequenceFlow('F3', 'G', 'B')}${sequenceFlow('F4', 'B', 'C')}${sequenceFlow('F5', 'C', 'E')}${sequenceFlow('F6', 'G', 'R')}${sequenceFlow('F7', 'R', 'RE')} + +${[ + sh('LMain', 70, 120, 798, 140), + sh('LRej', 70, 260, 798, 160), + ...(nested ? [sh('LInner', 100, 260, 768, 160)] : []), + ...(group ? [sh('Grp', 446, 285, 422, 115)] : []), + sh('S', 100, 182, 36, 36), + sh('A', 196, 160, 100, 80), + sh('G', 356, 175, 50, 50), + sh('B', 466, 160, 100, 80), + sh('C', 626, 160, 100, 80), + sh('E', 786, 182, 36, 36), + sh('R', 466, 300, 100, 80), + sh('RE', 626, 322, 36, 36), + edgeDi('F1', [[136, 200], [196, 200]]), + edgeDi('F2', [[296, 200], [356, 200]]), + edgeDi('F3', [[406, 200], [466, 200]]), + edgeDi('F4', [[566, 200], [626, 200]]), + edgeDi('F5', [[726, 200], [786, 200]]), + edgeDi('F6', [[381, 225], [381, 340], [466, 340]]), + edgeDi('F7', [[566, 340], [626, 340]]), + ].join('')}\n`; +}; + +const run = async (xml: string, ops: object[]) => { + const result = await mutateDoc(await Doc.fromXml(xml), ops, { dryRun: true, layout: 'incremental' }); + return { result, before: shapes(await Doc.fromXml(xml)), after: shapes(await Doc.fromXml(result.xml)) }; +}; + +describe('lanes without a pool grow only as far as their content reaches', () => { + for (const variant of [{}, { nested: true }, { listed: false }]) { + const lanes = ['LMain', 'LRej', ...(variant.nested ? ['LInner'] : [])]; + + it(`a task on the shorter lane leaves the lanes as they are ${JSON.stringify(variant)}`, async () => { + const { result, before, after } = await run(laned(variant), [{ op: 'add', kind: 'task', id: 'N', name: 'N', after: 'R' }]); + expect(after.get('RE')!.bounds.x).toBeGreaterThan(before.get('RE')!.bounds.x); + for (const id of lanes) expect(after.get(id)!.bounds, id).toEqual(before.get(id)!.bounds); + expect(result.layout.metrics!.added).toEqual([]); + }); + + it(`a task on the row that reaches the border widens every lane by the push, keeping the margin ${JSON.stringify(variant)}`, async () => { + const { result, before, after } = await run(laned(variant), [{ op: 'add', kind: 'task', id: 'N', name: 'N', after: 'C' }]); + const dx = after.get('E')!.bounds.x - before.get('E')!.bounds.x; + expect(dx).toBeGreaterThan(0); + for (const id of lanes) expect(after.get(id)!.bounds.width, id).toBe(before.get(id)!.bounds.width + dx); + expect(result.layout.metrics!.added).toEqual([]); + }); + } + + it('a group drawn in the lanes that grows with the push takes every lane along', async () => { + const { result, before, after } = await run(laned({ group: true }), [{ op: 'add', kind: 'task', id: 'N', name: 'N', after: 'R' }]); + const dx = after.get('Grp')!.bounds.width - before.get('Grp')!.bounds.width; + expect(dx).toBeGreaterThan(0); + for (const id of ['LMain', 'LRej']) expect(after.get(id)!.bounds.width, id).toBe(before.get(id)!.bounds.width + dx); + expect(result.layout.metrics!.added).toEqual([]); + }); +}); diff --git a/test/domain/diagram/layout/incremental/local-push.test.ts b/test/domain/diagram/layout/incremental/local-push.test.ts new file mode 100644 index 00000000..7b2c1849 --- /dev/null +++ b/test/domain/diagram/layout/incremental/local-push.test.ts @@ -0,0 +1,215 @@ +/** + * The space tool of a splice pushes only what follows the new node and what that would run into + * (src/domain/diagram/layout/incremental/PlacementObstacles.ts pushSet, SpaceTool.ts `push`): + * - another row beyond the line keeps its place (a rejection path gets a task, the main path + * above it stays) + * - a pushed node that would jump over a line takes the line and its end along + * - what hangs on a pushed shape's boundary events and content (annotations) moves with it + * - a pool grows only as far as its content reaches + * - a new flow into a drawn node left of its source pushes the target and what follows it right of + * the source (NeighbourPlacer pushBehind), as a single `connect` and in a batch alike; a loop + * return stays + */ +import { describe, expect, it } from 'vitest'; +import { applyToXml } from '../../../../../src/api.js'; +import { Doc } from '../../../../../src/domain/document/Doc.js'; +import { mutateDoc } from '../../../../../src/index.js'; +import { drawnProcess, edgeDi, sequenceFlow, shapeDi, shapes } from '../../../../helpers/drawing.js'; + + +/** S -> A -> G; "yes": B -> C -> E on the main row, "no": R -> RE on the row below, R under B */ +const branches = (): string => + drawnProcess( + ` + + + ${sequenceFlow('F1', 'S', 'A')}${sequenceFlow('F2', 'A', 'G')}${sequenceFlow('F3', 'G', 'B')}${sequenceFlow('F4', 'B', 'C')}${sequenceFlow('F5', 'C', 'E')}${sequenceFlow('F6', 'G', 'R')}${sequenceFlow('F7', 'R', 'RE')}`, + [ + shapeDi('S', 100, 182, 36, 36), + shapeDi('A', 196, 160, 100, 80), + shapeDi('G', 356, 175, 50, 50), + shapeDi('B', 466, 160, 100, 80), + shapeDi('C', 626, 160, 100, 80), + shapeDi('E', 786, 182, 36, 36), + shapeDi('R', 466, 300, 100, 80), + shapeDi('RE', 626, 322, 36, 36), + edgeDi('F1', [[136, 200], [196, 200]]), + edgeDi('F2', [[296, 200], [356, 200]]), + edgeDi('F3', [[406, 200], [466, 200]]), + edgeDi('F4', [[566, 200], [626, 200]]), + edgeDi('F5', [[726, 200], [786, 200]]), + edgeDi('F6', [[381, 225], [381, 340], [466, 340]]), + edgeDi('F7', [[566, 340], [626, 340]]), + ].join(''), + ); + +const run = async (xml: string, ops: object[]) => { + const result = await mutateDoc(await Doc.fromXml(xml), ops, { dryRun: true, layout: 'incremental' }); + return { result, before: shapes(await Doc.fromXml(xml)), after: shapes(await Doc.fromXml(result.xml)) }; +}; +const at = (map: Map, id: string): { x: number; y: number } => ({ x: map.get(id)!.bounds.x, y: map.get(id)!.bounds.y }); +const shapeLines = (xml: string): string[] => [...xml.matchAll(/bpmnElement="([^"]+)"[^>]*>\s*/g)].map((match) => `${match[1]} ${match[2]}`).sort(); + +describe('a splice pushes only what follows the new node', () => { + it('a task after the last task of the row below moves its end event, not the main row above', async () => { + const { result, before, after } = await run(branches(), [{ op: 'add', kind: 'task', id: 'N', name: 'N', after: 'R' }]); + expect(at(after, 'N')).toEqual({ x: 626, y: 300 }); + expect(at(after, 'RE').x).toBe(786); + for (const id of ['S', 'A', 'G', 'B', 'C', 'E', 'R']) expect(at(after, id), id).toEqual(at(before, id)); + expect(result.layout.moved).toEqual(['RE']); + expect(result.layout.metrics!.added).toEqual([]); + }); + + it('a task on the main row moves what follows it there; the row below stays', async () => { + const { result, before, after } = await run(branches(), [{ op: 'add', kind: 'task', id: 'N', name: 'N', after: 'B' }]); + expect(at(after, 'C').x).toBeGreaterThan(at(before, 'C').x); + expect(at(after, 'E').x - at(after, 'C').x).toBe(at(before, 'E').x - at(before, 'C').x); + for (const id of ['R', 'RE']) expect(at(after, id), id).toEqual(at(before, id)); + expect(result.layout.metrics!.added).toEqual([]); + }); + + it('a node pushed over a line takes the line and its end along instead of crossing it', async () => { + // Q (row below) goes up right of the end event B to T on the main row; B moves past that line + const xml = drawnProcess( + ` + + ${sequenceFlow('F1', 'S', 'A')}${sequenceFlow('F2', 'A', 'B')}${sequenceFlow('F3', 'S2', 'Q')}${sequenceFlow('F4', 'Q', 'T')}${sequenceFlow('F5', 'T', 'TE')}`, + [ + shapeDi('S', 100, 182, 36, 36), + shapeDi('A', 196, 160, 100, 80), + shapeDi('B', 356, 182, 36, 36), + shapeDi('S2', 100, 322, 36, 36), + shapeDi('Q', 196, 300, 100, 80), + shapeDi('T', 900, 160, 100, 80), + shapeDi('TE', 1060, 182, 36, 36), + edgeDi('F1', [[136, 200], [196, 200]]), + edgeDi('F2', [[296, 200], [356, 200]]), + edgeDi('F3', [[136, 340], [196, 340]]), + edgeDi('F4', [[296, 340], [476, 340], [476, 120], [950, 120], [950, 160]]), + edgeDi('F5', [[1000, 200], [1060, 200]]), + ].join(''), + ); + const { result, before, after } = await run(xml, [{ op: 'add', kind: 'task', id: 'N', name: 'N', after: 'A' }]); + const dx = at(after, 'B').x - at(before, 'B').x; + expect(dx).toBeGreaterThan(120); + expect(at(after, 'T').x).toBe(at(before, 'T').x + dx); + for (const id of ['S2', 'Q']) expect(at(after, id), id).toEqual(at(before, id)); + expect(result.layout.metrics!.added.map((problem) => problem.kind)).not.toContain('crossings'); + }); +}); + +describe('a pushed shape drags the artifacts of what hangs on it', () => { + it('an annotation on the boundary event of a pushed task moves with the task', async () => { + // S -> A -> B -> E; the timer BT on B leads to BE, the annotation AN under B explains BT + const xml = drawnProcess( + ` + + deadline + ${sequenceFlow('F1', 'S', 'A')}${sequenceFlow('F2', 'A', 'B')}${sequenceFlow('F3', 'B', 'E')}${sequenceFlow('F4', 'BT', 'BE')}`, + [ + shapeDi('S', 100, 182, 36, 36), + shapeDi('A', 196, 160, 100, 80), + shapeDi('B', 356, 160, 100, 80), + shapeDi('E', 516, 182, 36, 36), + shapeDi('BT', 388, 222, 36, 36), + shapeDi('BE', 516, 322, 36, 36), + shapeDi('AN', 356, 300, 100, 30), + edgeDi('F1', [[136, 200], [196, 200]]), + edgeDi('F2', [[296, 200], [356, 200]]), + edgeDi('F3', [[456, 200], [516, 200]]), + edgeDi('F4', [[406, 258], [406, 340], [516, 340]]), + edgeDi('AS', [[400, 258], [400, 300]]), + ].join(''), + ); + const { result, before, after } = await run(xml, [{ op: 'add', kind: 'task', id: 'N', name: 'N', after: 'A' }]); + const dx = at(after, 'B').x - at(before, 'B').x; + expect(dx).toBeGreaterThan(0); + for (const id of ['BT', 'BE', 'AN']) expect(at(after, id), id).toEqual({ x: at(before, id).x + dx, y: at(before, id).y }); + expect(result.layout.metrics!.added).toEqual([]); + }); + + it('an annotation on a task inside a pushed sub-process moves with the sub-process', async () => { + // S -> A -> SP -> E; the expanded sub-process SP holds SS -> T -> SE, the annotation AN under SP explains T + const xml = drawnProcess( + ` + ${sequenceFlow('G1', 'SS', 'T')}${sequenceFlow('G2', 'T', 'SE')} + check twice + ${sequenceFlow('F1', 'S', 'A')}${sequenceFlow('F2', 'A', 'SP')}${sequenceFlow('F3', 'SP', 'E')}`, + [ + shapeDi('S', 100, 182, 36, 36), + shapeDi('A', 196, 160, 100, 80), + shapeDi('SP', 356, 100, 350, 200, ' isExpanded="true"'), + shapeDi('SS', 386, 182, 36, 36), + shapeDi('T', 482, 160, 100, 80), + shapeDi('SE', 642, 182, 36, 36), + shapeDi('E', 766, 182, 36, 36), + shapeDi('AN', 482, 340, 100, 30), + edgeDi('G1', [[422, 200], [482, 200]]), + edgeDi('G2', [[582, 200], [642, 200]]), + edgeDi('F1', [[136, 200], [196, 200]]), + edgeDi('F2', [[296, 200], [356, 200]]), + edgeDi('F3', [[706, 200], [766, 200]]), + edgeDi('AS', [[532, 240], [532, 340]]), + ].join(''), + ); + const { result, before, after } = await run(xml, [{ op: 'add', kind: 'task', id: 'N', name: 'N', after: 'A' }]); + const dx = at(after, 'SP').x - at(before, 'SP').x; + expect(dx).toBeGreaterThan(0); + for (const id of ['T', 'E', 'AN']) expect(at(after, id), id).toEqual({ x: at(before, id).x + dx, y: at(before, id).y }); + expect(result.layout.metrics!.added).toEqual([]); + }); +}); + +describe('a pool grows only as far as its content reaches', () => { + /** the branches model in a pool: the main row in lane Main, the row below in lane Rejection */ + const pooled = (): string => { + const lane = (id: string, refs: string[]): string => `${refs.map((ref) => `${ref}`).join('')}`; + const xml = branches() + .replace('', `\n${lane('Main', ['S', 'A', 'G', 'B', 'C', 'E'])}${lane('Rejection', ['R', 'RE'])}`) + .replace('bpmnElement="P">', `bpmnElement="Collab">${shapeDi('Pool', 40, 120, 828, 300)}${shapeDi('Main', 70, 120, 798, 140)}${shapeDi('Rejection', 70, 260, 798, 160)}`); + return xml.replace(/(bpmnElement="(?:Pool|Main|Rejection)")/g, '$1 isHorizontal="true"'); + }; + + it('a task on the shorter lane leaves the pool as it is', async () => { + const { result, before, after } = await run(pooled(), [{ op: 'add', kind: 'task', id: 'N', name: 'N', after: 'R' }]); + expect(at(after, 'RE').x).toBe(786); + for (const id of ['Pool', 'Main', 'Rejection']) expect(after.get(id)!.bounds, id).toEqual(before.get(id)!.bounds); + expect(result.layout.metrics!.added).toEqual([]); + }); + + it('a task on the row that reaches the border widens the pool and its lanes by the push, keeping the margin', async () => { + const { result, before, after } = await run(pooled(), [{ op: 'add', kind: 'task', id: 'N', name: 'N', after: 'C' }]); + const dx = at(after, 'E').x - at(before, 'E').x; + expect(dx).toBeGreaterThan(0); + for (const id of ['Pool', 'Main', 'Rejection']) expect(after.get(id)!.bounds.width, id).toBe(before.get(id)!.bounds.width + dx); + expect(result.layout.metrics!.added).toEqual([]); + }); +}); + +describe('a new flow into a drawn node left of its source pushes the target', () => { + it('connect: the target and what follows it move right of the source, the rest stays', async () => { + const { result, before, after } = await run(branches(), [{ op: 'connect', source: 'C', target: 'R', id: 'Flow_CToR' }]); + expect(at(after, 'R').x).toBeGreaterThanOrEqual(at(before, 'C').x + 100 + 40); + expect(at(after, 'RE').x - at(after, 'R').x).toBe(at(before, 'RE').x - at(before, 'R').x); + for (const id of ['S', 'A', 'G', 'B', 'C', 'E']) expect(at(after, id), id).toEqual(at(before, id)); + expect(result.layout.metrics!.added.map((problem) => problem.kind)).not.toContain('backwardFlow'); + }); + + it('connect: a loop return leaves its target where it is', async () => { + const { before, after } = await run(branches(), [{ op: 'connect', source: 'C', target: 'A', id: 'Flow_CToA' }]); + for (const id of ['S', 'A', 'G', 'B', 'C', 'E', 'R', 'RE']) expect(at(after, id), id).toEqual(at(before, id)); + }); + + it('a batch adding the source and the flow draws what the single commands draw', async () => { + const ops = [ + { op: 'add', kind: 'exclusiveGateway', id: 'Gateway_D', name: 'D?', after: 'B', before: 'C' }, + { op: 'connect', source: 'Gateway_D', target: 'R', id: 'Flow_DToR', name: 'no' }, + ]; + let single = branches(); + for (const op of ops) single = (await applyToXml(single, [op], { force: true })).xml; + const batched = (await applyToXml(branches(), ops, { force: true })).xml; + expect(shapeLines(batched)).toEqual(shapeLines(single)); + const after = shapes(await Doc.fromXml(batched)); + expect(after.get('R')!.bounds.x).toBeGreaterThan(after.get('Gateway_D')!.bounds.x + after.get('Gateway_D')!.bounds.width); + }); +}); diff --git a/test/domain/diagram/layout/incremental/repair-on-drawn-lines.test.ts b/test/domain/diagram/layout/incremental/repair-on-drawn-lines.test.ts new file mode 100644 index 00000000..fc0a444d --- /dev/null +++ b/test/domain/diagram/layout/incremental/repair-on-drawn-lines.test.ts @@ -0,0 +1,34 @@ +/** + * The crossing repair does not lay a hand-drawn line on top of the others to save a crossing + * (src/domain/diagram/layout/incremental/CrossingRepair.ts reroutes): run up a gateway's branches + * into a join, it would read as their continuation. The crossing stays instead. + */ +import { readFileSync } from 'node:fs'; +import { join } from 'node:path'; +import { describe, expect, it } from 'vitest'; +import { applyToXml } from '../../../../../src/api.js'; +import { diffProblems } from '../../../../../src/domain/diagram/utils/problems.js'; +import { LayoutAnalysis } from '../../../../../src/domain/diagram/di/LayoutAnalysis.js'; + +const ROOT = join(import.meta.dirname, '..', '..', '..', '..', '..'); + +describe('the crossing repair keeps a hand-drawn line off the others', () => { + it('Pack items keeps its line into the join instead of running up the branches of Charged? (public scenario)', async () => { + const xml = readFileSync(join(ROOT, 'tools', 'scenarios', 'controlflow__s16.bpmn'), 'utf8'); + // a gateway on the "no" branch of Charged? whose rejection path crosses the lines into the join + const ops = [ + { op: 'add', kind: 'userTask', id: 'Activity_Approve', name: 'Approve', flow: 'Flow_24' }, + { op: 'add', kind: 'exclusiveGateway', id: 'Gateway_Approved', name: 'Approved?', after: 'Activity_Approve', before: 'Activity_RetryCharge' }, + { op: 'add', kind: 'sendTask', id: 'Activity_Reject', name: 'Reject', after: 'Gateway_Approved', flowName: 'no' }, + { op: 'add', kind: 'endEvent', id: 'Event_Rejected', name: 'Rejected', after: 'Activity_Reject' }, + { op: 'add', kind: 'boundaryEvent:timer', id: 'Event_3DaysApproval', name: '3 days', on: 'Activity_Approve', timer: 'P3D', nonInterrupting: true }, + { op: 'add', kind: 'sendTask', id: 'Activity_Remind', name: 'Remind', after: 'Event_3DaysApproval' }, + { op: 'add', kind: 'endEvent', id: 'Event_Reminded', name: 'Reminded', after: 'Activity_Remind' }, + ]; + const applied = await applyToXml(xml, ops, { force: true }); + expect(applied.result.layout?.mode).toBe('incremental'); + expect(applied.result.layout?.rerouted ?? []).not.toContain('Flow_14'); + const { added } = diffProblems((await LayoutAnalysis.ofXml(xml)).problems, (await LayoutAnalysis.ofXml(applied.xml)).problems); + expect(added.filter((problem) => problem.kind === 'parallelRun')).toEqual([]); + }); +}); diff --git a/test/domain/diagram/layout/incremental/row-label-charge.test.ts b/test/domain/diagram/layout/incremental/row-label-charge.test.ts new file mode 100644 index 00000000..3377c859 --- /dev/null +++ b/test/domain/diagram/layout/incremental/row-label-charge.test.ts @@ -0,0 +1,40 @@ +/** + * The row choice of a branch or exception path charges a trial route through a label enough that a + * clean row wins (src/domain/diagram/layout/incremental/RowChooser.ts connectionCost, LABEL_HIT): + * a timer's path does not run down through the name of an end event next to its circle. + */ +import { readFileSync } from 'node:fs'; +import { join } from 'node:path'; +import { describe, expect, it } from 'vitest'; +import { applyToXml } from '../../../../../src/api.js'; +import { Doc } from '../../../../../src/domain/document/Doc.js'; +import { shapes } from '../../../../helpers/drawing.js'; + +const ROOT = join(import.meta.dirname, '..', '..', '..', '..', '..'); + +describe('a trial route through a label', () => { + it('a boundary path takes its host\'s top border instead of running through an end event\'s name (public scenario)', async () => { + const xml = readFileSync(join(ROOT, 'tools', 'scenarios', 'agent__o2c.e4.bpmn'), 'utf8'); + // a task on the first flow with a gateway and a rejection path, then a timer on it with a reminder + const ops = [ + { op: 'add', kind: 'userTask', id: 'Activity_Approve', name: 'Approval', flow: 'Flow_3' }, + { op: 'add', kind: 'exclusiveGateway', id: 'Gateway_Approved', name: 'Approved?', after: 'Activity_Approve', before: 'Activity_CheckCredit' }, + { op: 'add', kind: 'sendTask', id: 'Activity_Reject', name: 'Rejection', after: 'Gateway_Approved', flowName: 'no' }, + { op: 'add', kind: 'endEvent', id: 'Event_Rejected', name: 'Rejected', after: 'Activity_Reject' }, + { op: 'add', kind: 'boundaryEvent:timer', id: 'Event_Timer', name: '3 days', on: 'Activity_Approve', timer: 'P3D', nonInterrupting: true }, + { op: 'add', kind: 'sendTask', id: 'Activity_Remind', name: 'Reminder', after: 'Event_Timer' }, + { op: 'add', kind: 'endEvent', id: 'Event_Reminded', name: 'Reminded', after: 'Activity_Remind' }, + ]; + const applied = await applyToXml(xml, ops, { force: true }); + expect((applied.result.layout.metrics?.added ?? []).map((problem) => problem.kind)).not.toContain('labelOnLine'); + // the end event below the new task keeps its name where it was + const before = shapes(await Doc.fromXml(xml)); + const after = shapes(await Doc.fromXml(applied.xml)); + expect(after.get('Event_OrderRejected')!.label).toEqual(before.get('Event_OrderRejected')!.label); + // the timer sits on the top border and its reminder above the task + const host = after.get('Activity_Approve')!.bounds; + const remind = after.get('Activity_Remind')!.bounds; + expect(after.get('Event_Timer')!.bounds.y + 18).toBe(host.y); + expect(remind.y + remind.height).toBeLessThan(host.y); + }); +}); diff --git a/test/domain/diagram/layout/incremental/visual-judge-regressions.test.ts b/test/domain/diagram/layout/incremental/visual-judge-regressions.test.ts index d1435e36..a629d756 100644 --- a/test/domain/diagram/layout/incremental/visual-judge-regressions.test.ts +++ b/test/domain/diagram/layout/incremental/visual-judge-regressions.test.ts @@ -240,9 +240,9 @@ describe('judged edits on hand drawings', () => { // mostly before the insertion line: untouched with its content and inner flows for (const id of ['ESP', 'ES', 'ET', 'EE']) expect(shapesAfter.get(id)!.bounds, id).toEqual(shapesBefore.get(id)!.bounds); expect(edges(after).get('EF2')!.points).toEqual(edges(before).get('EF2')!.points); - // mostly beyond it: moved as a whole - for (const id of ['ESP2', 'BS', 'BT', 'BE']) expect(shapesAfter.get(id)!.bounds, id).toEqual({ ...shapesBefore.get(id)!.bounds, x: shapesBefore.get(id)!.bounds.x + dx }); - expect(edges(after).get('BF1')!.points).toEqual(edges(before).get('BF1')!.points.map((point) => ({ x: point.x + dx, y: point.y }))); + // mostly beyond it, but neither following the new node nor in the way of what moves: kept as a whole + for (const id of ['ESP2', 'BS', 'BT', 'BE']) expect(shapesAfter.get(id)!.bounds, id).toEqual(shapesBefore.get(id)!.bounds); + expect(edges(after).get('BF1')!.points).toEqual(edges(before).get('BF1')!.points); }); it('rename: a long name grows its task (wider, then higher), the boundary event stays on the border, what follows keeps its distance', async () => { diff --git a/test/helpers/drawing.ts b/test/helpers/drawing.ts index 4bec4d64..cc331ff5 100644 --- a/test/helpers/drawing.ts +++ b/test/helpers/drawing.ts @@ -15,3 +15,19 @@ export function edges(doc: Doc): Map { for (const plane of new DiagramReader(doc.definitions).planes()) for (const edge of plane.edges.values()) out.set(edge.id, edge); return out; } + +/** The namespaces of a BPMN file with a diagram. */ +export const DI_NS = 'xmlns:bpmn="http://www.omg.org/spec/BPMN/20100524/MODEL" xmlns:bpmndi="http://www.omg.org/spec/BPMN/20100524/DI" xmlns:dc="http://www.omg.org/spec/DD/20100524/DC" xmlns:di="http://www.omg.org/spec/DD/20100524/DI"'; + +/** The DI shape of `id` in a hand-made drawing; `extra`: more attributes (` isHorizontal="true"`). */ +export const shapeDi = (id: string, left: number, top: number, width: number, height: number, extra = ''): string => + ``; + +/** The DI edge of `id` through `pts`. */ +export const edgeDi = (id: string, pts: Array<[number, number]>): string => `${pts.map(([left, top]) => ``).join('')}`; + +export const sequenceFlow = (id: string, source: string, target: string): string => ``; + +/** A file with process P holding `process`, drawn by `di`. */ +export const drawnProcess = (process: string, di: string): string => + `\n\n${process}\n${di}\n`;