Changeset 18539 in josm for trunk/src/org/openstreetmap

Timestamp:

2022-08-16T15:50:52+02:00 (2 years ago)

Author:

taylor.smock

Message:

Fix #21856: Split way: Wrong position of new member in PTv2 relation splitting a loop

Location:

trunk/src/org/openstreetmap/josm

Files:

• command/SplitWayCommand.java (modified) (6 diffs)
• data/osm/DataSet.java (modified) (1 diff)

trunk/src/org/openstreetmap/josm/command/SplitWayCommand.java

@@ -488 +488 @@
                             if (way.lastNode() == way.firstNode()) {
                                 // Self-closing way.
-                                direction = Direction.IRRELEVANT;
+                                direction = direction.merge(Direction.IRRELEVANT);
                             } else {
                                 // For ordered relations, looking beyond the nearest neighbour members is not required,
@@ -498 +498 @@
                                     else {
                                         if (w.lastNode() == way.firstNode() || w.firstNode() == way.firstNode()) {
-                                            direction = Direction.FORWARDS;
+                                            direction = direction.merge(Direction.FORWARDS);
                                         } else if (w.firstNode() == way.lastNode() || w.lastNode() == way.lastNode()) {
-                                            direction = Direction.BACKWARDS;
+                                            direction = direction.merge(Direction.BACKWARDS);
                                         }
                                     }
@@ -510 +510 @@
                                     else {
                                         if (w.lastNode() == way.firstNode() || w.firstNode() == way.firstNode()) {
-                                            direction = Direction.BACKWARDS;
+                                            direction = direction.merge(Direction.BACKWARDS);
                                         } else if (w.firstNode() == way.lastNode() || w.lastNode() == way.lastNode()) {
-                                            direction = Direction.FORWARDS;
+                                            direction = direction.merge(Direction.FORWARDS);
                                         }
                                     }
@@ -529 +529 @@
                                     Way w = r.getMember(ir - k).getWay();
                                     if (w.lastNode() == way.firstNode() || w.firstNode() == way.firstNode()) {
-                                        direction = Direction.FORWARDS;
+                                        direction = direction.merge(Direction.FORWARDS);
                                     } else if (w.firstNode() == way.lastNode() || w.lastNode() == way.lastNode()) {
-                                        direction = Direction.BACKWARDS;
+                                        direction = direction.merge(Direction.BACKWARDS);
                                     }
                                     break;
@@ -538 +538 @@
                                     Way w = r.getMember(ir + k).getWay();
                                     if (w.lastNode() == way.firstNode() || w.firstNode() == way.firstNode()) {
-                                        direction = Direction.BACKWARDS;
+                                        direction = direction.merge(Direction.BACKWARDS);
                                     } else if (w.firstNode() == way.lastNode() || w.lastNode() == way.lastNode()) {
-                                        direction = Direction.FORWARDS;
+                                        direction = direction.merge(Direction.FORWARDS);
                                     }
                                     break;
@@ -952 +952 @@
         BACKWARDS,
         UNKNOWN,
-        IRRELEVANT
+        IRRELEVANT;
+
+        /**
+         * Merge directions (this helps avoid overriding {@link #FORWARDS} with {@link #BACKWARDS}).
+         * @param other The other direction to merge. {@link #UNKNOWN} will be overridden.
+         * @return The merged direction
+         */
+        Direction merge(Direction other) {
+            if (this == other) {
+                return this;
+            }
+            if (this == IRRELEVANT || other == IRRELEVANT ||
+                    (this == FORWARDS && other == BACKWARDS) ||
+                    (other == FORWARDS && this == BACKWARDS)) {
+                return IRRELEVANT;
+            }
+            if (this == UNKNOWN) {
+                return other;
+            }
+            if (other == UNKNOWN) {
+                return this;
+            }
+            return UNKNOWN;
+        }
     }
 

trunk/src/org/openstreetmap/josm/data/osm/DataSet.java

@@ -531 +531 @@
      */
     public void addPrimitiveRecursive(OsmPrimitive primitive) {
+        Stream<OsmPrimitive> children;
         if (primitive instanceof Way) {
-            ((Way) primitive).getNodes().forEach(n -> addPrimitiveRecursive(n));
+            children = ((Way) primitive).getNodes().stream().map(OsmPrimitive.class::cast);
         } else if (primitive instanceof Relation) {
-            ((Relation) primitive).getMembers().forEach(m -> addPrimitiveRecursive(m.getMember()));
-        }
+            children = ((Relation) primitive).getMembers().stream().map(RelationMember::getMember);
+        } else {
+            children = Stream.empty();
+        }
+        // Relations may have the same member multiple times.
+        children.filter(p -> !Objects.equals(this, p.getDataSet())).forEach(this::addPrimitiveRecursive);
         addPrimitive(primitive);
     }