Changeset 15852 in josm


Ignore:
Timestamp:
2020-02-15T11:15:05+01:00 (6 weeks ago)
Author:
GerdP
Message:

fix #10511: Joining complex areas produces exception

  • detect case where a single closed way contains the same segment twice or more and fix the "inside" flag
  • enable and correct unit test (it "destroyed" preferences.xml)
  • init cmdsCount to prevent corrupted undo/redo stacks after a "Join areas internal error."
  • make sure that a single command is placed on the undo/redo stack in case of successfull execution. The old code sometimes created two, the latter one "Move tags from ways to relations". I found no good reason for that.
  • Try to clean up undo/redo stack if user cancelled operation in 2nd popup asking about relations. Still a dirty hack but a less dangerous one.
Location:
trunk
Files:
2 edited

Legend:

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

    r15627 r15852  
    183183                    Objects.equals(way, that.way);
    184184        }
     185
     186        @Override
     187        public String toString() {
     188            return "w" + way.getUniqueId() + " " + way.getNodesCount() + " nodes";
     189        }
    185190    }
    186191
     
    254259         */
    255260        WayTraverser(Collection<WayInPolygon> ways) {
    256             availableWays = new HashSet<>(ways);
     261            availableWays = new LinkedHashSet<>(ways);
    257262            lastWay = null;
    258263        }
     
    482487     */
    483488    public void join(Collection<Way> ways) {
     489        cmdsCount = 0;
    484490        addedRelations.clear();
    485491
     
    548554                commitCommands(tr("Move tags from ways to relations"));
    549555
     556                makeCommitsOneAction(marktr("Joined overlapping areas"));
     557
    550558                if (result.polygons != null && ds != null) {
    551559                    List<Way> allWays = new ArrayList<>();
     
    569577            if (addUndoRedo) {
    570578                UndoRedoHandler.getInstance().undo();
    571                 UndoRedoHandler.getInstance().getRedoCommands().clear();
     579                // add no-change commands to the stack to remove the half-done commands
     580                Way w = ways.iterator().next();
     581                cmds.add(new ChangeCommand(w, w));
     582                cmds.add(new ChangeCommand(w, w));
     583                commitCommands(tr("Reverting changes"));
     584                UndoRedoHandler.getInstance().undo();
    572585            }
    573586        }
     
    597610     * @return new area formed.
    598611     * @throws UserCancelException if user cancels the operation
    599      */
    600     public JoinAreasResult joinAreas(List<Multipolygon> areas) throws UserCancelException {
     612     * @since xxx : visibility changed from public to private
     613     */
     614    private JoinAreasResult joinAreas(List<Multipolygon> areas) throws UserCancelException {
    601615
    602616        // see #11026 - Because <ways> is a dynamic filtered (on ways) of a filtered (on selected objects) collection,
     
    705719            }
    706720        }
    707 
    708         makeCommitsOneAction(marktr("Joined overlapping areas"));
    709721
    710722        if (warnAboutRelations) {
     
    10081020        }
    10091021
     1022        revertDuplicateTwoNodeWays(result);
     1023
    10101024        return result;
     1025    }
     1026
     1027    /**
     1028     * Correct possible error in markWayInsideSide result when splitting a self-intersecting way.
     1029     * If we have two ways with the same two nodes and the same direction there must be a self intersection.
     1030     * Change the direction flag for the latter of the two ways. The result is that difference between the number
     1031     * of ways with insideToTheRight = {@code true} and those with insideToTheRight = {@code false}
     1032     * differs by 0 or 1, not more.
     1033     * <p>See #10511
     1034     * @param parts the parts of a single closed way
     1035     */
     1036    private static void revertDuplicateTwoNodeWays(List<WayInPolygon> parts) {
     1037        for (int i = 0; i < parts.size(); i++) {
     1038            WayInPolygon w1 = parts.get(i);
     1039            if (w1.way.getNodesCount() != 2)
     1040                continue;
     1041            for (int j = i + 1; j < parts.size(); j++) {
     1042                WayInPolygon w2 = parts.get(j);
     1043                if (w2.way.getNodesCount() == 2 && w1.insideToTheRight == w2.insideToTheRight
     1044                        && w1.way.firstNode() == w2.way.firstNode() && w1.way.lastNode() == w2.way.lastNode()) {
     1045                    w2.insideToTheRight = !w2.insideToTheRight;
     1046                }
     1047            }
     1048        }
    10111049    }
    10121050
     
    11621200                cleanMultigonWays.add(way);
    11631201        }
    1164 
    11651202        WayTraverser traverser = new WayTraverser(cleanMultigonWays);
    11661203        List<AssembledPolygon> result = new ArrayList<>();
  • trunk/test/unit/org/openstreetmap/josm/actions/JoinAreasActionTest.java

    r15034 r15852  
    1515import java.util.Set;
    1616
    17 import org.junit.Ignore;
    1817import org.junit.Rule;
    1918import org.junit.Test;
     
    5049    @Rule
    5150    @SuppressFBWarnings(value = "URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD")
    52     public JOSMTestRules test = new JOSMTestRules().main().projection();
     51    public JOSMTestRules test = new JOSMTestRules().main().projection().preferences();
    5352
    5453    /**
     
    5857     */
    5958    @Test
    60     @Ignore("disable this test, needs further working") // XXX
    6159    public void testTicket10511() throws IOException, IllegalDataException {
    6260        try (InputStream is = TestUtils.getRegressionDataStream(10511, "10511_mini.osm")) {
     
    6664            try {
    6765                new JoinAreasAction(false).join(ds.getWays());
     66                Collection<IPrimitive> found = SearchAction.searchAndReturn("type:way", SearchMode.replace);
     67                assertEquals(1, found.size());
    6868            } finally {
    6969                // Ensure we clean the place before leaving, even if test fails.
Note: See TracChangeset for help on using the changeset viewer.