Changeset 5098 in josm for trunk

Timestamp:

2012-03-18T11:42:54+01:00 (12 years ago)

Author:

xeen

Message:

Reduce repaints required when in draw mode. This should improve performance a bit with target highlighting activated. See #7503

File:

• trunk/src/org/openstreetmap/josm/actions/mapmode/DrawAction.java (modified) (19 diffs)

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

@@ -2 +2 @@
 package org.openstreetmap.josm.actions.mapmode;
 
-import javax.swing.JCheckBoxMenuItem;
-import javax.swing.JMenuItem;
+import static org.openstreetmap.josm.gui.help.HelpUtil.ht;
+import static org.openstreetmap.josm.tools.I18n.marktr;
 import static org.openstreetmap.josm.tools.I18n.tr;
 import static org.openstreetmap.josm.tools.I18n.trn;
-import static org.openstreetmap.josm.tools.I18n.marktr;
-import static org.openstreetmap.josm.gui.help.HelpUtil.ht;
 
 import java.awt.AWTEvent;
@@ -14 +12 @@
 import java.awt.Cursor;
 import java.awt.Graphics2D;
-import java.awt.MenuItem;
 import java.awt.Point;
 import java.awt.Stroke;
@@ -38 +35 @@
 import java.util.Set;
 import java.util.TreeSet;
+
 import javax.swing.AbstractAction;
+import javax.swing.JCheckBoxMenuItem;
+import javax.swing.JMenuItem;
 import javax.swing.JOptionPane;
 import javax.swing.JPopupMenu;
@@ -66 +66 @@
 import org.openstreetmap.josm.gui.layer.OsmDataLayer;
 import org.openstreetmap.josm.gui.widgets.PopupMenuLauncher;
+import org.openstreetmap.josm.tools.Geometry;
 import org.openstreetmap.josm.tools.ImageProvider;
 import org.openstreetmap.josm.tools.Pair;
 import org.openstreetmap.josm.tools.Shortcut;
 import org.openstreetmap.josm.tools.Utils;
