Modify

Opened 10 years ago

Closed 9 years ago

Last modified 9 years ago

#10454 closed enhancement (fixed)

[PATCH] Mapbox "empty" tile (imagery with zoom level > 17)

Reported by: Bernhard Hiller <bhil@…> Owned by: team
Priority: normal Milestone: 15.05
Component: Core imagery Version: tested
Keywords: mapbox zoom Cc: wiktorn

Description (last modified by Don-vip)

If tiles are not available at high zoom level, Mapbox sends some kind of "empty" tile, i.e. a black tile with a grey cross in it:

http://a.tiles.mapbox.com/v4/openstreetmap.map-inh7ifmo/18/148511/135995.png

Instead of a "no tiles at this zoom level" message and stretching of lower zoom tiles, JOSM shows those empty tiles.
This behavior can be seen e.g. in the Perlis region of Malaysia: maximum zoom level available is 17.
It seems that the machanism for detecting the maximum zoom level does not work with Mapbox. What about using the file size of the tile, followed by an MD5 checksum test to determine if a tile is such an empty tile or not?

Attachments (5)

mapbox-perlis.jos (12.2 KB ) - added by bastiK 9 years ago.
mapbox_no_tiles_handling.patch (14.7 KB ) - added by wiktorn 9 years ago.
ElevationProfile.patch (849 bytes ) - added by wiktorn 9 years ago.
10454_no_tiles.patch (39.2 KB ) - added by wiktorn 9 years ago.
10454_no_tiles_regex.patch (1.1 KB ) - added by wiktorn 9 years ago.

Download all attachments as: .zip

Change History (43)

comment:1 by Bernhard Hiller <bhil@…>, 10 years ago

See also ticket #10451

Last edited 10 years ago by skyper (previous) (diff)

comment:2 by Don-vip, 9 years ago

Description: modified (diff)

comment:3 by Don-vip, 9 years ago

Keywords: mapbox zoom added
Summary: Mapbox "empty" tileMapbox "empty" tile (imagery with zoom level > 17)

comment:4 by Don-vip, 9 years ago

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

comment:5 by Don-vip, 9 years ago

#5819 is related (same problem for Bing)

comment:6 by samusz, 9 years ago

This is also found in the Nepal imagery ie https://www.openstreetmap.org/#map=15/27.95306/85.22836

comment:7 by wiktorn, 9 years ago

Owner: changed from team to Bernhard Hiller <bhil@…>
Status: newneedinfo

comment:8 by wiktorn, 9 years ago

Owner: changed from Bernhard Hiller <bhil@…> to samusz

comment:9 by wiktorn, 9 years ago

Can you provide information, which imagery source you experience problems with?

comment:10 by wiktorn, 9 years ago

Cc: wiktorn added

by bastiK, 9 years ago

Attachment: mapbox-perlis.jos added

in reply to:  9 comment:11 by bastiK, 9 years ago

Replying to wiktorn:

Can you provide information, which imagery source you experience problems with?

See attached session file: Open and zoom in.

comment:12 by mdk, 9 years ago

As a workaround for disaster mapping in Nepal you can add your own imagery layer with URL

tms[17]:http://{switch:a,b,c}.tiles.mapbox.com/v4/openstreetmap.map-inh7ifmo/{zoom}/{x}/{y}.png?access_token=pk.eyJ1Ijoib3BlbnN0cmVldG1hcCIsImEiOiJncjlmd0t3In0.DmZsIeOW-3x-C5eX-wAqTw

Only difference to original Mapbox URL: [17] instead of [19] so JOSM will not try to download tiles for zoom level 18 or 19.
But if you use this imagery layer in regions where tiles for these zoom levels exists, you will not see them.

I think MapBox should stop delivering black tiles (with grey cross) for zoom levels where imagery is missing. JOSM could not really check, if a delivered tile contains useful data or not.

comment:13 by wiktorn, 9 years ago

Indeed. Workaround from mdk is the best for time being. And indeed, correction by MapBox would be far more better...

comment:14 by bastiK, 9 years ago

I always get the same etag value "067736a547cafe90014b4e59b6510abe" for the error image. This may or may not be stable over time.

in reply to:  14 comment:15 by mdk, 9 years ago

Replying to bastiK:

I always get the same etag value "067736a547cafe90014b4e59b6510abe" for the error image. This may or may not be stable over time.

Do you want to filter tiles and handle them as if the server returns an error so that JOSM would try to scale lower zoom tiles? I think this would work - even if I don't like the smell of such a solution.

Do you want to hard code this as a special Mapbox hack? Or do you think about a generic filter configuration? In a simple case this could be a etag map as blacklist.

by wiktorn, 9 years ago

comment:16 by wiktorn, 9 years ago

