From 50013bf2e8ae246e65099ce605588f8710ccb191 Mon Sep 17 00:00:00 2001 From: simonpoole Date: Sun, 1 Dec 2024 11:14:44 +0100 Subject: [PATCH] Support extracting a segment from closed ways --- .../blau/android/osm/GeometryEditsTest.java | 22 +- src/main/java/de/blau/android/Logic.java | 19 +- .../ClosedWaySplittingActionModeCallback.java | 29 ++- .../WaySelectionActionModeCallback.java | 2 +- ...nClosedWaySplittingActionModeCallback.java | 42 ++-- src/main/java/de/blau/android/osm/Result.java | 10 + .../de/blau/android/osm/StorageDelegator.java | 212 ++++++++++++------ 7 files changed, 222 insertions(+), 114 deletions(-) diff --git a/src/androidTest/java/de/blau/android/osm/GeometryEditsTest.java b/src/androidTest/java/de/blau/android/osm/GeometryEditsTest.java index a8887461a3..895c862249 100644 --- a/src/androidTest/java/de/blau/android/osm/GeometryEditsTest.java +++ b/src/androidTest/java/de/blau/android/osm/GeometryEditsTest.java @@ -324,10 +324,10 @@ public void closedWaySplit() { final Node n5 = nList1.get(4); assertEquals(n1, n5); assertTrue(w1.isClosed()); - Way[] ways = logic.performClosedWaySplit(main, w1, n2, n4, false); - assertEquals(2, ways.length); - assertEquals(3, ways[0].getNodes().size()); - assertEquals(3, ways[1].getNodes().size()); + List results = logic.performClosedWaySplit(main, w1, n2, n4, false); + assertEquals(2, results.size()); + assertEquals(3, ((Way) results.get(0).getElement()).getNodes().size()); + assertEquals(3, ((Way) results.get(1).getElement()).getNodes().size()); } catch (Exception igit) { fail(igit.getMessage()); } @@ -361,12 +361,14 @@ public void closedWaySplitToPolygons() { final Node n5 = nList1.get(4); assertEquals(n1, n5); assertTrue(w1.isClosed()); - Way[] ways = logic.performClosedWaySplit(main, w1, n1, n3, true); - assertEquals(2, ways.length); - assertEquals(4, ways[0].getNodes().size()); - assertTrue(ways[0].isClosed()); - assertEquals(4, ways[1].getNodes().size()); - assertTrue(ways[1].isClosed()); + List results = logic.performClosedWaySplit(main, w1, n1, n3, true); + assertEquals(2, results.size()); + final Way way0 = (Way) results.get(0).getElement(); + assertEquals(4, way0.getNodes().size()); + assertTrue(way0.isClosed()); + final Way way1 = (Way) results.get(1).getElement(); + assertEquals(4, way1.getNodes().size()); + assertTrue(way1.isClosed()); } catch (Exception igit) { fail(igit.getMessage()); } diff --git a/src/main/java/de/blau/android/Logic.java b/src/main/java/de/blau/android/Logic.java index 6f7a2fcbbb..79bd3cf18a 100644 --- a/src/main/java/de/blau/android/Logic.java +++ b/src/main/java/de/blau/android/Logic.java @@ -2144,17 +2144,18 @@ public synchronized List performSplit(@Nullable final FragmentActivity a * @param node1 first split point * @param node2 second split point * @param createPolygons create polygons by closing the split ways if true - * @return null if the split fails, the two ways otherwise + * @return a List of Result objects containing the original Way in the 1st element and the new Way in the 2ndand any + * issues * @throws OsmIllegalOperationException if the operation failed * @throws StorageException if we ran out of memory */ @NonNull - public synchronized Way[] performClosedWaySplit(@Nullable FragmentActivity activity, @NonNull Way way, @NonNull Node node1, @NonNull Node node2, + public synchronized List performClosedWaySplit(@Nullable FragmentActivity activity, @NonNull Way way, @NonNull Node node1, @NonNull Node node2, boolean createPolygons) { createCheckpoint(activity, R.string.undo_action_split_way); try { displayAttachedObjectWarning(activity, way); - Way[] result = getDelegator().splitAtNodes(way, node1, node2, createPolygons); + List result = getDelegator().splitAtNodes(way, node1, node2, createPolygons); invalidateMap(); return result; } catch (OsmIllegalOperationException | StorageException ex) { @@ -2167,10 +2168,10 @@ public synchronized Way[] performClosedWaySplit(@Nullable FragmentActivity activ * Extract a segment from a way (the way between two nodes of the same way) * * @param activity activity we were called fron - * @param way Unclosed Way to split + * @param way Way to split * @param node1 first split point * @param node2 second split point - * @return null if the split fails, the segment otherwise + * @return the segment in the 1st Result if successful, otherwise the results contain issues */ @NonNull public synchronized List performExtractSegment(@Nullable FragmentActivity activity, @NonNull Way way, @NonNull Node node1, @NonNull Node node2) { @@ -2178,15 +2179,15 @@ public synchronized List performExtractSegment(@Nullable FragmentActivit try { displayAttachedObjectWarning(activity, way); List result = null; - if (way.isEndNode(node1)) { + if (way.isClosed()) { + result = getDelegator().splitAtNodes(way, node1, node2, false); + return result.subList(1, result.size()); // extracted segment is in the 2nd result + } else if (way.isEndNode(node1)) { result = extractSegmentAtEnd(way, node1, node2); } else if (way.isEndNode(node2)) { result = extractSegmentAtEnd(way, node2, node1); } else { result = getDelegator().splitAtNode(way, node1, true); - if (result.isEmpty()) { - throw new OsmIllegalOperationException("Splitting way " + way.getOsmId() + " at node " + node1.getOsmId() + " failed"); - } Result first = result.get(0); boolean splitOriginal = way.hasNode(node2); Way newWay = (Way) first.getElement(); diff --git a/src/main/java/de/blau/android/easyedit/ClosedWaySplittingActionModeCallback.java b/src/main/java/de/blau/android/easyedit/ClosedWaySplittingActionModeCallback.java index c53a43f2ef..a86d95cd9d 100644 --- a/src/main/java/de/blau/android/easyedit/ClosedWaySplittingActionModeCallback.java +++ b/src/main/java/de/blau/android/easyedit/ClosedWaySplittingActionModeCallback.java @@ -7,10 +7,12 @@ import android.util.Log; import androidx.annotation.NonNull; +import de.blau.android.dialogs.ElementIssueDialog; import de.blau.android.exception.OsmIllegalOperationException; import de.blau.android.exception.StorageException; import de.blau.android.osm.Node; import de.blau.android.osm.OsmElement; +import de.blau.android.osm.Result; import de.blau.android.osm.Way; import de.blau.android.util.SerializableState; @@ -88,17 +90,24 @@ public boolean handleElementClick(OsmElement element) { // NOSONAR super.handleElementClick(element); try { if (element instanceof Node) { - Way[] result = logic.performClosedWaySplit(main, way, node, (Node) element, createPolygons); - if (result.length == 2) { - logic.setSelectedNode(null); - logic.setSelectedRelation(null); - logic.setSelectedWay(result[0]); - logic.addSelectedWay(result[1]); - List selection = new ArrayList<>(); - selection.addAll(logic.getSelectedWays()); - main.startSupportActionMode(new MultiSelectWithGeometryActionModeCallback(manager, selection)); - return true; + List results = logic.performClosedWaySplit(main, way, node, (Node) element, createPolygons); + logic.setSelectedNode(null); + logic.setSelectedRelation(null); + logic.setSelectedWay((Way) results.get(0).getElement()); + logic.addSelectedWay((Way) results.get(1).getElement()); + List selection = new ArrayList<>(); + selection.addAll(logic.getSelectedWays()); + main.startSupportActionMode(new MultiSelectWithGeometryActionModeCallback(manager, selection)); + List resultsWithIssue = new ArrayList<>(); + for (Result r : results) { + if (r.hasIssue()) { + resultsWithIssue.add(r); + } } + if (!resultsWithIssue.isEmpty()) { + ElementIssueDialog.showTagConflictDialog(main, resultsWithIssue); + } + return true; } } catch (OsmIllegalOperationException | StorageException ex) { // toast has already been displayed diff --git a/src/main/java/de/blau/android/easyedit/WaySelectionActionModeCallback.java b/src/main/java/de/blau/android/easyedit/WaySelectionActionModeCallback.java index 3dfa1aa96a..a7cc173f46 100644 --- a/src/main/java/de/blau/android/easyedit/WaySelectionActionModeCallback.java +++ b/src/main/java/de/blau/android/easyedit/WaySelectionActionModeCallback.java @@ -200,7 +200,7 @@ public boolean onPrepareActionMode(ActionMode mode, Menu menu) { updated |= setItemVisibility(joined, unjoinItem, false); updated |= setItemVisibility(joined, unjoinDissimilarItem, false); - updated |= setItemVisibility(size >= 3 && !closed, extractSegmentItem, false); + updated |= setItemVisibility(size >= 3, extractSegmentItem, false); if (updated) { arrangeMenu(menu); diff --git a/src/main/java/de/blau/android/easyedit/turnrestriction/RestrictionClosedWaySplittingActionModeCallback.java b/src/main/java/de/blau/android/easyedit/turnrestriction/RestrictionClosedWaySplittingActionModeCallback.java index da1c7b4c36..50cf803d77 100644 --- a/src/main/java/de/blau/android/easyedit/turnrestriction/RestrictionClosedWaySplittingActionModeCallback.java +++ b/src/main/java/de/blau/android/easyedit/turnrestriction/RestrictionClosedWaySplittingActionModeCallback.java @@ -1,5 +1,7 @@ package de.blau.android.easyedit.turnrestriction; +import static de.blau.android.contract.Constants.LOG_TAG_LEN; + import java.util.HashSet; import java.util.List; import java.util.Map; @@ -24,10 +26,13 @@ * */ public class RestrictionClosedWaySplittingActionModeCallback extends AbstractClosedWaySplittingActionModeCallback { - private static final String DEBUG_TAG = RestrictionClosedWaySplittingActionModeCallback.class.getSimpleName().substring(0, Math.min(23, RestrictionClosedWaySplittingActionModeCallback.class.getSimpleName().length())); - private final Way way; - private final Node node; - private final Way fromWay; + + private static final int TAG_LEN = Math.min(LOG_TAG_LEN, RestrictionClosedWaySplittingActionModeCallback.class.getSimpleName().length()); + private static final String DEBUG_TAG = RestrictionClosedWaySplittingActionModeCallback.class.getSimpleName().substring(0, TAG_LEN); + + private final Way way; + private final Node node; + private final Way fromWay; /** * Construct a new callback for splitting a closed way/polygon as part of a turn restriction @@ -58,22 +63,23 @@ public boolean handleElementClick(OsmElement element) { // NOSONAR super.handleElementClick(element); try { if (element instanceof Node) { - Way[] result = logic.performClosedWaySplit(main, way, node, (Node) element, false); - if (result.length == 2) { - if (fromWay == null) { - Set candidates = new HashSet<>(); - candidates.add(result[0]); - candidates.add(result[1]); - main.startSupportActionMode(new RestartFromElementActionModeCallback(manager, candidates, candidates, savedResults)); - } else { - Way viaWay = result[0]; - if (fromWay.hasCommonNode(result[1])) { - viaWay = result[1]; - } - main.startSupportActionMode(new ViaElementActionModeCallback(manager, fromWay, viaWay, savedResults)); + List results = logic.performClosedWaySplit(main, way, node, (Node) element, false); + // FIXME we currently don't display any issues as that would be confusing + Way way0 = (Way) results.get(0).getElement(); + Way way1 = (Way) results.get(1).getElement(); + if (fromWay == null) { + Set candidates = new HashSet<>(); + candidates.add(way0); + candidates.add(way1); + main.startSupportActionMode(new RestartFromElementActionModeCallback(manager, candidates, candidates, savedResults)); + } else { + Way viaWay = way0; + if (fromWay.hasCommonNode(way1)) { + viaWay = way1; } - return true; + main.startSupportActionMode(new ViaElementActionModeCallback(manager, fromWay, viaWay, savedResults)); } + return true; } } catch (OsmIllegalOperationException | StorageException ex) { // toast has already been displayed diff --git a/src/main/java/de/blau/android/osm/Result.java b/src/main/java/de/blau/android/osm/Result.java index 2bfae7b820..f3f6218b05 100644 --- a/src/main/java/de/blau/android/osm/Result.java +++ b/src/main/java/de/blau/android/osm/Result.java @@ -106,10 +106,20 @@ public Collection getIssues() { * * @return the element */ + @Nullable public OsmElement getElement() { return element; } + /** + * Check if this result contains an OsmElement + * + * @return true if an element is present + */ + public boolean hasElement() { + return element != null; + } + /** * Set the stored OsmElement * diff --git a/src/main/java/de/blau/android/osm/StorageDelegator.java b/src/main/java/de/blau/android/osm/StorageDelegator.java index 6e82f73d8d..4d2947a0b2 100755 --- a/src/main/java/de/blau/android/osm/StorageDelegator.java +++ b/src/main/java/de/blau/android/osm/StorageDelegator.java @@ -11,6 +11,7 @@ import java.io.Serializable; import java.net.ProtocolException; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.HashMap; @@ -53,6 +54,7 @@ import de.blau.android.util.SavingHelper.Exportable; import de.blau.android.util.ScreenMessage; import de.blau.android.util.Util; +import de.blau.android.util.Winding; import de.blau.android.util.collections.LongHashSet; import de.blau.android.util.collections.LongOsmElementMap; import de.blau.android.util.collections.MultiHashMap; @@ -65,7 +67,8 @@ public class StorageDelegator implements Serializable, Exportable, DataStorage { private static final long serialVersionUID = 11L; - public static final int MIN_NODES_CIRCLE = 3; + public static final int MIN_NODES_CIRCLE = 3; + private static final int MINIMUN_NODES_FOR_WAY_SPLIT = 3; private Storage currentStorage; @@ -1250,105 +1253,158 @@ public void removeNode(@NonNull final Node node) { } /** - * Split a (closed) way at two points + * Split a closed way at two points * * @param way way to split * @param node1 first node to split at * @param node2 second node to split at * @param createPolygons split in to two polygons - * @return null if split failed or wasn't possible, the two resulting ways otherwise + * @return the original Way in the 1st Result, the new Way in the 2nd Result, issues if not successful */ @NonNull - public Way[] splitAtNodes(@NonNull Way way, @NonNull Node node1, @NonNull Node node2, boolean createPolygons) { + public List splitAtNodes(@NonNull Way way, @NonNull Node node1, @NonNull Node node2, boolean createPolygons) { Log.d(DEBUG_TAG, "splitAtNodes way " + way.getOsmId() + " node1 " + node1.getOsmId() + " node2 " + node2.getOsmId()); + Result resultOrig = new Result(); + Result resultNew = new Result(); // undo - old way is saved here, new way is saved at insert dirty = true; undo.save(way); List nodes = way.getNodes(); - if (nodes.size() < 3) { + if (nodes.size() < MINIMUN_NODES_FOR_WAY_SPLIT) { throw new OsmIllegalOperationException("Closed way with less than three nodes cannot be split"); } + int winding = Winding.winding(nodes); + int pos1 = nodes.indexOf(node1); + int pos2 = nodes.indexOf(node2); + validateRelationMemberCount(way.getParentRelations(), 1); + List metricKeys = getMetricKeys(way); + double originalLength = getWayLength(way, metricKeys); + /* * convention iterate over list, copy everything between first split node found and 2nd split node found if 2nd * split node found first the same */ - List nodesForNewWay = new LinkedList<>(); - List nodesForOldWay1 = new LinkedList<>(); - List nodesForOldWay2 = new LinkedList<>(); + List nodesExtracted = new LinkedList<>(); + List nodesEnd1 = new LinkedList<>(); + List nodesEnd2 = new LinkedList<>(); boolean found1 = false; boolean found2 = false; - for (Iterator it = way.getNodeIterator(); it.hasNext();) { - Node wayNode = it.next(); - if (!found1 && wayNode.getOsmId() == node1.getOsmId()) { + final long node1Id = node1.getOsmId(); + final long node2Id = node2.getOsmId(); + for (Node wayNode : nodes) { + if (!found1 && wayNode.getOsmId() == node1Id) { found1 = true; - nodesForNewWay.add(wayNode); + nodesExtracted.add(wayNode); if (!found2) { - nodesForOldWay1.add(wayNode); + nodesEnd1.add(wayNode); } else { - nodesForOldWay2.add(wayNode); + nodesEnd2.add(wayNode); } - } else if (!found2 && wayNode.getOsmId() == node2.getOsmId()) { + } else if (!found2 && wayNode.getOsmId() == node2Id) { found2 = true; - nodesForNewWay.add(wayNode); + nodesExtracted.add(wayNode); if (!found1) { - nodesForOldWay1.add(wayNode); + nodesEnd1.add(wayNode); } else { - nodesForOldWay2.add(wayNode); + nodesEnd2.add(wayNode); } } else if ((found1 && !found2) || (!found1 && found2)) { - nodesForNewWay.add(wayNode); + nodesExtracted.add(wayNode); } else if (!found1) { - nodesForOldWay1.add(wayNode); + nodesEnd1.add(wayNode); } else { - nodesForOldWay2.add(wayNode); - } - } - - // shuffle the nodes around for the original way so that they are in sequence and the way isn't closed - Log.d(DEBUG_TAG, "nodesForNewWay " + nodesForNewWay.size() + " oldNodes1 " + nodesForOldWay1.size() + " oldNodes2 " + nodesForOldWay2.size()); - List oldNodes = way.getNodes(); - oldNodes.clear(); - if (nodesForOldWay1.isEmpty()) { - oldNodes.addAll(nodesForOldWay2); - } else if (nodesForOldWay2.isEmpty()) { - oldNodes.addAll(nodesForOldWay1); - } else if (nodesForOldWay1.get(0) == nodesForOldWay2.get(nodesForOldWay2.size() - 1)) { - oldNodes.addAll(nodesForOldWay2); - nodesForOldWay1.remove(0); - oldNodes.addAll(nodesForOldWay1); + nodesEnd2.add(wayNode); + } + } + + // shuffle the nodes around from the original way so that they are in sequence and the way isn't closed + Log.d(DEBUG_TAG, "nodesForNewWay " + nodesExtracted.size() + " oldNodes1 " + nodesEnd1.size() + " oldNodes2 " + nodesEnd2.size()); + + // create the new way + Way newWay = factory.createWayWithNewId(); + newWay.addTags(way.getTags()); + + // enforce behaviour that first and second node are relative to the winding of the ring + if ((winding == Winding.CLOCKWISE && pos2 > pos1) || (winding == Winding.COUNTERCLOCKWISE && pos1 < pos2)) { + addEndSegmentsToWay(way, nodesEnd1, nodesEnd2); + newWay.getNodes().clear(); + newWay.addNodes(nodesExtracted, false); } else { - oldNodes.addAll(nodesForOldWay1); - nodesForOldWay2.remove(0); - oldNodes.addAll(nodesForOldWay2); + addEndSegmentsToWay(newWay, nodesEnd1, nodesEnd2); + way.getNodes().clear(); + way.addNodes(nodesExtracted, false); } - List changedElements = new ArrayList<>(); - if (createPolygons && way.nodeCount() > Way.MINIMUM_NODES_IN_WAY) { // close the original way now - way.addNode(way.getFirstNode()); + + if (createPolygons) { + closeWay(way); + closeWay(newWay); } + + List changedElements = new ArrayList<>(); way.updateState(OsmElement.STATE_MODIFIED); apiStorage.insertElementSafe(way); changedElements.add(way); - // create the new way - Way newWay = factory.createWayWithNewId(); - newWay.addTags(way.getTags()); - newWay.addNodes(nodesForNewWay, false); - if (createPolygons && newWay.nodeCount() > Way.MINIMUM_NODES_IN_WAY) { // close the new way now - newWay.addNode(newWay.getFirstNode()); - } insertElementUnsafe(newWay); - addSplitWayToRelations(way, true, newWay, changedElements); + if (!metricKeys.isEmpty() && originalLength != 0) { + resultOrig.addIssue(SplitIssue.SPLIT_METRIC); + resultNew.addIssue(SplitIssue.SPLIT_METRIC); + for (String key : metricKeys) { + distributeMetric(key, originalLength, way); + distributeMetric(key, originalLength, newWay); + } + } + + List relationResults = addSplitWayToRelations(way, true, newWay, changedElements); onElementChanged(null, changedElements); - Way[] result = new Way[2]; - result[0] = way; - result[1] = newWay; - return result; + + resultOrig.setElement(way); + resultNew.setElement(newWay); + List resultList = Arrays.asList(resultOrig, resultNew); + resultList.addAll(relationResults); + return resultList; + } + + /** + * Add way nodes from two end segments of a (formerly) existing way to a way + * + * @param way the Way + * @param nodesEndSegment1 nodes from 1st segment + * @param nodesForEndSegment2 nodes from 2nd segment + */ + private void addEndSegmentsToWay(@NonNull Way way, @NonNull List nodesEndSegment1, @NonNull List nodesForEndSegment2) { + List nodes = way.getNodes(); + nodes.clear(); + if (nodesEndSegment1.isEmpty()) { + nodes.addAll(nodesForEndSegment2); + } else if (nodesForEndSegment2.isEmpty()) { + nodes.addAll(nodesEndSegment1); + } else if (nodesEndSegment1.get(0) == nodesForEndSegment2.get(nodesForEndSegment2.size() - 1)) { + nodes.addAll(nodesForEndSegment2); + nodesEndSegment1.remove(0); + nodes.addAll(nodesEndSegment1); + } else { + nodes.addAll(nodesEndSegment1); + nodesForEndSegment2.remove(0); + nodes.addAll(nodesForEndSegment2); + } + } + + /** + * Close a way by adding the 1st node to the end + * + * @param way the Way + */ + private void closeWay(@NonNull Way way) { + if (way.nodeCount() > Way.MINIMUM_NODES_IN_WAY) { // close the original way now + way.addNode(way.getFirstNode()); + } } /** @@ -1377,18 +1433,8 @@ public List splitAtNode(@NonNull final Way way, @NonNull final Node node } validateRelationMemberCount(way.getParentRelations(), 1); - // check tags for problematic keys - List metricKeys = new ArrayList<>(); - for (String key : way.getTags().keySet()) { - if (Tags.isWayMetric(key)) { - metricKeys.add(key); - } - } - // determine the length before we remove nodes - double originalLength = 1D; - if (!metricKeys.isEmpty()) { - originalLength = way.length(); - } + List metricKeys = getMetricKeys(way); + double originalLength = getWayLength(way, metricKeys); // we assume this node is only contained in the way once. // else the user needs to split the remaining way again. @@ -1424,6 +1470,40 @@ public List splitAtNode(@NonNull final Way way, @NonNull final Node node return resultList; } + /** + * Returnn the length of the way is thaere are metric keys + * + * @param way the way + * @param metricKeys a list of relevant keys + * @return the length + */ + private double getWayLength(@NonNull final Way way, @NonNull List metricKeys) { + // determine the length before we remove nodes + double originalLength = 1D; + if (!metricKeys.isEmpty()) { + originalLength = way.length(); + } + return originalLength; + } + + /** + * Get a list of length dependent keys that thw Way has + * + * @param way the Way + * @return a list of keys + */ + @NonNull + private List getMetricKeys(@NonNull final Way way) { + // check tags for problematic keys + List metricKeys = new ArrayList<>(); + for (String key : way.getTags().keySet()) { + if (Tags.isWayMetric(key)) { + metricKeys.add(key); + } + } + return metricKeys; + } + /** * Get nodes for the split of way keeping the initial ones *