Changeset 13309 in josm

Timestamp:

2018-01-13T00:52:54+01:00 (7 years ago)

Author:

Don-vip

Message:

fix #15766, see #15688 - fix performance regression introduced in r13229 when drawing a way of many nodes while the filter dialog is open

Location:

trunk/src/org/openstreetmap/josm

Files:

• actions/mapmode/DrawAction.java (modified) (22 diffs)
• actions/mapmode/MapMode.java (modified) (1 diff)
• data/osm/AbstractPrimitive.java (modified) (1 diff)
• data/osm/DataSet.java (modified) (1 diff)
• data/osm/FilterMatcher.java (modified) (2 diffs)
• data/osm/OsmPrimitive.java (modified) (2 diffs)

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

@@ -199 +199 @@
     /**
      * Checks if a map redraw is required and does so if needed. Also updates the status bar.
+     * @param e event, can be null
      * @return true if a repaint is needed
      */
-    private boolean redrawIfRequired() {
+    private boolean redrawIfRequired(Object e) {
         updateStatusLine();
         // repaint required if the helper line is active.
@@ -236 +237 @@
                 needsRepaint = true;
             } else if (!alt && continueFrom != null && !continueFrom.isSelected()) {
-                currentDataSet.addSelected(continueFrom);
+                addSelection(currentDataSet, continueFrom);
                 needsRepaint = true;
             }
+        }
+
+        if (!needsRepaint && e instanceof SelectionChangeEvent) {
+            SelectionChangeEvent event = (SelectionChangeEvent) e;
+            needsRepaint = !event.getOldSelection().isEmpty() && event.getSelection().isEmpty();
         }
 
@@ -250 +256 @@
         ds.beginUpdate(); // to prevent the selection listener to screw around with the state
         try {
-            ds.addSelected(toAdd);
-            ds.clearSelection(toRemove);
+            addSelection(ds, toAdd);
+            clearSelection(ds, toRemove);
         } finally {
             ds.endUpdate();
         }
+    }
+
+    private static void updatePreservedFlag(OsmPrimitive osm, boolean state) {
+        // Preserves selected primitives and selected way nodes
+        osm.setPreserved(state);
+        if (osm instanceof Way) {
+            for (Node n : ((Way) osm).getNodes()) {
+                n.setPreserved(state);
+            }
+        }
+    }
+
+    private static void setSelection(DataSet ds, Collection<OsmPrimitive> toSet) {
+        toSet.stream().forEach(x -> updatePreservedFlag(x, true));
+        ds.setSelected(toSet);
+    }
+
+    private static void setSelection(DataSet ds, OsmPrimitive toSet) {
+        updatePreservedFlag(toSet, true);
+        ds.setSelected(toSet);
+    }
+
+    private static void addSelection(DataSet ds, OsmPrimitive toAdd) {
+        updatePreservedFlag(toAdd, true);
+        ds.addSelected(toAdd);
+    }
+
+    private static void clearSelection(DataSet ds, OsmPrimitive toRemove) {
+        ds.clearSelection(toRemove);
+        updatePreservedFlag(toRemove, false);
     }
 
@@ -303 +339 @@
         map.statusLine.activateAnglePanel(false);
 
-        removeHighlighting();
+        DataSet ds = getLayerManager().getEditDataSet();
+        if (ds != null) {
+            ds.getSelected().stream().forEach(x -> updatePreservedFlag(x, false));
+        }
+
+        removeHighlighting(null);
         map.keyDetector.removeKeyListener(this);
         map.keyDetector.removeModifierExListener(this);
@@ -317 +358 @@
         updateKeyModifiersEx(modifiers);
         computeHelperLine();
-        addHighlighting();
+        addHighlighting(null);
     }
 
@@ -326 +367 @@
         snapHelper.setFixedMode();
         computeHelperLine();
-        redrawIfRequired();
+        redrawIfRequired(e);
     }
 
@@ -339 +380 @@
         snapHelper.unFixOrTurnOff();
         computeHelperLine();
-        redrawIfRequired();
+        redrawIfRequired(e);
     }
 
