Changeset 17141 in josm

Timestamp:

2020-10-10T11:48:49+02:00 (4 years ago)

Author:

GerdP

Message:

see #19885: memory leak with "temporary" objects in validator and actions
next bunch of changes:

• use ChangeNodesCommand instead of ChangeCommand
• further simplifications

Location:

trunk/src/org/openstreetmap/josm

Files:

• actions/FollowLineAction.java (modified) (2 diffs)
• actions/JoinAreasAction.java (modified) (3 diffs)
• actions/JoinNodeWayAction.java (modified) (2 diffs)
• actions/mapmode/ExtrudeAction.java (modified) (8 diffs)
• actions/mapmode/ImproveWayAccuracyAction.java (modified) (4 diffs)
• data/validation/tests/DuplicatedWayNodes.java (modified) (2 diffs)
• tools/Geometry.java (modified) (2 diffs)

trunk/src/org/openstreetmap/josm/actions/FollowLineAction.java

@@ -12 +12 @@
 
 import org.openstreetmap.josm.actions.mapmode.DrawAction;
-import org.openstreetmap.josm.command.ChangeCommand;
+import org.openstreetmap.josm.command.ChangeNodesCommand;
 import org.openstreetmap.josm.command.SelectCommand;
 import org.openstreetmap.josm.command.SequenceCommand;
@@ -109 +109 @@
         }
         if (newPoint != null) {
-            Way newFollower = new Way(follower);
+            List<Node> modNodes = follower.getNodes();
             if (reversed) {
-                newFollower.addNode(0, newPoint);
+                modNodes.add(0, newPoint);
             } else {
-                newFollower.addNode(newPoint);
+                modNodes.add(newPoint);
             }
+            boolean isClosed = modNodes.size() >= 3 && modNodes.get(modNodes.size()-1) == modNodes.get(0);
             UndoRedoHandler.getInstance().add(new SequenceCommand(tr("Follow line"),
-                    new ChangeCommand(ds, follower, newFollower),
-                    new SelectCommand(ds, newFollower.isClosed() // see #10028 - unselect last node when closing a way
+                    new ChangeNodesCommand(ds, follower, modNodes),
+                    new SelectCommand(ds, isClosed // see #10028 - unselect last node when closing a way
                             ? Arrays.<OsmPrimitive>asList(follower)
                             : Arrays.<OsmPrimitive>asList(follower, newPoint)

trunk/src/org/openstreetmap/josm/actions/JoinAreasAction.java

@@ -27 +27 @@
 import org.openstreetmap.josm.command.AddCommand;
 import org.openstreetmap.josm.command.ChangeCommand;
+import org.openstreetmap.josm.command.ChangeNodesCommand;
 import org.openstreetmap.josm.command.Command;
 import org.openstreetmap.josm.command.DeleteCommand;
@@ -793 +794 @@
             discardedWays.remove(oldest);
             discardedWays.add(way);
-            Way copy = new Way(oldest);
-            copy.setNodes(way.getNodes());
-            cmds.add(new ChangeCommand(oldest, copy));
+            cmds.add(new ChangeNodesCommand(oldest, way.getNodes()));
             return oldest;
         }
@@ -875 +874 @@
                 }
 
-                Way newWay = new Way(way);
-                newWay.setNodes(newNodes);
-                cmds.add(new ChangeCommand(way, newWay));
+                cmds.add(new ChangeNodesCommand(way, newNodes));
                 ++totalWaysModified;
             }

trunk/src/org/openstreetmap/josm/actions/JoinNodeWayAction.java

@@ -23 +23 @@
 import javax.swing.JOptionPane;
 
-import org.openstreetmap.josm.command.ChangeCommand;
+import org.openstreetmap.josm.command.ChangeNodesCommand;
 import org.openstreetmap.josm.command.Command;
 import org.openstreetmap.josm.command.MoveCommand;
@@ -192 +192 @@
                 wayNodes.addAll(segmentIndex + 1, nodesToAdd);
             }
-            Way wnew = new Way(w);
-            wnew.setNodes(wayNodes);
-            cmds.add(new ChangeCommand(ds, w, wnew));
+            cmds.add(new ChangeNodesCommand(ds, w, wayNodes));
         }
 

