Modify

Opened 4 years ago

Closed 3 years ago

Last modified 3 years ago

#12350 closed enhancement (fixed)

[patch] snap scale to native zoom levels

Reported by: kolesar Owned by: team
Priority: normal Milestone: 16.02
Component: Core Version: latest
Keywords: Cc: naoliv

Description

JOSM renders TMS tiles resized when scale does not match TMS zoom level. Resizing uses nearest neighbour algorithm, resulting image is noisy. There is an option to set zoom to layer's native resolution but it is a one-time action. After zooming to data/layer/selection scale is again set to a fractional value.

I have added a new option to keep scale snapped to Mercator zoom levels:

Edit / Preferences (F12) / Display Settings / Look and feel / [x] Snap zoom to Mercator zoom levels (imagery looks perfect, zoom steps are larger)

When this setting is enabled and projection is Mercator, zoom in and out keeps scale at powers of two. Adjusted MapSlider to have snapped levels at values 0, 5, 10, and so on. Requests to change zoom by a fraction are rounded. Zoom to bounding box works fine, no edges get outside of viewport.

In this mode maps look and zoom exactly the same as in usual web maps. Attached patch based on current snapshot.

Attachments (6)

snapZoomToMercator.patch (16.2 KB) - added by kolesar 4 years ago.
NativeScaleLayer.patch (39.4 KB) - added by kolesar 4 years ago.
scale.png (162 bytes) - added by kolesar 4 years ago.
scale.svg (796 bytes) - added by kolesar 4 years ago.
NativeScaleLayerChange.patch (1.2 KB) - added by kolesar 4 years ago.
ZoomPreferences.patch (4.7 KB) - added by kolesar 4 years ago.

Download all attachments as: .zip

Change History (57)

Changed 4 years ago by kolesar

Attachment: snapZoomToMercator.patch added

comment:1 Changed 4 years ago by bastiK

Works well, I like it!

comment:2 Changed 4 years ago by Klumbumbus

see also #11865

comment:3 Changed 4 years ago by wiktorn

Looks good. Though I dream of connecting the zoom scales to imagery sources. Though it would need some algorithm to elect which zoom scale to use, when different imagery scales are in use.

The only case, when it might get a bit tricky, is when there is WMTS tile source, that supports EPSG:3857 but has different tile sizes than standard TMS. Service:
wmts:http://www.ngi.be/cartoweb/1.0.0/WMTSCapabilities.xml

(choose layer topo, use it in Belgium) is one example of such imagery service.

There are no visible distortions, as the difference in scales are around 2e-10, but if you use "snap to tile size" from right-click-menu, you will notice, that it will always move you one zoom level out. This is not the case for standard TMS.

comment:4 in reply to:  3 ; Changed 4 years ago by kolesar

I know WMTS, I like its concept. We should keep in mind that a single WMTS source can have any number of resolutions. There is no rule to have equal steps between resolution levels. In a special case there can be only one resolution. Or two levels with a non-standard large step (for example 3x), then what should be happen when zooming in from outer level? And outside that range?

Another problem is that several raster layers can be loaded and can be visible at the same time with transparency. If they have different resolutions, which one should we choose automatically? When I use right-click function "snap to tile size" I choose one of the layers. It's not trivial to choose globally.

Funcion "snap to tile size" actually fails on this WMTS source, compare a downloaded tile with JOSM screenshot. It actually zooms out more and more on every subsequent call, check it in tested version of JOSM. This function needs a fix.

It is based on AbstractTileSourceLayer.getScaleFactor(int zoom) function that calculates a ratio between supposed resolution of a standard TMS (!) tileset and the current scale. This factor is applied on scale. It always snaps to TMS levels, not the WMTS. Obviously this works only for 2zoom based TMS layers in Mercator projection.

WMTS can have any projection, even more than one. Result looks best if source projection and display projection is the same. JOSM does not reproject rasters but does some magic hack to put tiles at (approximately) right position when projections do not match.

At first look I found unnecessary steps in getScaleFactor(). Transforms viewport corners to LatLon, then transforms then to "tile coordinates" that are actually Mercator coordinates divided by the power of two of given zoom levels. Then calculates ratio from number screen pixels with number of tiles.