-import org.openstreetmap.josm.tools.Geometry;
 
 /**
@@ -84 +84 @@
     private Node mouseOnExistingNode;
     private Set<Way> mouseOnExistingWays = new HashSet<Way>();
+    // old highlights store which primitives are currently highlighted. This
+    // is true, even if target highlighting is disabled since the status bar
+    // derives its information from this list as well.
     private Set<OsmPrimitive> oldHighlights = new HashSet<OsmPrimitive>();
+    // new highlights contains a list of primitives that should be highlighted
+    // but haven’t been so far. The idea is to compare old and new and only
+    // repaint if there are changes.
+    private Set<OsmPrimitive> newHighlights = new HashSet<OsmPrimitive>();
     private boolean drawHelperLine;
     private boolean wayIsFinished = false;
@@ -132 +139 @@
     private void redrawIfRequired() {
         updateStatusLine();
+        // repaint required if the helper line is active.
+        boolean needsRepaint = drawHelperLine && !wayIsFinished;
+        // move newHighlights to oldHighlights; only update changed primitives
+        for(OsmPrimitive x : newHighlights) {
+            if(oldHighlights.contains(x)) {
+                continue;
+            }
+            needsRepaint = true;
+            x.setHighlighted(true);
+        }
+        oldHighlights.removeAll(newHighlights);
+        for(OsmPrimitive x : oldHighlights) {
+            x.setHighlighted(false);
+            needsRepaint = true;
+        }
+        oldHighlights = newHighlights;
+
         if ((!drawHelperLine || wayIsFinished) && !drawTargetHighlight)
             return;
+
         // update selection to reflect which way being modified
         if (currentBaseNode != null && getCurrentDataSet().getSelected().isEmpty() == false) {
             Way continueFrom = getWayForNode(currentBaseNode);
-            if (alt && continueFrom != null) {
+            if (alt && continueFrom != null && (!currentBaseNode.isSelected() || continueFrom.isSelected())) {
                 getCurrentDataSet().beginUpdate(); // to prevent the selection listener to screw around with the state
                 getCurrentDataSet().addSelected(currentBaseNode);
                 getCurrentDataSet().clearSelection(continueFrom);
                 getCurrentDataSet().endUpdate();
-            } else if (!alt && continueFrom != null) {
+                needsRepaint = true;
+            } else if (!alt && continueFrom != null && !continueFrom.isSelected()) {
                 getCurrentDataSet().addSelected(continueFrom);
-            }
-        }
-        Main.map.mapView.repaint();
+                needsRepaint = true;
+            }
+        }
+
+        if(needsRepaint) {
+            Main.map.mapView.repaint();
+        }
     }
 
@@ -228 +258 @@
         computeHelperLine();
         addHighlighting();
-        redrawIfRequired();
     }
 
@@ -278 +307 @@
         computeHelperLine();
         addHighlighting();
-        redrawIfRequired();
     }
 
@@ -303 +331 @@
         computeHelperLine();
         removeHighlighting();
-        redrawIfRequired();
     }
 
@@ -579 +606 @@
         computeHelperLine();
         removeHighlighting();
-        redrawIfRequired();
     }
 
@@ -707 +733 @@
         computeHelperLine();
         addHighlighting();
-        redrawIfRequired();
     }
 
@@ -723 +748 @@
             return;
         }
-
-        double distance = -1;
-        double angle = -1;
 
         Collection<OsmPrimitive> selection = getCurrentDataSet().getSelected();
@@ -972 +994 @@
     /**
      * Takes the data from computeHelperLine to determine which ways/nodes should be highlighted
-     * (if feature enabled). Also sets the target cursor if appropriate.
+     * (if feature enabled). Also sets the target cursor if appropriate. It adds the to-be-
+     * highlighted primitives to newHighlights but does not actually highlight them. This work is
+     * done in redrawIfRequired. This means, calling addHighlighting() without redrawIfRequired()
+     * will leave the data in an inconsistent state.
+     * 
+     * The status bar derives its information from oldHighlights, so in order to update the status
+     * bar both addHighlighting() and repaintIfRequired() are needed, since former fills newHighlights
+     * and latter processes them into oldHighlights.
      */
     private void addHighlighting() {
-        removeHighlighting();
+        newHighlights = new HashSet<OsmPrimitive>();
+
         // if ctrl key is held ("no join"), don't highlight anything
         if (ctrl) {
             Main.map.mapView.setNewCursor(cursor, this);
+            redrawIfRequired();
             return;
         }
@@ -990 +1021 @@
         if (mouseOnExistingNode != null) {
             Main.map.mapView.setNewCursor(cursorJoinNode, this);
-            // We also need this list for the statusbar help text
-            oldHighlights.add(mouseOnExistingNode);
-            if(drawTargetHighlight) {
-                mouseOnExistingNode.setHighlighted(true);
-        }
+            newHighlights.add(mouseOnExistingNode);
+            redrawIfRequired();
             return;
         }
@@ -1001 +1029 @@
         if (mouseOnExistingWays.size() == 0) {
             Main.map.mapView.setNewCursor(cursor, this);
+            redrawIfRequired();
             return;
         }
 
         Main.map.mapView.setNewCursor(cursorJoinWay, this);
-
-        // We also need this list for the statusbar help text
-        oldHighlights.addAll(mouseOnExistingWays);
-        if (!drawTargetHighlight) return;
-        for (Way w : mouseOnExistingWays) {
-            w.setHighlighted(true);
-        }
+        newHighlights.addAll(mouseOnExistingWays);
+        redrawIfRequired();
     }
 
     /**
-     * Removes target highlighting from primitives
+     * Removes target highlighting from primitives. Issues repaint if required.
      */
     private void removeHighlighting() {
-        for(OsmPrimitive prim : oldHighlights) {
-            prim.setHighlighted(false);
-        }
-        oldHighlights = new HashSet<OsmPrimitive>();
+        newHighlights = new HashSet<OsmPrimitive>();
+        redrawIfRequired();
     }
 
@@ -1041 +1063 @@
             g2.setColor(selectedColor);
             g2.setStroke(new BasicStroke(3, BasicStroke.CAP_ROUND, BasicStroke.JOIN_ROUND));
-        } else if (!snapHelper.drawConstructionGeometry) {
-            return;
-        }
+        } else if (!snapHelper.drawConstructionGeometry)
+            return;
         GeneralPath b = new GeneralPath();
         Point p1=mv.getPoint(currentBaseNode);
@@ -1169 +1190 @@
                         wayIsFinished=false;
                     }  else {
-                    // if more than 1 node were affected by previous command,
-                    // we have no way to continue, so we forget about found node
+                        // if more than 1 node were affected by previous command,
+                        // we have no way to continue, so we forget about found node
                         n=null;
                         break;
@@ -1178 +1199 @@
             // select last added node - maybe we will continue drawing from it
             if (n!=null) getCurrentDataSet().addSelected(n);
-       }
+        }
     }
 
@@ -1616 +1637 @@
         @Override
         public void actionPerformed(ActionEvent e) {
-               if (snapHelper!=null) snapHelper.toggleSnapping();
+            if (snapHelper!=null) snapHelper.toggleSnapping();
         }
     }