trunk/src/org/openstreetmap/josm/actions/mapmode/ExtrudeAction.java

@@ -31 +31 @@
 import org.openstreetmap.josm.actions.MergeNodesAction;
 import org.openstreetmap.josm.command.AddCommand;
-import org.openstreetmap.josm.command.ChangeCommand;
+import org.openstreetmap.josm.command.ChangeNodesCommand;
 import org.openstreetmap.josm.command.Command;
 import org.openstreetmap.josm.command.MoveCommand;
@@ -586 +586 @@
             EastNorth b = ws.getSecondNode().getEastNorth();
             n.setEastNorth(Geometry.closestPointToSegment(a, b, n.getEastNorth()));
-            Way wnew = new Way(ws.way);
-            wnew.addNode(ws.lowerIndex+1, n);
+            List<Node> modnodes = ws.way.getNodes();
+            modnodes.add(ws.lowerIndex+1, n);
             DataSet ds = ws.way.getDataSet();
             UndoRedoHandler.getInstance().add(new SequenceCommand(tr("Add a new node to an existing way"),
-                    new AddCommand(ds, n), new ChangeCommand(ds, ws.way, wnew)));
+                    new AddCommand(ds, n), new ChangeNodesCommand(ds, ws.way, modnodes)));
         }
     }
@@ -634 +634 @@
         // create extrusion
         Collection<Command> cmds = new LinkedList<>();
-        Way wnew = new Way(selectedSegment.way);
+        List<Node> modNodes = selectedSegment.way.getNodes();
         boolean wayWasModified = false;
-        boolean wayWasSingleSegment = wnew.getNodesCount() == 2;
+        boolean wayWasSingleSegment = modNodes.size() == 2;
         int insertionPoint = selectedSegment.lowerIndex + 1;
 
@@ -656 +656 @@
             Node n1Old = selectedSegment.getFirstNode();
             Node n1New = new Node(ProjectionRegistry.getProjection().eastNorth2latlon(newN1en));
-            wnew.addNode(insertionPoint, n1New);
-            wnew.removeNode(n1Old);
+            modNodes.add(insertionPoint, n1New);
+            modNodes.remove(n1Old);
             wayWasModified = true;
             cmds.add(new AddCommand(ds, n1New));
@@ -664 +664 @@
             //introduce new node
             Node n1New = new Node(ProjectionRegistry.getProjection().eastNorth2latlon(newN1en));
-            wnew.addNode(insertionPoint, n1New);
+            modNodes.add(insertionPoint, n1New);
             wayWasModified = true;
             insertionPoint++;
@@ -686 +686 @@
             Node n2Old = selectedSegment.getSecondNode();
             Node n2New = new Node(ProjectionRegistry.getProjection().eastNorth2latlon(newN2en));
-            wnew.addNode(insertionPoint, n2New);
-            wnew.removeNode(n2Old);
+            modNodes.add(insertionPoint, n2New);
+            modNodes.remove(n2Old);
             wayWasModified = true;
             cmds.add(new AddCommand(ds, n2New));
@@ -694 +694 @@
             //introduce new node
             Node n2New = new Node(ProjectionRegistry.getProjection().eastNorth2latlon(newN2en));
-            wnew.addNode(insertionPoint, n2New);
+            modNodes.add(insertionPoint, n2New);
             wayWasModified = true;
             cmds.add(new AddCommand(ds, n2New));
@@ -702 +702 @@
         //the way was a single segment, close the way
         if (wayWasSingleSegment) {
-            wnew.addNode(selectedSegment.getFirstNode());
+            modNodes.add(modNodes.get(0));
             wayWasModified = true;
         }
         if (wayWasModified) {
             // we only need to change the way if its node list was really modified
-            cmds.add(new ChangeCommand(selectedSegment.way, wnew));
+            cmds.add(new ChangeNodesCommand(selectedSegment.way, modNodes));
         }
         Command c = new SequenceCommand(tr("Extrude Way"), cmds);

trunk/src/org/openstreetmap/josm/actions/mapmode/ImproveWayAccuracyAction.java

