[PR] [WIP] remove 'spacing.labelNode', add 'nodeLabels.margin' #621
Conversation
Additional changes: - retrieve margins using IndividualSpacings - retrieve margins locally instead of passing them via parameters
| @@ -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); | |||
There was a problem hiding this comment.
I removed this completely here as the code handles ports inside of the node.
uruuru
added
alg-layered
le-cds
changed the title
| // 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? |
There was a problem hiding this comment.
Yes, between multiple labels that all belong to the same graph element.
| @@ -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? | |||
There was a problem hiding this comment.
ELK Layered does its own end label processing in EndLabelPreprocessor and EndLabelPostprocessor. I think this code wasmeant for algorithms that do not implement their own sophisticated end label processing.
| 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? |
| + portLabelSpace.x | ||
| + labelSpacing; | ||
| + desiredNodeMargin.right; // TODO cds: I feel this is wrong (same below) |
There was a problem hiding this comment.
I think the idea here was that there's the width of the node, then a bit of space between it and the port labels, then the width of the port labels, and then another bit of space between them and the edge end labels. In fact, this might make the node label margin a little strange, perhaps. It needs to be applied between a node and its port labels, and then again between those and the end labels... Hm...
There was a problem hiding this comment.
Oh, and outside node labels as well...
@le-cds consider this PR a start to the task. I left a couple of
TODO cdsin the diff wherever I wasn't sure. Maybe you can find time to have a look at them.Also, are there any other tests apart from looking at several of the elk-models files?
There's one functional change in there as well: