Modify

Opened 4 months ago

Closed 2 months ago

Last modified 5 weeks ago

#18064 closed defect (fixed)

TMS layer by default didn't go beyond 21 zoom regardless of max zoom setting in properties

Reported by: SergeyAstakhov Owned by: team
Priority: major Milestone: 19.10
Component: Core imagery Version:
Keywords: zoom Cc: wiktorn

Description (last modified by Don-vip)

When adding a TMS layer without specifying the maximum zoom level (for example, via the remotec ontrol from OpenAerialMap "Open in JOSM"), it is limited to the default level of 21, despite the fact that the settings for TMS in preferences are set to a higher value.

This limitation is obtained in the AbstractTileSourceLayer.checkMaxZoomLvl() method when comparing the zoom with the return value from the ts.getMaxZoom() method:

if (ts != null && ts.getMaxZoom() != 0 && ts.getMaxZoom() < maxZoomLvl) {
   maxZoomLvl = ts.getMaxZoom();
}

which ends for the TMS layer in the AbstractTMSTileSource.getMaxZoom() method, which returns a hardcoded value of 21.

As workaround you can ether always specify max zoom level in url templates, ether use other layer types (if possible). However, this is inconvenient in some situations (in particular, OAM "Open in JOSM" didn't pass max zoom level to remotecontrol), so it would be much better if the default maximum zoom was taken from the settings and was not hardcoded.

Attachments (0)

Change History (17)

comment:1 Changed 4 months ago by SergeyAstakhov

Description: modified (diff)

comment:2 Changed 4 months ago by Don-vip

Cc: wiktorn added
Description: modified (diff)

@wiktorn can you please take a look?

comment:3 Changed 4 months ago by Don-vip

Keywords: zoom added

comment:4 Changed 4 months ago by wiktorn

We've two problems here:

  • (bug) we're not using max zoom setting from preferences panel
  • even if we'd use it, it would trigger check in JMapViewer, which allows only 22 as max zoom (triggered when opening download dialog, as all TMS tile sources could be used as background for download dialog)

The problem is, that even if we introduce this change without bumping JMapViewer.MAX_ZOOM up to 30 (the value that users were allowed to set in preferences through UI) we will render JOSM a bit unusable - can't open download dialog nor can't open Imagery Settings (as we have also JMapViewer there...)

comment:5 Changed 4 months ago by stoecker

Zomm 22 has a image length of about 10m, that's 4cm per pixel. Zoom 30 would have 0.1mm!

I'd agree that we could change max zoom 22 to 24 (about 2m per tile, 1cm per pixel) as we also already provide backgrounds with zoom 24, but everything else is simply senseless. If we already allow values up to 30 somewhere, that should be fixed to 24 as well.

P.S. NOTE, that the numbers are estimations, as scale depends on the real position of the tile.

So it is fix the bug you mention and change max values (up from 22 and down form 30) to 24?

comment:6 Changed 4 months ago by wiktorn

We can do that. But to support zoom level 24 we need also to promote all ints to longs in OsmMercator. Expression 256 * (1 << aZoomLevel) in getMaxPixels is overflowing at level 24.

comment:7 Changed 4 months ago by stoecker

Any negative side effects to expect?

comment:8 in reply to:  7 Changed 4 months ago by wiktorn

Replying to stoecker:

Any negative side effects to expect?

Some public interfaces of OsmMercator will change it's return signature:

public long getMaxPixels(int aZoomlevel);
public long falseEasting(int aZoomlevel);
public long falseNorthing(int aZoomlevel);

Probably, to be consistent, we should also promote aX and aY to be longs, as at this zoom levels those could also overflow.

public double getDistance(int x1, int y1, int x2, int y2, int zoomLevel);
public double xToLon(int aX, int aZoomlevel);
public double yToLat(int aY, int aZoomlevel);

And their uses. I fear, that this may open can of worms, as we use everywhere int as point coordinates. It looks though as it mainly works, I've noticed only that reopening download dialog for this (https://www.openstreetmap.org/node/1988920999#map=13/-16.8209/179.9947) location gives wired initial screen state. But other bugs may be lurking.

comment:9 Changed 4 months ago by stoecker

I'd say go ahead. "int" seems close to the edge anyway (e.g. we already have real-world 512 pixel tiles, as soon as it is 1024 it also fails), so I'd say change it and we'll be on the safe side. Only don't overdo it and change pixel coordinates which have a different scope :-)

comment:10 Changed 2 months ago by Don-vip

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

comment:11 Changed 2 months ago by Don-vip

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

comment:13 Changed 2 months ago by Don-vip

@wiktorn I have released 19.09, so we can start testing zoom 24. Can you please commit your int=>long changes?

comment:14 Changed 2 months ago by wiktorn

In 15456/josm:

Allow zoom in TMS layer up to 24.

See: #18064

comment:15 Changed 2 months ago by Don-vip

For reference: [o35190].

comment:16 Changed 2 months ago by Don-vip

Resolution: fixed
Status: newclosed

Seems to work fine :) Thanks!

comment:17 Changed 5 weeks ago by wiktorn

Another reference: [o35224]

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.