Changeset 15890 in josm for trunk/src


Ignore:
Timestamp:
2020-02-22T07:47:13+01:00 (5 years ago)
Author:
GerdP
Message:

see #18728: Join overlapping Areas is slow when combining many complex ways

  • don't use UndoRedoHandler to store lots of intermediate steps followed by slow undo/redo actions
  • catch Exceptions and try to undo actions instead of leaving the undo stack messed up

reduces execution time by ~50% for the complex case

File:
1 edited

Legend:

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

    r15887 r15890  
    5858public class JoinAreasAction extends JosmAction {
    5959    // This will be used to commit commands and unite them into one large command sequence at the end
     60    private final transient LinkedList<Command> executedCmds = new LinkedList<>();
    6061    private final transient LinkedList<Command> cmds = new LinkedList<>();
    61     private int cmdsCount;
    6262    private DataSet ds;
    6363    private final transient List<Relation> addedRelations = new LinkedList<>();
     
    485485    private void clearFields() {
    486486        ds = null;
    487         cmdsCount = 0;
    488487        cmds.clear();
    489488        addedRelations.clear();
     
    562561                commitCommands(tr("Move tags from ways to relations"));
    563562
    564                 makeCommitsOneAction(marktr("Joined overlapping areas"));
     563                commitExecuted();
    565564
    566565                if (result.polygons != null && ds != null) {
     
    580579        } catch (UserCancelException exception) {
    581580            Logging.trace(exception);
    582             //revert changes
    583             //FIXME: this is dirty hack
    584             makeCommitsOneAction(tr("Reverting changes"));
    585             if (addUndoRedo) {
    586                 UndoRedoHandler.getInstance().undo();
    587                 // add no-change commands to the stack to remove the half-done commands
    588                 Way w = ways.iterator().next();
    589                 cmds.add(new ChangeCommand(w, w));
    590                 cmds.add(new ChangeCommand(w, w));
    591                 commitCommands(tr("Reverting changes"));
    592                 UndoRedoHandler.getInstance().undo();
    593             }
     581            tryUndo();
     582        } catch (JosmRuntimeException | IllegalArgumentException exception) {
     583            Logging.trace(exception);
     584            tryUndo();
     585            throw exception;
     586        }
     587    }
     588
     589    private void tryUndo() {
     590        cmds.clear();
     591        if (!executedCmds.isEmpty()) {
     592            // revert all executed commands
     593            ds = executedCmds.getFirst().getAffectedDataSet();
     594            ds.beginUpdate();
     595            while (!executedCmds.isEmpty()) {
     596                executedCmds.removeLast().undoCommand();
     597            }
     598            ds.endUpdate();
    594599        }
    595600    }
     
    896901
    897902        cmds.clear();
    898         cmdsCount++;
    899903    }
    900904
    901905    private void commitCommand(Command c) {
    902         if (addUndoRedo) {
    903             UndoRedoHandler.getInstance().add(c);
    904         } else {
    905             c.executeCommand();
    906         }
     906        c.executeCommand();
     907        executedCmds.add(c);
     908    }
     909
     910    /**
     911     * Add all executed commands as one command to the undo stack without executing them again.
     912     */
     913    private void commitExecuted() {
     914        cmds.clear();
     915        if (addUndoRedo && !executedCmds.isEmpty()) {
     916            UndoRedoHandler ur = UndoRedoHandler.getInstance();
     917            if (executedCmds.size() == 1) {
     918                ur.add(executedCmds.getFirst(), false);
     919            } else {
     920                ur.add(new JoinAreaCommand(executedCmds), false);
     921            }
     922        }
     923        executedCmds.clear();
    907924    }
    908925
     
    14361453        // the user about this.
    14371454
    1438         //TODO: ReverseWay and Combine way are really slow and we use them a lot here. This slows down large joins.
    14391455        List<Way> actionWays = new ArrayList<>(ways.size());
    14401456        int oldestPos = 0;
     
    14501466                ReverseWayResult res = ReverseWayAction.reverseWay(way.way);
    14511467                commitCommand(res.getReverseCommand());
    1452                 cmdsCount++;
    14531468            }
    14541469        }
     
    14621477        }
    14631478        commitCommand(result.b);
    1464         cmdsCount++;
    14651479
    14661480        return result.a;
     
    17081722    }
    17091723
    1710     /**
    1711      * Takes the last cmdsCount actions back and combines them into a single action
    1712      * (for when the user wants to undo the join action)
    1713      * @param message The commit message to display
    1714      */
    1715     private void makeCommitsOneAction(String message) {
    1716         cmds.clear();
    1717         if (addUndoRedo) {
    1718             UndoRedoHandler ur = UndoRedoHandler.getInstance();
    1719             List<Command> commands = ur.getUndoCommands();
    1720             int i = Math.max(commands.size() - cmdsCount, 0);
    1721             for (; i < commands.size(); i++) {
    1722                 cmds.add(commands.get(i));
    1723             }
    1724             ur.undo(cmds.size());
    1725         }
    1726 
    1727         commitCommands(message == null ? marktr("Join Areas Function") : message);
    1728         cmdsCount = 0;
    1729     }
    1730 
    17311724    @Override
    17321725    protected void updateEnabledState() {
     
    17381731        updateEnabledStateOnModifiableSelection(selection);
    17391732    }
     1733
     1734    private static class JoinAreaCommand extends SequenceCommand {
     1735        JoinAreaCommand(Collection<Command> sequenz) {
     1736            super(tr("Joined overlapping areas"), sequenz, true);
     1737            setSequenceComplete(true);
     1738        }
     1739
     1740        @Override
     1741        public void undoCommand() {
     1742            getAffectedDataSet().beginUpdate();
     1743            super.undoCommand();
     1744            getAffectedDataSet().endUpdate();
     1745        }
     1746
     1747        @Override
     1748        public boolean executeCommand() {
     1749            getAffectedDataSet().beginUpdate();
     1750            boolean rc = super.executeCommand();
     1751            getAffectedDataSet().endUpdate();
     1752            return rc;
     1753        }
     1754    }
    17401755}
Note: See TracChangeset for help on using the changeset viewer.