cyclestreets / ios

iPhone app
https://www.cyclestreets.net/mobile/iphone/
179 stars 149 forks source link

Map text/size unreadable #81

Closed mvl22 closed 8 years ago

mvl22 commented 9 years ago

Several reports that, in the new 3.0 release, the text size is too small to read.

mvl22 commented 9 years ago

The problem and solution is basically as described here: http://stackoverflow.com/questions/22712897/tiles-from-openstreetmap-in-retina-display

I think the underlying problem is that the tiles are served as 256x256px files. When a retina display shows those, the 256 pixels are half the size of a non-retina display. Presumably the old RouteMe library didn't know anything about retina so just showed at 2x2 the size. Now that Apple MapKit is handling the tiles, it's just showing the 256 pixels one-to-one, so overall looks smaller.

I think it's finally time to bite the bullet and install a tileserver and serve a set at 512x512. I'll have a go. I've been wanting to do this for ages anyway.

Neil, if the suggested solution of doing "implement your own custom MKTileOverlayRenderer and draw each 256px tile into a 512px CGContext. The tiles will appear blurry, though." is easy to achieve quickly as a workaround, that is probably worth attempting as an interim solution.

neilkachu commented 9 years ago

Yes, there's some emails on this on the CS mobile list. It's pretty much down to the map requestors (MapKit & RouteMe act the same) being intelligent and requesting retina sized images but the server not supporting this and returning non retina images that then are rendered into too small a pixel area. It's a pixels/inch issue difference between displays.

Good explanation of the problem: http://myseamap.budszuhn.com/mapkit-mktileoverlay-and-retina-displays-part-2/

neilkachu commented 9 years ago

Left is new 3.0 app; right is old 2.5 app: i.e.: Left: Retina Right: Non Retina

img_0023

mvl22 commented 9 years ago

Are there any non-retina devices that the app will run under, nowadays?

I.e. for the purposes of the app, if we serve tiles of size 512x512 would we also need to have tiles of 256x256? (I will need to create the latter anyway, as I want to remove reliance on third-party hosting that we are currently proxying for the website.)

neilkachu commented 9 years ago

Not for new users, but if a user is running OS6 they can only use v2.5 my 3GS pictured is non retina. Note we can send the scale in the tile template ( and default to 1 if not there) so it should be able to support both apps and resolutions. i.e.: http://myserver/tile?x={x}&y={y}&z={z}&scale={scale}

mvl22 commented 9 years ago

OK, I'll see whether mod_tile supports scale={scale} when I install it.

If not, it would have to be a different tileset, i.e.

/mapnik/ /mapnik-retina/

which the app would need to select dynamically based on the OS.

neilkachu commented 9 years ago

OK can be done if required, let me know.

mvl22 commented 9 years ago

I've been reading up on the issue of tile sizing, in trying to understand why the maps are appearing difficult-to-read in the new iPhone app, which is a pixel density issue.

I believe this is also an issue on the Android app. Basically it seems it will be an issue on any phone with a pixel density greater than 1, which of course is now most phones these days.

Pixel density table: http://web.archive.org/web/20140805102049/http://en.wikipedia.org/wiki/List_of_displays_by_pixel_density (the important column to look at is "CSS pixel ratio")

Using a Samsung S3 phone (pixel density =2), if I look at the CycleStreets app for my street compared the same location using the Chrome app, the text is just about readable, but the whole map is indeed twice as dense.

However, it appears that simply making tiles at twice the size ('Retina tiles') doesn't actually fully solve the problem, because there are other densities, e.g. 1, 1.325, 1.5, 1.7, 2, 1.8, 2.2, 3, as explained here:

https://github.com/openlayers/ol3/issues/1905 (This is the best discussion I've found in trying to understand this general issue.)

Basically it seems to be saying that the correct solution is for the size (width and height) of each tile to be 256px times the pixel density of the screen. I can't quite work out what the phone does it with it then or how the mapping libraries adjust, but I'm going to give this a try on the server-side.

So an old iPhone (=1) needs 256x256. An iPhone 5 (or Samsung Galaxy S3, or many other phones for instance) (=2) needs 512x512. A Samsung Galaxy S2 (=1.5) needs 384x384. etc.