Instead of doing etag blacklisting, I decided to refactor a bit "no tiles at zoom level" detection, as for Bing we had already a bit out of place support.

Right now it is defined per each TileSource, so it give fine control, how we treat each of the tile provider. The only drawback is, that you need now to define your Mapbox URL with "mapbox:" prefix instead of "tms:", such as:

mapbox[19]:http://{switch:a,b,c}.tiles.mapbox.com/v4/openstreetmap.map-inh7ifmo/{zoom}/{x}/{y}.png?access_token=pk.eyJ1Ijoib3BlbnN0cmVldG1hcCIsImEiOiJncjlmd0t3In0.DmZsIeOW-3x-C5eX-wAqTw

Though maybe it's better just to make tms support etags blacklisting which is easy to implement right now.

comment:17 by bastiK, 9 years ago

It is good to move X-VE-Tile-Info to Bing class, as this is a Bing specific header. But is it really needed to add a new imagery type (mapbox:) just for this? Anyway, the class MapboxTileSource is missing from the patch.

I'd rather have new XML parameters in the imagery definition, for example

<entry>
  <name>Mapbox Satellite</name>

  <blacklist>
    <etag value="067736a547cafe90014b4e59b6510abe"/>
  </blacklist>

  <id>Mapbox</id>
  <type>tms</type>
  <url>http://{switch:a,b,c}.tiles.mapbox.com/v4/openstreetmap.map-inh7ifmo/{zoom}/{x}/{y}.png?access_token=pk.eyJ1Ijoib3BlbnN0cmVldG1hcCIsImEiOiJncjlmd0t3In0.DmZsIeOW-3x-C5eX-wAqTw</url>
</entry>

in reply to:  17 ; comment:18 by wiktorn, 9 years ago

Replying to bastiK:

It is good to move X-VE-Tile-Info to Bing class, as this is a Bing specific header. But is it really needed to add a new imagery type (mapbox:) just for this? Anyway, the class MapboxTileSource is missing from the patch.

I'd rather have new XML parameters in the imagery definition, for example

Indeed, this might be the best way.

Are:

Available somewhere in repository? Or shall I provide just separate patch for these two?

I also prefer such XML structure:

<blacklist>
  <header name="ETag" value="&quot;067736a547cafe90014b4e59b6510abe&quot;" />
</blacklist>

PS. Sorry for missing file in patch

in reply to:  18 comment:19 by bastiK, 9 years ago

Replying to wiktorn:

Are:

Available somewhere in repository? Or shall I provide just separate patch for these two?

See trunk/data/maps.xsd

I also prefer such XML structure:

<blacklist>
  <header name="ETag" value="&quot;067736a547cafe90014b4e59b6510abe&quot;" />
</blacklist>

Yes, this is better. Then it should even work for the Bing header. Let's also avoid the term "blacklist" as it might be associated with imagery that we have no permission to use in OSM. Alternative would be "no-tile".

comment:20 by wiktorn, 9 years ago

This is second try. I finally decided for <no-tile-header name="..." value="..." /> structure.

For Bing, the neccessary addition is:

<no-tile-header name="X-VE-Tile-Info" value="no-tile" />

And for Mapbox:

<no-tile-header name="ETag" value="&quot;067736a547cafe90014b4e59b6510abe&quot;" />

Some remarks:

  • I couldn't resist to cleanup some of the TileSource classes and refactor the constructors.
  • I didn't verify anywhere XSD changes
  • I smuggled in some performance improvement for no-tile situation in BufferedImageCacheEntry
  • And smuggled in some changes in ICachedLoaderListener (and in implementations) fixing detection of no-tile situation
  • I did some migration code in ImageryLayerInfo, we need to decide (and maybe flag), how long this migration code should be there. It just moves the <no-tile-header ...> information from defaults to user presets, so user doesn't need to redefine its map sources
  • I did not yet verified, if my TileSource changes affect any plugins

As this is a bit intrusive, I would wait till point-release is done before commiting this to trunk.

Last edited 9 years ago by wiktorn (previous) (diff)

by wiktorn, 9 years ago

Attachment: ElevationProfile.patch added

comment:21 by wiktorn, 9 years ago

It looks like ElevationProfile was only affected by TileSource changes.

comment:22 by wiktorn, 9 years ago

Owner: changed from samusz to team
Status: needinfonew

comment:23 by wiktorn, 9 years ago

Summary: Mapbox "empty" tile (imagery with zoom level > 17)[PATCH] Mapbox "empty" tile (imagery with zoom level > 17)

comment:24 by bastiK, 9 years ago

Thanks!

  • The jmapviewer class AbstractOsmTileSource should not include JOSM classes (ImageryInfo).
  • It may be less trouble to support more than one no-tile-header element right from the start. Or at least have a plan how to upgrade without messing up preferences and keeping backwards compatibility with older JOSM versions still in use.