It would be easy to replace this function with a much simpler one. getScaleFactor() could divide scale of the given zoom level with current scale. I have actually rewritten this function and worked fine for Mercator projection and Mercator TMS but when I changed projection to EPSG:23700 (Hungarian EOV) it gave totally false results. Before I got tiles that were slightly distorted but more or less the right position.

First step is to fix "snap to tile size" for WMTS layers. The real challenge is to keep backwards compatibility with the magic hack above.

Replying to wiktorn:

There are no visible distortions, as the difference in scales are around 2e-10, but if you use "snap to tile size" from right-click-menu, you will notice, that it will always move you one zoom level out. This is not the case for standard TMS.

comment:5 Changed 4 years ago by kolesar

Before modifying "Snap to tile size" I have merged two implementations of the same function. Right click menu on a map tile had "Snap to tile size" while right click menu in layer dialog had "Zoom to native resolution", implemented twice. Finally I have refactored both context menus, see #12363.

comment:6 in reply to:  4 Changed 4 years ago by bastiK

Replying to kolesar:

I know WMTS, I like its concept. We should keep in mind that a single WMTS source can have any number of resolutions. There is no rule to have equal steps between resolution levels. In a special case there can be only one resolution. Or two levels with a non-standard large step (for example 3x), then what should be happen when zooming in from outer level? And outside that range?

It gets slightly complicated, but it should be possible to fill in missing levels such that two consecutive levels do not have ratio greater 2x. E.g. for WMTS levels

10, 20, 50, 100, 750

extend to

..., 2.5, 5, 10, 20, 20*sqrt(2.5)=31.6, 50, 100, 100*(7.5^(1/3))=195.7, 100*(7.5^(2/3))=383.2, 750, 1500, 3000, ...

Another problem is that several raster layers can be loaded and can be visible at the same time with transparency. If they have different resolutions, which one should we choose automatically? When I use right-click function "snap to tile size" I choose one of the layers. It's not trivial to choose globally.

But also a use-case that is not as common (TMS/WMS is still dominant), so we don't need to worry too much about it. If the user is doing it on purpose (i.e. not keeping lower layer visible by accident / out of laziness), then the lowest visible layer should be the most relevant.

comment:7 in reply to:  4 ; Changed 4 years ago by wiktorn

Replying to bastiK:

But also a use-case that is not as common (TMS/WMS is still dominant), so we don't need to worry too much about it. If the user is doing it on purpose (i.e. not keeping lower layer visible by accident / out of laziness), then the lowest visible layer should be the most relevant.

I agree, that WMTS is not so much widespread.

When thinking about this, what came to my mind, that maybe proper solution would be just to introduce some icon (magnifying glass with a lock?) on Layer List, that would mark to which layer zoom levels are bound. This would solve some disambiguity to which layer we should snap.

With such solution, I'd do that regardless of the projection used by the user. I guess that there might be some bugs lurking when someone uses imagery using Polar Stenographic projection.

comment:8 in reply to:  7 Changed 4 years ago by kolesar

I have totally rewritten this. Implemented all asked features and some more:

  • large steps between imagery levels can be filled with intermediate steps
  • zoom steps are user-configurable (as a double precision number)
  • intermediate steps are available also for always-2x TMS levels if zoom step is smaller
  • zoom steps outside native resolution range are adjusted to top/bottom resolutions
  • user can select imagery layer to be used for adjusting (icon in layer list)
  • when a new imagery layer is added, setting is applied automatically
  • if another layer is chosen to snap, non-transparent imagery layers above are set to invisible
  • supports any projection (WMTS)

I need some time to add checks for extreme cases, clean the code, and test it deeply. I will attach patch when ready.

Replying to wiktorn:

When thinking about this, what came to my mind, that maybe proper solution would be just to introduce some icon (magnifying glass with a lock?) on Layer List, that would mark to which layer zoom levels are bound. This would solve some disambiguity to which layer we should snap.

With such solution, I'd do that regardless of the projection used by the user. I guess that there might be some bugs lurking when someone uses imagery using Polar Stenographic projection.

