From e9ee138279e257fd1bd3ee313c3596bd40a3e250 Mon Sep 17 00:00:00 2001 From: Iliyan Velichkov Date: Wed, 15 Jul 2026 16:24:18 +0300 Subject: [PATCH] fix(intent): lay out generated BPMN/EDM diagrams so they read cleanly The intent generators emitted diagram interchange (bpmndi for .bpmn, mxGraphModel for .edm) with naive placement, so branching processes and larger entity models opened badly formatted and had to be reordered by hand. BPMN (BpmnIntentGenerator): replace the single-line, declaration-order placement with a deterministic layered (Sugiyama-style) layout. A node's column (X) is its longest-path distance from the start event, so a gateway's then/else branches share a column and the flow reads strictly left-to-right; its lane (Y) is assigned per column by the barycentre of its predecessors' lanes and spread symmetrically around a centre lane, so branches fan out above and below instead of piling onto the two fixed lanes (140/300) the old code used. Edges are routed orthogonally (right-angle L/Z) when their endpoints are on different lanes, straight when aligned. The obsolete secondaryBranchTargets single-drop heuristic is removed. EDM (EdmIntentGenerator.appendMxGraphModel): place entities in a relationship-aware order (breadth-first over the FK graph so connected entities cluster) and pack each into the currently shortest column instead of a blind index % 3, so columns stay balanced regardless of card height and FK-linked entities land near each other. Cells are still emitted in declaration order, keeping regeneration diffs stable. Both layouts are pure functions of the model, so output stays byte-stable; the modeler re-routes on first manual edit. Existing engine-intent unit tests and IntentEngineIT/IntentEmissionCoverageIT assertions key on element/flow presence, not coordinates, so they are unaffected. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../generator/bpmn/BpmnIntentGenerator.java | 235 +++++++++++------- .../generator/edm/EdmIntentGenerator.java | 109 +++++++- 2 files changed, 250 insertions(+), 94 deletions(-) diff --git a/components/engine/engine-intent/src/main/java/org/eclipse/dirigible/components/intent/generator/bpmn/BpmnIntentGenerator.java b/components/engine/engine-intent/src/main/java/org/eclipse/dirigible/components/intent/generator/bpmn/BpmnIntentGenerator.java index fc5686d3846..8fc3237a1b7 100644 --- a/components/engine/engine-intent/src/main/java/org/eclipse/dirigible/components/intent/generator/bpmn/BpmnIntentGenerator.java +++ b/components/engine/engine-intent/src/main/java/org/eclipse/dirigible/components/intent/generator/bpmn/BpmnIntentGenerator.java @@ -9,12 +9,11 @@ */ package org.eclipse.dirigible.components.intent.generator.bpmn; -import java.util.ArrayDeque; import java.util.ArrayList; -import java.util.Deque; import java.util.HashMap; import java.util.HashSet; import java.util.LinkedHashMap; +import java.util.LinkedHashSet; import java.util.List; import java.util.Locale; import java.util.Map; @@ -67,8 +66,10 @@ *