At this stage, I think the best I can do is add a server-side script which resizes the tiles on request. For instance, if a client needs an x2 size tile (512x512) I can upscale the 256x256 to 512x512 using Imagemagick.

Although this will look a bit blurry, in the longer term tile hosting providers might start supplying proper 512x512 tiles as a result of Vector storage: http://blog.gravitystorm.co.uk/2014/03/28/a-sneak-peak-at-thunderforest-lightning-maps/

The question then is what the URL structure should. It looks like there are three choices:

Google support only scales of 1,2 (and 4 for paying customers). I will whitelist a set of values (e.g. 1,2) to avoid creating a DoS scenario.

The useful mySeaMap link that Neil gave, implies that scale=2 is being sent by iOS Mapkit. However, our tileserver logs show that the iOS app (v3) is not sending this.

I see that our apps define the tileservers as follows:

neilkachu commented 9 years ago

Hi,

Id says it should be http://servername/stylename/{z}/{x}/{y}/{s}.png (though obviously these odd android scales will break this clean format) in which case id plumb for http://servername/stylename/{z}/{x}/{y}.png?scale=2 iOS will send the extra parameter but currently the tile template is not set up to do so, that is all. iOS is only ever going to be sending whole numbers, so 2 or 3 (for iP6/6+)

From the blog post I sent it look like iOS is adjusting the zoom for a specific bounds and requesting more images to fill it. So where now we have too much data on screen, with correct setup we would see 1/2 as much but more pixels. Ive tried faking this this end but because we can't request 512x512 tiles it won't work without server support.

Will have to see what the result of the upscale is visually, on a non retina screen it looks quite ropey anyway, but on retina should be better.

Yes vector does solve this but seems a long way off still for these OS sources.

cheers.

mvl22 commented 9 years ago

I've now added support to the tileserver cache we are using:

https://github.com/cyclestreets/tilecache/commit/d36be58c75812713f0921c377db11b3f2369a72f

This is merely upscaling the images rather than generating them properly from the geographical data at a higher resolution. In due course, where providers support native higher-resolution tiles, I'll add support to pass those through the cache without the rescaling kicking in.

For now I've only added support for the @2x.png URL format as used by Mapbox (unfortunately ?scale=2 adds surprising complexity because Apache's -f RewriteCond check will not notice the difference).

I've whitelisted support for 1.5 and 2 scale factors only. (Jez, let me know if you think more would be useful.)

So the following URL formats now work:

/layername/{z}/{x}/{y}.png /layername/{z}/{x}/{y}@1.5x.png /layername/{z}/{x}/{y}@2x.png

