Changeset 14628 in josm

Timestamp:

2019-01-03T20:06:44+01:00 (6 years ago)

Author:

simon04

Message:

fix #16706 - Zoom to selection should not zoom out for zoom on a node

Simplify bound enlargements in org.openstreetmap.josm.actions.AutoScaleAction#modeSelectionOrConflict

Location:

trunk/src/org/openstreetmap/josm

Files:

• actions/AutoScaleAction.java (modified) (4 diffs)
• data/ProjectionBounds.java (modified) (1 diff)
• data/osm/visitor/BoundingXYVisitor.java (modified) (5 diffs)
• gui/NavigatableComponent.java (modified) (1 diff)

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

@@ -292 +292 @@
                 mapView.zoomNext();
                 break;
-            default:
-                BoundingXYVisitor bbox = getBoundingBox();
-                if (bbox != null && bbox.getBounds() != null) {
-                    mapView.zoomTo(bbox);
-                }
-            }
-        }
-        putValue("active", Boolean.TRUE);
+            case PROBLEM:
+                modeProblem(new ValidatorBoundingXYVisitor());
+                break;
+            case DATA:
+                modeData(new BoundingXYVisitor());
+                break;
+            case LAYER:
+                modeLayer(new BoundingXYVisitor());
+                break;
+            case SELECTION:
+            case CONFLICT:
+                modeSelectionOrConflict(new BoundingXYVisitor());
+                break;
+            case DOWNLOAD:
+                modeDownload(new BoundingXYVisitor());
+                break;
+            }
+            putValue("active", Boolean.TRUE);
+        }
     }
 
@@ -328 +339 @@
     }
 
-    private BoundingXYVisitor getBoundingBox() {
-        switch (mode) {
-        case PROBLEM:
-            return modeProblem(new ValidatorBoundingXYVisitor());
-        case DATA:
-            return modeData(new BoundingXYVisitor());
-        case LAYER:
-            return modeLayer(new BoundingXYVisitor());
-        case SELECTION:
-        case CONFLICT:
-            return modeSelectionOrConflict(new BoundingXYVisitor());
-        case DOWNLOAD:
-            return modeDownload(new BoundingXYVisitor());
-        default:
-            return new BoundingXYVisitor();
-        }
-    }
-
-    private static BoundingXYVisitor modeProblem(ValidatorBoundingXYVisitor v) {
+    private static void modeProblem(ValidatorBoundingXYVisitor v) {
         TestError error = MainApplication.getMap().validatorDialog.getSelectedError();
         if (error == null)
-            return null;
+            return;
         v.visit(error);
         if (v.getBounds() == null)
-            return null;
-        v.enlargeBoundingBox(Config.getPref().getDouble("validator.zoom-enlarge-bbox", 0.0002));
-        return v;
-    }
-
-    private static BoundingXYVisitor modeData(BoundingXYVisitor v) {
+            return;
+        MainApplication.getMap().mapView.zoomTo(v);
+    }
+
+    private static void modeData(BoundingXYVisitor v) {
         for (Layer l : MainApplication.getLayerManager().getLayers()) {
             l.visitBoundingBox(v);
         }
-        return v;
-    }
-
-    private BoundingXYVisitor modeLayer(BoundingXYVisitor v) {
+        MainApplication.getMap().mapView.zoomTo(v);
+    }
+
+    private void modeLayer(BoundingXYVisitor v) {
         // try to zoom to the first selected layer
         Layer l = getFirstSelectedLayer();
         if (l == null)
-            return null;
+            return;
         l.visitBoundingBox(v);
-        return v;
-    }
-
-    private BoundingXYVisitor modeSelectionOrConflict(BoundingXYVisitor v) {
+        MainApplication.getMap().mapView.zoomTo(v);
+    }
+
+    private void modeSelectionOrConflict(BoundingXYVisitor v) {
         Collection<IPrimitive> sel = new HashSet<>();
         if (AutoScaleMode.SELECTION == mode) {
@@ -395 +387 @@
                     tr("Information"),
                     JOptionPane.INFORMATION_MESSAGE);
-            return null;
+            return;
         }
         for (IPrimitive osm : sel) {
             osm.accept(v);
         }
-
-        // Increase the bounding box by up to 100% to give more context.
-        v.enlargeBoundingBoxLogarithmically(100);
-        // Make the bounding box at least 100 meter wide to
-        // ensure reasonable zoom level when zooming onto single nodes.
-        v.enlargeToMinSize(Config.getPref().getDouble("zoom_to_selection_min_size_in_meter", 100));
-        return v;
-    }
-
-    private BoundingXYVisitor modeDownload(BoundingXYVisitor v) {
+        if (v.getBounds() == null) {
+            return;
+        }
+
+        // Do not zoom if the current scale covers the selection, #16706
+        final MapView mapView = MainApplication.getMap().mapView;
+        final double mapScale = mapView.getScale();
+        final double minScale = v.getBounds().getScale(mapView.getWidth(), mapView.getHeight());
+        v.enlargeBoundingBoxLogarithmically();
+        final double maxScale = v.getBounds().getScale(mapView.getWidth(), mapView.getHeight());
+        if (minScale <= mapScale && mapScale < maxScale) {
+            mapView.zoomTo(v.getBounds().getCenter());
+        } else {
+            mapView.zoomTo(v);
+        }
+    }
+
+    private void modeDownload(BoundingXYVisitor v) {
         if (lastZoomTime > 0 &&
                 System.currentTimeMillis() - lastZoomTime > Config.getPref().getLong("zoom.bounds.reset.time", TimeUnit.SECONDS.toMillis(10))) {
@@ -438 +438 @@
             }
         }
