Fix always existing bug in model order port sorting. (#1067)

* 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.

* Fixed all but one case for port model order and feedback nodes.

* Check whether model order exists and do not sort in this case.

* Removed comment for unnecessary backward edges below.

* Do insertionsort to handle comparator errors.

* Fixed typos in node comparator.

* Always use insertion sort and more small fixes.

* Sort nodes before sorting ports since in-layer feedback dummies exist.

* Found the last inconsistency in port ordering.

* Define all corner cases of node model order.

* Use normal sorting instead of insertion sort but keep insertion sort.

* Make sure that exchanging ports does not lead to bugs by using a scalar.

* Readded second node sorting.

I could solve this maybe by doing the inversePortProcessor after this.

* Added first patch of sortbyinputmodel correctness examples.

* Added correctness tests with multiple nodes.

* Port model order basic concrete tests.

* Nodes have to be sorted with insertion sort.

* Added optimization fixme to model order port comparator.

* long edge and feedback edge corner cases.

* Handle unconnected ports.

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();
+    }
 }