(note there is no @1x.png URL format - I've tried to keep to the same format as Mapbox uses.)

Please could you try this and let me know if this actually works!

neilkachu commented 9 years ago

Hmm, not quite:

request example: http://tile.cyclestreets.net/mapnik/16/32740/21789@2x.png results in 512px image but MapKit isn't handling these correctly, the on screen tiles are 256px Even if I upscale the values as indicated on the Seaman post (to http://tile.cyclestreets.net/mapnik/17/65480/43578@2x.png) it's still not right, further investigation is required it seems. If I change the tile size to 512, I just get sea images for OSM, Apple maps show the correct location but the xyz value are always different screen shot 2014-11-17 at 22 37 49

ios simulator screen shot 17 nov 2014 22 11 23

mvl22 commented 9 years ago

Andy @Gravitystorm, I wonder whether (as an expert tiles person!) you might be able to advise us on whether the actual output of what I've done seems to be correct?

http://tile.cyclestreets.net/mapnik/16/32740/21789@2x.png

Currently this is outputting a tile at 72dpi, with 144px across and 144px down. Do you know if this is the correct 'spec', so to speak, of a 'retina tile'?

gravitystorm commented 9 years ago

There's no real spec for retina tiles. The best guide is the mapbox api, which recently changed between v3 and v4. I've no idea what mapkit itself is expecting.

Normal tile http://api.tiles.mapbox.com/v3/examples.map-zr0njcqy/13/2275/3306.png:

3306

API v3 retina tile http://api.tiles.mapbox.com/v3/examples.map-zr0njcqy/13/2275/3306@2x.png:

3306 2x

API v4 retina tile http://api.tiles.mapbox.com/v4/examples.map-zr0njcqy/13/2275/3306@2x.png?access_token=...

3306 2x-v4

To create a "retina" map there are two choices. When you start with a normal 256x256 tile, you can either replace it with four 256x256 tiles that cover the same area, or you can replace it with one 512x512 tile. These are v3 and v4 respectively. If you look carefully at the number of labels and especially the lakes on the v3 tile above, you'll see it's actually supposed to be one of the 4 tiles needed to replace a zoom 12 tile:

1653

You can easily see how using v3, you'd need 4 retina tiles to cover the same area and show the same features. Since you need so many more tiles you also twiddle the zoom level that you're displaying.

Anyway, this is a long winded way to say that by creating 512x512 px tiles you're implementing mapbox api v4. You'll need to check very carefully what mapkit is expecting, and bear in mind that half of what you read on the internet refers to the v3 way of doing things and half to the v4 way.

mvl22 commented 9 years ago

Thanks, Andy. That is a very useful summary.

I've re-read the OpenLayers3 discussion on this (at https://github.com/openlayers/ol3/issues/1905) which is a useful discussion also. I note however that it says: "Mapbox's approach is a crude hack the tile grid and only works with pixel ratios of exactly 2. It will become increasingly limiting as non-pixel-ratio-2 devices become more common."

I have indeed tried that approach of creating 512px x 512px tiles, as per Mapbox API V4. However, as shown in Neil's latest screenshot, Apple Mapkit doesn't seem to work with that size. So the Mapbox solution (which the OL3 discussion suggests is problematic anyway) isn't immediately usable in Mapkit anyway.

It seems there may be a few possible approaches:

1) Create a custom canvas in Mapkit, as per this example: http://stackoverflow.com/a/25920523 However, that points out that this will not work in iOS8. I see there is also https://github.com/mapbox/mbxmapkit which possibly could be adapted.

2) Mapkit seems to have a way of setting the tilesize for the overlay canvas, so that could be set at 512: http://stackoverflow.com/questions/25095512/ios7-mktileoverlayrenderer-with-tile-size-greater-than-256 However, the (unanswered) post there seems to suggest this only works for 128 and 256, not 512. I guess that would be the ideal solution, because that would work for 2x, and other scaling factors too.

Both of those involve trying to get Mapkit to adjust to 512x512 tiles. The alternative seems to be this:

3) Take the approach implied by the mySeaMap blog posting at: http://myseamap.budszuhn.com/mapkit-mktileoverlay-and-retina-displays-part-2/ and override the loadTileAtPath, zooming in by 1 level. This will need a maths function to convert the incoming path to the new path. The zoom is incremented by one, and x and y are doubled but with a bit of offsetting in practice. Helpfully, there is boilerplate code for this on the OSM website: http://wiki.openstreetmap.org/wiki/Slippy_map_tilenames#Lon..2Flat._to_tile_numbers

I think we should try (3) first.

NB I had earlier misunderstood what the scale=2 parameter in the mySeaMap posting was about. It does not have any effect on the size of the tile itself, which remains 256x256. Instead, it is a hint to the cartography rendering system that the labelling within that tile should be adjusted to be more readable when displayed at half-by-half the size. This graphic: https://farm9.staticflickr.com/8142/7210334896_64ac0b3788.jpg from https://www.mapbox.com/blog/retina-tiles-available-ios-sdk/ demonstrates this notion of a hint to the cartography subsystem of a rendering stack.

mvl22 commented 9 years ago

Take the approach implied by the mySeaMap blog posting This will need a maths function to convert the incoming path to the new path The zoom is incremented by one, and x and y are doubled but with a bit of offsetting in practice.

I've written a quick class as a proof of concept, based on the OSM wiki page noted above. This does seem to convert the paths correctly.

@neilkachu, could you try this? (Obviously converted to Objective-C.)

<?php