* The {@code bpmndi:BPMNDiagram} block IS emitted (see {@link #appendBpmnDiagram}): the * Flowable/Oryx modeler renders the canvas only from the diagram interchange, so a process with no - * shapes opens empty. Nodes are laid out left-to-right on a fixed lane for deterministic, - * byte-stable output; the modeler re-routes on first manual edit. + * shapes opens empty. Nodes are laid out with a deterministic layered (Sugiyama-style) placement - + * column by longest-path distance from the start event, lane by predecessor barycentre - with + * orthogonally-routed edges, so branching processes read cleanly without manual reordering; the + * output stays byte-stable and the modeler re-routes on first manual edit. * *

* {@code flowable:formKey} is the form page URL. The Inbox / Process perspective opens a @@ -771,42 +772,34 @@ private static StepIntent decisionOf(String stepId, List steps) { // ----- BPMN diagram interchange (bpmndi) --------------------------------------------------- - private static final int LANE_Y = 140; - /** - * The lane a decision's secondary (default / {@code else}) branch target sits on, dropped below the - * main lane so the gateway's two outgoing flows diverge visibly instead of overlapping on one line. - */ - private static final int SECONDARY_LANE_Y = 300; - private static final int NODE_SPACING = 160; + /** X of the first (start) column's centre. */ private static final int FIRST_NODE_CENTER_X = 100; + /** Horizontal distance between the centres of adjacent rank columns (task width 100 + gap). */ + private static final int COLUMN_PITCH = 180; + /** Y of the centre lane (lane offset 0); branches fan out above and below it. */ + private static final int CENTER_Y = 260; + /** Vertical distance between adjacent lanes. */ + private static final int LANE_HEIGHT = 130; /** * Append the {@code bpmndi:BPMNDiagram} block. The Flowable/Oryx modeler renders the canvas * only from this diagram interchange (a process with no {@code BPMNShape}s opens empty), so - * it is mandatory. Nodes are laid out left-to-right along the linear chain at a fixed lane, except - * a decision's secondary ({@code else}) branch target, which drops to a lower lane so the gateway's - * two outgoing flows are visibly distinct rather than overlapping on one line; edges connect the - * right edge of the source to the left edge of the target. The layout is deterministic so - * re-generation is byte-stable; the modeler re-routes on first manual edit. + * it is mandatory. + * + *

+ * The layout is a deterministic layered (Sugiyama-style) placement rather than a single + * line: each node's column (X) is its longest-path distance from the start event, so + * parallel branches of a gateway share a column and the flow always reads left-to-right; each + * node's lane (Y) is assigned per column by the barycentre of its predecessors' lanes and + * centred on {@link #CENTER_Y}, so branches fan out above and below the main line instead of piling + * onto one of two fixed lanes. Edges are routed orthogonally (an L/Z of right-angle + * segments) when the endpoints are on different lanes, and as a straight segment when they line up + * - the same shape a modeler draws by hand. The whole computation is a pure function of the step + * graph, so re-generation stays byte-stable; the modeler re-routes on first manual edit. */ private static void appendBpmnDiagram(StringBuilder sb, String processId, List effectiveIds, List flows, List steps) { - // A decision's secondary branch target (the default / `else` flow's target) is dropped to a lower - // lane so the gateway's two outgoing flows are visibly distinct instead of running along one line. - Set secondaryNodes = secondaryBranchTargets(flows); - Map bounds = new java.util.LinkedHashMap<>(); - for (int i = 0; i < effectiveIds.size(); i++) { - String id = effectiveIds.get(i); - if (bounds.containsKey(id)) { - continue; - } - int[] size = nodeSize(id, steps); - int centerX = FIRST_NODE_CENTER_X + i * NODE_SPACING; - int laneY = secondaryNodes.contains(id) ? SECONDARY_LANE_Y : LANE_Y; - int x = centerX - size[0] / 2; - int y = laneY - size[1] / 2; - bounds.put(id, new int[] {x, y, size[0], size[1]}); - } + Map bounds = layout(effectiveIds, flows, steps); sb.append(" \n \n \n \n"); + .append("\">\n"); + for (int[] point : edgeWaypoints(source, target)) { + sb.append(" \n"); + } + sb.append(" \n"); } sb.append(" \n"); sb.append(" \n"); } /** - * The nodes of each decision's default ({@code else}) branch - its "second option" - placed on the - * lower lane so the gateway's flows diverge and a later main-lane edge (e.g. a {@code next: end} - * jump) does not visually cross them. Starts from each default-flow target and walks the branch - * forward along the sequence flows, collecting every node until it reaches {@code end} or rejoins - * the main path (a node entered by a conditioned {@code then} flow). The end event is never - * dropped. - *

- * Without the full walk, only the immediate target dropped: a multi-node reject branch (e.g. - * {@code else -> cancel} followed by more steps) left those steps on the main lane between - * {@code send} and {@code end}, so the {@code send -> end} edge ran straight through them and - * looked like {@code send -> cancel}. + * Compute the {@code [x, y, width, height]} bounds of every node with a layered layout: column by + * longest-path rank from the start event, lane by predecessor-barycentre within the column. The + * returned map preserves {@code effectiveIds} order so the emitted shapes are byte-stable. */ - private static Set secondaryBranchTargets(List flows) { - Map> adjacency = new HashMap<>(); - Set thenTargets = new HashSet<>(); - Deque frontier = new ArrayDeque<>(); + private static Map layout(List effectiveIds, List flows, List steps) { + // Forward adjacency and predecessor lists, restricted to nodes that actually have a shape. + Set nodes = new LinkedHashSet<>(effectiveIds); + Map> predecessors = new LinkedHashMap<>(); + for (String id : effectiveIds) { + predecessors.put(id, new ArrayList<>()); + } for (SequenceFlow flow : flows) { - adjacency.computeIfAbsent(flow.source(), k -> new ArrayList<>()) - .add(flow.target()); - if (flow.id() == null) { - continue; + if (nodes.contains(flow.source()) && nodes.contains(flow.target()) && !flow.source() + .equals(flow.target())) { + predecessors.get(flow.target()) + .add(flow.source()); + } + } + + Map rank = rankByLongestPath(effectiveIds, flows, nodes); + // Group nodes by rank in a stable (effectiveIds) order, then compress ranks to contiguous + // columns so an empty rank leaves no visual gap. + Map> byRank = new java.util.TreeMap<>(); + for (String id : effectiveIds) { + byRank.computeIfAbsent(rank.get(id), k -> new ArrayList<>()) + .add(id); + } + Map columnOf = new HashMap<>(); + int column = 0; + for (Integer r : byRank.keySet()) { + columnOf.put(r, column++); + } + + // Lane assignment, one column at a time in rank order: sort a column's nodes by the average + // lane of their already-placed predecessors (barycentre - the classic crossing-reduction + // heuristic), then spread them symmetrically around the centre lane (offset 0). + Map laneOf = new HashMap<>(); + for (List columnNodes : byRank.values()) { + columnNodes.sort(java.util.Comparator.comparingDouble(id -> barycentre(id, predecessors, laneOf)) + .thenComparingInt(effectiveIds::indexOf)); + double first = -(columnNodes.size() - 1) / 2.0; + for (int i = 0; i < columnNodes.size(); i++) { + laneOf.put(columnNodes.get(i), first + i); } - if (flow.id() - .endsWith("_then")) { - thenTargets.add(flow.target()); - } else if (flow.id() - .endsWith("_default") - && !END_ID.equals(flow.target())) { - frontier.add(flow.target()); - } - } - // Forward walk of the else branch(es). Stop at the end event and where the branch rejoins the - // main path (a `then` target), so a shared convergence/end node stays on the main lane. - Set secondary = new HashSet<>(); - while (!frontier.isEmpty()) { - String node = frontier.poll(); - if (node == null || END_ID.equals(node) || thenTargets.contains(node) || !secondary.add(node)) { + } + + Map bounds = new LinkedHashMap<>(); + for (String id : effectiveIds) { + if (bounds.containsKey(id)) { continue; } - for (String next : adjacency.getOrDefault(node, List.of())) { - if (!END_ID.equals(next) && !thenTargets.contains(next)) { - frontier.add(next); + int[] size = nodeSize(id, steps); + int centerX = FIRST_NODE_CENTER_X + columnOf.get(rank.get(id)) * COLUMN_PITCH; + int centerY = CENTER_Y + (int) Math.round(laneOf.get(id) * LANE_HEIGHT); + bounds.put(id, new int[] {centerX - size[0] / 2, centerY - size[1] / 2, size[0], size[1]}); + } + return bounds; + } + + /** + * Longest-path rank of each node from {@link #START_ID}, computed by Bellman-Ford-style relaxation + * (bounded to {@code |nodes|} passes so a stray back edge cannot loop forever). The end event is + * pinned to the deepest rank so nothing sits to its right. + */ + private static Map rankByLongestPath(List effectiveIds, List flows, Set nodes) { + Map rank = new HashMap<>(); + for (String id : effectiveIds) { + rank.put(id, 0); + } + for (int pass = 0; pass < nodes.size(); pass++) { + boolean changed = false; + for (SequenceFlow flow : flows) { + Integer source = rank.get(flow.source()); + Integer target = rank.get(flow.target()); + if (source == null || target == null || flow.source() + .equals(flow.target())) { + continue; + } + if (target < source + 1) { + rank.put(flow.target(), source + 1); + changed = true; } } + if (!changed) { + break; + } + } + int deepest = rank.values() + .stream() + .mapToInt(Integer::intValue) + .max() + .orElse(0); + rank.put(END_ID, deepest); + return rank; + } + + /** Average lane of a node's already-placed predecessors, or {@code 0} when none are placed yet. */ + private static double barycentre(String id, Map> predecessors, Map laneOf) { + double sum = 0; + int count = 0; + for (String predecessor : predecessors.getOrDefault(id, List.of())) { + Double lane = laneOf.get(predecessor); + if (lane != null) { + sum += lane; + count++; + } + } + return count == 0 ? 0 : sum / count; + } + + /** + * Waypoints for an edge from the {@code source} bounds to the {@code target} bounds: a straight + * right-to-left segment when the two shapes share a lane, otherwise an orthogonal L/Z stepping out + * of the source's right side, across to the midpoint column, up or down to the target's lane, and + * into the target's left side. + */ + private static List edgeWaypoints(int[] source, int[] target) { + int x1 = source[0] + source[2]; + int y1 = source[1] + source[3] / 2; + int x2 = target[0]; + int y2 = target[1] + target[3] / 2; + if (y1 == y2) { + return List.of(new int[] {x1, y1}, new int[] {x2, y2}); } - return secondary; + int midX = (x1 + x2) / 2; + return List.of(new int[] {x1, y1}, new int[] {midX, y1}, new int[] {midX, y2}, new int[] {x2, y2}); } /** Width/height of a node's shape by its element id / step kind. */ diff --git a/components/engine/engine-intent/src/main/java/org/eclipse/dirigible/components/intent/generator/edm/EdmIntentGenerator.java b/components/engine/engine-intent/src/main/java/org/eclipse/dirigible/components/intent/generator/edm/EdmIntentGenerator.java index c4746e56f28..89025bb2d4b 100644 --- a/components/engine/engine-intent/src/main/java/org/eclipse/dirigible/components/intent/generator/edm/EdmIntentGenerator.java +++ b/components/engine/engine-intent/src/main/java/org/eclipse/dirigible/components/intent/generator/edm/EdmIntentGenerator.java @@ -19,6 +19,7 @@ import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.Queue; import java.util.Set; import org.eclipse.dirigible.components.base.helpers.JsonHelper; @@ -1745,24 +1746,22 @@ private static void appendMxGraphModel(StringBuilder sb, EdmDocument document, L propertyCellId.put(name, props); } + // Placement pass: pick an entity order that clusters FK-related entities, then pack each into + // the currently shortest column. Emission below still walks `entities` in declaration order (so + // the XML element order stays stable across regenerations); positions come from this map. + Map placement = placeEntities(entities, document); + sb.append(" \n \n"); sb.append(" \n"); sb.append(" \n"); - int[] columnY = new int[GRID_COLUMNS]; - for (int i = 0; i < GRID_COLUMNS; i++) { - columnY[i] = GRID_ORIGIN; - } - int index = 0; for (Map entity : entities) { String name = (String) entity.get("name"); List> properties = propertiesOf(entity); - int column = index % GRID_COLUMNS; - int x = GRID_ORIGIN + column * (CELL_WIDTH + COLUMN_GAP); - int y = columnY[column]; - int height = TITLE_HEIGHT + ROW_HEIGHT * Math.max(properties.size(), 1); - columnY[column] = y + height + ROW_GAP; - index++; + int[] pos = placement.get(name); + int x = pos[0]; + int y = pos[1]; + int height = pos[2]; sb.append(" \n \n"); } + /** + * Compute the {@code [x, y, height]} grid position of every entity. Entities are visited in a + * relationship-aware order - a breadth-first walk over the (undirected) FK graph seeded in + * declaration order - so connected entities are laid out consecutively and land near each other; + * each is then packed into the currently shortest of the {@link #GRID_COLUMNS} columns so the + * columns stay balanced regardless of how tall individual cards are. Deterministic: the seed order + * and the shortest-column tie-break (lowest index) are both stable. + */ + private static Map placeEntities(List> entities, EdmDocument document) { + Map>> byName = new LinkedHashMap<>(); + for (Map entity : entities) { + byName.put((String) entity.get("name"), propertiesOf(entity)); + } + List order = relationshipOrder(byName.keySet(), document); + + int[] columnBottom = new int[GRID_COLUMNS]; + for (int i = 0; i < GRID_COLUMNS; i++) { + columnBottom[i] = GRID_ORIGIN; + } + Map placement = new HashMap<>(); + for (String name : order) { + int column = shortestColumn(columnBottom); + int x = GRID_ORIGIN + column * (CELL_WIDTH + COLUMN_GAP); + int y = columnBottom[column]; + int height = TITLE_HEIGHT + ROW_HEIGHT * Math.max(byName.get(name) + .size(), + 1); + columnBottom[column] = y + height + ROW_GAP; + placement.put(name, new int[] {x, y, height}); + } + return placement; + } + + /** + * A breadth-first ordering of the entities over the undirected FK graph, seeded in declaration + * order, so each connected cluster of entities comes out contiguously (and any entity with no + * relations still appears, in declaration order). + */ + private static List relationshipOrder(Set names, EdmDocument document) { + Map> adjacency = new LinkedHashMap<>(); + for (String name : names) { + adjacency.put(name, new ArrayList<>()); + } + for (Map.Entry>> entry : document.relationsByEntity.entrySet()) { + String owner = entry.getKey(); + for (Map relation : entry.getValue()) { + String referenced = (String) relation.get("referenced"); + if (adjacency.containsKey(owner) && adjacency.containsKey(referenced)) { + adjacency.get(owner) + .add(referenced); + adjacency.get(referenced) + .add(owner); + } + } + } + List order = new ArrayList<>(names.size()); + Set seen = new HashSet<>(); + for (String seed : names) { + if (seen.contains(seed)) { + continue; + } + Queue queue = new java.util.ArrayDeque<>(); + queue.add(seed); + seen.add(seed); + while (!queue.isEmpty()) { + String current = queue.poll(); + order.add(current); + for (String neighbour : adjacency.get(current)) { + if (seen.add(neighbour)) { + queue.add(neighbour); + } + } + } + } + return order; + } + + /** Index of the column whose stacked cards currently reach the smallest Y (ties: lowest index). */ + private static int shortestColumn(int[] columnBottom) { + int best = 0; + for (int i = 1; i < columnBottom.length; i++) { + if (columnBottom[i] < columnBottom[best]) { + best = i; + } + } + return best; + } + @SuppressWarnings("unchecked") private static List> propertiesOf(Map entity) { return (List>) entity.getOrDefault("properties", List.of());