Ticket #24741: 24741.patch

src/org/openstreetmap/josm/actions/relation/ExportRelationToGpxAction.java

@@ -24 +24 @@
 
 import org.openstreetmap.josm.actions.GpxExportAction;
 import org.openstreetmap.josm.actions.IPrimitiveAction;
+import org.openstreetmap.josm.data.coor.LatLon;
 import org.openstreetmap.josm.data.gpx.GpxData;
 import org.openstreetmap.josm.data.gpx.GpxTrack;
 import org.openstreetmap.josm.data.gpx.WayPoint;
@@ -31 +32 @@
 import org.openstreetmap.josm.data.osm.Node;
 import org.openstreetmap.josm.data.osm.Relation;
 import org.openstreetmap.josm.data.osm.RelationMember;
+import org.openstreetmap.josm.data.osm.Way;
 import org.openstreetmap.josm.gui.MainApplication;
 import org.openstreetmap.josm.gui.dialogs.relation.sort.WayConnectionType;
 import org.openstreetmap.josm.gui.dialogs.relation.sort.WayConnectionTypeCalculator;
@@ -151 +153 @@
 
             List<WayConnectionType> wct = new WayConnectionTypeCalculator().updateLinks(flat);
             final HashMap<String, Integer> names = new HashMap<>();