comment:9 Changed 4 years ago by bastiK

Ticket #12481 has been marked as a duplicate of this ticket.

comment:10 Changed 4 years ago by naoliv

Cc: naoliv added

comment:11 Changed 4 years ago by bastiK

Wiktor, what is the status?

comment:12 in reply to:  11 ; Changed 4 years ago by wiktorn

Replying to bastiK:

Wiktor, what is the status?

@kolesar: what's the status? Can we help?

comment:13 Changed 4 years ago by kolesar

It's all about free time.

comment:14 in reply to:  12 Changed 4 years ago by kolesar

Replying to wiktorn:

@kolesar: what's the status? Can we help?

The only missing piece is zoom slider at top left corner. It has a range from 35 to 150, unit of this number is not well defined. It divides screen extents with world extents then converts to a logaritmic value with base 1.1 (!) where calculation does not use logarithm function but a loop that repeatedly divides a number with 1.1, see MapSlider.propertyChange method.

This calculation makes displayed value depending on window size. It you only resize JOSM window and then zoom in and back to original zoom, you will get a different number here. Strange, isn't it?

JOSM defines maximum zoom in meters/pixels (1 cm == 100 px) but this unit can't be represented in scale (EastNorth/pixel) for the whole area of projection because meters/pixels (or meters/EastNorth) varies in some projections, including EPSG:3857 Spherical Mercator. In this projection EastNorth unit equals to a meters only at Equator. Moving towards North or South one EasthNort unit means less than a meter.

Projections do not have a method to give maximum of this unit. JOSM currently uses a different way to limit zoom: checks meters/pixels at the current viewport center and if it's too large, then forces this limit calculated for the current place. See NavigatableComponent.zoomTo.

Minimum zoom is also problematic when projection does not cover the whole world. Even Mercator leaves out north/south poles.

In my first patch I modified this slider to display exactly 0, 5, 10, 15, ... at TMS zoom levels 0, 1, 2, ... respectively. This does not work for other projections, and can't have round numbers for irregular WMTS zoom levels.

This control can be adjusted with keyboard. When focused, keys right and left adjust one step (integer value plus/minus). Step with 1.1 based logaritm was too small to step to 2x zoom change, snapping moved this slider back to original position, zoom did not change. That's why I have modified this control.

Definitions like 'minimum zoom', 'maximum zoom', 'world extents', 'meters/pixels' vary from projection to another and also for different places of the world.

Question: can we discuss this topic here or can I leave this control as it is?

Last edited 4 years ago by kolesar (previous) (diff)

comment:15 Changed 4 years ago by kolesar

I have finished cleaning code, added javadoc.

Modified MapSlider to use snapped scales and calculate value from scale regardless of window size.

Added an option to switch off intermediate steps between native zoom levels if needed. Default values: 2x zoom step and intermediate steps are on. As most tile sources have 2x steps additional intermediate steps are needed rarely.

Tested with Mercator TMS layers and belgian topo WMTS in both projections.

Attached NativeScaleLayer.patch and image to be placed at images/dialogs/layerlist directory.

Last edited 4 years ago by kolesar (previous) (diff)

Changed 4 years ago by kolesar

Attachment: NativeScaleLayer.patch added

Changed 4 years ago by kolesar

Attachment: scale.png added

comment:16 Changed 4 years ago by stoecker

SVG instead of PNG for new icons is preferred. :-)

comment:17 Changed 4 years ago by kolesar

Attached SVG image with same dimensions and content.

Changed 4 years ago by kolesar

Attachment: scale.svg added

comment:18 Changed 4 years ago by wiktorn

In 9818/josm:

Snap scale to mercator zoom levels.

See #12350

Patch submitted by: kolesar

comment:19 Changed 4 years ago by wiktorn

@kolesar: thank you for your submission. I've applied slightly modified patch.

