Index: src/org/openstreetmap/josm/actions/CombineWayAction.java =================================================================== --- src/org/openstreetmap/josm/actions/CombineWayAction.java (revision 18644) +++ src/org/openstreetmap/josm/actions/CombineWayAction.java (working copy) @@ -232,9 +232,14 @@ } protected static List tryJoin(Collection ways) { - List path = joinWithMultipolygonCode(ways); + List path; + NodeGraph graph = NodeGraph.createDirectedGraphFromWays(ways); + path = graph.buildSpanningPathNoRemove(); if (path.isEmpty()) { - NodeGraph graph = NodeGraph.createNearlyUndirectedGraphFromNodeWays(ways); + path = joinWithMultipolygonCode(ways); + } + if (path.isEmpty()) { + graph = NodeGraph.createNearlyUndirectedGraphFromNodeWays(ways); path = graph.buildSpanningPathNoRemove(); } return path; Index: src/org/openstreetmap/josm/data/osm/NodeGraph.java =================================================================== --- src/org/openstreetmap/josm/data/osm/NodeGraph.java (revision 18644) +++ src/org/openstreetmap/josm/data/osm/NodeGraph.java (working copy) @@ -11,6 +11,7 @@ import java.util.HashSet; import java.util.LinkedHashMap; import java.util.LinkedHashSet; +import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.Map.Entry; @@ -140,6 +141,9 @@ private int numUndirectedEges; /** counts the number of edges that were added */ private int addedEdges; + private LinkedList[] adj; + private final List allNodes = new ArrayList<>(); + private final HashMap nodeIdMap = new HashMap<>(); private final Map> successors = new LinkedHashMap<>(); private final Map> predecessors = new LinkedHashMap<>(); @@ -182,6 +186,26 @@ rememberPredecessors(pair); } numUndirectedEges = undirectedEdges.size(); + + // calculate an index for each node contained in this graph + allNodes.clear(); + allNodes.addAll(getNodes()); + // calculate the adjen + nodeIdMap.clear(); + adj = new LinkedList[allNodes.size()]; + for (int i = 0; i < allNodes.size(); i++) { + adj[i] = new LinkedList<>(); + } + // map the nodes in this graph to an Integer + for (int i = 0; i < allNodes.size(); i++) { + nodeIdMap.put(allNodes.get(i), i); + } + + for (NodePair edge : undirectedEdges) { + int idxA = nodeIdMap.get(edge.getA()); + int idxB = nodeIdMap.get(edge.getB()); + addEdge(idxA, idxB); + } } /** @@ -210,10 +234,6 @@ } } - protected Set getTerminalNodes() { - return getNodes().stream().filter(this::isTerminalNode).collect(Collectors.toCollection(LinkedHashSet::new)); - } - private List getConnectedPairs(Node node) { List connected = new ArrayList<>(); connected.addAll(Optional.ofNullable(successors.get(node)).orElseGet(Collections::emptyList)); @@ -287,21 +307,24 @@ */ public List buildSpanningPath() { prepare(); - if (numUndirectedEges > 0 && isConnected()) { - // try to find a path from each "terminal node", i.e. from a - // node which is connected by exactly one undirected edges (or - // two directed edges in opposite direction) to the graph. A - // graph built up from way segments is likely to include such - // nodes, unless the edges build one or more closed rings. - // We order the nodes to start with the best candidates, but - // it might take very long if there is no valid path as we iterate over all nodes - // to find out. - Set nodes = getTerminalNodes(); - nodes = nodes.isEmpty() ? getMostFrequentVisitedNodesFirst() : nodes; - return nodes.stream() - .map(this::buildSpanningPath) - .filter(path -> !path.isEmpty()) - .findFirst().orElse(null); + if (numUndirectedEges > 0) { + int indicator = isEulerian(); + if (indicator > 0) { + // try to find a path from each "terminal node", i.e. from a + // node which is connected by exactly one undirected edges (or + // two directed edges in opposite direction) to the graph. A + // graph built up from way segments is likely to include such + // nodes, unless the edges build one or more closed rings. + // We order the nodes to start with the best candidates, but + // it might take very long if there is no valid path as we iterate over all nodes + // to find out. + Collection startNodes = getOddNodes(); + startNodes = startNodes.isEmpty() ? getMostFrequentVisitedNodesFirst() : startNodes; + return startNodes.stream() + .map(this::buildSpanningPath) + .filter(path -> !path.isEmpty()) + .findFirst().orElse(null); + } } return null; } @@ -327,12 +350,11 @@ * @return true if it is connected. */ private boolean isConnected() { - Set nodes = getNodes(); - if (nodes.isEmpty()) + if (allNodes.isEmpty()) return false; Deque toVisit = new ArrayDeque<>(); HashSet visited = new HashSet<>(); - toVisit.add(nodes.iterator().next()); + toVisit.add(allNodes.iterator().next()); while (!toVisit.isEmpty()) { Node n = toVisit.pop(); if (!visited.contains(n)) { @@ -345,7 +367,7 @@ visited.add(n); } } - return nodes.size() == visited.size(); + return allNodes.size() == visited.size(); } /** @@ -377,4 +399,46 @@ return Collections.unmodifiableSet(result); } + private void addEdge(int v, int w) { + //Function to add an edge into the graph + adj[v].add(w); // Add w to v's list. + adj[w].add(v); //The graph is undirected + } + + private List getOddNodes() { + List odd = new ArrayList<>(); + // Count vertices with odd degree + for (int i = 0; i < allNodes.size(); i++) { + if (adj[i].size() % 2 != 0) + odd.add(allNodes.get(i)); + } + return odd; + } + + /** + * Check if graph has Eulerian path or Circuit or none of both + * Code taken from https://www.geeksforgeeks.org/eulerian-path-and-circuit/ + * @return 0 if graph is not Eulerian, 1 if there is a Eulerian Path, 2 if there is an Eulerian Circuit + */ + private int isEulerian() { + // Check if all vertices are connected + if (!isConnected()) + return 0; + + // Count vertices with odd degree + int odd = 0; + for (int i = 0; i < allNodes.size(); i++) { + if (adj[i].size() % 2 != 0) + odd++; + } + + // If count is more than 2, then graph is not Eulerian + if (odd > 2) + return 0; + + // If odd count is 2, then semi-eulerian. + // If odd count is 0, then eulerian + // Note that odd count can never be 1 for undirected graph + return (odd == 2) ? 1 : 2; + } } Index: src/org/openstreetmap/josm/data/osm/NodePair.java =================================================================== --- src/org/openstreetmap/josm/data/osm/NodePair.java (revision 18644) +++ src/org/openstreetmap/josm/data/osm/NodePair.java (working copy) @@ -77,9 +77,9 @@ public String toString() { return new StringBuilder() .append('[') - .append(a.getId()) + .append(a.getUniqueId()) .append(',') - .append(b.getId()) + .append(b.getUniqueId()) .append(']') .toString(); } Index: test/unit/org/openstreetmap/josm/actions/CombineWayActionTest.java =================================================================== --- test/unit/org/openstreetmap/josm/actions/CombineWayActionTest.java (revision 18644) +++ test/unit/org/openstreetmap/josm/actions/CombineWayActionTest.java (working copy) @@ -3,6 +3,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; import java.io.IOException; import java.io.InputStream; @@ -14,8 +15,8 @@ import java.util.List; import java.util.Set; +import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; -import org.junit.jupiter.api.Test; import org.openstreetmap.josm.TestUtils; import org.openstreetmap.josm.data.osm.DataSet; import org.openstreetmap.josm.data.osm.Node; @@ -137,7 +138,7 @@ List reversedWays = new LinkedList<>(); List unreversedWays = new LinkedList<>(); CombineWayAction.detectReversedWays(selection, path, reversedWays, unreversedWays); - assertFalse(reversedWays.isEmpty()); + assertTrue(reversedWays.isEmpty()); } }