fix: comprehensive ElkDiagramRenderer cleanup and Y-offset fix
All checks were successful
CI / cleanup-branch (push) Has been skipped
CI / build (push) Successful in 55s
CI / docker (push) Successful in 38s
CI / deploy-feature (push) Has been skipped
CI / deploy (push) Successful in 35s

Based on thorough code review, fixes all identified issues:

1. **Y-offset root cause**: Added post-layout normalization that shifts
   all positioned nodes and edges so the bounding box starts at (0,0).
   ELK can place nodes at arbitrary positions within its root graph;
   normalizing compensates regardless of what ELK computes internally.

2. **Bounding box**: Compute from recursively flattened node tree +
   edge point bounds. Removes double-counting of compound children
   (children have absolute coords, not relative to parent).

3. **SVG double-drawing**: Compound children were drawn both inside
   drawCompoundContainer and again in the allNodes loop. Now collects
   compound child IDs and skips them in the second pass.

4. **findNode**: Now recurses into children for nested compound lookup.

5. **colorForType**: Removed redundant double-check on EIP_TYPES.

6. **Dead code removed**: routeNodeMap/indexNodeRecursive (populated but
   never read), MIN_NODE_WIDTH/CHAR_WIDTH/LABEL_PADDING (unused).

7. **Static initialization**: LayoutMetaDataProvider registration moved
   from constructor to static block (runs once, not per instance).

8. **Debug logging removed**: Removed diagnostic System.out.println.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
hsiegeln
2026-03-27 22:20:33 +01:00
parent 7926179ed9
commit 0df7735d20

View File