@@ -351 +392 @@
         // Make sure helper line is computed later (causes deadlock in selection event chain otherwise)
         SwingUtilities.invokeLater(() -> {
+            event.getOldSelection().stream().forEach(x -> updatePreservedFlag(x, false));
+            event.getSelection().stream().forEach(x -> updatePreservedFlag(x, true));
             computeHelperLine();
-            addHighlighting();
+            addHighlighting(event);
         });
     }
 
     private void tryAgain(MouseEvent e) {
-        getLayerManager().getEditDataSet().setSelected();
+        getLayerManager().getEditDataSet().clearSelection();
         mouseReleased(e);
     }
@@ -374 +417 @@
         // Redraw to remove the helper line stub
         computeHelperLine();
-        removeHighlighting();
+        removeHighlighting(null);
     }
 
@@ -445 +488 @@
                 // have to switch modes)
 
-                ds.setSelected(n);
+                setSelection(ds, n);
                 // If we extend/continue an existing way, select it already now to make it obvious
                 Way continueFrom = getWayForNode(n);
                 if (continueFrom != null) {
-                    ds.addSelected(continueFrom);
+                    addSelection(ds, continueFrom);
                 }
 
@@ -627 +670 @@
         }
 
-        ds.setSelected(newSelection);
+        setSelection(ds, newSelection);
 
         // "viewport following" mode for tracing long features
@@ -635 +678 @@
         }
         computeHelperLine();
-        removeHighlighting();
+        removeHighlighting(e);
     }
 
@@ -730 +773 @@
                 (posn0 >= 1                            && targetNode.equals(selectedWay.getNode(posn0-1)))) || // previous node
                 (posn0 < selectedWay.getNodesCount()-1 && targetNode.equals(selectedWay.getNode(posn0+1)))) {  // next node
-                getLayerManager().getEditDataSet().setSelected(targetNode);
+                setSelection(getLayerManager().getEditDataSet(), targetNode);
                 lastUsedNode = targetNode;
                 return true;
@@ -789 +832 @@
 
         computeHelperLine();
-        addHighlighting();
+        addHighlighting(e);
     }
 
@@ -961 +1004 @@
         mousePos = e.getPoint();
         snapHelper.noSnapNow();
-        boolean repaintIssued = removeHighlighting();
+        boolean repaintIssued = removeHighlighting(e);
         // force repaint in case snapHelper needs one. If removeHighlighting
         // caused one already, don't do it again.
@@ -1123 +1166 @@
      * bar both addHighlighting() and repaintIfRequired() are needed, since former fills newHighlights
      * and latter processes them into oldHighlights.
-     */
-    private void addHighlighting() {
+     * @param event event, can be null
+     */
+    private void addHighlighting(Object event) {
         newHighlights = new HashSet<>();
         MapView mapView = MainApplication.getMap().mapView;
@@ -1131 +1175 @@
         if (ctrl) {
             mapView.setNewCursor(cursor, this);
-            redrawIfRequired();
+            redrawIfRequired(event);
             return;
         }
@@ -1143 +1187 @@
             mapView.setNewCursor(cursorJoinNode, this);
             newHighlights.add(mouseOnExistingNode);
-            redrawIfRequired();
+            redrawIfRequired(event);
             return;
         }
@@ -1150 +1194 @@
         if (mouseOnExistingWays.isEmpty()) {
             mapView.setNewCursor(cursor, this);
-            redrawIfRequired();
+            redrawIfRequired(event);
             return;
         }
@@ -1156 +1200 @@
         mapView.setNewCursor(cursorJoinWay, this);
         newHighlights.addAll(mouseOnExistingWays);
-        redrawIfRequired();
+        redrawIfRequired(event);
     }
 
     /**
      * Removes target highlighting from primitives. Issues repaint if required.
+     * @param event event, can be null
      * @return true if a repaint has been issued.
      */