-        return v;
+        MainApplication.getMap().mapView.zoomTo(v);
     }
 

trunk/src/org/openstreetmap/josm/data/ProjectionBounds.java

@@ -192 +192 @@
         return !Utils.equalsEpsilon(minEast, maxEast) || !Utils.equalsEpsilon(minNorth, maxNorth);
     }
+
+    /**
+     * Computes the scale of this bounds with respect to the given width/height.
+     * @param width the width
+     * @param height the height
+     * @return the computed scale
+     */
+    public double getScale(final int width, final int height) {
+        // -20 to leave some border
+        int w = width - 20;
+        if (w < 20) {
+            w = 20;
+        }
+        int h = height - 20;
+        if (h < 20) {
+            h = 20;
+        }
+
+        double scaleX = getDeltaEast() / w;
+        double scaleY = getDeltaNorth() / h;
+        return Math.max(scaleX, scaleY);
+    }
+
+    private double getDeltaNorth() {
+        return maxNorth - minNorth;
+    }
+
+    private double getDeltaEast() {
+        return maxEast - minEast;
+    }
 }

trunk/src/org/openstreetmap/josm/data/osm/visitor/BoundingXYVisitor.java

@@ -3 +3 @@
 
 import java.util.Collection;
+import java.util.function.DoubleUnaryOperator;
 
 import org.openstreetmap.josm.data.Bounds;
@@ -19 +20 @@
 import org.openstreetmap.josm.data.osm.Way;
 import org.openstreetmap.josm.data.projection.ProjectionRegistry;
-import org.openstreetmap.josm.gui.MainApplication;
-import org.openstreetmap.josm.gui.MapFrame;
 import org.openstreetmap.josm.spi.preferences.Config;
 
@@ -141 +140 @@
 
     /**
-     * Enlarges the calculated bounding box by 0.002 degrees.
+     * Enlarges the calculated bounding box by 0.001 degrees.
      * If the bounding box has not been set (<code>min</code> or <code>max</code>
      * equal <code>null</code>) this method does not do anything.
      */
     public void enlargeBoundingBox() {
-        enlargeBoundingBox(Config.getPref().getDouble("edit.zoom-enlarge-bbox", 0.002));
+        final double enlarge = Config.getPref().getDouble("edit.zoom-enlarge-bbox", 0.001);
+        enlargeBoundingBox(enlarge, enlarge);
     }
 
@@ -154 +154 @@
      * equal <code>null</code>) this method does not do anything.
      *
