search

amelroua / osmdroid

Automatically exported from code.google.com/p/osmdroid
0 stars 0 forks source link

MapBoxTileSource design issues #521

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago 
Adding a MapBoxTileSource was a great idea. 
However, the current implementation has the following issues:

1) Default zoom max level = 20 is too high. At this level, it shows empty tiles 
with just a cross. Should be 19. 

2) Handling of MapBox map ID with retrieveMapBoxMapId is not good. MapBox can 
provide various types of maps (satellite, satellite labelled,...). 
No way to handle more than 1 map type in an application, even using the 
"full-featured" constructor. 

I would suggest to change the default constructor, to pass it the map ID. 
And to avoid various map types mixed in the same caching area, we also have to 
pass a user-defined name. 

public MapBoxTileSource(String name, String aMapBoxMapId)
{
  mapBoxMapId = aMapBoxMapId;
  super(name, ResourceProxy.string.base, 1, 19, 256, ".png", mapBoxBaseUrl);
}

The caller can then create it for instance this way:
OnlineTileSourceBase myMBSatTileSource = new MapBoxTileSource(
  "MapBoxSatellite", 
  ManifestUtil.retrieveKey(this, "MAPBOX_SAT_MAPID"));

3) It's using this base url "http://api.tiles.mapbox.com/v3/"
=> It doesn't use the usual "farming" mechanism: 
"http://a.tiles.mapbox.com/v3/", 
"http://b.tiles.mapbox.com/v3/",
"http://c.tiles.mapbox.com/v3/", 
"http://d.tiles.mapbox.com/v3/"
Not sure if it's a potential issue or not. 

4) It's using this resourceId: ResourceProxy.string.base, which is also the id 
of BASE tile source (http://topo.openstreetmap.de/base/), and used at various 
places in osmdroid. 
Strange, but again, not sure if it's a potential issue or not.

Original issue reported on code.google.com by mathieu....@gmail.com on 27 Jan 2014 at 5:14

GoogleCodeExporter commented 9 years ago 
I would also like to ask to make String getMapBoxMapId() non-static and 
overridable.
It will allows not to depend on the AndroidManifest, context and on 
retrieveMapBoxMapId().

Original comment by ls.illar...@gmail.com on 27 Jan 2014 at 5:31

GoogleCodeExporter commented 9 years ago 
Do you know where the docs for this are?
I looked at https://www.mapbox.com/developers/api/ but it wasn't clear what the 
tile url should be or what is the highest zoom level.

Original comment by neilboyd on 31 Jan 2014 at 6:01

GoogleCodeExporter commented 9 years ago 
[deleted comment]
GoogleCodeExporter commented 9 years ago 
@ls.illarionov Or would you prefer a setter for mapBoxMapId? Then you don't 
need to extend this class.
I don't really see the point of the getMapBoxMapId method as it is now.

Original comment by neilboyd on 31 Jan 2014 at 6:09

GoogleCodeExporter commented 9 years ago 
This page https://www.mapbox.com/developers/api-overview/
mentions the tiles base farming: "http://a.tiles.mapbox.com/v3/", etc. 

Highest zoom level: I've not found in the doc, it's experimental. 

Patch for suggested changes available here: 
https://drive.google.com/file/d/0B_8VOmRyW664Sk9CcEJtM2k3YU0/edit?usp=sharing

Original comment by mathieu....@gmail.com on 1 Feb 2014 at 4:57

GoogleCodeExporter commented 9 years ago 
There are examples which sometimes mention a,b,c,d and sometimes don't.  I was 
looking for a spec rather than examples.

Original comment by neilboyd on 2 Feb 2014 at 7:40

GoogleCodeExporter commented 9 years ago 
No spec found... 

Searching for a doc, I discovered that MapBox guys are working on their own 
fork of osmdroid. See: https://www.mapbox.com/blog/mapbox-android/

Original comment by mathieu....@gmail.com on 3 Feb 2014 at 11:02

GoogleCodeExporter commented 9 years ago 
> mentions the tiles base farming: "http://a.tiles.mapbox.com/v3/", etc. 

For tiles we recommend the [a, b, c, d] set that are under the api level. api 
is also supported, but less emphasized for tiles.

> Highest zoom level: I've not found in the doc, it's experimental. 

There isn't a service-wide maximum zoom level - we can support higher zooms on 
road data than on satellite, and higher zooms for satellite in well-covered 
metro areas than the country or the middle of the ocean, etc.

> Searching for a doc, I discovered that MapBox guys are working on their own 
fork of osmdroid.

Yes, the repo is https://github.com/mapbox/mapbox-android-sdk

Original comment by t...@macwright.org on 8 Feb 2014 at 3:59

GoogleCodeExporter commented 9 years ago 
[deleted comment]
GoogleCodeExporter commented 9 years ago 
I fixed point 1 and 3 in revision 1430. I chose zoom level 18 since that is the 
highest zoom level that any of the other providers have.

I fixed point 4  in revision 1431, although it does highlight a fundamental 
limitation of the resource proxy idea for external tile sources. A solution for 
that would also help with point 2.

Original comment by neilboyd on 9 Feb 2014 at 9:26