Changeset 15883 in josm


Ignore:
Timestamp:
2020-02-18T15:02:41+01:00 (6 weeks ago)
Author:
GerdP
Message:

fix #9599: Join Overlapping Areas" creates a new way when joining 2 buildings
Two different problems are solved with this:

  • when inner loops are removed it was possible that the old way was removed and only a new part remained
  • when ways are joined with CombineWaysAction, the first old way was kept, not the oldest one
Location:
trunk
Files:
3 added
2 edited

Legend:

Unmodified
Added
Removed
  • trunk/src/org/openstreetmap/josm/actions/JoinAreasAction.java

    r15875 r15883  
    480480    public void actionPerformed(ActionEvent e) {
    481481        join(getLayerManager().getEditDataSet().getSelectedWays());
     482        clearFields();
     483    }
     484
     485    private void clearFields() {
     486        ds = null;
     487        cmdsCount = 0;
     488        cmds.clear();
     489        addedRelations.clear();
    482490    }
    483491
     
    488496     */
    489497    public void join(Collection<Way> ways) {
    490         cmdsCount = 0;
    491         addedRelations.clear();
     498        clearFields();
    492499
    493500        if (ways.isEmpty()) {
     
    675682        List<WayInPolygon> preparedWays = new ArrayList<>();
    676683
     684        // maps oldest way referring to start of each part
     685        Map<Node, Way> oldestWayMap = new HashMap<>();
     686
    677687        for (Way way : outerStartingWays) {
    678688            List<Way> splitWays = splitWayOnNodes(way, nodes);
     689
     690            // see #9599
     691            if (!way.isNew() && splitWays.size() > 1) {
     692                for (Way part : splitWays) {
     693                    Node n = part.firstNode();
     694                    Way old = oldestWayMap.get(n);
     695                    if (old == null || old.getUniqueId() > way.getUniqueId()) {
     696                        oldestWayMap.put(n, way);
     697                    }
     698                }
     699            }
     700
    679701            preparedWays.addAll(markWayInsideSide(splitWays, false));
    680702        }
     
    721743
    722744        commitCommands(marktr("Delete relations"));
     745
     746        // see #9599: result should contain original way(s) where possible
     747        if (discardedWays.stream().anyMatch(w -> !w.isNew())) {
     748            for (int i = 0; i < polygons.size(); i++) {
     749                Multipolygon mp = polygons.get(i);
     750                for (int k = 0; k < mp.getInnerWays().size(); k++) {
     751                    Way inner = mp.getInnerWays().get(k);
     752                    Way older = keepOlder(inner, oldestWayMap, discardedWays);
     753                    if (inner != older) {
     754                        mp.getInnerWays().set(k, older);
     755                    }
     756                }
     757                Way older = keepOlder(mp.outerWay, oldestWayMap, discardedWays);
     758                if (older != mp.outerWay) {
     759                    Multipolygon mpNew = new Multipolygon(older);
     760                    mpNew.innerWays.addAll(mp.getInnerWays());
     761                    polygons.set(i, mpNew);
     762                }
     763            }
     764            commitCommands(marktr("Keep older versions"));
     765        }
    723766
    724767        // Delete the discarded inner ways
     
    740783
    741784        return new JoinAreasResult(true, polygons);
     785    }
     786
     787    /**
     788     * Create copy of given way using an older id so that we don't create a new way instead of a modified old one.
     789     * @param way the way to check
     790     * @param oldestWayMap  nodes from old ways
     791     * @param discardedWays collection of ways which will be deleted (modified)
     792     * @return a copy of the way with an older id or the way itself
     793     */
     794    private Way keepOlder(Way way, Map<Node, Way> oldestWayMap, List<Way> discardedWays) {
     795        Way oldest = null;
     796        for (Node n : way.getNodes()) {
     797            Way orig = oldestWayMap .get(n);
     798            if (orig != null && (oldest == null || oldest.getUniqueId() > orig.getUniqueId())
     799                    && discardedWays.contains(orig)) {
     800                oldest = orig;
     801            }
     802        }
     803        if (oldest != null) {
     804            discardedWays.remove(oldest);
     805            discardedWays.add(way);
     806            Way copy = new Way(oldest);
     807            copy.setNodes(way.getNodes());
     808            cmds.add(new ChangeCommand(oldest, copy));
     809            return copy;
     810        }
     811        return way;
    742812    }
    743813
     
    10561126     * @param way way to split
    10571127     * @param nodes split points
    1058      * @return list of split ways (or original ways if no splitting is done).
     1128     * @return list of split ways (or original way if no splitting is done).
    10591129     */
    10601130    private List<Way> splitWayOnNodes(Way way, Set<Node> nodes) {
     
    13731443        //TODO: ReverseWay and Combine way are really slow and we use them a lot here. This slows down large joins.
    13741444        List<Way> actionWays = new ArrayList<>(ways.size());
    1375 
     1445        int oldestPos = 0;
     1446        Way oldest = ways.get(0).way;
    13761447        for (WayInPolygon way : ways) {
    13771448            actionWays.add(way.way);
     1449            if (oldest.isNew() || !way.way.isNew() && oldest.getUniqueId() > way.way.getUniqueId()) {
     1450                oldest = way.way;
     1451                oldestPos = actionWays.size() - 1;
     1452            }
    13781453
    13791454            if (!way.insideToTheRight) {
     
    13831458            }
    13841459        }
     1460
     1461        // see #9599: Help CombineWayAction to use the oldest way
     1462        Collections.rotate(actionWays, actionWays.size() - oldestPos);
    13851463
    13861464        Pair<Way, Command> result = CombineWayAction.combineWaysWorker(actionWays);
  • trunk/test/unit/org/openstreetmap/josm/actions/JoinAreasActionTest.java

    r15852 r15883  
    5252
    5353    /**
     54     * Non-regression test for bug #9599.
     55     * @throws IOException if any I/O error occurs
     56     * @throws IllegalDataException if OSM parsing fails
     57     */
     58    @Test
     59    public void testTicket9599() throws IOException, IllegalDataException {
     60        try (InputStream is = TestUtils.getRegressionDataStream(9599, "ex5.osm")) {
     61            DataSet ds = OsmReader.parseDataSet(is, null);
     62            Layer layer = new OsmDataLayer(ds, null, null);
     63            MainApplication.getLayerManager().addLayer(layer);
     64            try {
     65                new JoinAreasAction(false).join(ds.getWays());
     66                Collection<IPrimitive> found = SearchAction.searchAndReturn("type:way", SearchMode.replace);
     67                assertEquals(1, found.size());
     68                assertEquals(257786939, found.iterator().next().getUniqueId());
     69            } finally {
     70                // Ensure we clean the place before leaving, even if test fails.
     71                MainApplication.getLayerManager().removeLayer(layer);
     72            }
     73        }
     74    }
     75
     76    /**
     77     * Non-regression test for bug #9599.
     78     * @throws IOException if any I/O error occurs
     79     * @throws IllegalDataException if OSM parsing fails
     80     */
     81    @Test
     82    public void testTicket9599Simple() throws IOException, IllegalDataException {
     83        try (InputStream is = TestUtils.getRegressionDataStream(9599, "three_old.osm")) {
     84            DataSet ds = OsmReader.parseDataSet(is, null);
     85            Layer layer = new OsmDataLayer(ds, null, null);
     86            MainApplication.getLayerManager().addLayer(layer);
     87            try {
     88                new JoinAreasAction(false).join(ds.getWays());
     89                Collection<IPrimitive> found = SearchAction.searchAndReturn("type:way", SearchMode.replace);
     90                assertEquals(1, found.size());
     91                assertEquals(31567319, found.iterator().next().getUniqueId());
     92            } finally {
     93                // Ensure we clean the place before leaving, even if test fails.
     94                MainApplication.getLayerManager().removeLayer(layer);
     95            }
     96        }
     97    }
     98
     99    /**
    54100     * Non-regression test for bug #10511.
    55101     * @throws IOException if any I/O error occurs
Note: See TracChangeset for help on using the changeset viewer.