Index: src/org/openstreetmap/josm/actions/JoinAreasAction.java =================================================================== --- src/org/openstreetmap/josm/actions/JoinAreasAction.java (Revision 11780) +++ src/org/openstreetmap/josm/actions/JoinAreasAction.java (Arbeitskopie) @@ -5,6 +5,7 @@ import static org.openstreetmap.josm.tools.I18n.tr; import static org.openstreetmap.josm.tools.I18n.trn; +import java.awt.GridBagLayout; import java.awt.event.ActionEvent; import java.awt.event.KeyEvent; import java.util.ArrayList; @@ -12,6 +13,7 @@ import java.util.Collections; import java.util.Comparator; import java.util.EnumSet; +import java.util.HashMap; import java.util.HashSet; import java.util.LinkedHashSet; import java.util.LinkedList; @@ -31,6 +33,7 @@ import java.util.stream.Stream; import javax.swing.JOptionPane; +import javax.swing.JPanel; import org.openstreetmap.josm.Main; import org.openstreetmap.josm.actions.ReverseWayAction.ReverseWayResult; @@ -37,6 +40,7 @@ import org.openstreetmap.josm.actions.SplitWayAction.SplitWayResult; import org.openstreetmap.josm.command.AddCommand; import org.openstreetmap.josm.command.ChangeCommand; +import org.openstreetmap.josm.command.ChangePropertyCommand; import org.openstreetmap.josm.command.Command; import org.openstreetmap.josm.command.DeleteCommand; import org.openstreetmap.josm.command.SequenceCommand; @@ -46,13 +50,17 @@ import org.openstreetmap.josm.data.osm.Node; import org.openstreetmap.josm.data.osm.NodePositionComparator; import org.openstreetmap.josm.data.osm.OsmPrimitive; +import org.openstreetmap.josm.data.osm.OsmPrimitiveType; import org.openstreetmap.josm.data.osm.Relation; import org.openstreetmap.josm.data.osm.RelationMember; import org.openstreetmap.josm.data.osm.TagCollection; import org.openstreetmap.josm.data.osm.Way; +import org.openstreetmap.josm.gui.ConditionalOptionPaneUtil; +import org.openstreetmap.josm.gui.DefaultNameFormatter; import org.openstreetmap.josm.gui.Notification; import org.openstreetmap.josm.gui.conflict.tags.CombinePrimitiveResolverDialog; import org.openstreetmap.josm.gui.layer.OsmDataLayer; +import org.openstreetmap.josm.gui.widgets.JMultilineLabel; import org.openstreetmap.josm.tools.Geometry; import org.openstreetmap.josm.tools.JosmRuntimeException; import org.openstreetmap.josm.tools.Pair; @@ -72,11 +80,525 @@ private final transient List addedRelations = new LinkedList<>(); /** + * Defines an exception while joining areas. + * @author Michael Zangl + */ + static class JoinAreasException extends Exception { + public JoinAreasException(String message) { + super(message); + } + } + + static class UnclosedAreaException extends JoinAreasException { + + private Pair gap; + + public UnclosedAreaException(Pair gap) { + super("Gap found between: " + gap.a + " and " + gap.b); + this.gap = gap; + } + + } + + static class SelfIntersectingAreaException extends JoinAreasException { + + private Pair intersect; + + public SelfIntersectingAreaException(Pair intersect) { + super("Intersection found between: " + intersect.a + " and " + intersect.b); + this.intersect = intersect; + } + + } + + static class UndirectedWaySegment { + private Node a; + private Node b; + + UndirectedWaySegment(Node a, Node b) { + if (a == b) { + throw new IllegalArgumentException("Way segment cannot start and end at the same node."); + } + this.a = a; + this.b = b; + } + + public boolean hasEnd(Node current) { + return a == current || b == current; + } + + public Node getOtherEnd(Node current) { + if (current == a) { + return b; + } else if (current == b) { + return a; + } else { + throw new IllegalArgumentException(current + " is not an endpoint"); + } + } + + public boolean intersects(UndirectedWaySegment other) { + EastNorth intersection = getIntersectionPoint(other); + return intersection != null; + } + + private EastNorth getIntersectionPoint(UndirectedWaySegment other) { + EastNorth intersection = null; + if (!hasEnd(other.a) && !hasEnd(other.b)) { + // ignore just touching. + intersection = Geometry.getSegmentSegmentIntersection( + a.getEastNorth(), b.getEastNorth(), + other.a.getEastNorth(), other.b.getEastNorth()); + } + return intersection; + } + + @Override + public int hashCode() { + return a.hashCode() + b.hashCode(); + } + + @Override + public boolean equals(Object obj) { + if (this.getClass() == obj.getClass()) { + UndirectedWaySegment other = (UndirectedWaySegment) obj; + return (a.equals(other.a) && b.equals(other.b)) || (a.equals(other.b) && b.equals(other.a)); + } else { + return false; + } + } + + @Override + public String toString() { + return "UndirectedWaySegment [" + a + ", " + b + "]"; + } + + } + + /** + * This class defines an area that might be joined. + * @author Michael Zangl + */ + static class JoinableArea { + /** + * A list of Node->Node segments that compose this area. + * You can reconstruct the interior of this area by XORing those lines. + */ + private final HashSet waySegments = new HashSet<>(); + private final List ways = new ArrayList<>(); + private final List relations = new ArrayList<>(); + private final Map tags; + private final OsmPrimitive basePrimitive; + + JoinableArea(Way way) throws JoinAreasException { + this(way, Collections.singleton(way), Collections.emptyList()); + } + + JoinableArea(Relation relation) throws JoinAreasException { + this(relation, getMembers(relation, "outer"), getMembers(relation, "inner")); + relations.add(relation); + } + + /** + * Creates a new joinable area. + * @param base The primitive this area is for. + * @param outer The ways that should be outer ways. + * @param inner The ways that should be inner ways. + * @throws JoinAreasException If the area is invalid + */ + JoinableArea(OsmPrimitive base, Collection outer, Collection inner) throws JoinAreasException { + basePrimitive = base; + tags = new HashMap<>(base.getInterestingTags()); + tags.remove("type", "multipolygon"); + + try { + for (Way o : outer) { + addWayForceNonintersecting(o); + } + Pair outerGap = findGap(); + if (outerGap != null) { + throw new UnclosedAreaException(outerGap); + } + + for (Way i : inner) { + addWayForceNonintersecting(i); + } + Pair innerGap = findGap(); + if (innerGap != null) { + throw new UnclosedAreaException(innerGap); + } + } catch (RuntimeException e) { + throw BugReport.intercept(e).put("outer", outer).put("inner", inner); + } + } + + /** + * Check if this area is a valid closed area + * @return The gap if there is one, null for closed areas. + */ + private Pair findGap() { + HashSet leftOver = new HashSet<>(waySegments); + while (!leftOver.isEmpty()) { + LinkedList part = removeOutlinePart(leftOver); + if (part.getFirst() != part.getLast()) { + return new Pair<>(part.getFirst(), part.getLast()); + } + } + return null; + } + + /** + * Add a new Way to the outline (outer or inner) of this area. + * @param way The way. + * @throws SelfIntersectingAreaException If the way self-intersects + */ + private void addWayForceNonintersecting(Way way) throws SelfIntersectingAreaException { + for (Pair pair : way.getNodePairs(false)) { + this.addWayForceNonintersecting(new UndirectedWaySegment(pair.a, pair.b)); + } + ways .add(way); + } + + private void addWayForceNonintersecting(UndirectedWaySegment s) throws SelfIntersectingAreaException { + if (waySegments.contains(s)) { + // We add a way segment twice. This means that the outline of the area contains this segment twice. + // This cancels out, so we remove the segment, + waySegments.remove(s); + } else { + // Now check for intersections + Optional intersection = waySegments.stream().filter(s::intersects).findAny(); + if (intersection.isPresent()) { + throw new SelfIntersectingAreaException(new Pair<>(intersection.get(), s)); + } + waySegments.add(s); + } + } + + private static Collection getMembers(Relation relation, String role) { + return relation.getMembers().stream().filter(m -> role.equals(m.getRole())) + .filter(m -> OsmPrimitiveType.WAY.equals(m.getType())).map(m -> m.getWay()) + .collect(Collectors.toList()); + } + + /** + * Check if the area contains a segment. + * @param segment The segment. Assumed to not intersect any of our borders. + * @return true if the segment is inside. False if it is on the outline or outside. + */ + public boolean contains(UndirectedWaySegment segment) { + if (waySegments.contains(segment)) { + return false; + } + // To find out which side of the way the outer side is, we can follow a ray starting anywhere at the way in any direction. + // Computation is done in East/North space. + // We use a ray at a fixed yRay coordinate that ends at xRay; + // we need to make sure this ray does not go into the same direction the way is going. + // This is done by rotating by 90° if we need to. + + int intersections = 0; + // Use some "random" start point on the segment + EastNorth rayNode1 = segment.a.getEastNorth(); + EastNorth rayNode2 = segment.b.getEastNorth(); + EastNorth rayFrom = rayNode1.getCenter(rayNode2); + + // Now find the x/y mapping function. We need to ensure that rayNode1->rayNode2 is not parallel to our x axis. + ToDoubleFunction x; + ToDoubleFunction y; + if (Math.abs(rayNode1.east() - rayNode2.east()) < Math.abs(rayNode1.north() - rayNode2.north())) { + x = en -> en.east(); + y = en -> en.north(); + } else { + x = en -> -en.north(); + y = en -> en.east(); + } + + double xRay = x.applyAsDouble(rayFrom); + double yRay = y.applyAsDouble(rayFrom); + + for (UndirectedWaySegment part : waySegments) { + // intersect against all way segments + EastNorth n1 = part.a.getEastNorth(); + EastNorth n2 = part.b.getEastNorth(); + if ((rayNode1.equals(n1) && rayNode2.equals(n2)) || (rayNode2.equals(n1) && rayNode1.equals(n2))) { + // This is the segment we are starting the ray from. + // We ignore this to avoid rounding errors. + continue; + } + + double x1 = x.applyAsDouble(n1); + double x2 = x.applyAsDouble(n2); + double y1 = y.applyAsDouble(n1); + double y2 = y.applyAsDouble(n2); + + if (!((y1 <= yRay && yRay < y2) || (y2 <= yRay && yRay < y1))) { + // No intersection, since segment is above/below ray + continue; + } + double xIntersect = x1 + (x2 - x1) * (yRay - y1) / (y2 - y1); + double onLine = xIntersect / xRay; + if (Math.abs(onLine - 1) < 1e-10) { + // Lines that are directly on each other are considered outside. + return false; + } + if (xIntersect < xRay) { + intersections++; + } + } + + return intersections % 2 == 1; + } + + public Collection getSegments() { + return Collections.unmodifiableCollection(waySegments); + } + } + + /** + * A hash set with an xor method. + * @param element type + */ + private static class XOrHashSet extends HashSet { + public XOrHashSet() { + super(); + } + + public XOrHashSet(Collection c) { + super(c); + } + + public void xor(T e) { + if (!this.add(e)) { + this.remove(e); + } + } + } + + /** + * This class collects the areas to be joined. + */ + static class JoinAreasCollector { + private final List possibleNewNodes = new ArrayList<>(); + private final List unionOf = new ArrayList<>(); + /** + * All hash sets that may be + */ + private final XOrHashSet waySegments = new XOrHashSet<>(); + private final DataSet ds; + private final Map tags; + + JoinAreasCollector(DataSet ds, Collection waysAndRelations) throws JoinAreasException { + this.ds = ds; + Collection collectAreas = collectAreas(waysAndRelations); + collectAreas.forEach(this::unionWithArea); + + tags = unionOf.isEmpty() ? Collections.emptyMap() : unionOf.iterator().next().tags; + } + + private static Collection collectAreas(Collection waysAndRelations) throws JoinAreasException { + Collection areas = new ArrayList<>(); + for(OsmPrimitive osm : waysAndRelations) { + if (osm instanceof Way) { + areas.add(new JoinableArea((Way) osm)); + } else if (osm instanceof Relation) { + areas.add(new JoinableArea((Relation) osm)); + } + } + return areas; + } + + void unionWithArea(JoinableArea area) { + Collection segments = area.getSegments(); + + // Our worker list. Once a way is split, it is re-added to the wroker to check for more splits. + XOrHashSet toAdd = new XOrHashSet<>(segments); + while (!toAdd.isEmpty()) { + UndirectedWaySegment s = toAdd.iterator().next(); + toAdd.remove(s); + Optional intersects = waySegments.stream().filter(s::intersects).findAny(); + if (intersects.isPresent()) { + EastNorth intersection = s.getIntersectionPoint(intersects.get()); + // Now generate two segments around the intersection. + waySegments.remove(intersects.get()); + // TODO: Find a node close to newNode to handle intersections of 3 or more lines. + Node newNode = new Node(intersection); + possibleNewNodes.add(newNode); + // We use xor here to fix ways that e.g. reverse on themselves. + waySegments.xor(new UndirectedWaySegment(intersects.get().a, newNode)); + waySegments.xor(new UndirectedWaySegment(newNode, intersects.get().b)); + + toAdd.xor(new UndirectedWaySegment(s.a, newNode)); + toAdd.xor(new UndirectedWaySegment(newNode, s.b)); + } else { + // No more intersections - we add that segment to our geometry + waySegments.xor(s); + } + } + + unionOf.add(area); + } + + private boolean allAreasHaveSameTags() { + return unionOf.stream().allMatch(area -> area.tags.equals(tags)); + } + + /** + * Gets the commands that are required to join the areas. + * @return The join commands. + */ + public List getCommands() { + if (unionOf.isEmpty()) { + return Collections.emptyList(); + } + Collection outline = computeOutline(); + + List commands = new ArrayList<>(); + // The primitives of which we should remove the tags. + List toRemoveTags = new ArrayList<>(); + unionOf.stream().map(area -> area.basePrimitive).forEach(toRemoveTags::add); + + // Add the split nodes + // Remove nodes of interior segments. + possibleNewNodes.stream() + .filter(n -> outline.stream().filter(w -> w.hasEnd(n)).findAny().isPresent()) + .map(n -> new AddCommand(ds, n)) + .forEach(commands::add); + + // Now search all ways which are completely used in our new geometry (e.g. multipolygon inners, ...) + // We should not change those ways. + List outlineWays = new ArrayList<>(); + List segmentsToContain = new ArrayList<>(outline); + for (Way preserve : findOutlinesToPreserve(segmentsToContain)) { + List preservedSegments = segmentsForWay(preserve); + if (preservedSegments.size() != preservedSegments.stream().distinct().count()) { + // This way contains a segment twice. Skip it, we want to fix this. + continue; + } + if (!segmentsToContain.containsAll(preservedSegments)) { + // it may happen that two outlines that should be preserved happen to be on the same segment + // We need to ignore the second one then. + continue; + } + outlineWays.add(preserve); + segmentsToContain.removeAll(preservedSegments); + } + + // Multipolygons that were selected and can now be removed + List relationsToRemove = unionOf.stream().flatMap(area -> area.relations.stream()) + .distinct().collect(Collectors.toList()); + toRemoveTags.removeAll(relationsToRemove); + + // Compute the ways that need to be removed. + // Those are all ways of the old geometry that are not used in any other place. + List waysToRemove = unionOf.stream().flatMap(area -> area.ways.stream()) + .distinct() + .filter(way -> !outlineWays.contains(way)) + // Preserve ways that are member in any relation that we did not modify + .filter(way -> way.getReferrers().stream().allMatch(relationsToRemove::contains)) + // Preserve ways that have tags + .filter(way -> toRemoveTags.contains(way) || way.getInterestingTags().isEmpty()) + .collect(Collectors.toList()); + toRemoveTags.removeAll(waysToRemove); + + // Now we are left with the remaining outline in the segmentsToContain array. + // For each chunk in that outline, we create a new way + // TODO: We can reuse the ways we would delete otherwise. + while (!segmentsToContain.isEmpty()) { + List wayToCreate = removeOutlinePart(segmentsToContain); + Way osm = new Way(); + osm.setNodes(wayToCreate); + outlineWays.add(osm); + commands.add(new AddCommand(ds, osm)); + } + + // Now it is time to generate the final area. + if (outlineWays.isEmpty()) { + throw new AssertionError("No outline ways found."); + } else if (outlineWays.size() == 1) { + // We only have one way. Add the tags to that way. + outlineWays.get(0).setKeys(tags); + } else { + // find a relation. Use the more complex multipolygon when merging two of them. + Relation multipolygon = relationsToRemove.stream().sorted(Comparator.comparingInt(r -> -r.getMembersCount())) + .findFirst().orElseGet(Relation::new); + multipolygon.setKeys(tags); + Pair update = CreateMultipolygonAction.updateMultipolygonRelation(outlineWays, multipolygon); + if (update == null) { + throw new AssertionError("The outline ways should be continuous but no multipolygon could be created."); + } + if (update.a.getDataSet() == null) { + // used the fake relation. + commands.add(new AddCommand(update.b)); + } else { + commands.add(new ChangeCommand(update.a, update.b)); + } + relationsToRemove.remove(multipolygon); + } + + // Apply deletion of the primitives we don't need any more. + if (!relationsToRemove.isEmpty()) { + commands.add(new DeleteCommand(ds, relationsToRemove)); + } + if (!waysToRemove.isEmpty()) { + commands.add(new DeleteCommand(ds, waysToRemove)); + } + for(OsmPrimitive osm : toRemoveTags) { + for (String key : osm.getKeys().keySet()) { + commands.add(new ChangePropertyCommand(osm, key, "")); + } + } + + return commands; + } + + private List segmentsForWay(Way way) { + return way.getNodePairs(false).stream() + .map(pair -> new UndirectedWaySegment(pair.a, pair.b)).collect(Collectors.toList()); + } + + private List findOutlinesToPreserve(List segmentsToContain) { + return unionOf.stream().flatMap(u -> u.ways.stream()) + .filter(w -> segmentsToContain.containsAll(segmentsForWay(w))).collect(Collectors.toList()); + } + + private Collection computeOutline() { + return waySegments.stream().filter( + seg -> unionOf.stream().noneMatch(area -> area.contains(seg)) + ).collect(Collectors.toList()); + } + } + + private static LinkedList removeOutlinePart(Collection segmentsToContain) { + Node start = segmentsToContain.iterator().next().a; + LinkedList nodes = new LinkedList<>(); + nodes.add(start); + // Move in one direction on that way. + while ((nodes.size() < 2 || nodes.getFirst() != nodes.getLast()) && !segmentsToContain.isEmpty()) { + Optional traverse = segmentsToContain.stream().filter(s -> s.hasEnd(nodes.getLast())).findAny(); + if (!traverse.isPresent()) { + break; + } + segmentsToContain.remove(traverse.get()); + nodes.addLast(traverse.get().getOtherEnd(nodes.getLast())); + } + + // Now move in the other direction - as far as we can go. + while ((nodes.size() < 2 || nodes.getFirst() != nodes.getLast()) && !segmentsToContain.isEmpty()) { + Optional traverse = segmentsToContain.stream().filter(s -> s.hasEnd(nodes.getFirst())).findAny(); + if (!traverse.isPresent()) { + break; + } + segmentsToContain.remove(traverse.get()); + nodes.addFirst(traverse.get().getOtherEnd(nodes.getFirst())); + } + + return nodes; + } + + /** * This helper class describes join areas action result. * @author viesturs */ public static class JoinAreasResult { - private final boolean hasChanges; private final List polygons; @@ -155,11 +677,12 @@ @Override public boolean equals(Object other) { - if (this == other) return true; - if (other == null || getClass() != other.getClass()) return false; + if (this == other) + return true; + if (other == null || getClass() != other.getClass()) + return false; RelationRole that = (RelationRole) other; - return Objects.equals(rel, that.rel) && - Objects.equals(role, that.role); + return Objects.equals(rel, that.rel) && Objects.equals(role, that.role); } } @@ -185,11 +708,12 @@ @Override public boolean equals(Object other) { - if (this == other) return true; - if (other == null || getClass() != other.getClass()) return false; + if (this == other) + return true; + if (other == null || getClass() != other.getClass()) + return false; WayInPolygon that = (WayInPolygon) other; - return insideToTheRight == that.insideToTheRight && - Objects.equals(way, that.way); + return insideToTheRight == that.insideToTheRight && Objects.equals(way, that.way); } @Override @@ -232,7 +756,7 @@ * Inverse inside and outside */ public void reverse() { - for (WayInPolygon way: ways) { + for (WayInPolygon way : ways) { way.insideToTheRight = !way.insideToTheRight; } Collections.reverse(ways); @@ -338,13 +862,13 @@ EastNorth en1 = n1.getEastNorth(); EastNorth en2 = n2.getEastNorth(); EastNorth en3 = n3.getEastNorth(); - double angle = Math.atan2(en3.getY() - en1.getY(), en3.getX() - en1.getX()) - - Math.atan2(en2.getY() - en1.getY(), en2.getX() - en1.getX()); - while (angle >= 2*Math.PI) { - angle -= 2*Math.PI; + double angle = Math.atan2(en3.getY() - en1.getY(), en3.getX() - en1.getX()) + - Math.atan2(en2.getY() - en1.getY(), en2.getX() - en1.getX()); + while (angle >= 2 * Math.PI) { + angle -= 2 * Math.PI; } while (angle < 0) { - angle += 2*Math.PI; + angle += 2 * Math.PI; } return angle; } @@ -362,28 +886,26 @@ // Pairs of (way, nextNode) lastWay = Stream.concat( - availableWays.stream() - .filter(way -> way.way.firstNode().equals(headNode) && way.insideToTheRight) - .map(way -> new Pair<>(way, way.way.getNode(1))), - availableWays.stream() - .filter(way -> way.way.lastNode().equals(headNode) && !way.insideToTheRight) - .map(way -> new Pair<>(way, way.way.getNode(way.way.getNodesCount() - 2)))) + availableWays.stream().filter(way -> way.way.firstNode().equals(headNode) && way.insideToTheRight) + .map(way -> new Pair<>(way, way.way.getNode(1))), + availableWays.stream().filter(way -> way.way.lastNode().equals(headNode) && !way.insideToTheRight) + .map(way -> new Pair<>(way, way.way.getNode(way.way.getNodesCount() - 2)))) - // now find the way with the best angle - .min(Comparator.comparingDouble(wayAndNext -> { - Node nextNode = wayAndNext.b; - if (nextNode == prevNode) { - // we always prefer going back. - return Double.POSITIVE_INFINITY; - } - double angle = Math.atan2(nextNode.getEastNorth().east() - headNode.getEastNorth().east(), - nextNode.getEastNorth().north() - headNode.getEastNorth().north()) - headAngle; - if (angle > Math.PI) - angle -= 2*Math.PI; - if (angle <= -Math.PI) - angle += 2*Math.PI; - return angle; - })).map(wayAndNext -> wayAndNext.a).orElse(null); + // now find the way with the best angle + .min(Comparator.comparingDouble(wayAndNext -> { + Node nextNode = wayAndNext.b; + if (nextNode == prevNode) { + // we always prefer going back. + return Double.POSITIVE_INFINITY; + } + double angle = Math.atan2(nextNode.getEastNorth().east() - headNode.getEastNorth().east(), + nextNode.getEastNorth().north() - headNode.getEastNorth().north()) - headAngle; + if (angle > Math.PI) + angle -= 2 * Math.PI; + if (angle <= -Math.PI) + angle += 2 * Math.PI; + return angle; + })).map(wayAndNext -> wayAndNext.a).orElse(null); lastWayReverse = lastWay != null && !lastWay.insideToTheRight; return lastWay; } @@ -398,7 +920,7 @@ WayInPolygon mostLeft = null; // most left way connected to head node boolean comingToHead = false; // true if candidate come to head node - double angle = 2*Math.PI; + double angle = 2 * Math.PI; for (WayInPolygon candidateWay : availableWays) { boolean candidateComingToHead; @@ -408,8 +930,8 @@ candidateComingToHead = !candidateWay.insideToTheRight; candidatePrevNode = candidateWay.way.getNode(1); } else if (candidateWay.way.lastNode().equals(headNode)) { - candidateComingToHead = candidateWay.insideToTheRight; - candidatePrevNode = candidateWay.way.getNode(candidateWay.way.getNodesCount() - 2); + candidateComingToHead = candidateWay.insideToTheRight; + candidatePrevNode = candidateWay.way.getNode(candidateWay.way.getNodesCount() - 2); } else continue; if (candidateComingToHead && candidateWay.equals(lastWay)) @@ -417,7 +939,8 @@ double candidateAngle = getAngle(headNode, candidatePrevNode, prevNode); - if (mostLeft == null || candidateAngle < angle || (Utils.equalsEpsilon(candidateAngle, angle) && !candidateComingToHead)) { + if (mostLeft == null || candidateAngle < angle + || (Utils.equalsEpsilon(candidateAngle, angle) && !candidateComingToHead)) { // Candidate is most left mostLeft = candidateWay; comingToHead = candidateComingToHead; @@ -462,9 +985,10 @@ * @since 11611 */ public JoinAreasAction(boolean addShortcut) { - super(tr("Join overlapping Areas"), "joinareas", tr("Joins areas that overlap each other"), addShortcut ? - Shortcut.registerShortcut("tools:joinareas", tr("Tool: {0}", tr("Join overlapping Areas")), KeyEvent.VK_J, Shortcut.SHIFT) - : null, true); + super(tr("Join overlapping Areas"), "joinareas", tr("Joins areas that overlap each other"), + addShortcut ? Shortcut.registerShortcut("tools:joinareas", + tr("Tool: {0}", tr("Join overlapping Areas")), KeyEvent.VK_J, Shortcut.SHIFT) : null, + true); } /** @@ -473,153 +997,132 @@ */ @Override public void actionPerformed(ActionEvent e) { - join(Main.getLayerManager().getEditDataSet().getSelectedWays()); + join(Main.getLayerManager().getEditDataSet().getSelected()); } /** * Joins the given ways. - * @param ways Ways to join + * @param waysAndRelations Ways / Multipolygons to join * @since 7534 */ - public void join(Collection ways) { - addedRelations.clear(); - - if (ways.isEmpty()) { - new Notification( - tr("Please select at least one closed way that should be joined.")) - .setIcon(JOptionPane.INFORMATION_MESSAGE) - .show(); + public void join(Collection waysAndRelations) { + if (waysAndRelations.isEmpty()) { + new Notification(tr("Please select at least one closed area that should be joined.")) + .setIcon(JOptionPane.INFORMATION_MESSAGE).show(); return; } - List allNodes = new ArrayList<>(); - for (Way way : ways) { - if (!way.isClosed()) { - new Notification( - tr("One of the selected ways is not closed and therefore cannot be joined.")) - .setIcon(JOptionPane.INFORMATION_MESSAGE) - .show(); - return; - } - - allNodes.addAll(way.getNodes()); + if (!ofSameDataset(waysAndRelations)) { + throw new IllegalArgumentException("Not in same DataSet"); } + waysAndRelations = selectRelationsInsteadOfMembers(waysAndRelations); + DataSet ds = waysAndRelations.iterator().next().getDataSet(); - // TODO: Only display this warning when nodes outside dataSourceArea are deleted - boolean ok = Command.checkAndConfirmOutlyingOperation("joinarea", tr("Join area confirmation"), - trn("The selected way has nodes outside of the downloaded data region.", - "The selected ways have nodes outside of the downloaded data region.", - ways.size()) + "
" - + tr("This can lead to nodes being deleted accidentally.") + "
" - + tr("Are you really sure to continue?") - + tr("Please abort if you are not sure"), - tr("The selected area is incomplete. Continue?"), - allNodes, null); - if (!ok) return; - - //analyze multipolygon relations and collect all areas - List areas = collectMultipolygons(ways); - - if (areas == null) - //too complex multipolygon relations found + if (!Command.checkAndConfirmOutlyingOperation("joinarea", tr("Join area confirmation"), + trn("The selected area has nodes outside of the downloaded data region.", + "The selected areas have nodes outside of the downloaded data region.", waysAndRelations.size()) + "
" + + tr("This can lead to nodes being deleted accidentally.") + "
" + + tr("Are you really sure to continue?") + tr("Please abort if you are not sure"), + tr("The selected area is incomplete. Continue?"), waysAndRelations, null)) { return; - - if (!testJoin(areas)) { - new Notification( - tr("No intersection found. Nothing was changed.")) - .setIcon(JOptionPane.INFORMATION_MESSAGE) - .show(); - return; } - if (!resolveTagConflicts(areas)) - return; - //user canceled, do nothing. - try { - // see #11026 - Because is a dynamic filtered (on ways) of a filtered (on selected objects) collection, - // retrieve effective dataset before joining the ways (which affects the selection, thus, the collection) - // Dataset retrieving allows to call this code without relying on Main.getCurrentDataSet(), thus, on a mapview instance - DataSet ds = ways.iterator().next().getDataSet(); + JoinAreasCollector collector = new JoinAreasCollector(ds, waysAndRelations); + if (!collector.allAreasHaveSameTags()) { + new Notification(tr("Only areas with the same tags can be joined.")) + .setIcon(JOptionPane.INFORMATION_MESSAGE).show(); + return; + } - // Do the job of joining areas - JoinAreasResult result = joinAreas(areas); + List commands = collector.getCommands(); + commitCommand(new SequenceCommand(tr("Join Areas"), commands)); - if (result.hasChanges) { - // move tags from ways to newly created relations - // TODO: do we need to also move tags for the modified relations? - for (Relation r: addedRelations) { - cmds.addAll(CreateMultipolygonAction.removeTagsFromWaysIfNeeded(r)); - } - commitCommands(tr("Move tags from ways to relations")); - - List allWays = new ArrayList<>(); - for (Multipolygon pol : result.polygons) { - allWays.add(pol.outerWay); - allWays.addAll(pol.innerWays); - } - if (ds != null) { - ds.setSelected(allWays); - } - } else { - new Notification( - tr("No intersection found. Nothing was changed.")) - .setIcon(JOptionPane.INFORMATION_MESSAGE) - .show(); - } - } catch (UserCancelException exception) { - Main.trace(exception); - //revert changes - //FIXME: this is dirty hack - makeCommitsOneAction(tr("Reverting changes")); - Main.main.undoRedo.undo(); - Main.main.undoRedo.redoCommands.clear(); + } catch (UnclosedAreaException e) { + new Notification(tr("One of the selected areas is not closed and therefore cannot be joined.")) + .setIcon(JOptionPane.INFORMATION_MESSAGE).show(); + return; + } catch (JoinAreasException e) { + new Notification(tr("One of the selected areas has an invalid geomerty.")) + .setIcon(JOptionPane.INFORMATION_MESSAGE).show(); + return; } } /** - * Tests if the areas have some intersections to join. - * @param areas Areas to test - * @return {@code true} if areas are joinable + * If all members of a multipolygon are selected, ask the user to select the polygon instead of the ways. + * @param currentSelection + * @return The new list of primitives the user selected */ - private boolean testJoin(List areas) { - List allStartingWays = new ArrayList<>(); + private Collection selectRelationsInsteadOfMembers(Collection currentSelection) { + List selectableMultipolygons = currentSelection.stream() + .filter(osm -> osm.getType() == OsmPrimitiveType.WAY) + // Get all multipolygons refferred by the way + .flatMap(osm -> osm.getReferrers().stream()) + .distinct() + .filter(osm -> osm.isMultipolygon()) + .map(osm -> ((Relation) osm)) + // Filter for those that are completely selected + .filter(r -> r.getMembers().stream().map(m -> m.getMember()).allMatch(currentSelection::contains)) + .collect(Collectors.toList()); - for (Multipolygon area : areas) { - allStartingWays.add(area.outerWay); - allStartingWays.addAll(area.innerWays); + if (!selectableMultipolygons.isEmpty()) { + JPanel msg = new JPanel(new GridBagLayout()); + msg.add(new JMultilineLabel("" + + tr("You selected the members of the following multipolygons. " + + "Do you want to join the polygons instead?") + + "
    " + selectableMultipolygons.stream() + .map(r -> "
  • " + r.getDisplayName(DefaultNameFormatter.getInstance()) + "
    • ") + .collect(Collectors.joining()) + + "
")); + boolean answer = ConditionalOptionPaneUtil.showConfirmationDialog( + "join_areas_on_polygons", + Main.parent, + msg, + tr("Join multipolygons?"), + JOptionPane.YES_NO_OPTION, + JOptionPane.QUESTION_MESSAGE, + JOptionPane.YES_OPTION); + if (answer) { + HashSet select = new HashSet<>(selectableMultipolygons); + currentSelection.stream().filter( + w -> !(w instanceof Way && ((Way) w).getReferrers().stream().allMatch(selectableMultipolygons::contains)) + ).forEach(select::add); + return select; + } } + return currentSelection; + } - //find intersection points - Set nodes = Geometry.addIntersections(allStartingWays, true, cmds); - return !nodes.isEmpty(); + private boolean ofSameDataset(Collection waysAndRelations) { + return waysAndRelations.stream().map(OsmPrimitive::getDataSet).distinct().count() <= 1; } private static class DuplicateWayCollectorAccu { - private List currentWays = new ArrayList<>(); - private List duplicatesFound = new ArrayList<>(); + private List currentWays = new ArrayList<>(); + private List duplicatesFound = new ArrayList<>(); - private void add(Way way) { - List wayNodes = way.getNodes(); - List wayNodesReversed = way.getNodes(); - Collections.reverse(wayNodesReversed); - Optional duplicate = currentWays.stream() - .filter(current -> current.getNodes().equals(wayNodes) || current.getNodes().equals(wayNodesReversed)) - .findFirst(); - if (duplicate.isPresent()) { - currentWays.remove(duplicate.get()); - duplicatesFound.add(duplicate.get()); - duplicatesFound.add(way); - } else { - currentWays.add(way); - } - } + private void add(Way way) { + List wayNodes = way.getNodes(); + List wayNodesReversed = way.getNodes(); + Collections.reverse(wayNodesReversed); + Optional duplicate = currentWays.stream().filter( + current -> current.getNodes().equals(wayNodes) || current.getNodes().equals(wayNodesReversed)) + .findFirst(); + if (duplicate.isPresent()) { + currentWays.remove(duplicate.get()); + duplicatesFound.add(duplicate.get()); + duplicatesFound.add(way); + } else { + currentWays.add(way); + } + } - private DuplicateWayCollectorAccu combine(DuplicateWayCollectorAccu a2) { - duplicatesFound.addAll(a2.duplicatesFound); - a2.currentWays.forEach(this::add); - return this; - } + private DuplicateWayCollectorAccu combine(DuplicateWayCollectorAccu a2) { + duplicatesFound.addAll(a2.duplicatesFound); + a2.currentWays.forEach(this::add); + return this; + } } /** @@ -709,13 +1212,14 @@ List preparedWays = new ArrayList<>(); // Split the nodes on the - List splitOuterWays = outerStartingWays.stream() - .flatMap(way -> splitWayOnNodes(way, nodes).stream()).collect(Collectors.toList()); - List splitInnerWays = innerStartingWays.stream() - .flatMap(way -> splitWayOnNodes(way, nodes).stream()).collect(Collectors.toList()); + List splitOuterWays = outerStartingWays.stream().flatMap(way -> splitWayOnNodes(way, nodes).stream()) + .collect(Collectors.toList()); + List splitInnerWays = innerStartingWays.stream().flatMap(way -> splitWayOnNodes(way, nodes).stream()) + .collect(Collectors.toList()); // remove duplicate ways (A->B->C and C->B->A) - List duplicates = Stream.concat(splitOuterWays.stream(), splitInnerWays.stream()).collect(new DuplicateWayCollector()); + List duplicates = Stream.concat(splitOuterWays.stream(), splitInnerWays.stream()) + .collect(new DuplicateWayCollector()); splitOuterWays.removeAll(duplicates); splitInnerWays.removeAll(duplicates); @@ -754,7 +1258,7 @@ commitCommands(marktr("Assemble new polygons")); - for (Relation rel: relationsToDelete) { + for (Relation rel : relationsToDelete) { cmds.add(new DeleteCommand(rel)); } @@ -774,9 +1278,7 @@ if (warnAboutRelations) { new Notification( tr("Some of the ways were part of relations that have been modified.
Please verify no errors have been introduced.")) - .setIcon(JOptionPane.INFORMATION_MESSAGE) - .setDuration(Notification.TIME_LONG) - .show(); + .setIcon(JOptionPane.INFORMATION_MESSAGE).setDuration(Notification.TIME_LONG).show(); } return new JoinAreasResult(true, polygons); @@ -788,7 +1290,7 @@ * @return {@code true} if all conflicts are resolved, {@code false} if conflicts remain. */ private boolean resolveTagConflicts(List polygons) { - + //TODO: Use this List ways = new ArrayList<>(); for (Multipolygon pol : polygons) { @@ -879,7 +1381,7 @@ * @param description The description of what the commands do */ private void commitCommands(String description) { - switch(cmds.size()) { + switch (cmds.size()) { case 0: return; case 1: @@ -904,6 +1406,9 @@ /** * This method analyzes the way and assigns each part what direction polygon "inside" is. + * + * It uses an even/odd winding rule. + * * @param parts the split parts of the way * @param isInner - if true, reverts the direction (for multipolygon islands) * @return list of parts, marked with the inside orientation. @@ -983,7 +1488,7 @@ if (chunks.size() > 1) { SplitWayResult split = SplitWayAction.splitWay(getLayerManager().getEditLayer(), way, chunks, - Collections.emptyList(), SplitWayAction.Strategy.keepFirstChunk()); + Collections. emptyList(), SplitWayAction.Strategy.keepFirstChunk()); if (split != null) { //execute the command, we need the results @@ -1116,8 +1621,7 @@ // This seems to appear when is apply over invalid way like #9911 test-case // Remove all of these way to make the next work. List cleanMultigonWays = multigonWays.stream() - .filter(way -> way.way.getNodesCount() != 2 || !way.way.isClosed()) - .collect(Collectors.toList()); + .filter(way -> way.way.getNodesCount() != 2 || !way.way.isClosed()).collect(Collectors.toList()); WayTraverser traverser = new WayTraverser(cleanMultigonWays); List result = new ArrayList<>(); @@ -1133,8 +1637,8 @@ return fixTouchingPolygons(result); } - private static void findBoundaryPolygonsStartingWith(List discardedResult, WayTraverser traverser, List result, - WayInPolygon startWay) { + private static void findBoundaryPolygonsStartingWith(List discardedResult, WayTraverser traverser, + List result, WayInPolygon startWay) { List path = new ArrayList<>(); List startWays = new ArrayList<>(); try { @@ -1158,7 +1662,7 @@ if (ring.getNodes().size() <= 2) { // Invalid ring (2 nodes) -> remove traverser.removeWays(path); - for (WayInPolygon way: path) { + for (WayInPolygon way : path) { discardedResult.add(way.way); } } else { @@ -1177,7 +1681,7 @@ traverser.removeWay(currentWay); path.remove(index); } - traverser.setStartWay(path.get(index-1)); + traverser.setStartWay(path.get(index - 1)); } else { path.add(nextWay); } @@ -1358,10 +1862,8 @@ } if (outerWays.size() > 1) { - new Notification( - tr("Sorry. Cannot handle multipolygon relations with multiple outer ways.")) - .setIcon(JOptionPane.INFORMATION_MESSAGE) - .show(); + new Notification(tr("Sorry. Cannot handle multipolygon relations with multiple outer ways.")) + .setIcon(JOptionPane.INFORMATION_MESSAGE).show(); return null; } @@ -1371,35 +1873,28 @@ innerWays.retainAll(selectedWays); if (processedOuterWays.contains(outerWay)) { - new Notification( - tr("Sorry. Cannot handle way that is outer in multiple multipolygon relations.")) - .setIcon(JOptionPane.INFORMATION_MESSAGE) - .show(); + new Notification(tr("Sorry. Cannot handle way that is outer in multiple multipolygon relations.")) + .setIcon(JOptionPane.INFORMATION_MESSAGE).show(); return null; } if (processedInnerWays.contains(outerWay)) { - new Notification( - tr("Sorry. Cannot handle way that is both inner and outer in multipolygon relations.")) - .setIcon(JOptionPane.INFORMATION_MESSAGE) - .show(); + new Notification(tr("Sorry. Cannot handle way that is both inner and outer in multipolygon relations.")) + .setIcon(JOptionPane.INFORMATION_MESSAGE).show(); return null; } - for (Way way :innerWays) { + for (Way way : innerWays) { if (processedOuterWays.contains(way)) { new Notification( tr("Sorry. Cannot handle way that is both inner and outer in multipolygon relations.")) - .setIcon(JOptionPane.INFORMATION_MESSAGE) - .show(); + .setIcon(JOptionPane.INFORMATION_MESSAGE).show(); return null; } if (processedInnerWays.contains(way)) { - new Notification( - tr("Sorry. Cannot handle way that is inner in multiple multipolygon relations.")) - .setIcon(JOptionPane.INFORMATION_MESSAGE) - .show(); + new Notification(tr("Sorry. Cannot handle way that is inner in multiple multipolygon relations.")) + .setIcon(JOptionPane.INFORMATION_MESSAGE).show(); return null; } } @@ -1431,7 +1926,8 @@ * @return The list of relation with roles to add own relation to */ private RelationRole addOwnMultipolygonRelation(Collection inner) { - if (inner.isEmpty()) return null; + if (inner.isEmpty()) + return null; OsmDataLayer layer = Main.getLayerManager().getEditLayer(); // Create new multipolygon relation and add all inner ways to it Relation newRel = new Relation(); @@ -1439,8 +1935,8 @@ for (Way w : inner) { newRel.addMember(new RelationMember("inner", w)); } - cmds.add(layer != null ? new AddCommand(layer, newRel) : - new AddCommand(inner.iterator().next().getDataSet(), newRel)); + cmds.add(layer != null ? new AddCommand(layer, newRel) + : new AddCommand(inner.iterator().next().getDataSet(), newRel)); addedRelations.add(newRel); // We don't add outer to the relation because it will be handed to fixRelations() @@ -1492,7 +1988,8 @@ * @param ownMultipol elements to directly add as outer * @param relationsToDelete set of relations to delete. */ - private void fixRelations(List rels, Way outer, RelationRole ownMultipol, Set relationsToDelete) { + private void fixRelations(List rels, Way outer, RelationRole ownMultipol, + Set relationsToDelete) { List multiouters = new ArrayList<>(); if (ownMultipol != null) {