Fix always existing bug in model order port sorting.

plugins/org.eclipse.elk.alg.layered/src/org/eclipse/elk/alg/layered/intermediate/NorthSouthPortPreprocessor.java

@@ -23,6 +23,7 @@
 import org.eclipse.elk.alg.layered.graph.Layer;
 import org.eclipse.elk.alg.layered.options.InternalProperties;
 import org.eclipse.elk.alg.layered.options.LayeredOptions;
+import org.eclipse.elk.alg.layered.options.OrderingStrategy;
 import org.eclipse.elk.core.alg.ILayoutProcessor;
 import org.eclipse.elk.core.options.PortConstraints;
 import org.eclipse.elk.core.options.PortSide;
@@ -169,7 +170,8 @@ public void process(final LGraph layeredGraph, final IElkProgressMonitor monitor
                 }
 
                 // Sort the port list if we have control over the port order
-                if (!node.getProperty(LayeredOptions.PORT_CONSTRAINTS).isOrderFixed()) {
+                if (!node.getProperty(LayeredOptions.PORT_CONSTRAINTS).isOrderFixed()
+                        && node.getGraph().getProperty(LayeredOptions.CONSIDER_MODEL_ORDER_STRATEGY) == OrderingStrategy.NONE) {
                     sortPortList(node);
                 }
 
@@ -188,6 +190,10 @@ public void process(final LGraph layeredGraph, final IElkProgressMonitor monitor
                 LinkedList<LPort> portList = Lists.newLinkedList();
                 Iterables.addAll(portList, node.getPorts(PortSide.NORTH));
 
+                if (node.getGraph().getProperty(LayeredOptions.CONSIDER_MODEL_ORDER_STRATEGY) != OrderingStrategy.NONE) {                    
+                    portList = modelOrderNorthSouthInputReversing(portList, node);
+                }
+
                 createDummyNodes(layeredGraph, portList, northDummyNodes, southDummyNodes,
                         barycenterAssociates);
 
@@ -241,6 +247,10 @@ public void process(final LGraph layeredGraph, final IElkProgressMonitor monitor
                     portList.addFirst(port);
                 }
 
+                if (node.getGraph().getProperty(LayeredOptions.CONSIDER_MODEL_ORDER_STRATEGY) != OrderingStrategy.NONE) {                    
+                    portList = modelOrderNorthSouthInputReversing(portList, node);
+                }
+
                 createDummyNodes(layeredGraph, portList, southDummyNodes, null,
                         barycenterAssociates);
 
@@ -358,6 +368,22 @@ private void sortPortList(final LNode node) {
             }
         });
     }
+
+    private LinkedList<LPort> modelOrderNorthSouthInputReversing(LinkedList<LPort> portList, LNode node) {
+        // Reverse port list if edges are incoming edges.
+        LinkedList<LPort> incoming = new LinkedList<LPort>();
+        LinkedList<LPort> outgoing = new LinkedList<LPort>();
+        for (LPort port : portList) {
+            if (!port.getIncomingEdges().isEmpty()) {
+                // Incoming edge
+                incoming.add(port);
+            } else {
+                outgoing.add(port);
+            }
+        }
+        Lists.reverse(incoming).addAll(outgoing);
+        return incoming;
+    }
 
     // /////////////////////////////////////////////////////////////////////////////
     // DUMMY NODE CREATION

plugins/org.eclipse.elk.alg.layered/src/org/eclipse/elk/alg/layered/intermediate/SortByInputModelProcessor.java

@@ -11,6 +11,7 @@
 
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 
 import org.eclipse.elk.alg.layered.graph.LEdge;