by wiktorn, 9 years ago

Attachment: 10454_no_tiles.patch added

in reply to:  24 comment:25 by wiktorn, 9 years ago

Replying to bastiK:

Thanks!

  • The jmapviewer class AbstractOsmTileSource should not include JOSM classes (ImageryInfo).

I introduec TileSourceInfo in JMapViewer that groups a bit of information about TileSource to reduce number of parameters in constructors. ImageryInfo now extends TileSourceInfo.

  • It may be less trouble to support more than one no-tile-header element right from the start. Or at least have a plan how to upgrade without messing up preferences and keeping backwards compatibility with older JOSM versions still in use.

I introduced JSON serializer/deserializer for Map's in Preferences, to keep the objects within one XML entry, though I don't find this a perfect solution. But doing proper XML serialization of Maps in Preferences looks for me as too much effort for functionality, that, I hope, will never be used.

comment:26 by bastiK, 9 years ago

Resolution: fixed
Status: newclosed

In 8344/josm:

applied #10454 - Mapbox "empty" tile (imagery with zoom level > 17) (patch by wiktorn)

comment:27 by bastiK, 9 years ago

In [o31122]:

applied ​#10454 - Mapbox "empty" tile (imagery with zoom level > 17) (patch by wiktorn)

comment:28 by aceman, 9 years ago

I also noticed this bug long ago and had to limit mapbox to level 17 to avoid the black tiles. Thanks for working on it!
Let's see if it works now.

I assume the current patch does not need the workaround from comment 16.

comment:29 by stoecker, 9 years ago

Changes to the XML structure must be documented here: https://josm.openstreetmap.de/wiki/Maps#Documentation

comment:30 by wiktorn, 9 years ago

Done in version 366.

comment:31 by bastiK, 9 years ago

This could be used for MapQuest Open Aerial as well. Header for normal image:

HTTP/1.1 200 OK
Content-Type: image/jpeg
Last-Modified: Fri, 10 Feb 2012 06:12:55 GMT
Server: Mapnik2/0.8.0
Access-Control-Allow-Origin: *
ETag: "sat/11/1485/928:4f34b567:Mutx4s/rIkJikBM2knOp+yhVjdMA"
Content-Length: 15966
X-Varnish: 2099522830
Cache-Control: max-age=600
Expires: Tue, 12 May 2015 15:22:41 GMT
Date: Tue, 12 May 2015 15:12:41 GMT
Connection: keep-alive

Header for no-tile image:

HTTP/1.1 200 OK
Content-Type: image/jpeg
Last-Modified: Mon, 18 Aug 2014 14:44:05 GMT
Server: Mapnik2/0.8.0
Access-Control-Allow-Origin: *
ETag: "sat/15/23771/14841:53f21135:OC4CjEZcEJv8Az5u7fOqgEP+NpkA"
Content-Length: 2935
X-Varnish: 1119527010
Cache-Control: max-age=120
Expires: Tue, 12 May 2015 15:15:53 GMT
Date: Tue, 12 May 2015 15:13:53 GMT
Connection: keep-alive

ETag-Values for other no-tile images:

ETag: "sat/17/95221/59324:53f21135:OC4CjEZcEJv8Az5u7fOqgEP+NpkA"
ETag: "sat/17/95222/59324:53f21135:OC4CjEZcEJv8Az5u7fOqgEP+NpkA"

comment:32 by bastiK, 9 years ago

Resolution: fixed
Status: closedreopened

comment:33 by wiktorn, 9 years ago

Here we have a special case. I guess that the form of the header is:
"${tilenumber}:${md5hash}"

So we can't use this mechanism to support this. We have two possibilities - either do SHA1 digest of byte content, and add <no-tile-sha1> tag, of provide specific, MapQuest TileSource class.

This would cover also Mapbox case, but it wouldn't cover Bing case, so either way, I guess we need to have these two methods (header based and content-digest based).

comment:34 by bastiK, 9 years ago

What about to redefine the value of no-tile-header to be a regular expression? This would cover everything.

by wiktorn, 9 years ago

Attachment: 10454_no_tiles_regex.patch added

comment:35 by wiktorn, 9 years ago

Indeed.

And as they say, now we have two problems :-)

Tested with following regex:

<no-tile-header name="ETag" value=".*?:OC4CjEZcEJv8Az5u7fOqgEP\+NpkA&quot;" />

comment:36 by bastiK, 9 years ago

Resolution: fixed
Status: reopenedclosed

See [o31124]:

applied #10454 - change header value to regex (patch by wiktorn)

comment:37 by Don-vip, 9 years ago

Milestone: 15.05

comment:38 by wiktorn, 9 years ago

Ticket #11554 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. Next status will be 'reopened'.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.