Modify

Opened 7 months ago

Closed 7 weeks ago

#16706 closed enhancement (fixed)

[Patch] zoom to selection should not zoom out for zoom on a node

Reported by: dieterdreist Owned by: simon04
Priority: normal Milestone: 19.01
Component: Core Version: latest
Keywords: zoom to selection, zoom Cc:

Description

If you click "zoom to selection" (from the selection window, right click), on a node, JOSM zooms to a "reasonable zoom level", but it should only zoom in (if necessary to reach this reasonable zoom level), it should not zoom out (if you were already zoomed in).

this is my system info:

Relative:URL: ^/trunk
Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Last:Changed Date: 2018-09-02 22:28:43 +0200 (Sun, 02 Sep 2018)
Revision:14220
Build-Date:2018-09-03 01:32:21
URL:https://josm.openstreetmap.de/svn/trunk

Identification: JOSM/1.5 (14220 en) Mac OS X 10.13.6
OS Build number: Mac OS X 10.13.6 (17G65)
Memory Usage: 438 MB / 2048 MB (92 MB allocated, but free)
Java version: 10.0.2+13, Oracle Corporation, Java HotSpot(TM) 64-Bit Server VM
Screen: Display 188875522 1920x1080, Display 458628992 1920x1080
Maximum Screen Size: 1920x1080
VM arguments: [-Dsun.java2d.opengl=true]
Dataset consistency test: No problems found

Attachments (4)

16706.patch (14.6 KB) - added by simon04 5 months ago.
16706-improve.patch (1.8 KB) - added by GerdP 8 weeks ago.
sample.osm (2.0 KB) - added by GerdP 8 weeks ago.
sample file to test different zoom to situations
16706-improve-v2.patch (7.7 KB) - added by GerdP 8 weeks ago.

Download all attachments as: .zip

Change History (23)

comment:1 Changed 7 months ago by Don-vip

Milestone: 18.09

comment:2 Changed 7 months ago by Don-vip

In 14221/josm:

see #16706 - use documented enum for AutoScaleMode instead of undocumented string constants

comment:3 Changed 6 months ago by Don-vip

Milestone: 18.0918.10

comment:4 Changed 5 months ago by Don-vip

Milestone: 18.1018.11

Changed 5 months ago by simon04

Attachment: 16706.patch added

comment:5 Changed 5 months ago by simon04

Owner: changed from team to simon04
Status: newassigned
Summary: zoom to selection should not zoom out for zoom on a node[Patch] zoom to selection should not zoom out for zoom on a node

The current behaviour is due to #4413 and #9669.

At the moment, there are various bound enlargements going on in org.openstreetmap.josm.actions.AutoScaleAction#modeSelectionOrConflict. I propose to simplify those to a single enlargement by a lat/lon degree value.

comment:6 Changed 4 months ago by Don-vip

Milestone: 18.1118.12

comment:7 Changed 3 months ago by Don-vip

Milestone: 18.1219.01

comment:8 Changed 2 months ago by simon04

Resolution: fixed
Status: assignedclosed

In 14628/josm:

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

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

comment:9 Changed 2 months ago by GerdP

I am not happy with this change. When I select a single node or a single way while zoomed out far and press 3 to zoom to selected element I always have to zoom in again.
If this is intended maybe there is an option to configure how much data should be visible next to the selected object(s)?

comment:10 Changed 2 months ago by stoecker

Resolution: fixed
Status: closedreopened

comment:11 Changed 2 months ago by simon04

I wonder whether auto scaling will ever make everyone happy. :/

Sure, we can introduce a few advanced preference options for fine-grained control. I currently struggle with the naming of the preference keys. We are talking of those constants: https://github.com/openstreetmap/josm/commit/49da6844f43304596c7dea909e80ba934ab8a474#diff-ed25bb6e13db597f76e5d9325e172e47R189

  • 0.01° is the minimum bbox size after enlarging. Name the preference key zoom.bounds.minimum-size-after-enlarge or zoom.bounds.enlarge.lower-bound or …?
  • No enlargement is performed for bbox with size of 0.1° or more. Name the preference key zoom.bounds.no-enlarge-above or zoom.bounds.enlarge.threshold or …?
  • Between those two values, a linear interpolation is performed according to 0.001 - deg / 100. How to name the factor 100?

comment:12 Changed 2 months ago by GerdP

No idea yet what the values mean. I tried different values as a replacement for the factor 100 but none improved my case (zoom in on a single selected node.

comment:13 Changed 2 months ago by simon04

For the single node case you'd have to lower the 0.01 value. At the moment, the bounds around the node are enlarged to be at least 0.01° (lat/lon) in size.

comment:14 Changed 2 months ago by GerdP

Ah, okay. For me this works quite well:

        final DoubleUnaryOperator enlargement = deg -> deg < 0.0002
                ? 0.0002
                : deg < 0.1
                ? 0.0002 - deg / 100
                : 0.0;

To be honest, the nested code is too complex for me. Maybe you can create a patch with named constants and then I might be able to understand what this code does ;-)

Version 0, edited 2 months ago by GerdP (next)

Changed 8 weeks ago by GerdP

Attachment: 16706-improve.patch added

comment:15 Changed 8 weeks ago by GerdP

Not sure if I got the meaning of the threshold value right?
I've just noticed that there are two different methods to zoom to element now. When I select a node and press 3 to zoom to it I'm happy with my patch. When I select a node in the validator tree and press 6 (zoom to problem) it also seems to work fine.
BUT when I zoom to a rather short horizontal way using 6 there is nearly no place left and right of it, while zooming in with 3 shows a reasonable part of the map.
Why is there a difference?

Changed 8 weeks ago by GerdP

Attachment: sample.osm added

sample file to test different zoom to situations

Changed 8 weeks ago by GerdP

Attachment: 16706-improve-v2.patch added

comment:16 Changed 8 weeks ago by GerdP

with 16706-improve-v2.patch JOSM core always uses enlargeBoundingBoxLogarithmically()
I've also changed some variable names from box to v where they are instances of BoundingXYVisitor.

I am not yet sure if we need the logarithmic calculation. In my configuration the real zoom is dominated by method NavigatableComponent.scaleSnap().

comment:17 in reply to:  15 ; Changed 8 weeks ago by simon04

"edit.zoom-enlarge-bbox" and 0.0002 are fine for me. I didn't review the remainder.

Replying to GerdP:

Why is there a difference?

Lkely due to org.openstreetmap.josm.gui.dialogs.ValidatorDialog.ValidatorBoundingXYVisitor#visit(org.openstreetmap.josm.data.osm.WaySegment) only being present for the ValidatorBoundingXYVisitor.

comment:18 in reply to:  17 Changed 8 weeks ago by GerdP

Replying to GerdP:

Why is there a difference?

Lkely due to org.openstreetmap.josm.gui.dialogs.ValidatorDialog.ValidatorBoundingXYVisitor#visit(org.openstreetmap.josm.data.osm.WaySegment) only being present for the ValidatorBoundingXYVisitor.

No, reason is in class AutoScaleAction. Only the method modeSelectionOrConflict uses enlargeBoundingBoxLogarithmically. This is changed in v2 of the patch so that all modes (keys 1..6) use it.
I think it should be comitted before next release.

comment:19 Changed 7 weeks ago by GerdP

Resolution: fixed
Status: reopenedclosed

In 14734/josm:

fix #16706 (16706-improve-v2.patch)

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain simon04.
as The resolution will be set.
The resolution will be deleted.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.