@@ -55,8 +56,16 @@ public void process(final LGraph graph, final IElkProgressMonitor progressMonito
                 + graph.getProperty(LayeredOptions.CONSIDER_MODEL_ORDER_STRATEGY), 1);
         int layerIndex = 0;
         for (Layer layer : graph) {
+            // The layer.id is necessary to check whether nodes really connect to the previous layer.
+            // Feedback long edge dummies do might have a long-edge dummy in the same layer.
+            layer.id = layerIndex;
             final int previousLayerIndex = layerIndex == 0 ? 0 : layerIndex - 1;
             Layer previousLayer = graph.getLayers().get(previousLayerIndex);
+            // Sort nodes before port sorting to have sorted nodes for in-layer feedback edge dummies.
+            ModelOrderNodeComparator comparator = new ModelOrderNodeComparator(previousLayer,
+                    graph.getProperty(LayeredOptions.CONSIDER_MODEL_ORDER_STRATEGY),
+                    graph.getProperty(LayeredOptions.CONSIDER_MODEL_ORDER_LONG_EDGE_STRATEGY), true);
+            SortByInputModelProcessor.insertionSort(layer.getNodes(), comparator);
             for (LNode node : layer.getNodes()) {
                 if (node.getProperty(LayeredOptions.PORT_CONSTRAINTS) != PortConstraints.FIXED_ORDER
                         && node.getProperty(LayeredOptions.PORT_CONSTRAINTS) != PortConstraints.FIXED_POS) {
@@ -65,7 +74,6 @@ public void process(final LGraph graph, final IElkProgressMonitor progressMonito
                     // Therefore all ports that connect to the same node should have the same
                     // (their minimal) model order.
                     // Get minimal model order for target node
-
                     Collections.sort(node.getPorts(),
                             new ModelOrderPortComparator(previousLayer,
                                     graph.getProperty(LayeredOptions.CONSIDER_MODEL_ORDER_STRATEGY),
@@ -74,11 +82,12 @@ public void process(final LGraph graph, final IElkProgressMonitor progressMonito
                     progressMonitor.log("Node " + node + " ports: " + node.getPorts());
                 }
             }
-            // Sort nodes.
-            Collections.sort(layer.getNodes(),
-                    new ModelOrderNodeComparator(previousLayer,
-                            graph.getProperty(LayeredOptions.CONSIDER_MODEL_ORDER_STRATEGY),
-                            graph.getProperty(LayeredOptions.CONSIDER_MODEL_ORDER_LONG_EDGE_STRATEGY)));
+            // Sort nodes after port sorting to also sort dummy feedback nodes from the current layer correctly.
+            comparator = new ModelOrderNodeComparator(previousLayer,
+                    graph.getProperty(LayeredOptions.CONSIDER_MODEL_ORDER_STRATEGY),
+                    graph.getProperty(LayeredOptions.CONSIDER_MODEL_ORDER_LONG_EDGE_STRATEGY), false);
+            SortByInputModelProcessor.insertionSort(layer.getNodes(), comparator);
+
             progressMonitor.log("Layer " + layerIndex + ": " + layer);
             layerIndex++;
         }
@@ -138,5 +147,35 @@ public static LNode getTargetNode(final LPort port) {
         } while (node != null && node.getType() != NodeType.NORMAL);
         return node;
     }
+
+    public static void insertionSort(final List<LNode> layer,
+            final ModelOrderNodeComparator comparator) {
+        LNode temp;
+        for (int i = 1; i < layer.size(); i++) {
+            temp = layer.get(i);
+            int j = i;
+            while (j > 0 && comparator.compare(layer.get(j - 1), temp) > 0) {
+                layer.set(j, layer.get(j - 1));
+                j--;
+            }
+            layer.set(j, temp);
+        }
+        comparator.clearTransitiveOrdering();
+    }
+
+    public static void insertionSortPort(final List<LPort> layer,
+            final ModelOrderPortComparator comparator) {
+        LPort temp;
+        for (int i = 1; i < layer.size(); i++) {
+            temp = layer.get(i);
+            int j = i;
+            while (j > 0 && comparator.compare(layer.get(j - 1), temp) > 0) {
+                layer.set(j, layer.get(j - 1));
+                j--;
+            }
+            layer.set(j, temp);
+        }
+        comparator.clearTransitiveOrdering();
+    }
 }