-     * @param enlargeDegree number of degrees to enlarge on each side
-     */
-    public void enlargeBoundingBox(double enlargeDegree) {
+     * @param enlargeDegreeX number of degrees to enlarge on each side along X
+     * @param enlargeDegreeY number of degrees to enlarge on each side along Y
+     */
+    public void enlargeBoundingBox(double enlargeDegreeX, double enlargeDegreeY) {
         if (bounds == null)
             return;
@@ -162 +163 @@
         LatLon maxLatlon = ProjectionRegistry.getProjection().eastNorth2latlon(bounds.getMax());
         bounds = new ProjectionBounds(new LatLon(
-                        Math.max(-90, minLatlon.lat() - enlargeDegree),
-                        Math.max(-180, minLatlon.lon() - enlargeDegree)).getEastNorth(ProjectionRegistry.getProjection()),
+                        Math.max(-90, minLatlon.lat() - enlargeDegreeY),
+                        Math.max(-180, minLatlon.lon() - enlargeDegreeX)).getEastNorth(ProjectionRegistry.getProjection()),
                 new LatLon(
-                        Math.min(90, maxLatlon.lat() + enlargeDegree),
-                        Math.min(180, maxLatlon.lon() + enlargeDegree)).getEastNorth(ProjectionRegistry.getProjection()));
-    }
-
-    /**
-     * Enlarges the bounding box up to <code>maxEnlargePercent</code>, depending on
+                        Math.min(90, maxLatlon.lat() + enlargeDegreeY),
+                        Math.min(180, maxLatlon.lon() + enlargeDegreeX)).getEastNorth(ProjectionRegistry.getProjection()));
+    }
+
+    /**
+     * Enlarges the bounding box up to 0.001 degrees, depending on
      * its size. If the bounding box is small, it will be enlarged more in relation
      * to its beginning size. The larger the bounding box, the smaller the change,
-     * down to the minimum of 1% enlargement.
-     *
-     * Warning: if the bounding box only contains a single node, no expansion takes
-     * place because a node has no width/height. Use {@link #enlargeBoundingBox(double)}
-     * instead.
-     *
-     * Example: You specify enlargement to be up to 100%.
-     *
-     *          Bounding box is a small house: enlargement will be 95–100%, i.e.
-     *          making enough space so that the house fits twice on the screen in
-     *          each direction.
-     *
-     *          Bounding box is a large landuse, like a forest: Enlargement will
-     *          be 1–10%, i.e. just add a little border around the landuse.
+     * down to 0.0 degrees.
      *
      * If the bounding box has not been set (<code>min</code> or <code>max</code>
      * equal <code>null</code>) this method does not do anything.
-     *
-     * @param maxEnlargePercent maximum enlargement in percentage (100.0 for 100%)
-     */
-    public void enlargeBoundingBoxLogarithmically(double maxEnlargePercent) {
+     */
+    public void enlargeBoundingBoxLogarithmically() {
         if (bounds == null)
             return;
-
-        double diffEast = bounds.getMax().east() - bounds.getMin().east();
-        double diffNorth = bounds.getMax().north() - bounds.getMin().north();
-
-        double enlargeEast = Math.min(maxEnlargePercent - 10*Math.log(diffEast), 1)/100;
-        double enlargeNorth = Math.min(maxEnlargePercent - 10*Math.log(diffNorth), 1)/100;
-
-        visit(bounds.getMin().add(-enlargeEast/2, -enlargeNorth/2));
-        visit(bounds.getMax().add(+enlargeEast/2, +enlargeNorth/2));
-    }
-
-    /**
-     * Specify a degree larger than 0 in order to make the bounding box at least
-     * the specified size in width and height. The value is ignored if the
-     * bounding box is already larger than the specified amount.
-     *
-     * If the bounding box has not been set (<code>min</code> or <code>max</code>
-     * equal <code>null</code>) this method does not do anything.
-     *
-     * If the bounding box contains objects and is to be enlarged, the objects
-     * will be centered within the new bounding box.
-     *
-     * @param size minimum width and height in meter
-     */
-    public void enlargeToMinSize(double size) {
-        if (bounds == null)
-            return;
-        // convert size from meters to east/north units
-        MapFrame map = MainApplication.getMap();
-        double enSize = size * map.mapView.getScale() / map.mapView.getDist100Pixel() * 100;
-        visit(bounds.getMin().add(-enSize/2, -enSize/2));
-        visit(bounds.getMax().add(+enSize/2, +enSize/2));
+        final LatLon min = ProjectionRegistry.getProjection().eastNorth2latlon(bounds.getMin());
+        final LatLon max = ProjectionRegistry.getProjection().eastNorth2latlon(bounds.getMax());
+        final double deltaLat = max.lat() - min.lat();
+        final double deltaLon = max.lon() - min.lon();
+        // [0.001, 0.1] degree -> [0.001, 0.0] degree enlargement
+        final DoubleUnaryOperator enlargement = deg -> deg < 0.001
+                ? 0.001
+                : deg < 0.1
+                ? 0.001 - deg / 100
+                : 0.0;
+        enlargeBoundingBox(enlargement.applyAsDouble(deltaLon), enlargement.applyAsDouble(deltaLat));
     }
 

trunk/src/org/openstreetmap/josm/gui/NavigatableComponent.java

@@ -790 +790 @@
      */
     public void zoomTo(ProjectionBounds box) {
-        // -20 to leave some border
-        int w = getWidth()-20;
-        if (w < 20) {
-            w = 20;
-        }
-        int h = getHeight()-20;
-        if (h < 20) {
-            h = 20;
-        }
-
-        double scaleX = (box.maxEast-box.minEast)/w;
-        double scaleY = (box.maxNorth-box.minNorth)/h;
-        double newScale = Math.max(scaleX, scaleY);
-
+        double newScale = box.getScale(getWidth(), getHeight());
         newScale = scaleFloor(newScale);
         zoomTo(box.getCenter(), newScale);