@@ -9,12 +9,12 @@ import com.cameleer3.server.core.diagram.DiagramRenderer;
import com.cameleer3.server.core.diagram.PositionedEdge;
import com.cameleer3.server.core.diagram.PositionedNode;
import org.eclipse.elk.alg.layered.options.LayeredMetaDataProvider;
import org.eclipse.elk.alg.layered.options.NodePlacementStrategy;
import org.eclipse.elk.core.RecursiveGraphLayoutEngine;
import org.eclipse.elk.core.options.CoreOptions;
import org.eclipse.elk.core.options.Direction;
import org.eclipse.elk.core.options.EdgeRouting;
import org.eclipse.elk.core.options.HierarchyHandling;
import org.eclipse.elk.alg.layered.options.NodePlacementStrategy;
import org.eclipse.elk.core.util.BasicProgressMonitor;
import org.eclipse.elk.graph.ElkBendPoint;
import org.eclipse.elk.graph.ElkEdge;
@@ -40,17 +40,20 @@ import java.util.Set;
/**
* ELK + JFreeSVG implementation of {@link DiagramRenderer}.
* <p>
* Uses Eclipse ELK layered algorithm for top-to-bottom layout computation
* Uses Eclipse ELK layered algorithm for layout computation
* and JFreeSVG for SVG document generation with color-coded nodes.
*/
public class ElkDiagramRenderer implements DiagramRenderer {
// Register ELK provider once (not per-instance)
static {
org.eclipse.elk.core.data.LayoutMetaDataService.getInstance()
.registerLayoutMetaDataProviders(new LayeredMetaDataProvider());
}
private static final int PADDING = 20;
private static final int NODE_HEIGHT = 40;
private static final int NODE_WIDTH = 160;
private static final int MIN_NODE_WIDTH = 80;
private static final int CHAR_WIDTH = 8;
private static final int LABEL_PADDING = 32;
private static final int COMPOUND_TOP_PADDING = 30;
private static final int COMPOUND_SIDE_PADDING = 10;
private static final int CORNER_RADIUS = 8;
@@ -108,20 +111,11 @@ public class ElkDiagramRenderer implements DiagramRenderer {
NodeType.ON_COMPLETION
);
/** Top-level handler types that are laid out in their own separate ELK graph
* to prevent them from affecting the main flow's node positioning. */
/** Top-level handler types laid out in their own separate ELK graph. */
private static final Set<NodeType> HANDLER_SECTION_TYPES = EnumSet.of(
NodeType.ON_EXCEPTION, NodeType.ERROR_HANDLER, NodeType.ON_COMPLETION
);
public ElkDiagramRenderer() {
// Ensure the layered algorithm meta data provider is registered.
// LayoutMetaDataService uses ServiceLoader, but explicit registration
// guarantees availability regardless of classpath ordering.
org.eclipse.elk.core.data.LayoutMetaDataService.getInstance()
.registerLayoutMetaDataProviders(new LayeredMetaDataProvider());
}
@Override
public String renderSvg(RouteGraph graph) {
LayoutResult result = computeLayout(graph, Direction.DOWN);
@@ -144,7 +138,17 @@ public class ElkDiagramRenderer implements DiagramRenderer {
Font labelFont = new Font("SansSerif", Font.PLAIN, 12);
g2.setFont(labelFont);
// Draw compound containers first (background)
// Collect IDs of nodes drawn inside compounds (to avoid double-drawing)
Set<String> compoundChildIds = new HashSet<>();
for (PositionedNode node : allNodes(layout.nodes())) {
if (result.compoundInfos.containsKey(node.id()) && node.children() != null) {
for (PositionedNode child : node.children()) {
collectAllIds(child, compoundChildIds);
}
}
}
// Draw compound containers first (background + children inside)
for (Map.Entry<String, CompoundInfo> entry : result.compoundInfos.entrySet()) {
CompoundInfo ci = entry.getValue();
PositionedNode pn = findNode(layout.nodes(), ci.nodeId);
@@ -153,8 +157,9 @@ public class ElkDiagramRenderer implements DiagramRenderer {
}
}
// Draw leaf nodes
// Draw leaf nodes (skip compounds and their children — already drawn above)
for (PositionedNode node : allNodes(layout.nodes())) {
if (compoundChildIds.contains(node.id())) continue;
if (!result.compoundInfos.containsKey(node.id()) || node.children().isEmpty()) {
drawNode(g2, node, result.nodeColors.getOrDefault(node.id(), PURPLE));
}
@@ -181,7 +186,7 @@ public class ElkDiagramRenderer implements DiagramRenderer {
private LayoutResult computeLayout(RouteGraph graph, Direction rootDirection) {
ElkGraphFactory factory = ElkGraphFactory.eINSTANCE;
// Create root node
// Create root node for main flow
ElkNode rootNode = factory.createElkNode();
rootNode.setIdentifier("root");
rootNode.setProperty(CoreOptions.ALGORITHM, "org.eclipse.elk.layered");
@@ -193,14 +198,6 @@ public class ElkDiagramRenderer implements DiagramRenderer {
rootNode.setProperty(org.eclipse.elk.alg.layered.options.LayeredOptions.NODE_PLACEMENT_STRATEGY,
NodePlacementStrategy.LINEAR_SEGMENTS);
// Build index of all RouteNodes (flat list from graph + recursive children)
Map<String, RouteNode> routeNodeMap = new HashMap<>();
if (graph.getNodes() != null) {
for (RouteNode rn : graph.getNodes()) {
indexNodeRecursive(rn, routeNodeMap);
}
}
// Track which nodes are children of a compound (at any depth)
Set<String> childNodeIds = new HashSet<>();
@@ -209,9 +206,7 @@ public class ElkDiagramRenderer implements DiagramRenderer {
Map<String, Color> nodeColors = new HashMap<>();
Set<String> compoundNodeIds = new HashSet<>();
// Separate handler sections (ON_EXCEPTION, ON_COMPLETION, ERROR_HANDLER)
// from main flow nodes. Handler sections are laid out in their own ELK
// graphs to prevent them from affecting the main flow's Y positioning.
// Separate handler sections from main flow nodes
List<RouteNode> mainNodes = new ArrayList<>();
List<RouteNode> handlerNodes = new ArrayList<>();
if (graph.getNodes() != null) {
@@ -233,9 +228,7 @@ public class ElkDiagramRenderer implements DiagramRenderer {
}
}
// Process handler sections into their OWN separate ELK root graphs.
// This prevents them from affecting the main flow's Y positioning.
// Each handler gets its own independent layout.
// Process handler sections into their OWN separate ELK root graphs
List<ElkNode> handlerRoots = new ArrayList<>();
for (RouteNode rn : handlerNodes) {
ElkNode handlerRoot = factory.createElkNode();
@@ -245,6 +238,7 @@ public class ElkDiagramRenderer implements DiagramRenderer {
handlerRoot.setProperty(CoreOptions.SPACING_NODE_NODE, NODE_SPACING * 0.5);
handlerRoot.setProperty(CoreOptions.SPACING_EDGE_NODE, EDGE_SPACING * 0.5);
handlerRoot.setProperty(CoreOptions.HIERARCHY_HANDLING, HierarchyHandling.INCLUDE_CHILDREN);
handlerRoot.setProperty(CoreOptions.EDGE_ROUTING, EdgeRouting.POLYLINE);
handlerRoot.setProperty(org.eclipse.elk.alg.layered.options.LayeredOptions.NODE_PLACEMENT_STRATEGY,
NodePlacementStrategy.LINEAR_SEGMENTS);
@@ -254,7 +248,6 @@ public class ElkDiagramRenderer implements DiagramRenderer {
}
// Create ELK edges — skip edges that cross between different ELK root graphs
// (e.g., main flow → handler section). These cannot be laid out by ELK.
if (graph.getEdges() != null) {
for (RouteEdge re : graph.getEdges()) {
ElkNode sourceElk = elkNodeMap.get(re.getSource());
@@ -270,9 +263,7 @@ public class ElkDiagramRenderer implements DiagramRenderer {
continue;
}
// Determine the containing node for the edge
ElkNode containingNode = findCommonParent(sourceElk, targetElk);
ElkEdge elkEdge = factory.createElkEdge();
elkEdge.setContainingNode(containingNode);
elkEdge.getSources().add(sourceElk);
@@ -280,46 +271,32 @@ public class ElkDiagramRenderer implements DiagramRenderer {
}
}
// Run layout — main flow
// Run layout
RecursiveGraphLayoutEngine engine = new RecursiveGraphLayoutEngine();
engine.layout(rootNode, new BasicProgressMonitor());
// Debug: log root dimensions and children
System.out.println("[ELK DEBUG] rootNode: " + rootNode.getWidth() + "x" + rootNode.getHeight()
+ " children=" + rootNode.getChildren().size());
for (ElkNode child : rootNode.getChildren()) {
System.out.println("[ELK DEBUG] child " + child.getIdentifier()
+ " x=" + child.getX() + " y=" + child.getY()
+ " w=" + child.getWidth() + " h=" + child.getHeight());
}
// Run layout — each handler section independently
for (ElkNode handlerRoot : handlerRoots) {
engine.layout(handlerRoot, new BasicProgressMonitor());
}
// Extract results — only top-level nodes (children collected recursively)
// Extract positioned nodes
List<PositionedNode> positionedNodes = new ArrayList<>();
Map<String, CompoundInfo> compoundInfos = new HashMap<>();
if (graph.getNodes() != null) {
for (RouteNode rn : graph.getNodes()) {
if (childNodeIds.contains(rn.getId())) {
// Skip — collected under its parent compound
continue;
}
ElkNode elkNode = elkNodeMap.get(rn.getId());
if (elkNode == null) continue;
// Use the correct root for coordinate calculation:
// handler nodes use their handler root, main flow uses rootNode
ElkNode coordRoot = getElkRoot(elkNode);
positionedNodes.add(extractPositionedNode(rn, elkNode, elkNodeMap,
compoundNodeIds, compoundInfos, coordRoot));
}
}
// Extract edges from main root + all handler roots
// Extract edges from main root + handler roots
List<PositionedEdge> positionedEdges = new ArrayList<>();
List<ElkEdge> allEdges = collectAllEdges(rootNode);
for (ElkNode hr : handlerRoots) {
@@ -329,7 +306,6 @@ public class ElkDiagramRenderer implements DiagramRenderer {
String sourceId = elkEdge.getSources().isEmpty() ? "" : elkEdge.getSources().get(0).getIdentifier();
String targetId = elkEdge.getTargets().isEmpty() ? "" : elkEdge.getTargets().get(0).getIdentifier();
// Determine which root this edge belongs to for coordinate calculation
ElkNode edgeRoot = getElkRoot(elkEdge.getContainingNode());
List<double[]> points = new ArrayList<>();
@@ -350,7 +326,6 @@ public class ElkDiagramRenderer implements DiagramRenderer {
});
}
// Find label from original edge
String label = "";
if (graph.getEdges() != null) {
for (RouteEdge re : graph.getEdges()) {
@@ -364,23 +339,24 @@ public class ElkDiagramRenderer implements DiagramRenderer {
positionedEdges.add(new PositionedEdge(sourceId, targetId, label, points));
}
// Compute bounding box from actual positioned node coordinates
// (not rootNode dimensions, which ignore handler roots)
// Normalize: shift all nodes and edges so bounding box starts at (0, 0).
// ELK can place nodes at arbitrary positions within the root graph;
// normalizing ensures the output starts at the origin regardless.
normalizePositions(positionedNodes, positionedEdges);
// Compute bounding box from all positioned nodes and edges
double totalWidth = 0;
double totalHeight = 0;
for (PositionedNode pn : positionedNodes) {
for (PositionedNode pn : allNodes(positionedNodes)) {
double right = pn.x() + pn.width();
double bottom = pn.y() + pn.height();
if (right > totalWidth) totalWidth = right;
if (bottom > totalHeight) totalHeight = bottom;
// Also check children bounds for compounds
if (pn.children() != null) {
for (PositionedNode child : pn.children()) {
double cr = pn.x() + child.x() + child.width();
double cb = pn.y() + child.y() + child.height();
if (cr > totalWidth) totalWidth = cr;
if (cb > totalHeight) totalHeight = cb;
}
}
for (PositionedEdge pe : positionedEdges) {
for (double[] pt : pe.points()) {
if (pt[0] > totalWidth) totalWidth = pt[0];
if (pt[1] > totalHeight) totalHeight = pt[1];
}
}
@@ -388,6 +364,61 @@ public class ElkDiagramRenderer implements DiagramRenderer {
return new LayoutResult(layout, nodeColors, compoundInfos);
}
/**
* Shift all positioned nodes and edge points so the bounding box starts at (0, 0).
* This compensates for ELK placing nodes at non-zero origins within its root graph.
*/
private void normalizePositions(List<PositionedNode> nodes, List<PositionedEdge> edges) {
// Find minimum x/y across all nodes (recursively including children)
double minX = Double.MAX_VALUE;
double minY = Double.MAX_VALUE;
for (PositionedNode pn : allNodes(nodes)) {
if (pn.x() < minX) minX = pn.x();
if (pn.y() < minY) minY = pn.y();
}
for (PositionedEdge pe : edges) {
for (double[] pt : pe.points()) {
if (pt[0] < minX) minX = pt[0];
if (pt[1] < minY) minY = pt[1];
}
}
if (minX <= 0 && minY <= 0) return; // Already at or past origin
// Shift nodes — PositionedNode is a record, so we must rebuild
double shiftX = minX > 0 ? minX : 0;
double shiftY = minY > 0 ? minY : 0;
for (int i = 0; i < nodes.size(); i++) {
nodes.set(i, shiftNode(nodes.get(i), shiftX, shiftY));
}
// Shift edge points in-place
for (PositionedEdge pe : edges) {
for (double[] pt : pe.points()) {
pt[0] -= shiftX;
pt[1] -= shiftY;
}
}
}
/** Recursively shift a PositionedNode and its children by (dx, dy). */
private PositionedNode shiftNode(PositionedNode pn, double dx, double dy) {
List<PositionedNode> shiftedChildren = List.of();
if (pn.children() != null && !pn.children().isEmpty()) {
shiftedChildren = new ArrayList<>();
for (PositionedNode child : pn.children()) {
shiftedChildren.add(shiftNode(child, dx, dy));
}
}
return new PositionedNode(
pn.id(), pn.label(), pn.type(),
pn.x() - dx, pn.y() - dy,
pn.width(), pn.height(),
shiftedChildren
);
}
// ----------------------------------------------------------------
// SVG drawing helpers
// ----------------------------------------------------------------
@@ -410,7 +441,7 @@ public class ElkDiagramRenderer implements DiagramRenderer {
private void drawCompoundContainer(SVGGraphics2D g2, PositionedNode node, Color color) {
// Semi-transparent background
Color bg = new Color(color.getRed(), color.getGreen(), color.getBlue(), 38); // ~15% alpha
Color bg = new Color(color.getRed(), color.getGreen(), color.getBlue(), 38);
g2.setColor(bg);
g2.fill(new RoundRectangle2D.Double(
node.x(), node.y(), node.width(), node.height(),
@@ -425,7 +456,6 @@ public class ElkDiagramRenderer implements DiagramRenderer {
// Label at top
g2.setColor(color);
FontMetrics fm = g2.getFontMetrics();
float labelX = (float) (node.x() + COMPOUND_SIDE_PADDING);
float labelY = (float) (node.y() + 18);
g2.drawString(node.label(), labelX, labelY);
@@ -479,7 +509,7 @@ public class ElkDiagramRenderer implements DiagramRenderer {
if (ENDPOINT_TYPES.contains(type)) return BLUE;
if (PROCESSOR_TYPES.contains(type)) return GREEN;
if (ERROR_TYPES.contains(type)) return RED;
if (EIP_TYPES.contains(type)) return EIP_TYPES.contains(type) ? PURPLE : PURPLE;
if (EIP_TYPES.contains(type)) return PURPLE;
if (CROSS_ROUTE_TYPES.contains(type)) return CYAN;
return PURPLE;
}
@@ -497,16 +527,6 @@ public class ElkDiagramRenderer implements DiagramRenderer {
// Recursive node building
// ----------------------------------------------------------------
/** Index a RouteNode and all its descendants into the map. */
private void indexNodeRecursive(RouteNode node, Map<String, RouteNode> map) {
map.put(node.getId(), node);
if (node.getChildren() != null) {
for (RouteNode child : node.getChildren()) {
indexNodeRecursive(child, map);
}
}
}
/**
* Recursively create ELK nodes. Compound nodes become ELK containers
* with their children nested inside. Non-compound nodes become leaf nodes.
@@ -535,7 +555,6 @@ public class ElkDiagramRenderer implements DiagramRenderer {
new org.eclipse.elk.core.math.ElkPadding(COMPOUND_TOP_PADDING,
COMPOUND_SIDE_PADDING, COMPOUND_SIDE_PADDING, COMPOUND_SIDE_PADDING));
// Recursively create children inside this compound
for (RouteNode child : rn.getChildren()) {
childNodeIds.add(child.getId());
createElkNodeRecursive(child, elkNode, factory, elkNodeMap, nodeColors,
@@ -552,7 +571,7 @@ public class ElkDiagramRenderer implements DiagramRenderer {
/**
* Recursively extract a PositionedNode from the ELK layout result.
* Compound nodes include their children with absolute coordinates.
* All coordinates are absolute (relative to the ELK root).
*/
private PositionedNode extractPositionedNode(
RouteNode rn, ElkNode elkNode, Map<String, ElkNode> elkNodeMap,
@@ -600,14 +619,12 @@ public class ElkDiagramRenderer implements DiagramRenderer {
/** Proper lowest common ancestor of two ELK nodes. */
private ElkNode findCommonParent(ElkNode a, ElkNode b) {
// Collect all ancestors of 'a' (including a itself)
Set<ElkNode> ancestorsOfA = new HashSet<>();
ElkNode current = a;
while (current != null) {
ancestorsOfA.add(current);
current = current.getParent();
}
// Walk up from 'b' until we find a common ancestor
current = b;
while (current != null) {
if (ancestorsOfA.contains(current)) {
@@ -615,7 +632,6 @@ public class ElkDiagramRenderer implements DiagramRenderer {
}
current = current.getParent();
}
// Fallback: root of 'a'
return getElkRoot(a);
}
@@ -647,13 +663,19 @@ public class ElkDiagramRenderer implements DiagramRenderer {
return edges;
}
/** Recursively find a PositionedNode by ID. */
private PositionedNode findNode(List<PositionedNode> nodes, String id) {
for (PositionedNode n : nodes) {
if (n.id().equals(id)) return n;
if (n.children() != null) {
PositionedNode found = findNode(n.children(), id);
if (found != null) return found;
}
}
return null;
}
/** Recursively flatten a PositionedNode tree. */
private List<PositionedNode> allNodes(List<PositionedNode> nodes) {
List<PositionedNode> all = new ArrayList<>();
for (PositionedNode n : nodes) {
@@ -665,6 +687,16 @@ public class ElkDiagramRenderer implements DiagramRenderer {
return all;
}
/** Recursively collect all node IDs from a tree. */
private void collectAllIds(PositionedNode node, Set<String> ids) {
ids.add(node.id());
if (node.children() != null) {
for (PositionedNode child : node.children()) {
collectAllIds(child, ids);
}
}
}
// ----------------------------------------------------------------
// Internal data classes
// ----------------------------------------------------------------