@@ -24 +24 @@
 
 import org.openstreetmap.josm.command.AddCommand;
-import org.openstreetmap.josm.command.ChangeCommand;
+import org.openstreetmap.josm.command.ChangeNodesCommand;
 import org.openstreetmap.josm.command.Command;
 import org.openstreetmap.josm.command.DeleteCommand;
@@ -461 +461 @@
                 for (WaySegment virtualSegment : virtualSegments) {
                     Way w = virtualSegment.way;
-                    Way wnew = new Way(w);
-                    wnew.addNode(virtualSegment.lowerIndex + 1, virtualNode);
-                    virtualCmds.add(new ChangeCommand(w, wnew));
+                    List<Node> modNodes = w.getNodes();
+                    modNodes.add(virtualSegment.lowerIndex + 1, virtualNode);
+                    virtualCmds.add(new ChangeNodesCommand(w, modNodes));
                 }
 
@@ -481 +481 @@
                 if (referrersCount != 1 || referrerWayCount != 1) {
                     // detach node from way
-                    final Way newWay = new Way(targetWay);
-                    final List<Node> nodes = newWay.getNodes();
+                    final List<Node> nodes = targetWay.getNodes();
                     nodes.remove(candidateNode);
-                    newWay.setNodes(nodes);
                     if (nodes.size() < 2) {
                         final Command deleteCmd = DeleteCommand.delete(Collections.singleton(targetWay), true);
@@ -490 +488 @@
                             UndoRedoHandler.getInstance().add(deleteCmd);
                         }
-                        newWay.setNodes(null);
                     } else {
-                        UndoRedoHandler.getInstance().add(new ChangeCommand(targetWay, newWay));
+                        UndoRedoHandler.getInstance().add(new ChangeNodesCommand(targetWay, nodes));
                     }
                 } else if (candidateNode.isTagged()) {

trunk/src/org/openstreetmap/josm/data/validation/tests/DuplicatedWayNodes.java

@@ -4 +4 @@
 import static org.openstreetmap.josm.tools.I18n.tr;
 
+import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Iterator;
+import java.util.List;
 
-import org.openstreetmap.josm.command.ChangeCommand;
+import org.openstreetmap.josm.command.ChangeNodesCommand;
 import org.openstreetmap.josm.command.Command;
 import org.openstreetmap.josm.data.osm.Node;
@@ -59 +61 @@
         if (it.hasNext()) {
             Way w = (Way) it.next();
-            Way wnew = new Way(w);
-            wnew.setNodes(null);
             Node lastN = null;
+            List<Node> modNodes = new ArrayList<>();
             for (Node n : w.getNodes()) {
-                if (lastN == null) {
-                    wnew.addNode(n);
-                } else if (n == lastN) {
-                    // Skip this node
-                } else {
-                    wnew.addNode(n);
+                if (n != lastN) {
+                    modNodes.add(n);
                 }
                 lastN = n;
             }
-            if (wnew.getNodesCount() < 2)
+            if (modNodes.size() < 2)
                 // Empty way, delete
                 return deletePrimitivesIfNeeded(Collections.singleton(w));
             else
-                return new ChangeCommand(w, wnew);
+                return new ChangeNodesCommand(w, modNodes);
         }
         return null;

trunk/src/org/openstreetmap/josm/tools/Geometry.java

@@ -22 +22 @@
 
 import org.openstreetmap.josm.command.AddCommand;
-import org.openstreetmap.josm.command.ChangeCommand;
+import org.openstreetmap.josm.command.ChangeNodesCommand;
 import org.openstreetmap.josm.command.Command;
 import org.openstreetmap.josm.data.coor.EastNorth;
@@ -231 +231 @@
 
         for (int pos = 0; pos < ways.size(); pos++) {
-            if (!changedWays[pos]) {
-                continue;
-            }
-
-            Way way = ways.get(pos);
-            Way newWay = new Way(way);
-            newWay.setNodes(newNodes[pos]);
-
-            cmds.add(new ChangeCommand(dataset, way, newWay));
+            if (changedWays[pos]) {
+                cmds.add(new ChangeNodesCommand(dataset, ways.get(pos), newNodes[pos]));
+            }
         }