-    private boolean removeHighlighting() {
+    private boolean removeHighlighting(Object event) {
         newHighlights = new HashSet<>();
-        return redrawIfRequired();
+        return redrawIfRequired(event);
     }
 
@@ -1314 +1359 @@
     public Collection<? extends OsmPrimitive> getPreservedPrimitives() {
         DataSet ds = getLayerManager().getEditDataSet();
-        if (ds != null) {
-            // Preserves selected primitives and selected way nodes
-            Set<OsmPrimitive> result = new HashSet<>();
-            for (Way w : ds.getSelectedWays()) {
-                result.addAll(w.getNodes());
-            }
-            result.addAll(ds.getSelected());
-            return result;
-        }
-        return Collections.emptySet();
+        return ds != null ? ds.allPreservedPrimitives() : Collections.emptySet();
     }
 
@@ -1368 +1404 @@
             // select last added node - maybe we will continue drawing from it
             if (n != null) {
-                getLayerManager().getEditDataSet().addSelected(n);
+                addSelection(getLayerManager().getEditDataSet(), n);
             }
         }

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

@@ -233 +233 @@
      * Gets a collection of primitives that should not be hidden by the filter.
      * @return The primitives that the filter should not hide.
+     * @deprecated use {@link org.openstreetmap.josm.data.osm.DataSet#allPreservedPrimitives}
      * @since 11993
      */
+    @Deprecated
     public Collection<? extends OsmPrimitive> getPreservedPrimitives() {
         return Collections.emptySet();

trunk/src/org/openstreetmap/josm/data/osm/AbstractPrimitive.java

@@ -169 +169 @@
      */
     protected static final short FLAG_ANNOTATED = 1 << 12;
+
+    /**
+     * Determines if the primitive is preserved from the filter mechanism.
+     */
+    protected static final short FLAG_PRESERVED = 1 << 13;
 
     /**

trunk/src/org/openstreetmap/josm/data/osm/DataSet.java

@@ -518 +518 @@
 
     /**
+     * Returns a collection containing all primitives preserved from filtering.
+     * @return A collection containing all primitives preserved from filtering.
+     * @see OsmPrimitive#isPreserved
+     * @since 13309
+     */
+    public Collection<OsmPrimitive> allPreservedPrimitives() {
+        return getPrimitives(OsmPrimitive::isPreserved);
+    }
+
+    /**
      * Adds a primitive to the dataset.
      *

trunk/src/org/openstreetmap/josm/data/osm/FilterMatcher.java

@@ -11 +11 @@
 import org.openstreetmap.josm.data.osm.search.SearchMode;
 import org.openstreetmap.josm.data.osm.search.SearchParseError;
-import org.openstreetmap.josm.gui.MainApplication;
-import org.openstreetmap.josm.gui.MapFrame;
 import org.openstreetmap.josm.tools.SubclassFilteredCollection;
 
@@ -225 +223 @@
 
     private static FilterType test(List<FilterInfo> filters, OsmPrimitive primitive, boolean hidden) {
-        MapFrame map = MainApplication.getMap();
-        if (primitive.isIncomplete() ||
-                (map != null && map.mapMode != null && map.mapMode.getPreservedPrimitives().contains(primitive)))
+        if (primitive.isIncomplete() || primitive.isPreserved())
             return FilterType.NOT_FILTERED;
 

trunk/src/org/openstreetmap/josm/data/osm/OsmPrimitive.java

@@ -428 +428 @@
 
     /**
+     * Set binary property used internally by the filter mechanism.
+     * @param isPreserved new "preserved" flag value
+     * @since 13309
+     */
+    public void setPreserved(boolean isPreserved) {
+        updateFlags(FLAG_PRESERVED, isPreserved);
+    }
+
+    /**
      * Replies true, if this primitive is disabled. (E.g. a filter applies)
      * @return {@code true} if this object has the "disabled" flag enabled
@@ -457 +466 @@
     public boolean getDisabledType() {
         return (flags & FLAG_DISABLED_TYPE) != 0;
+    }
+
+    /**
+     * Replies true, if this primitive is preserved from filtering.
+     * @return {@code true} if this object has the "preserved" flag enabled
+     * @since 13309
+     */
+    public boolean isPreserved() {
+        return (flags & FLAG_PRESERVED) != 0;
     }