From 74b69bca81090c2a876cd4dffaf21a568d1f5b2e Mon Sep 17 00:00:00 2001 From: Soeren Domroes Date: Thu, 15 Aug 2024 12:28:58 +0200 Subject: [PATCH] Make sure that the order of incoming ports matches the order given by the previous layer. Updated WEST side handling to fix this and fixed typo as well and makes port model order non constraining for input ports. --- .../ModelOrderPortComparator.java | 39 +++++++++++-------- 1 file changed, 23 insertions(+), 16 deletions(-) diff --git a/plugins/org.eclipse.elk.alg.layered/src/org/eclipse/elk/alg/layered/intermediate/preserveorder/ModelOrderPortComparator.java b/plugins/org.eclipse.elk.alg.layered/src/org/eclipse/elk/alg/layered/intermediate/preserveorder/ModelOrderPortComparator.java index 20241c4a4..db003b5e6 100644 --- a/plugins/org.eclipse.elk.alg.layered/src/org/eclipse/elk/alg/layered/intermediate/preserveorder/ModelOrderPortComparator.java +++ b/plugins/org.eclipse.elk.alg.layered/src/org/eclipse/elk/alg/layered/intermediate/preserveorder/ModelOrderPortComparator.java @@ -90,7 +90,7 @@ public int compare(final LPort originalP1, final LPort originalP2) { LPort p1 = originalP1; LPort p2 = originalP2; - if (portModelOrder && p1.getSide() == PortSide.WEST && p2.getSide() == PortSide.WEST) { + if (p1.getSide() == PortSide.WEST && p2.getSide() == PortSide.WEST) { // Both nodes have the same port side. // Hence I need to determine their ordering based on the side. // WEST sort descending @@ -134,19 +134,7 @@ public int compare(final LPort originalP1, final LPort originalP2) { // Sort incoming edges by sorting their ports by the order of the nodes they connect to. if (!p1.getIncomingEdges().isEmpty() && !p2.getIncomingEdges().isEmpty()) { - // If port order is used instead of edge order, consult it to make decisions. - // If not both ports have a model order fall back to the edge order. - if (portModelOrder) { - int result = checkPortModelOrder(p1, p2); - if (result != 0) { - if (result == -1) { - updateBiggerAndSmallerAssociations(p2, p1); - } else if (result == 1) { - updateBiggerAndSmallerAssociations(p1, p2); - } - return result; - } - } + LNode p1Node = p1.getIncomingEdges().get(0).getSource().getNode(); LNode p2Node = p2.getIncomingEdges().get(0).getSource().getNode(); if (p1Node.equals(p2Node)) { @@ -160,6 +148,7 @@ public int compare(final LPort originalP1, final LPort originalP2) { // In this case both incoming edges must have a model order set. Check it. return Integer.compare(p1MO, p2MO); } + // Check which of the nodes connects first to the previous layer. for (LNode previousNode : previousLayer) { if (previousNode.equals(p1Node)) { updateBiggerAndSmallerAssociations(p1, p2); @@ -169,6 +158,20 @@ public int compare(final LPort originalP1, final LPort originalP2) { return -1; } } + // If both ports do not connect to the previous layer, use the port model order. + // If port order is used instead of edge order, consult it to make decisions. + // If not both ports have a model order fall back to the edge order. + if (portModelOrder) { + int result = checkPortModelOrder(p1, p2); + if (result != 0) { + if (result == -1) { + updateBiggerAndSmallerAssociations(p2, p1); + } else if (result == 1) { + updateBiggerAndSmallerAssociations(p1, p2); + } + return result; + } + } } // Sort outgoing edges by sorting their ports based on the model order of their edges. @@ -210,10 +213,10 @@ public int compare(final LPort originalP1, final LPort originalP2) { p1Order = p1.getOutgoingEdges().get(0).getProperty(InternalProperties.MODEL_ORDER); } if (p2.getOutgoingEdges().get(0).hasProperty(InternalProperties.MODEL_ORDER)) { - p2Order = p1.getOutgoingEdges().get(0).getProperty(InternalProperties.MODEL_ORDER); + p2Order = p2.getOutgoingEdges().get(0).getProperty(InternalProperties.MODEL_ORDER); } - // Same target node + // If both ports have the same target nodes, make sure that the backward edge is below the normal edge. if (p1TargetNode != null && p1TargetNode.equals(p2TargetNode)) { // Backward edges below if (p1.getOutgoingEdges().get(0).getProperty(InternalProperties.REVERSED) @@ -225,6 +228,7 @@ public int compare(final LPort originalP1, final LPort originalP2) { updateBiggerAndSmallerAssociations(p2, p1); return -1; } + // If both edges are reversed or not reversed, just use their model order. if (p1Order > p2Order) { updateBiggerAndSmallerAssociations(p1, p2); } else { @@ -232,6 +236,8 @@ public int compare(final LPort originalP1, final LPort originalP2) { } return Integer.compare(p1Order, p2Order); } + // Use precomputed ordering value if possible. + // FIXME why do I need this? if (targetNodeModelOrder != null) { if (targetNodeModelOrder.containsKey(p1TargetNode)) { p1Order = targetNodeModelOrder.get(p1TargetNode); @@ -240,6 +246,7 @@ public int compare(final LPort originalP1, final LPort originalP2) { p2Order = targetNodeModelOrder.get(p2TargetNode); } } + // If the nodes have different targets just use their order. if (p1Order > p2Order) { updateBiggerAndSmallerAssociations(p1, p2); } else {