@all:
I still some space for improvement, that I haven't had yet time to implement:

  • make ScaleList an immutable object
  • reduce number of ScaleList object creation (use +/- what's done in initProjection in WM(T)S TileSources)
  • make WMS layer also NativeScale aware
  • make NativeScale an abstract class instead of interface and introduce it in the ImageryLayer hierarchy (? - I'm not sure about this)

But I guess, that I can move on with these changes gradually later on, as time permits.

comment:20 in reply to:  19 Changed 4 years ago by kolesar

Replying to wiktorn:

  • make WMS layer also NativeScale aware
  • make NativeScale an abstract class instead of interface and introduce it in the ImageryLayer hierarchy (? - I'm not sure about this)
  • WMS layers do not have native resolutions (or they don't introduce them)
  • Prefer interfaces to abstract classes (Effective Java, Joshua Bloch, item 18)

comment:21 Changed 4 years ago by wiktorn

In 9820/josm:

Fix unit tests after [9818]. Move all zoom levels one level lower.

See: #12350

comment:22 Changed 4 years ago by kolesar

Summary: [patch] snap scale to Mercator zoom levels[patch] snap scale to native zoom levels

comment:23 Changed 4 years ago by Klumbumbus

Before this patch you needed two mouse wheel ticks to zoom one zoom step. Now (even without background) the zoom level changes with every mouse wheel tick. (I mean the zoom steps, which are used in mapcss). So the zooming is now twice as fast as before. I don't know if this is good or bad, I just want to mention it.

comment:24 in reply to:  23 ; Changed 4 years ago by kolesar

Replying to Klumbumbus:

Before this patch you needed two mouse wheel ticks to zoom one zoom step. Now (even without background) the zoom level changes with every mouse wheel tick. (I mean the zoom steps, which are used in mapcss). So the zooming is now twice as fast as before. I don't know if this is good or bad, I just want to mention it.

Change is intentional. Previous step was square root of 2 that would make an intermediate step between 2x step tms zoom levels. Another possible solution to step whole zoom levels: default zoom ratio sqrt 2 and intermediate steps disabled. Which one would you prefer?

Last edited 4 years ago by kolesar (previous) (diff)

comment:25 in reply to:  24 ; Changed 4 years ago by bastiK

Replying to kolesar:

Replying to Klumbumbus:

Before this patch you needed two mouse wheel ticks to zoom one zoom step. Now (even without background) the zoom level changes with every mouse wheel tick. (I mean the zoom steps, which are used in mapcss). So the zooming is now twice as fast as before. I don't know if this is good or bad, I just want to mention it.

Another possible solution to step whole zoom levels: default zoom ratio sqrt 2 and intermediate steps disabled.

It seems strange to disable intermediate steps. Then why implement them in the first place? I think factor 2 is not too much and we should keep it like it is.

comment:26 Changed 4 years ago by wiktorn

In 9825/josm:

Improvements for native scales.

  • make ScaleList immutable object
  • reduce number of ScaleList object creation
  • use ScaleList.snapZoom for getBestZoom in WMTS

See: #12350

comment:27 Changed 4 years ago by Don-vip

There's a problem in r9825:

        Collection<Double> scales = new ArrayList<>(currentTileMatrixSet.tileMatrix.size());
        if (currentTileMatrixSet != null) {
            for (TileMatrix tileMatrix : currentTileMatrixSet.tileMatrix) {
                scales.add(tileMatrix.scaleDenominator * 0.28e-03);
            }
        }
        this.nativeScaleList = new ScaleList(scales);

currentTileMatrixSet is first dereferenced, then checked if null. It must either be checked before, or not checked at all (causes a Findbugs warning)

comment:28 in reply to:  27 ; Changed 4 years ago by kolesar

Replying to Don-vip:

There's a problem in r9825:
currentTileMatrixSet is first dereferenced, then checked if null. It must either be checked before, or not checked at all (causes a Findbugs warning)

Problem is more complex and not related with recent patches. In what case can currentTileMatrixSet be null? I think that means invalid status for WMTS layer. Condition newLayer != null is unnecessary because userSelectLayer throws an exception instead of returning null. This exception is catched when adding a new WMTS layer but not catched when changing projection.

If current layer exists in new projection with the same name it is not a problem because there is only one candidate and userSelectLayer returns that. But if there are no candidates or there are more candidates but user selects none, it throws an exception.

What is expected behaviour when WMTS layer does not have data in selected projection or user does not select layer? Layer should be removed from layer list? Transparent overlay with error message on map? Or empty display?

First we should answer this question and then adapt code to the decision.

comment:29 in reply to:  25 ; Changed 4 years ago by kolesar

Replying to bastiK:

It seems strange to disable intermediate steps. Then why implement them in the first place? I think factor 2 is not too much and we should keep it like it is.

Disabling intermediate steps allows user to snap zoom levels 1:1 (without resizing) and have a smaller zoom ratio when there is no nativeScaleLayer selected. Or do the same with large step (more than 2x) WMTS layers and 2x zoom ratio.

I'm thinking about creating controls for these settings at look-and-feel preferences page:

[  2] zoom ratio
  [x] allow intermediate steps between native zoom levels

What do you think?

comment:30 Changed 4 years ago by wiktorn

In 9830/josm:

Fix findbugs warning / NPE.

See: #12350

comment:31 in reply to:  28 Changed 4 years ago by wiktorn

Replying to kolesar:

Problem is more complex and not related with recent patches. In what case can currentTileMatrixSet be null? I think that means invalid status for WMTS layer.
What is expected behaviour when WMTS layer does not have data in selected projection or user does not select layer? Layer should be removed from layer list? Transparent overlay with error message on map? Or empty display?

currentTileMatrixSet should be null (it's not right now), when user changes to the projection that it's not supported by WMTS service. It will already popups a warning and usually gives empty/black screen (modulo differences between projections). I'll test that, correct and document.

comment:32 in reply to:  29 ; Changed 4 years ago by bastiK

Replying to kolesar:

Replying to bastiK:

It seems strange to disable intermediate steps. Then why implement them in the first place? I think factor 2 is not too much and we should keep it like it is.

Disabling intermediate steps allows user to snap zoom levels 1:1 (without resizing) and have a smaller zoom ratio when there is no nativeScaleLayer selected. Or do the same with large step (more than 2x) WMTS layers and 2x zoom ratio.

For (hypothetical) WMTS with only few levels (e.g. z20, z16, z10 in terms of slippymap zoom levels) this leads to rather impractical zoom behavior. It is still an editor, not a tile map viewer.

It is good to have consistent zoom steps with imagery background enabled and without.

I'm thinking about creating controls for these settings at look-and-feel preferences page:

[  2] zoom ratio
  [x] allow intermediate steps between native zoom levels

What do you think?

Maybe a slider with about 5 positions from small to large (21/5, 21/4, 21/3, sqrt(2), 2)? Second option can be Einstein in my opinion.

comment:33 Changed 4 years ago by kolesar

I have found a bug in recent change in LayerListDialog. When there are more tms/wmts layers and I change native scale layer to another, icon at old layer is not refreshed until list is repainted. Attached patch that fixes this issue, please commit.

Changed 4 years ago by kolesar

comment:34 Changed 4 years ago by naoliv

Where exactly do I configure this new behavior in JOSM, please?
I am missing a more fine-grained zoom level like we had before and I couldn't find where I can change this.

comment:35 Changed 4 years ago by kolesar

Preferences, [x] Expert mode, last tab (setting preferences directly, advanced preferences), set value of zoom.ratio to 1.4142135623730951 (square root of 2).

Last edited 4 years ago by kolesar (previous) (diff)

comment:36 Changed 4 years ago by naoliv

Right.
What I was expecting, from an user point of view, was an option here in the menu (sorry for the photo quality):

http://i.imgur.com/zGRylFf.png

Something like "Use native resolution", where we could enable/disable as needed.

comment:37 Changed 4 years ago by kolesar

There is no entry for this setting in that menu. It is a good idea, thanks.

You can choose a row in layer list with the new black/white mark between green active layer mark and visibility switch. You can also disable using native resolutions by clicking mark again.

comment:38 in reply to:  32 ; Changed 4 years ago by kolesar

Replying to bastiK:

[  2] zoom ratio
  [x] allow intermediate steps between native zoom levels

What do you think?

Maybe a slider with about 5 positions from small to large (21/5, 21/4, 21/3, sqrt(2), 2)? Second option can be Einstein in my opinion.

What about a checkbox that is visible in expert mode only?

comment:39 in reply to:  37 Changed 4 years ago by naoliv

Replying to kolesar:

You can choose a row in layer list with the new black/white mark between green active layer mark and visibility switch. You can also disable using native resolutions by clicking mark again.

I tried this, but it doesn't get the old behavior back.
Shouldn't I see the old zoom levels when it is disabled?

Or better, what I think that should happen: if it's enabled, use the new behavior (with zoom.ratio = 2); if disabled, act exactly like before (with zoom.ratio = 1.4142135623730951)

comment:40 Changed 4 years ago by kolesar

There were two changes at the same time.

  • Zoom ratio was moved from hardcoded sqrt(2) to preference option with default value 2.
  • A layer can be selected to adjust zoom levels to its native resolutions. When there is no layer selected, zoom ratio is used relatively from previous zoom.

If you set zoom ratio to sqrt(2) and disable matching native resolution, you will get the original behaviour.

Does it work for you as descibed?

comment:41 in reply to:  38 Changed 4 years ago by kolesar

Replying to bastiK:

[  2] zoom ratio
  [x] allow intermediate steps between native zoom levels

What do you think?

Maybe a slider with about 5 positions from small to large (21/5, 21/4, 21/3, sqrt(2), 2)? Second option can be Einstein in my opinion.

Created expert-only checkbox and a spinner control between 1-5 where you can type any decimal value if you need a different value. Zoom ratio is Math.pow(2, 1/number). Default is 1, previous behavior can be set by choosing 2.

[x] Intermediate steps between native resolutions
    Zoom steps to get double scale [   1]±

Attached patch. Please apply NativeScaleLayerChange.patch, too.

Changed 4 years ago by kolesar

Attachment: ZoomPreferences.patch added

comment:42 Changed 4 years ago by wiktorn

In 9840/josm:

ZoomPreferences.patch and NativeScaleLayerChange.patch by kolesar.

See: #12350

comment:43 in reply to:  40 Changed 4 years ago by naoliv

Replying to kolesar:

There were two changes at the same time.

  • Zoom ratio was moved from hardcoded sqrt(2) to preference option with default value 2.
  • A layer can be selected to adjust zoom levels to its native resolutions. When there is no layer selected, zoom ratio is used relatively from previous zoom.

If you set zoom ratio to sqrt(2) and disable matching native resolution, you will get the original behaviour.

Does it work for you as descibed?

Right, it works.
But from what I was originally thinking it seems that there isn't a quick way to change between the old and the new behavior.

When using Bing I would like to have the old behavior (more and smaller zoom steps).
But when mapping with the IBGE layer, for example, I prefer the new behavior.

It it possible to have a quick way to change between these two modes, without needing to change zoom ratio everytime?

comment:44 Changed 4 years ago by kolesar

Yes, it is possible. Set a small zoom ratio (increase number of zoom steps in preferences / look and feel) and uncheck intermediate steps between native resolutions (needs expert mode). You need to set these only once.

When you use Bing uncheck black/white mark of using native resolutions. Small zoom steps will be used. For IBGE layer keep black/white mark checked, zoom steps will follow native resolutions.

comment:45 Changed 4 years ago by wiktorn

In 9846/josm:

Divde by zero found by Coverity Scan.

See: #12350

comment:46 Changed 3 years ago by Klumbumbus

Milestone: 16.02

comment:47 Changed 3 years ago by Don-vip

Status of this?

comment:48 Changed 3 years ago by kolesar

Works fine. Ticket was not closed when wiktorn committed patches.

comment:49 Changed 3 years ago by Don-vip

Resolution: fixed
Status: newclosed

comment:50 Changed 3 years ago by Don-vip

In 9914/josm:

see #12350 - fix coverity 1352420, 1352421, 1352422, 1352423, 1352424 (potential NPEs)

comment:51 Changed 3 years ago by Klumbumbus

Ticket #11865 has been marked as a duplicate of this ticket.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain team.
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.