From aaf32e10a6849d53b9154b82ae621ef1b439754e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ulf=20R=C3=BCegg?= Date: Tue, 19 May 2020 21:05:56 +0200 Subject: [PATCH 1/4] core: #576 turned 'spacing.labelNode' into 'nodeLabels.margin' --- .../org/eclipse/elk/alg/layered/Layered.melk | 4 ++-- .../src/org/eclipse/elk/core/Core.melk | 24 ++++++++++--------- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/plugins/org.eclipse.elk.alg.layered/src/org/eclipse/elk/alg/layered/Layered.melk b/plugins/org.eclipse.elk.alg.layered/src/org/eclipse/elk/alg/layered/Layered.melk index 519eee1056..a399ecdd6a 100644 --- a/plugins/org.eclipse.elk.alg.layered/src/org/eclipse/elk/alg/layered/Layered.melk +++ b/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 diff --git a/plugins/org.eclipse.elk.core/src/org/eclipse/elk/core/Core.melk b/plugins/org.eclipse.elk.core/src/org/eclipse/elk/core/Core.melk index 9e90b2d1a7..ed5ae9edcb 100644 --- a/plugins/org.eclipse.elk.core/src/org/eclipse/elk/core/Core.melk +++ b/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 } From e2bcc045c1714166ce5ab6e8e7941c0cfaea3f17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ulf=20R=C3=BCegg?= Date: Tue, 19 May 2020 21:19:40 +0200 Subject: [PATCH 2/4] layered: #576 adjustment after 'labelNode' removal --- .../src/org/eclipse/elk/alg/layered/options/Spacings.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/org.eclipse.elk.alg.layered/src/org/eclipse/elk/alg/layered/options/Spacings.java b/plugins/org.eclipse.elk.alg.layered/src/org/eclipse/elk/alg/layered/options/Spacings.java index dfd61bc5c7..e2ff45bf7a 100644 --- a/plugins/org.eclipse.elk.alg.layered/src/org/eclipse/elk/alg/layered/options/Spacings.java +++ b/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, From 2402d9901c404e23f8f82ec316e1d57b93146f93 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ulf=20R=C3=BCegg?= Date: Tue, 19 May 2020 21:20:31 +0200 Subject: [PATCH 3/4] core.nodespacing: #576 adjusted NodeMarginCalculator Additional changes: - retrieve margins using IndividualSpacings - retrieve margins locally instead of passing them via parameters --- .../nodespacing/NodeMarginCalculator.java | 82 +++++++++---------- 1 file changed, 38 insertions(+), 44 deletions(-) diff --git a/plugins/org.eclipse.elk.alg.common/src/org/eclipse/elk/alg/common/nodespacing/NodeMarginCalculator.java b/plugins/org.eclipse.elk.alg.common/src/org/eclipse/elk/alg/common/nodespacing/NodeMarginCalculator.java index 6dd893174d..04d6f470fa 100644 --- a/plugins/org.eclipse.elk.alg.common/src/org/eclipse/elk/alg/common/nodespacing/NodeMarginCalculator.java +++ b/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,12 +225,10 @@ 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> outgoingEdges, final Iterable> 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(); @@ -243,7 +236,7 @@ private void processEdgeHeadTailLabels(final ElkRectangle boundingBox, 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; } } From c105e7b85006d0103a375ed2b8e7c48180091063 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ulf=20R=C3=BCegg?= Date: Tue, 19 May 2020 21:26:54 +0200 Subject: [PATCH 4/4] core.nodespacing: #576 further adjustments --- .../elk/alg/common/nodespacing/internal/NodeContext.java | 4 ++-- .../internal/algorithm/NodeLabelCellCreator.java | 9 +++++---- .../internal/algorithm/PortLabelPlacementCalculator.java | 6 ++---- 3 files changed, 9 insertions(+), 10 deletions(-) diff --git a/plugins/org.eclipse.elk.alg.common/src/org/eclipse/elk/alg/common/nodespacing/internal/NodeContext.java b/plugins/org.eclipse.elk.alg.common/src/org/eclipse/elk/alg/common/nodespacing/internal/NodeContext.java index 1f3f227d36..d3edddd5cf 100644 --- a/plugins/org.eclipse.elk.alg.common/src/org/eclipse/elk/alg/common/nodespacing/internal/NodeContext.java +++ b/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); diff --git a/plugins/org.eclipse.elk.alg.common/src/org/eclipse/elk/alg/common/nodespacing/internal/algorithm/NodeLabelCellCreator.java b/plugins/org.eclipse.elk.alg.common/src/org/eclipse/elk/alg/common/nodespacing/internal/algorithm/NodeLabelCellCreator.java index abd69696ea..9a503fe33a 100644 --- a/plugins/org.eclipse.elk.alg.common/src/org/eclipse/elk/alg/common/nodespacing/internal/algorithm/NodeLabelCellCreator.java +++ b/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); } } diff --git a/plugins/org.eclipse.elk.alg.common/src/org/eclipse/elk/alg/common/nodespacing/internal/algorithm/PortLabelPlacementCalculator.java b/plugins/org.eclipse.elk.alg.common/src/org/eclipse/elk/alg/common/nodespacing/internal/algorithm/PortLabelPlacementCalculator.java index 3e1a098ef8..3ca4466519 100644 --- a/plugins/org.eclipse.elk.alg.common/src/org/eclipse/elk/alg/common/nodespacing/internal/algorithm/PortLabelPlacementCalculator.java +++ b/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