# Class to convert a tile path to a new zoom level
# Example usage:
$path = '/18/131175/86347';    // http://b.tile.openstreetmap.org/18/131175/86347.png
$newTilePath = convertTilePath::convert ($path, 19);
echo $newUrl = 'http://b.tile.openstreetmap.org' . $newTilePath . '.png';
// Should be: http://b.tile.openstreetmap.org/19/262350/172694.png

# Class to convert a tile path to a new zoom level
# See: http://wiki.openstreetmap.org/wiki/Slippy_map_tilenames#PHP
class convertTilePath
{
    # Main
    public static function convert ($path, $newZoomLevel)
    {
        # Extract zoom, x, y
        preg_match ('|^/([0-9]+)/([0-9]+)/([0-9]+)$|', $path, $matches);
        list ($fullString, $zoom, $x, $y) = $matches;

        # Convert to latlon
        list ($lat, $lon) = self::tileToLatlon ($zoom, $x, $y);

        # Determine the new tile path
        list ($x, $y) = self::latlonToTile ($lat, $lon, $newZoomLevel);

        # Assemble the path
        $path = "/{$newZoomLevel}/{$x}/{$y}";

        # Return the new path
        return $path;
    }

    # Convert zoom/x/y to latlon
    public static function tileToLatlon ($zoom, $x, $y)
    {
        $n = pow (2, $zoom);
        $lon = $x / $n * 360.0 - 180.0;
        $lat = rad2deg(atan(sinh(pi() * (1 - 2 * $y / $n))));
        return array ($lat, $lon);
    }

    # Convert latlon at a specified zoom level to x/y
    public static function latlonToTile ($lat, $lon, $zoom)
    {
        $x = floor((($lon + 180) / 360) * pow(2, $zoom));
        $y = floor((1 - log(tan(deg2rad($lat)) + 1 / cos(deg2rad($lat))) / pi()) /2 * pow(2, $zoom));
        return array ($x, $y);
    }
}

?>
gravitystorm commented 9 years ago

So the Mapbox solution (which the OL3 discussion suggests is problematic anyway) isn't immediately usable in Mapkit anyway.

Careful - you've got the two things mixed up. It's the mapbox v3 approach (256px + zoom changes) which is being criticized in the OL3 thread. Since it's based on changing integer zoom levels, you can only handle multiples-of-2 pixel densities.

The mapbox v4 approach is more flexible, since you could (in theory) have @3x as 768px tiles, or @1.5x as 384px tiles, whereas the mapbox v3 approach only allows @2x (one zoom level change) @4x (two zoom level changes), @8x (three zooms) etc.

mvl22 commented 9 years ago

Good point, Andy.

Neil, on balance I suggest then that looking at what is being done in https://github.com/mapbox/mbxmapkit/blob/master/MBXMapKit/MBXRasterTileOverlay.m might be the best way forward - presumably that is overriding the standard canvas with a new one of the new size.

neilkachu commented 9 years ago

Nope, this is just a TileOverlay subclass, same as our MapSources, there's no image rendering going on here. Option 1) is possible, the issue with OS8 is that the methods they reference have been deprecated and moved to another that is all. We can try this with tiles set to 512, might work.

jezhiggins commented 9 years ago

Martin's asked me to outline what I'm doing over on the Android side. We have four different tile sources - OCM and OSM pulled from the CycleStreets tile server, Ordnance Survey OpenData tiles pulled from OpenStreetMap, and vector maps rendered on device. I use the OSMDroid library to splat the whole lot on the screen. The default tile size is 256x256 pixels.

At start up, I query the display metrics and check if it's a high density screen. If so, we set the tile size to 512x512 pixels. For the tiles pulled from the CycleStreets tile server, I pull the @2.png tile rather than the .png tile. For the OpenData tiles I scale up the tiles locally. I do the same for the vector map rendering, scaling up 256x256 tiles to 512x512px. (I tried getting it to render 512x512 but it's clearly not as configurable as it pretends to be, and I ended up with something like Neil's screenshot above.)

mvl22 commented 8 years ago

Just to say that thanks to @gravitystorm, the Android app is now seeing real retina tiles (at 2x) for the OpenCycleMap layer, as of https://github.com/cyclestreets/tilecache/commit/6469ae1bebe04f48bba567d34d9a955321d16f9d

