[v32.0-alpha] Satellite imagery: Download not working for offline

[westnordost]

When downloading the map (either manual or automatic), the map tiles are also downloaded and cached in the upcoming major version.

However, this does currently not happen for the satellite imagery, when that map style is selected.

Furthermore, a link to the privacy statement for the ESRI tiles is missing.

What to do?

VectorTileProvider should be refactored to look like this:

data class MapScene(
    val sceneFilePath: String,
    val sources: List<TileSource>
)

abstract class TileSource(
    val title: String,
    val maxZoom: Int,
    val copyrightText: String,
    val copyrightLink: String,
    val privacyStatementLink: String,
    val apiKey: String
) {
    abstract fun getTileUrl(zoom: Int, x: Int, y: Int): String
}

The jawg MapScene then has two sources, the jawg tiles and the esri tiles. Classes where the TileSource information is used need to be adapted. For example, if there are several tile sources which both require a copyrightText (that field and others could be made nullable if each none is required), they need to both turn up in the appropriate view.

The MapTilesDownloader will then accept a list of TileSources (or a MapScene - not the best name perhaps) and download all the tiles for all the sources in the list.

This approach is not very economic with data as it always downloads the satellite data also if the style is not activated. So additional work is needed to only download the satellite imagery conditionally (=if the setting is active?). I guesst he most straightforward impelementation would be to exchange the MapScene when the setting is changed.

[smichel17]

I feel I've not contributed as much code to SC as I would like (relative to amount of bikeshedding in PRs 😅), and this looks like it is both important and should not be too much work, so I'm up to take a pass at it, unless @matkoniecz has already started.

[matkoniecz]

so I'm up to take a pass at it, unless @matkoniecz has already started.

Sadly not yet started and likely will not today.

Though I am planning to work on it. Implementing things and then dumping on @westnordost task of actually completing it is not a good strategy.

So if @smichel17 wants: feel free to work on it! It would be appreciated.

[smichel17]

Ok. I will change the interface as suggested at the top, fix the compiler errors, and then open a draft PR and you can decide if you want to do anything else from there or if that's all that is needed.

[westnordost]

Wait wait, there are a few considerations that aren't in my original deliberations:

1. satellite imagery changes very infrequently. So, the satellite imagery needs a different cacheControl than the vector tiles, one where tiles are considered fresh for - I don't know - 6 months? Didn't think it through yet, but perhaps the information how long tiles are considered fresh and after what time they should be disregarded could be part of the TileSource instead of hardcoded into the MapTilesDownloadCacheConfig
2. if the satellite imagery is only fetched once and then not again for a very long time, I think probably no extra effort needs to go into conditionally not downloading satellite imagery but instead always download it. Depends on the size of the tiles too.

[smichel17]

1. Seems like this is a more-or-less independent chunk of the refactor / isn't a reason not to go ahead? It wasn't in VectorTileProvider before, so we can do this change after [v32.0-alpha] Satellite imagery: Download not working for offline #2772 (comment)?

   I'd also not mind waiting on this part because I am admittedly a bit fuzzy on how the cache actually works — fresh/stale vs maxAge, and how those values (and the max size) actually affect when cached tiles are used and when new ones are fetched. I can spend some time figuring it out, but I'd rather isolate "touching code I'm not as comfortable with" to a different commit.

2. Is this in reply to [v32.0-alpha] Satellite imagery: Download not working for offline #2772 (comment)?

[westnordost]

It wasn't in VectorTileProvider before

Because it was the only tile provider.

Is this in reply to #2772 (comment)?

No, it is a reply to

This approach is not very economic with data as it always downloads the satellite data also if the style is not activated. So additional work is needed to only download the satellite imagery conditionally (=if the setting is active?). I guesst he most straightforward impelementation would be to exchange the MapScene when the setting is changed.

[smichel17]

What's the relationship between Jawg and Esri? I thought they were independent, but the satellite scene/styles are inside the jawg folder and your suggested interface has the file path at the MapScene level (but maybe this just reflects the folder structure, not any other meaning).

Aside: First impression of Dagger

I'm comfortable with manual dependency injection, but have not used Dagger or other DI frameworks before. I've read through enough of the Dagger docs now to understand how it works for our case, but not enough to understand the full available functionality.

It's not totally clear to me how @Inject constructor(private val x: Y) is saving much over constructor(private val x: Y = Y()) 🤷

[matkoniecz]

What's the relationship between Jawg and Esri?

Basically none.

I thought they were independent, but the satellite scene/styles are inside the jawg folder and your suggested interface has the file path at the MapScene level (but maybe this just reflects the folder structure, not any other meaning).

Satellite style has also labels/roads displayed from Jawg vector tiles.

satellite scene/styles are inside the jawg folder

jawg folder has jawg compatible map styles, Esri aerial is added as raster layer

And even aerial is also using vector data anyway

[smichel17]

Satellite style has also labels/roads displayed from Jawg vector tiles.

I guess this means that the api key is also the same between the tile sources? Should that be pulled up into MapScene?

