Index: src/org/openstreetmap/josm/actions/CreateMultipolygonAction.java =================================================================== --- src/org/openstreetmap/josm/actions/CreateMultipolygonAction.java (revision 15137) +++ src/org/openstreetmap/josm/actions/CreateMultipolygonAction.java (working copy) @@ -11,6 +11,7 @@ import java.util.Collections; import java.util.HashMap; import java.util.HashSet; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Map.Entry; @@ -28,13 +29,14 @@ import org.openstreetmap.josm.command.SequenceCommand; import org.openstreetmap.josm.data.UndoRedoHandler; import org.openstreetmap.josm.data.osm.DataSet; -import org.openstreetmap.josm.data.osm.MultipolygonBuilder; -import org.openstreetmap.josm.data.osm.MultipolygonBuilder.JoinedPolygon; import org.openstreetmap.josm.data.osm.OsmPrimitive; import org.openstreetmap.josm.data.osm.OsmUtils; 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.data.osm.visitor.paint.relations.Multipolygon; +import org.openstreetmap.josm.data.validation.TestError; +import org.openstreetmap.josm.data.validation.tests.MultipolygonTest; import org.openstreetmap.josm.gui.MainApplication; import org.openstreetmap.josm.gui.Notification; import org.openstreetmap.josm.gui.dialogs.relation.DownloadRelationMemberTask; @@ -210,15 +212,58 @@ Set ways = new HashSet<>(selectedWays); ways.addAll(selectedMultipolygonRelation.getMemberPrimitives(Way.class)); - final MultipolygonBuilder polygon = analyzeWays(ways, true); - if (polygon == null) { - return null; //could not make multipolygon. - } else { - return Pair.create(selectedMultipolygonRelation, createRelation(polygon, selectedMultipolygonRelation)); + // even if no way was added the inner/outer roles might be different + MultipolygonTest mpTest = new MultipolygonTest(); + Pair tested = mpTest.makeFromWays(ways); + if (mpTest.getErrors().isEmpty()) { + return mergeRelationsMembers(selectedMultipolygonRelation, tested.b); } + showErrors(mpTest.getErrors()); + return null; //could not make multipolygon. } /** + * Merge members of multipolygon relation. Maintains the order of the old relation. May change roles, + * removes duplicate and non-way members and adds new members found in {@code calculated}. + * @param old old multipolygon relation + * @param calculated calculated multipolygon relation + * @return pair of old and new multipolygon relation + */ + private static Pair mergeRelationsMembers(Relation old, Relation calculated) { + Set merged = new LinkedHashSet<>(); + boolean foundDiff = false; + boolean foundNonWayMember = false; + // maintain order of members in updated relation + for (RelationMember oldMem :old.getMembers()) { + if (oldMem.isNode() || oldMem.isRelation()) { + foundNonWayMember = true; + continue; + } + for (RelationMember newMem : calculated.getMembers()) { + if (newMem.getMember().equals(oldMem.getMember())) { + if (!newMem.getRole().equals(oldMem.getRole())) { + foundDiff = true; + } + foundDiff |= !merged.add(newMem); // detect duplicate members in old relation + break; + } + } + } + if (foundNonWayMember) { + foundDiff = true; + GuiHelper.runInEDT(() -> new Notification(tr("Non-Way removed from multipolygon")).setIcon(JOptionPane.WARNING_MESSAGE).show()); + } + foundDiff |= merged.addAll(calculated.getMembers()); + if (!foundDiff) { + GuiHelper.runInEDT(() -> new Notification(tr("Nothing changed")).setDuration(Notification.TIME_SHORT).setIcon(JOptionPane.INFORMATION_MESSAGE).show()); + return null; + } + Relation toModify = new Relation(old); + toModify.setMembers(new ArrayList<>(merged)); + return Pair.create(old, toModify); + } + + /** * Returns a {@link Pair} null and the newly created/modified multipolygon {@link Relation}. * @param selectedWays selected ways * @param showNotif if {@code true}, shows a notification if an error occurs @@ -225,13 +270,23 @@ * @return pair of null and new multipolygon relation */ public static Pair createMultipolygonRelation(Collection selectedWays, boolean showNotif) { + MultipolygonTest mpTest = new MultipolygonTest(); + Pair tested = mpTest.makeFromWays(selectedWays); + tested.b.setMembers(RelationSorter.sortMembersByConnectivity(tested.b.getMembers())); + if (mpTest.getErrors().isEmpty()) + return Pair.create(null, tested.b); + if (showNotif) { + showErrors(mpTest.getErrors()); + } + return null; //could not make multipolygon. + } - final MultipolygonBuilder polygon = analyzeWays(selectedWays, showNotif); - if (polygon == null) { - return null; //could not make multipolygon. - } else { - return Pair.create(null, createRelation(polygon, null)); + private static void showErrors(List errors) { + StringBuilder sb = new StringBuilder(); + for (TestError e : errors) { + sb.append(e.getMessage()).append('\n'); } + GuiHelper.runInEDT(() -> new Notification(sb.toString()).setIcon(JOptionPane.INFORMATION_MESSAGE).show()); } /** @@ -287,71 +342,6 @@ } } - /** - * This method analyzes ways and creates multipolygon. - * @param selectedWays list of selected ways - * @param showNotif if {@code true}, shows a notification if an error occurs - * @return null, if there was a problem with the ways. - */ - private static MultipolygonBuilder analyzeWays(Collection selectedWays, boolean showNotif) { - - MultipolygonBuilder pol = new MultipolygonBuilder(); - final String error = pol.makeFromWays(selectedWays); - - if (error != null) { - if (showNotif) { - GuiHelper.runInEDT(() -> - new Notification(error) - .setIcon(JOptionPane.INFORMATION_MESSAGE) - .show()); - } - return null; - } else { - return pol; - } - } - - /** - * Builds a relation from polygon ways. - * @param pol data storage class containing polygon information - * @param clone relation to clone, can be null - * @return multipolygon relation - */ - private static Relation createRelation(MultipolygonBuilder pol, Relation clone) { - // Create new relation - Relation rel = clone != null ? new Relation(clone) : new Relation(); - rel.put("type", "multipolygon"); - // Add ways to it - for (JoinedPolygon jway:pol.outerWays) { - addMembers(jway, rel, "outer"); - } - - for (JoinedPolygon jway:pol.innerWays) { - addMembers(jway, rel, "inner"); - } - - if (clone == null) { - rel.setMembers(RelationSorter.sortMembersByConnectivity(rel.getMembers())); - } - - return rel; - } - - private static void addMembers(JoinedPolygon polygon, Relation rel, String role) { - final int count = rel.getMembersCount(); - final Set ways = new HashSet<>(polygon.ways); - for (int i = 0; i < count; i++) { - final RelationMember m = rel.getMember(i); - if (ways.contains(m.getMember()) && !role.equals(m.getRole())) { - rel.setMember(i, new RelationMember(role, m.getMember())); - } - } - ways.removeAll(rel.getMemberPrimitivesList()); - for (final Way way : ways) { - rel.addMember(new RelationMember(role, way)); - } - } - private static final List DEFAULT_LINEAR_TAGS = Arrays.asList("barrier", "fence_type", "source"); /** Index: src/org/openstreetmap/josm/data/validation/tests/MultipolygonTest.java =================================================================== --- src/org/openstreetmap/josm/data/validation/tests/MultipolygonTest.java (revision 15137) +++ src/org/openstreetmap/josm/data/validation/tests/MultipolygonTest.java (working copy) @@ -38,6 +38,7 @@ import org.openstreetmap.josm.tools.Geometry; import org.openstreetmap.josm.tools.Geometry.PolygonIntersection; import org.openstreetmap.josm.tools.Logging; +import org.openstreetmap.josm.tools.Pair; /** * Checks if multipolygons are valid @@ -78,6 +79,9 @@ private static final int FOUND_INSIDE = 1; private static final int FOUND_OUTSIDE = 2; + /** set when used to build a multipolygon relation */ + private Relation createdRelation; + /** * Constructs a new {@code MultipolygonTest}. */ @@ -463,9 +467,20 @@ if (list == null || list.isEmpty()) { return; } - + if (r == createdRelation) { + List modMembers = new ArrayList<>(); + for (PolygonLevel pol : list) { + final String calculatedRole = (pol.level % 2 == 0) ? "outer" : "inner"; + for (long wayId : pol.outerWay.getWayIds()) { + RelationMember member = wayMap.get(wayId); + modMembers.add(new RelationMember(calculatedRole, member.getMember())); + } + } + r.setMembers(modMembers); + return; + } for (PolygonLevel pol : list) { - String calculatedRole = (pol.level % 2 == 0) ? "outer" : "inner"; + final String calculatedRole = (pol.level % 2 == 0) ? "outer" : "inner"; for (long wayId : pol.outerWay.getWayIds()) { RelationMember member = wayMap.get(wayId); if (!calculatedRole.equals(member.getRole())) { @@ -893,4 +908,29 @@ return null; } } + + /** + * Create a multipolygon relation from the given ways and test it. + * @param ways the collection of ways + * @return a pair of a {@link Multipolygon} instance and the relation. + */ + public Pair makeFromWays(Collection ways) { + Relation r = new Relation(); + createdRelation = r; + r.put("type", "multipolygon"); + for (Way w : ways) { + r.addMember(new RelationMember("", w)); + } + errors.clear(); + Multipolygon polygon = null; + boolean hasRepeatedMembers = checkRepeatedWayMembers(r); + // Rest of checks is only for complete multipolygon + if (!hasRepeatedMembers && !r.hasIncompleteMembers()) { + polygon = new Multipolygon(r); + checkGeometryAndRoles(r, polygon); + } + errors.removeIf(e->e.getSeverity() == Severity.OTHER); + return new Pair<>(polygon, r); + } + }