+            List<Way> roundabouts = new ArrayList<>();
             for (int i = 0; i < flat.size(); i++) {
                 WayConnectionType wayConnectionType = wct.get(i);
-                if (!wayConnectionType.isOnewayLoopBackwardPart && !wayConnectionType.direction.isRoundabout()) {
-                    if (!wayConnectionType.linkPrev && !trkseg.isEmpty()) {
-                        gpxData.addTrack(new GpxTrack(trk, trkAttr));
-                        trkAttr.clear();
-                        trk.clear();
-                        trkseg.clear();
-                        trk.add(trkseg);
+                if (!wayConnectionType.linkPrev && !trkseg.isEmpty()) {
+                    checkRoundabouts(trkseg, roundabouts);
+                    roundabouts.clear();
+                    time = setTime(trkseg, time);
+                    gpxData.addTrack(new GpxTrack(trk, trkAttr));
+                    trkAttr.clear();
+                    trk.clear();
+                    trkseg.clear();
+                    trk.add(trkseg);
+                }
+                if (trkAttr.isEmpty()) {
+                    flat.get(i).getWay().referrers(Relation.class)
+                            .filter(relsFound::contains)
+                            .findFirst()
+                            .ifPresent(r -> {
+                                trkAttr.put("name", r.getName() != null ? r.getName() : Long.toString(r.getId()));
+                                trkAttr.put("desc", tr("based on osm route relation data, timestamps are synthetic"));
+                            });
+                    GpxData.ensureUniqueName(trkAttr, names, (String) trkAttr.get("name"));
+                }
+                if (wayConnectionType.isOnewayLoopBackwardPart)
+                    continue;
+                if (wayConnectionType.direction.isRoundabout()) {
+                    roundabouts.add(flat.get(i).getWay());
+                    continue;
+                }
+                List<Node> ln = flat.get(i).getWay().getNodes();
+                if (wayConnectionType.direction == WayConnectionType.Direction.BACKWARD)
+                    Collections.reverse(ln);
+                for (Node n: ln) {
+                    WayPoint point = OsmDataLayer.nodeToWayPoint(n, Long.MIN_VALUE);
+                    if (!trkseg.isEmpty()) {
+                        // see #24745 don't add connecting way nodes twice
+                        WayPoint last = trkseg.get(trkseg.size() - 1);
+                        if (point.getCoor().equals(last.getCoor()))
+                            continue;
                     }
-                    if (trkAttr.isEmpty()) {
-                        flat.get(i).getWay().referrers(Relation.class)
-                                .filter(relsFound::contains)
-                                .findFirst()
-                                .ifPresent(r -> {
-                                    trkAttr.put("name", r.getName() != null ? r.getName() : Long.toString(r.getId()));
-                                    trkAttr.put("desc", tr("based on osm route relation data, timestamps are synthetic"));
-                                });
-                        GpxData.ensureUniqueName(trkAttr, names, (String) trkAttr.get("name"));
-                    }
-                    List<Node> ln = flat.get(i).getWay().getNodes();
-                    if (wayConnectionType.direction == WayConnectionType.Direction.BACKWARD)
-                        Collections.reverse(ln);
-                    for (Node n: ln) {
-                        WayPoint point = OsmDataLayer.nodeToWayPoint(n, TimeUnit.SECONDS.toMillis(time));
-                        if (!trkseg.isEmpty()) {
-                            // see #24745 don't add connecting way nodes twice
-                            WayPoint last = trkseg.get(trkseg.size() - 1);
-                            if (point.getCoor().equals(last.getCoor()))
-                                continue;
-                        }
-                        trkseg.add(point);
-                        time += 1;
-                    }
+                    trkseg.add(point);
                 }
             }
-            gpxData.addTrack(new GpxTrack(trk, trkAttr));
+            if (!trkseg.isEmpty()) {
+                checkRoundabouts(trkseg, roundabouts);
+                time = setTime(trkseg, time);
+                gpxData.addTrack(new GpxTrack(trk, trkAttr));
+            }
 
             String lprefix = relations.iterator().next().getName();
             if (lprefix == null || relations.size() > 1)
@@ -200 +213 @@
         return new GpxLayer(gpxData, layerName, true);
     }
 
+    /**
+     * Set time for the points in the given track segment and return last used value. For each point the time is incremented by 1.
+     * @param trkseg the track segment
+     * @param time the time value to start with
+     * @return last used time value
+     */
+    private long setTime(List<WayPoint> trkseg, long time) {
+        for (WayPoint p : trkseg) {
+            p.setTimeInMillis(TimeUnit.SECONDS.toMillis(time));
+            time++;
+        }
+        return time;
+    }
+
+    /**
+     * Handle closed roundabout at beginning and/or end of track segment
+     * @param trkseg the track segment
+     * @param roundabouts list of roundabout ways
+     */
+    private void checkRoundabouts(List<WayPoint> trkseg, List<Way> roundabouts) {
+        for (Way roundabout : roundabouts) {
+            checkRoundabout(trkseg, roundabout);
+        }
+    }
+
+    /**
+     * Check if roundabout is at the beginning and/or end of track segment and possibly add further nodes.
+     * See #24741
+     * @param trkseg the track segment
+     * @param roundabout the (closed) roundabout
+     */
+    private void checkRoundabout(List<WayPoint> trkseg, Way roundabout) {
+        if (trkseg.isEmpty())
+            return;
+        LatLon first = trkseg.get(0).getCoor();
+        LatLon last = trkseg.get(trkseg.size() - 1).getCoor();
+        boolean hasFirst = roundabout.getNodes().stream().anyMatch(p -> p.getCoor().equals(first));
+        boolean hasLast = roundabout.getNodes().stream().anyMatch(p -> p.getCoor().equals(last));
+        if (hasFirst && hasLast) {
+            // track starts and ends with the same closed roundabout and thus is a round trip, we want to add part of the circle or
+            // just the connecting line between first and last to close the gap
+            if (mode.contains(FROM_FIRST_MEMBER)) {
+                trkseg.add(0, new WayPoint(trkseg.get(trkseg.size() - 1)));
+            } else {
+                trkseg.add(new WayPoint(trkseg.get(0)));
+            }
+        } else if (hasFirst || hasLast) {
+            // a closed roundabout is before or after a gap, we want to add the full circle but we have to rotate first
+            int pos = -1;
+            List<Node> ln = new ArrayList<>(roundabout.getNodes());
+            ln.remove(0);
+            for (int i = 0; i < ln.size(); i++) {
+                LatLon current = roundabout.getNode(i).getCoor();
+                if (current.equals(first) || current.equals(last)) {
+                    pos = i;
+                    break;
+                }
+            }
+            Collections.rotate(ln, -pos);
+            ln.add(ln.get(0));
+            for (Node n: ln) {
+                WayPoint point = OsmDataLayer.nodeToWayPoint(n, Long.MIN_VALUE);
+                if (hasFirst)
+                    trkseg.add(0, point);
+                else
+                    trkseg.add(point);
+            }
+        }
+    }
+
     private <T> Iterator<T> modeAwareIterator(List<T> list) {
         return mode.contains(FROM_FIRST_MEMBER)
                 ? list.iterator()