Brainstorming on naming: maybe the top-level data class should still be named VectorTileProvider, and the lower level is RasterLayer? This doesn't feel quite right yet, but closer. Should the base layer, which provides labels/roads, get special treatment? Maybe we don't need this abstraction yet, and we should just keep VectorTileProvider and add a satelliteLayer: RasterTileProvider property.

I'm just about done understanding and am going to start writing code soon, so maybe let's wait to continue this discussion until I open a draft PR. I'm leaning toward "just add a satelliteLayer property" for the initial draft and then we can discuss from there.

One other thing though: I want to understand the path that tiles take to get to Tangram. For the ESRI tiles, the download url is specified in the yaml scene file, which seems to be passed to Tangram directly. Josm tiles are instead…

• Somewhere along the way, we determine that we need to download more tiles and call Downloader::download() on that area (which calls MapTilesDownloader::download())
• The MapTilesDownloader gets the url from the VectorTileProvider and stores the results into the global cacheConfig
• The tangram mapView is initialized with a http handler that uses this cacheConfig.

Does tangram will ever fetch tiles itself over http? I'm guessing that it does, and the above process is only used for offline tiles? In this case, where does Tangram get the jawg url from? MapTilesDownloader::downloadTile seems to be the only place where VectorTileProvider::getTileUrl() is called.

[matkoniecz]

I guess this means that the api key is also the same between the tile sources?

Esri requires no API key.

[smichel17]

Satellite style has also labels/roads displayed from Jawg vector tiles.

So when using the ESRI layer, both the ESRI and Jawg maxZooms are limiting factors, correct?

[westnordost]

On naming:

Within on style for tangram-es, you could have any number of tile sources. It is possible to combine different vector tile and raster tile sources in one style. Then, all these sources are used together. The map scene is like a CSS stylesheet: In the CSS stylesheet, you can refer to any number of image URLs (mediocre comparison).

[westnordost]

Jawg tiles are only available up til zoom 16 I think. This does not mean that you can't zoom in more, just the z16 tiles are the "highest resolution" tiles of Jawg.
ESRI raster tiles may be available till other max zoom levels, I didn't check.

[westnordost]

One other thing though: I want to understand the path that tiles take to get to Tangram. For the ESRI tiles, the download url is specified in the yaml scene file, which seems to be passed to Tangram directly. Josm tiles are instead…

All map tile sources are actually specified in the scene yaml file(s). You also find the jawg one there, in global.yaml. Why is it then mentioned another time in the VectorTileProvider class? The reason is that this url is used by the MapDataDownloader.

tangram-es downloads the map tiles that are in view automatically (of course). But for StreetComplete offline-capability, we want to pre-download all the tiles in a given area when downloading a rectangle. So that is what the MapDataDownloader does: It downloads the same data as tangram-es would automatically. The "trick" here is that Streetcomplete tells tangram-es to use a certain cache directory (and config) and uses the exact same cache directory (and config) for the MapDataDownloader. So, things that are downloaded by this class can be used by tangram-es and the other way round.

[smichel17]

Ah, I missed global.yaml 🤦

I found https://www.esri.com/en-us/privacy/privacy-statements for a privacy policy link, but I think this is for use of their website, not necessarily their api.

[smichel17]

Ah, good 'old Android Studio

---

Anyway, aside, accessing the preferences seems a little boilerplate-y. Would you be interested in something like this? https://github.com/LibreShift/red-moon/blob/master/app/src/main/java/com/jmstudios/redmoon/model/Config.kt It's a Config singleton which uses the application context, so you can import it from anywhere and then access/set preference values like Config.whateverValue:

https://github.com/LibreShift/red-moon/blob/221e66728c6ca1838509ae55e7e7538706e5b87b/app/src/main/java/com/jmstudios/redmoon/schedule/ScheduleReceiver.kt#L59-L65

edit: How does one get embedded code snippets to actually work? :/

I would do this as a separate PR; it's not really related to this PR except I bumped into preferences. Also note: the particular implementation uses a little library I wrote to eliminate boilerplate inside the singleton, but that's not necessary for the singleton pattern / I'm not sure I'd use it today.

[westnordost]

@matkoniecz @smichel17 maybe we could back out the Satellite imagery feature from v32.0-alpha again and add it back as one package with #2794? I would feel a lot more confident with that, as currently v32.0-alpha is still quite unstable and some question marks left and I want to focus my efforts rather on that than attending to yet new (unstable) features.

[smichel17]

Sure. I'm working on the download now so we can probably get it in for a release, but I think it's a large enough feature (from the user's perspective) to warrant its own release anyway.

[matkoniecz]

(1) Obviously, feel free to do this!

(2) I would prefer for it to be released, but I think that I can avoid any negative consequences of that, and chance that it would cause problems are tiny anyway

[westnordost]

Ok, I removed the option from the settings but left the rest of the code, hoping that it can be finished soon.

[westnordost]

I'll close this issue as this is now not an issue for v32-alpha anymore but will be implemented by that PR