[PR] [WIP] remove 'spacing.labelNode', add 'nodeLabels.margin'

plugins/org.eclipse.elk.alg.common/src/org/eclipse/elk/alg/common/nodespacing/NodeMarginCalculator.java

@@ -15,6 +15,7 @@
 import org.eclipse.elk.core.options.CoreOptions;
 import org.eclipse.elk.core.options.EdgeLabelPlacement;
 import org.eclipse.elk.core.options.PortLabelPlacement;
+import org.eclipse.elk.core.util.IndividualSpacings;
 import org.eclipse.elk.core.util.adapters.GraphAdapters.EdgeAdapter;
 import org.eclipse.elk.core.util.adapters.GraphAdapters.GraphAdapter;
 import org.eclipse.elk.core.util.adapters.GraphAdapters.LabelAdapter;
@@ -109,30 +110,20 @@ public NodeMarginCalculator excludeEdgeHeadTailLabels() {
      * Calculates and assigns margins to all nodes.
      */
     public void process() {
-        double spacing = adapter.getProperty(CoreOptions.SPACING_LABEL_NODE);
-
         // Iterate through all nodes
         for (NodeAdapter<?> node : adapter.getNodes()) {
-            processNode(node, spacing);
+            processNode(node);
         }
     }
-    
+
     /**
-     * Calculates and assigns margins to the given node. The node is expected to be part of the graph the calculator
-     * was created for.
+     * Calculates and assigns margins to the given node. The node is expected to be part of the graph the calculator was
+     * created for.
      */
     public void processNode(final NodeAdapter<?> node) {
-        double spacing = adapter.getProperty(CoreOptions.SPACING_LABEL_NODE);
-        processNode(node, spacing);
-    }
 
-    /**
-     * Calculates the margin of the given node.
-     * 
-     * @param node the node whose margin to calculate.
-     * @param labelSpacing label spacing set on the layered graph.
-     */
-    private void processNode(final NodeAdapter<?> node, final double labelSpacing) {
+        ElkMargin desiredNodeMargin = IndividualSpacings.getIndividualOrInherited(node, CoreOptions.NODE_LABELS_MARGIN);
+
         // This will be our bounding box. We'll start with one that's the same size
         // as our node, and at the same position.
         ElkRectangle boundingBox = new ElkRectangle(
@@ -185,29 +176,33 @@ private void processNode(final NodeAdapter<?> node, final double labelSpacing) {
             }
 
             // End labels of edges connected to the port
+            // TODO cds: layered calls #excludeEdgeHeadTailLabels, is this ever active?
             if (includeEdgeHeadTailLabels) {
-                KVector requiredPortLabelSpace = new KVector(-labelSpacing, -labelSpacing);
-
-                // TODO: maybe leave space for manually placed ports 
+                KVector requiredPortLabelSpace = new KVector(-desiredNodeMargin.left, -desiredNodeMargin.top);
+
+                // TODO cds: it's not margin but spacing between pairs of labels, right?
+                double labelLabelSpacing =
+                        IndividualSpacings.getIndividualOrInherited(node, CoreOptions.SPACING_LABEL_LABEL);
+                // TODO: maybe leave space for manually placed ports
                 if (node.getProperty(CoreOptions.PORT_LABELS_PLACEMENT) == PortLabelPlacement.OUTSIDE) {
                     for (LabelAdapter<?> label : port.getLabels()) {
-                        requiredPortLabelSpace.x += label.getSize().x + labelSpacing;
-                        requiredPortLabelSpace.y += label.getSize().y + labelSpacing;
+                        // TODO cds: why spacing in both directions? Doesn't it depend on the stacking direction?
+                        requiredPortLabelSpace.x += label.getSize().x + labelLabelSpacing;
+                        requiredPortLabelSpace.y += label.getSize().y + labelLabelSpacing;
                     }
                 }
 
                 requiredPortLabelSpace.x = Math.max(requiredPortLabelSpace.x, 0.0);
                 requiredPortLabelSpace.y = Math.max(requiredPortLabelSpace.y, 0.0);
 
-                processEdgeHeadTailLabels(boundingBox, port.getOutgoingEdges(), port.getIncomingEdges(),
-                        node, port, requiredPortLabelSpace, labelSpacing);
+                processEdgeHeadTailLabels(boundingBox, port.getOutgoingEdges(), port.getIncomingEdges(), node, port,
+                        requiredPortLabelSpace);
             }
         }
 
         // Process end labels of edges directly connected to the node
         if (includeEdgeHeadTailLabels) {
-            processEdgeHeadTailLabels(boundingBox, node.getOutgoingEdges(), node.getIncomingEdges(),
-                    node, null, null, labelSpacing);
+            processEdgeHeadTailLabels(boundingBox, node.getOutgoingEdges(), node.getIncomingEdges(), node, null, null);
         }
 
         // Reset the margin
@@ -230,20 +225,18 @@ private void processNode(final NodeAdapter<?> node, final double labelSpacing) {
      * @param port the port if the edges are connected to one.
      * @param portLabelSpace if the edges are connected to a port, this is the space required to
      *                               place the port's labels.
-     * @param labelSpacing label spacing.
      */
     private void processEdgeHeadTailLabels(final ElkRectangle boundingBox,
             final Iterable<EdgeAdapter<?>> outgoingEdges, final Iterable<EdgeAdapter<?>> incomingEdges,
-            final NodeAdapter<?> node, final PortAdapter<?> port, final KVector portLabelSpace,
-            final double labelSpacing) {
+            final NodeAdapter<?> node, final PortAdapter<?> port, final KVector portLabelSpace) {
 
         ElkRectangle labelBox = new ElkRectangle();
 
         // For each edge, the tail labels of outgoing edges ...
         for (EdgeAdapter<?> edge : outgoingEdges) {
             for (LabelAdapter<?> label : edge.getLabels()) {
                 if (label.getProperty(CoreOptions.EDGE_LABELS_PLACEMENT) == EdgeLabelPlacement.TAIL) {
-                    computeLabelBox(labelBox, label, false, node, port, portLabelSpace, labelSpacing);
+                    computeLabelBox(labelBox, label, false, node, port, portLabelSpace);
                     boundingBox.union(labelBox);
                 }
             }
@@ -253,7 +246,7 @@ private void processEdgeHeadTailLabels(final ElkRectangle boundingBox,
         for (EdgeAdapter<?> edge : incomingEdges) {
             for (LabelAdapter<?> label : edge.getLabels()) {
                 if (label.getProperty(CoreOptions.EDGE_LABELS_PLACEMENT) == EdgeLabelPlacement.HEAD) {
-                    computeLabelBox(labelBox, label, true, node, port, portLabelSpace, labelSpacing);
+                    computeLabelBox(labelBox, label, true, node, port, portLabelSpace);
                     boundingBox.union(labelBox);
                 }
             }
@@ -272,11 +265,10 @@ private void processEdgeHeadTailLabels(final ElkRectangle boundingBox,
      *             directly to the node.
      * @param portLabelSpace if the edges are connected to a port, this is the space required to
      *                       place the port's labels.
-     * @param labelSpacing label spacing.
      */
     private void computeLabelBox(final ElkRectangle labelBox, final LabelAdapter<?> label,
             final boolean incomingEdge, final NodeAdapter<?> node, final PortAdapter<?> port,
-            final KVector portLabelSpace, final double labelSpacing) {
+            final KVector portLabelSpace) {
 
         labelBox.x = node.getPosition().x;
         labelBox.y = node.getPosition().y;
@@ -288,48 +280,50 @@ private void computeLabelBox(final ElkRectangle labelBox, final LabelAdapter<?>
         labelBox.width = label.getSize().x;
         labelBox.height = label.getSize().y;
 
+        ElkMargin desiredNodeMargin = IndividualSpacings.getIndividualOrInherited(node, CoreOptions.NODE_LABELS_MARGIN);
+
         if (port == null) {
             // The edge is connected directly to the node
             if (incomingEdge) {
                 // Assume the edge enters the node at its western side
-                labelBox.x -= labelSpacing + label.getSize().x;
+                labelBox.x -= desiredNodeMargin.left + label.getSize().x;
             } else {
                 // Assume the edge leaves the node at its eastern side
-                labelBox.x += node.getSize().x + labelSpacing;
+                labelBox.x += node.getSize().x + desiredNodeMargin.right;
             }
         } else {
             switch (port.getSide()) {
             case UNDEFINED:
             case EAST:
                 labelBox.x += port.getSize().x
-                           + labelSpacing
+                           + desiredNodeMargin.left
                            + portLabelSpace.x
-                           + labelSpacing;
+                           + desiredNodeMargin.right; // TODO cds: I feel this is wrong (same below)
                 break;
 
             case WEST:
-                labelBox.x -= labelSpacing
+                labelBox.x -= desiredNodeMargin.left
                            + portLabelSpace.x
-                           + labelSpacing
+                           + desiredNodeMargin.right
                            + label.getSize().x;
                 break;
 
             case NORTH:
                 labelBox.x += port.getSize().x
-                           + labelSpacing;
-                labelBox.y -= labelSpacing
+                           + desiredNodeMargin.left;
+                labelBox.y -= desiredNodeMargin.top
                            + portLabelSpace.y
-                           + labelSpacing
+                           + desiredNodeMargin.bottom
                            + label.getSize().y;
                 break;
 
             case SOUTH:
                 labelBox.x += port.getSize().x
-                           + labelSpacing;
+                           + desiredNodeMargin.left;
                 labelBox.y += port.getSize().y
-                           + labelSpacing
+                           + desiredNodeMargin.top
                            + portLabelSpace.y
-                           + labelSpacing;
+                           + desiredNodeMargin.bottom;
                 break;
             }
         }

plugins/org.eclipse.elk.alg.common/src/org/eclipse/elk/alg/common/nodespacing/internal/NodeContext.java

@@ -74,7 +74,7 @@ public final class NodeContext {
     /** Space to leave around the node label area. */
     public final ElkPadding nodeLabelsPadding;
     /** Space between a node and its outside labels. */
-    public final double nodeLabelSpacing;
+    public final ElkMargin nodeLabelsMargin;
     /** Space between two labels. */
     public final double labelLabelSpacing;
     /** Space between two different label cells. */
@@ -144,7 +144,7 @@ public NodeContext(final GraphAdapter<?> parentGraph, final NodeAdapter<?> node)
 
         // Copy spacings for convenience
         nodeLabelsPadding = IndividualSpacings.getIndividualOrInherited(node, CoreOptions.NODE_LABELS_PADDING);
-        nodeLabelSpacing = IndividualSpacings.getIndividualOrInherited(node, CoreOptions.SPACING_LABEL_NODE);
+        nodeLabelsMargin = IndividualSpacings.getIndividualOrInherited(node, CoreOptions.NODE_LABELS_MARGIN);
         labelLabelSpacing = IndividualSpacings.getIndividualOrInherited(node, CoreOptions.SPACING_LABEL_LABEL);
         portPortSpacing = IndividualSpacings.getIndividualOrInherited(node, CoreOptions.SPACING_PORT_PORT);
         portLabelSpacing = IndividualSpacings.getIndividualOrInherited(node, CoreOptions.SPACING_LABEL_PORT);

plugins/org.eclipse.elk.alg.common/src/org/eclipse/elk/alg/common/nodespacing/internal/algorithm/NodeLabelCellCreator.java

@@ -108,22 +108,23 @@ private static void createNodeLabelCellContainers(final NodeContext nodeContext,
         if (!onlyInside) {
             StripContainerCell northContainer = new StripContainerCell(
                     Strip.HORIZONTAL, symmetry, nodeContext.labelCellSpacing);
-            northContainer.getPadding().bottom = nodeContext.nodeLabelSpacing;
+            // Note that top margin translates to bottom padding (likewise for the cases below)
+            northContainer.getPadding().bottom = nodeContext.nodeLabelsMargin.top;
             nodeContext.outsideNodeLabelContainers.put(PortSide.NORTH, northContainer);
 
             StripContainerCell southContainer = new StripContainerCell(
                     Strip.HORIZONTAL, symmetry, nodeContext.labelCellSpacing);
-            southContainer.getPadding().top = nodeContext.nodeLabelSpacing;
+            southContainer.getPadding().top = nodeContext.nodeLabelsMargin.bottom;
             nodeContext.outsideNodeLabelContainers.put(PortSide.SOUTH, southContainer);
 
             StripContainerCell westContainer = new StripContainerCell(
                     Strip.VERTICAL, symmetry, nodeContext.labelCellSpacing);
-            westContainer.getPadding().right = nodeContext.nodeLabelSpacing;
+            westContainer.getPadding().right = nodeContext.nodeLabelsMargin.left;
             nodeContext.outsideNodeLabelContainers.put(PortSide.WEST, westContainer);
 
             StripContainerCell eastContainer = new StripContainerCell(
                     Strip.VERTICAL, symmetry, nodeContext.labelCellSpacing);
-            eastContainer.getPadding().left = nodeContext.nodeLabelSpacing;
+            eastContainer.getPadding().left = nodeContext.nodeLabelsMargin.right;
             nodeContext.outsideNodeLabelContainers.put(PortSide.EAST, eastContainer);
         }
     }

plugins/org.eclipse.elk.alg.common/src/org/eclipse/elk/alg/common/nodespacing/internal/algorithm/PortLabelPlacementCalculator.java

@@ -241,12 +241,10 @@ private static void constrainedInsidePortLabelPlacement(final NodeContext nodeCo
         ElkRectangle labelContainerRect = insidePortLabelContainer.getCellRectangle();
         double leftBorder = labelContainerRect.x + ElkMath.maxd(
                 insidePortLabelContainer.getPadding().left,
-                nodeContext.surroundingPortMargins.left,
-                nodeContext.nodeLabelSpacing);
+                nodeContext.surroundingPortMargins.left);
         double rightBorder = labelContainerRect.x + labelContainerRect.width - ElkMath.maxd(
                 insidePortLabelContainer.getPadding().right,
-                nodeContext.surroundingPortMargins.right,
-                nodeContext.nodeLabelSpacing);
+                nodeContext.surroundingPortMargins.right);
 
         // Obtain a rectangle strip overlap remover, which will actually do most of the work
         RectangleStripOverlapRemover overlapRemover = RectangleStripOverlapRemover

plugins/org.eclipse.elk.alg.layered/src/org/eclipse/elk/alg/layered/Layered.melk

@@ -47,7 +47,6 @@ algorithm layered(LayeredLayoutProvider) {
     supports org.eclipse.elk.spacing.edgeNode
     supports org.eclipse.elk.spacing.labelLabel
     supports org.eclipse.elk.spacing.labelPort
-    supports org.eclipse.elk.spacing.labelNode
     supports org.eclipse.elk.spacing.nodeNode
     supports org.eclipse.elk.spacing.nodeSelfLoop
     supports org.eclipse.elk.spacing.portPort
@@ -103,8 +102,9 @@ algorithm layered(LayeredLayoutProvider) {
     supports org.eclipse.elk.nodeSize.constraints
     supports org.eclipse.elk.nodeSize.options
     supports org.eclipse.elk.direction = Direction.UNDEFINED
-    supports org.eclipse.elk.nodeLabels.placement
+    supports org.eclipse.elk.nodeLabels.margin
     supports org.eclipse.elk.nodeLabels.padding
+    supports org.eclipse.elk.nodeLabels.placement
     supports org.eclipse.elk.portLabels.placement
     supports org.eclipse.elk.portLabels.nextToPortIfPossible
     supports org.eclipse.elk.portLabels.treatAsGroup

plugins/org.eclipse.elk.alg.layered/src/org/eclipse/elk/alg/layered/options/Spacings.java

@@ -89,7 +89,7 @@ private void precalculateNodeTypeSpacings() {
         nodeTypeSpacing(NodeType.NORTH_SOUTH_PORT, NodeType.EXTERNAL_PORT, 
                 LayeredOptions.SPACING_EDGE_EDGE); // TODO
         nodeTypeSpacing(NodeType.NORTH_SOUTH_PORT, NodeType.LABEL, 
-                LayeredOptions.SPACING_LABEL_NODE);
+                LayeredOptions.SPACING_NODE_NODE_BETWEEN_LAYERS);
 
         // external
         nodeTypeSpacing(NodeType.EXTERNAL_PORT, 

plugins/org.eclipse.elk.core/src/org/eclipse/elk/core/Core.melk

@@ -337,16 +337,6 @@ group spacing {
         targets parents
     }
 
-    option labelNode: double {
-        label "Label Node Spacing"
-        description
-            "Spacing to be preserved between labels and the border of node they are associated with. 
-            Note that the placement of a label is influenced by the 'nodelabels.placement' option."
-        default = 5
-        lowerBound = 0.0
-        targets parents
-    }
-
     option labelPort: double {
         label "Label Port Spacing"
         description
@@ -430,11 +420,23 @@ group partitioning {
 
 // --- NODE LABELS
 group nodeLabels {
+
+    advanced option margin: ElkMargin {
+        label "Node Label Margin"
+        description
+            "Define margin for node labels that are placed outside of a node (see 'nodeLabels.placement').
+             The margin affects not only the node's direct labels but also the labels of the node's 
+             ports and certain edges. "
+        default = new ElkMargin(5)
+        targets nodes
+    }
 
     advanced option padding: ElkPadding {
         label "Node Label Padding"
         description
-            "Define padding for node labels that are placed inside of a node."
+            "Define padding for node labels that are placed inside of a node (see 'nodeLabels.placement').
+             The padding affects not only the node's direct labels but also the labels of the node's 
+             ports and certain edges. "
         default = new ElkPadding(5)
         targets nodes
     }