Changeset 17386 in josm for trunk


Ignore:
Timestamp:
2020-12-02T16:43:36+01:00 (3 months ago)
Author:
GerdP
Message:

fix #20041: Align nodes in Circle creates Command which changes nothing

  • only create move command if node is visibly moved
  • reject a self-intersecting way
  • reject old nodes outside of download area (the original code only shows an info and continues)
  • add some unit tests to improve coverage
Location:
trunk
Files:
6 added
2 edited

Legend:

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

    r17188 r17386  
    2828import org.openstreetmap.josm.data.osm.OsmPrimitive;
    2929import org.openstreetmap.josm.data.osm.Way;
     30import org.openstreetmap.josm.data.osm.visitor.paint.relations.Multipolygon;
     31import org.openstreetmap.josm.data.osm.visitor.paint.relations.Multipolygon.JoinedWay;
     32import org.openstreetmap.josm.data.validation.tests.SelfIntersectingWay;
    3033import org.openstreetmap.josm.gui.Notification;
    3134import org.openstreetmap.josm.tools.Geometry;
     35import org.openstreetmap.josm.tools.Logging;
    3236import org.openstreetmap.josm.tools.Shortcut;
    3337
     
    5559
    5660    /**
    57      * Create a {@link MoveCommand} to move a node to a PolarCoor.
     61     * InvalidSelection exception has to be raised when action can't be performed
     62     */
     63    public static class InvalidSelection extends Exception {
     64
     65        /**
     66         * Create an InvalidSelection exception with default message
     67         */
     68        InvalidSelection() {
     69            super(tr("Selection could not be used to align in circle."));
     70        }
     71
     72        /**
     73         * Create an InvalidSelection exception with specific message
     74         * @param msg Message that will be displayed to the user
     75         */
     76        InvalidSelection(String msg) {
     77            super(msg);
     78        }
     79    }
     80
     81    /**
     82     * Add a {@link MoveCommand} to move a node to a PolarCoor if there is a significant move.
    5883     * @param n Node to move
    5984     * @param coor polar coordinate where to move the node
    60      * @return new MoveCommand
    61      * @since 13107
    62      */
    63     public static MoveCommand createMoveCommand(Node n, PolarCoor coor) {
     85     * @param cmds list of commands
     86     * @since 17386
     87     */
     88    public static void addMoveCommandIfNeeded(Node n, PolarCoor coor, List<Command> cmds) {
    6489        EastNorth en = coor.toEastNorth();
    65         return new MoveCommand(n, en.east() - n.getEastNorth().east(), en.north() - n.getEastNorth().north());
     90        double deltaEast = en.east() - n.getEastNorth().east();
     91        double deltaNorth = en.north() - n.getEastNorth().north();
     92
     93        if (Math.abs(deltaEast) > 5e-6 || Math.abs(deltaNorth) > 5e-6) {
     94            cmds.add(new MoveCommand(n, deltaEast, deltaNorth));
     95        }
    6696    }
    6797
     
    6999     * Perform AlignInCircle action.
    70100     *
     101     */
     102    @Override
     103    public void actionPerformed(ActionEvent e) {
     104        if (!isEnabled())
     105            return;
     106
     107        try {
     108            Command cmd = buildCommand(getLayerManager().getEditDataSet());
     109            UndoRedoHandler.getInstance().add(cmd);
     110        } catch (InvalidSelection except) {
     111            Logging.debug(except);
     112            new Notification(except.getMessage())
     113                .setIcon(JOptionPane.INFORMATION_MESSAGE)
     114                .setDuration(Notification.TIME_SHORT)
     115                .show();
     116        }
     117
     118    }
     119
     120    /**
     121     * Builds "align in circle" command depending on the selected objects.
    71122     * A fixed node is a node for which it is forbidden to change the angle relative to center of the circle.
    72123     * All other nodes are uniformly distributed.
    73      *
     124     * <p>
    74125     * Case 1: One unclosed way.
    75126     * --&gt; allow action, and align selected way nodes
    76127     * If nodes contained by this way are selected, there are fix.
    77128     * If nodes outside from the way are selected there are ignored.
    78      *
     129     * <p>
    79130     * Case 2: One or more ways are selected and can be joined into a polygon
    80131     * --&gt; allow action, and align selected ways nodes
     
    85136     * In all cases, selected nodes are fix, nodes with more than one referrers are fix
    86137     * (first referrer is the selected way)
    87      *
     138     * <p>
    88139     * Case 3: Only nodes are selected
    89140     * --&gt; Align these nodes, all are fix
    90      */
    91     @Override
    92     public void actionPerformed(ActionEvent e) {
    93         if (!isEnabled())
    94             return;
    95 
    96         Collection<OsmPrimitive> sel = getLayerManager().getEditDataSet().getSelected();
     141     * @param ds data set in which the command operates
     142     * @return the resulting command to execute to perform action
     143     * @throws InvalidSelection if selection cannot be used
     144     * @since 17386
     145     *
     146     */
     147    public static Command buildCommand(DataSet ds) throws InvalidSelection {
     148        Collection<OsmPrimitive> sel = ds.getSelected();
    97149        List<Node> nodes = new LinkedList<>();
    98150        // fixNodes: All nodes for which the angle relative to center should not be modified
     
    113165            // Case 1
    114166            Way w = ways.get(0);
     167            if (SelfIntersectingWay.isSelfIntersecting(w)) {
     168                throw new InvalidSelection(tr("Self-intersecting way"));
     169            }
     170
    115171            fixNodes.add(w.firstNode());
    116172            fixNodes.add(w.lastNode());
     
    121177            closedWay.addNode(w.firstNode());
    122178            nodes = collectNodesAnticlockwise(Collections.singletonList(closedWay));
    123             closedWay.setNodes(null);
     179            closedWay.setNodes(null); // see #19885
    124180        } else if (!ways.isEmpty() && checkWaysArePolygon(ways)) {
    125181            // Case 2
     
    152208            nodes = collectNodesAnticlockwise(ways);
    153209            if (nodes.size() < 4) {
    154                 new Notification(
    155                         tr("Not enough nodes in selected ways."))
    156                 .setIcon(JOptionPane.INFORMATION_MESSAGE)
    157                 .setDuration(Notification.TIME_SHORT)
    158                 .show();
    159                 return;
     210                throw new InvalidSelection(tr("Not enough nodes in selected ways."));
    160211            }
    161212        } else if (ways.isEmpty() && nodes.size() > 3) {
     
    164215            // No need to reorder nodes since all are fix
    165216        } else {
    166             // Invalid action
    167             new Notification(
    168                     tr("Please select at least four nodes."))
    169                     .setIcon(JOptionPane.INFORMATION_MESSAGE)
    170                     .setDuration(Notification.TIME_SHORT)
    171                     .show();
    172             return;
    173         }
     217            if (ways.isEmpty() && nodes.size() <= 3)
     218                throw new InvalidSelection(tr("Please select at least four nodes."));
     219            throw new InvalidSelection();
     220        }
     221
     222        // Check if one or more nodes are outside of download area
     223        if (nodes.stream().anyMatch(Node::isOutsideDownloadArea))
     224            throw new InvalidSelection(tr("One or more nodes involved in this action is outside of the downloaded area."));
    174225
    175226        if (center == null) {
     
    177228            center = Geometry.getCenter(nodes);
    178229            if (center == null) {
    179                 new Notification(tr("Cannot determine center of selected nodes."))
    180                     .setIcon(JOptionPane.INFORMATION_MESSAGE)
    181                     .setDuration(Notification.TIME_SHORT)
    182                     .show();
    183                 return;
     230                throw new InvalidSelection(tr("Cannot determine center of selected nodes."));
    184231            }
    185232        }
     
    195242        }
    196243
    197         if (!actionAllowed(nodes)) return;
    198 
    199         Collection<Command> cmds = new LinkedList<>();
     244        List<Command> cmds = new LinkedList<>();
    200245
    201246        // Move each node to that distance from the center.
     
    217262            Node first = nodes.get(i % nodeCount);
    218263            PolarCoor pcFirst = new PolarCoor(radius, PolarCoor.computeAngle(first.getEastNorth(), center), center);
    219             cmds.add(createMoveCommand(first, pcFirst));
     264            addMoveCommandIfNeeded(first, pcFirst, cmds);
    220265            if (j > i + 1) {
    221266                double delta;
     
    231276                for (int k = i+1; k < j; k++) {
    232277                    PolarCoor p = new PolarCoor(radius, pcFirst.angle + (k-i)*delta, center);
    233                     cmds.add(createMoveCommand(nodes.get(k % nodeCount), p));
     278                    addMoveCommandIfNeeded(nodes.get(k % nodeCount), p, cmds);
    234279                }
    235280            }
    236281            i = j; // Update start point for next iteration
    237282        }
    238 
    239         UndoRedoHandler.getInstance().add(new SequenceCommand(tr("Align Nodes in Circle"), cmds));
     283        if (cmds.isEmpty())
     284            throw new InvalidSelection(tr("nothing changed"));
     285        return new SequenceCommand(tr("Align Nodes in Circle"), cmds);
    240286    }
    241287
     
    253299     * @param ways List of ways to be joined
    254300     * @return Nodes anticlockwise ordered
    255      */
    256     private static List<Node> collectNodesAnticlockwise(List<Way> ways) {
    257         List<Node> nodes = new ArrayList<>();
    258         Node firstNode = ways.get(0).firstNode();
    259         Node lastNode = null;
    260         Way lastWay = null;
    261         while (firstNode != lastNode) {
    262             if (lastNode == null) lastNode = firstNode;
    263             for (Way way: ways) {
    264                 if (way == lastWay) continue;
    265                 if (way.firstNode() == lastNode) {
    266                     List<Node> wayNodes = way.getNodes();
    267                     for (int i = 0; i < wayNodes.size() - 1; i++) {
    268                         nodes.add(wayNodes.get(i));
    269                     }
    270                     lastNode = way.lastNode();
    271                     lastWay = way;
    272                     break;
    273                 }
    274                 if (way.lastNode() == lastNode) {
    275                     List<Node> wayNodes = way.getNodes();
    276                     for (int i = wayNodes.size() - 1; i > 0; i--) {
    277                         nodes.add(wayNodes.get(i));
    278                     }
    279                     lastNode = way.firstNode();
    280                     lastWay = way;
    281                     break;
    282                 }
    283             }
    284         }
    285         // Check if nodes are in anticlockwise order
    286         int nc = nodes.size();
    287         double area = 0;
    288         for (int i = 0; i < nc; i++) {
    289             EastNorth p1 = nodes.get(i).getEastNorth();
    290             EastNorth p2 = nodes.get((i+1) % nc).getEastNorth();
    291             area += p1.east()*p2.north() - p2.east()*p1.north();
    292         }
    293         if (area < 0)
     301     * @throws InvalidSelection if selection cannot be used
     302     */
     303    private static List<Node> collectNodesAnticlockwise(List<Way> ways) throws InvalidSelection {
     304        Collection<JoinedWay> rings = Multipolygon.joinWays(ways);
     305        if (rings.size() != 1)
     306            throw new InvalidSelection();
     307        List<Node> nodes = new ArrayList<>(rings.iterator().next().getNodes());
     308        if (nodes.get(0) != nodes.get(nodes.size()-1))
     309            throw new InvalidSelection();
     310        if (Geometry.isClockwise(nodes))
    294311            Collections.reverse(nodes);
     312        nodes.remove(nodes.size() - 1);
    295313        return nodes;
    296     }
    297 
    298     /**
    299      * Check if one or more nodes are outside of download area
    300      * @param nodes Nodes to check
    301      * @return true if action can be done
    302      */
    303     private static boolean actionAllowed(Collection<Node> nodes) {
    304         boolean outside = nodes.stream().anyMatch(Node::isOutsideDownloadArea);
    305         if (outside)
    306             new Notification(
    307                     tr("One or more nodes involved in this action is outside of the downloaded area."))
    308                     .setIcon(JOptionPane.WARNING_MESSAGE)
    309                     .setDuration(Notification.TIME_SHORT)
    310                     .show();
    311         return true;
    312314    }
    313315
     
    324326
    325327    /**
    326      * Determines if ways can be joined into a polygon.
     328     * Determines if ways can be joined into a single polygon.
    327329     * @param ways The ways collection to check
    328      * @return true if all ways can be joined into a polygon
     330     * @return true if all ways can be joined into a single polygon
    329331     */
    330332    private static boolean checkWaysArePolygon(Collection<Way> ways) {
     333        if (Multipolygon.joinWays(ways).size() != 1)
     334            return false;
    331335        // For each way, nodes strictly between first and last should't be reference by an other way
    332336        for (Way way: ways) {
     
    338342            }
    339343        }
    340         // Test if ways can be joined
    341         Way currentWay = null;
    342         Node startNode = null, endNode = null;
    343         int used = 0;
    344         while (true) {
    345             Way nextWay = null;
    346             for (Way w: ways) {
    347                 if (w.isClosed()) return ways.size() == 1;
    348                 if (w == currentWay) continue;
    349                 if (currentWay == null) {
    350                     nextWay = w;
    351                     startNode = w.firstNode();
    352                     endNode = w.lastNode();
    353                     break;
    354                 }
    355                 if (w.firstNode() == endNode) {
    356                     nextWay = w;
    357                     endNode = w.lastNode();
    358                     break;
    359                 }
    360                 if (w.lastNode() == endNode) {
    361                     nextWay = w;
    362                     endNode = w.firstNode();
    363                     break;
    364                 }
    365             }
    366             if (nextWay == null) return false;
    367             used += 1;
    368             currentWay = nextWay;
    369             if (endNode == startNode) return used == ways.size();
    370         }
    371     }
     344        return true;
     345    }
     346
    372347}
  • trunk/src/org/openstreetmap/josm/data/validation/tests/SelfIntersectingWay.java

    r11441 r17386  
    1212import org.openstreetmap.josm.data.validation.Test;
    1313import org.openstreetmap.josm.data.validation.TestError;
     14import org.openstreetmap.josm.data.validation.tests.CrossingWays.SelfCrossing;
    1415
    1516/**
     
    6263        }
    6364    }
     65
     66    /**
     67     * Check if the given way is self-intersecting
     68     * @param way the way to check
     69     * @return {@code true} if way contains some nodes more than once
     70     * @see SelfCrossing
     71     * @since 17386
     72     */
     73    public static boolean isSelfIntersecting(Way way) {
     74        SelfIntersectingWay test = new SelfIntersectingWay();
     75        test.visit(way);
     76        return !test.errors.isEmpty();
     77    }
    6478}
Note: See TracChangeset for help on using the changeset viewer.