@neilkachu I'm still very keen to get this working as the app shows tiles painfully small - I think your Dec 4, 2014 posting suggested something might be possible.

neilkachu commented 8 years ago

I'll have another look at this, the SO issue was just a method name, not a removal or anything like that. so the MBXRasterTileRenderer.m not MBXRasterTileOverlay.m might just work, they are setting the tileSize to 512 and downloading x2 512px tiles but it's the overriding of how to draw these into the graphics layer that is key. I'll cover this into CS logic and get back to you.

mvl22 commented 8 years ago

Thanks, Neil.

The ideal is definitely that you request from the server either standard 1x tiles (and the server returns 256x256) or request ....x2.png tiles and get back 512x512. (And in future x3 as screens become higher resolution, etc.). If possible avoid the option of upscaling on the device.

This means the tileserver at our end can then work out whether it is supposed to upscale or whether to pass on real Retina tiles - more and more providers now are providing proper x2 tiles, with the actual cartography modified to have more appropriate label sizes, etc.

neilkachu commented 8 years ago

Well that appears to-just-work! simulator screen shot 5 jan 2016 11 31 15

mvl22 commented 8 years ago

Wow, that is so much better.

Are other tilesets like Vector still fine, and does the OSM style (which we are upscaling) also appear the same way (albeit more pixelated because the server is upscaling)?

neilkachu commented 8 years ago

OSM looks like it works fine too, others look ok too. Currently all raster tile sources are passing through this renderer even if they are not retina enabled this side, I'll do some tests with this on/off but as its looking up the tileSize for the source before re-rendering it should be fine as a global replacement for raster tile sets. The Apple vector tiles don't use this system at all so aren't affected, they do the correct thing anyway.

mvl22 commented 8 years ago

Great; that sounds perfect. Definitely best if the server can do all the decision-making. Let's get this into a release soon-as (preferably with all the other new stuff - will contact you on e-mail re that).

What scaling factors need to be supported? Currently we have 1, 1.5, 2, 3 enabled. Do you need any more? Are you able to look this up automatically from the device, rather than hard-code it to 2?

https://github.com/cyclestreets/tilecache/blob/master/.config.php.template#L30-L31

neilkachu commented 8 years ago

The retina scale is derived from the device's screen scale, iOS only supports 1,2 & 3 no odd half sizes. But as the app is not enabled for x3 devices natively this shouldnt be an issue at the mo (it requires the entire app converting over to Auto layout). Is there a way to ask the tile source server what scales it supports? currently we have the switch this on this end so requires a build.

OSM looks ok on device if a bit pixely, you can be the judge.

mvl22 commented 8 years ago

it requires the entire app converting over to Auto layout

Sounds fun. Presumably this would enable iPad at last though :)

Is there a way to ask the tile source server what scales it supports?

No. We don't plan to add an API for this. Most tile providers are the same: they publish a list of what they support, and apps just add support for those levels, so:

currently we have the switch this on this end so requires a build.

yes, that's therefore fine.

OSM looks ok on device if a bit pixely, you can be the judge.

Yes, it will do a bit. I'll have a look when a build comes through on Testflight. I think it is likely to be better than the unreadability problem though!

neilkachu commented 8 years ago

OK, leave it with me, so for now only OCM & OSM will have this enabled.

mvl22 commented 8 years ago

only OCM & OSM will have this enabled.

Plus CNS and Ordnance Survey - any reason not to add those? They are all coming from the same tileserver so have the 2x upscaling support.

neilkachu commented 8 years ago

OK, I'll switch all these on too then! Build on it's way with some other fixes too.

mvl22 commented 8 years ago

OK, I'll switch all these on too then!

Thanks. Please work on the principle that if a source is coming from the CycleStreets tileserver, then support should be present for 1x, 2x, etc.

mvl22 commented 8 years ago

Now in latest release and is such a massive improvement - thanks for all the comments and work on this. Thanks to to @gravitystorm for such a nice 2x implementation in OCM!

We'll have to wait for the Mapnik layer and OS layers to be improved upstream, but the app for now has the code in place.

mvl22 commented 8 years ago

Mapnik (OSM) style issue: https://github.com/gravitystorm/openstreetmap-carto/issues/2038