Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Clustered style layers disappear when offline #12167

Closed
captainbarbosa opened this issue Jun 19, 2018 · 19 comments
Closed

Clustered style layers disappear when offline #12167

captainbarbosa opened this issue Jun 19, 2018 · 19 comments
Labels
archived Archived because of inactivity bug clustering Core The cross-platform C++ core, aka mbgl

Comments

@captainbarbosa
Copy link
Contributor

Platform: iOS, Android
Mapbox SDK version: iOS 4.0.1, Android 6.1.3

Steps to trigger behavior

  1. Create a clustered style layer from a local GeoJSON source
  2. Initialize an offline map that envelops the region containing the clustered style layer
  3. Remove network connection, force quit application, and reopen - all style layers related to clustering are gone.

Expected behavior

Clustered sources should continue to appear if they lie within the bounds of an offline map and are referencing a local GeoJSON file.

Actual behavior

Clustered sources disappear when viewing an offline map. Interestingly, this doesn't happen if the source is added as an unclustered style layer.

giphy2

A sample minimal test case for iOS can be found here.

@1ec5
Copy link
Contributor

1ec5 commented Jun 19, 2018

The ports symbol layer probably doesn’t work b/c nothing in the style is using the harbor-15 image. As @jfirebaugh pointed out, the clusteredPortsNumbers might not work if it’s using the default font stack and nothing else in the style is using that font stack. But we’re unsure why the clusteredPorts circle layer isn’t showing up. Perhaps we can’t cluster invisible symbols (since the symbol image is missing)?

Interestingly, this doesn't happen if the source is added as an unclustered style layer.

If clustering is disabled in this example, everything shows up, including the harbor-15 icons and labels?

It would also help to clarify whether MGLMapViewDelegate.mapView(_:didFinishLoading:) is getting called as expected when clustering is enabled.

@jeffm19
Copy link

jeffm19 commented Jun 20, 2018

I’ve also experienced this issue with Android 6.1.3. I’m also working with offline maps and am trying to load a local GeoJSON source when the device has no internet and found that the clusters do no appear. Here are some observations I have seen while testing.

I followed this sample and replaced the GeoJSON source URL with a local GeoJson file that my app reads in.
https://github.com/mapbox/mapbox-android-demo/blob/master/MapboxAndroidDemo/src/main/java/com/mapbox/mapboxandroiddemo/examples/dds/GeoJsonClusteringActivity.java

  1. If I comment out the SymbolLayer that is used to display the text (cluster count) inside the cluster, then the cluster circles are drawn on the screen while the device is offline. From my perspective, it seems to be an issue with the SymbolLayer.

  2. When I load my GeoJSON file I observe that my clusters are not shown. However, if I toggle Wi-Fi on, all of a sudden the clusters appear on the screen with the proper cluster count numbers. It seems that something with the device being online triggers it.

Unfortunately, since I’m working with offline maps, this is a big issue for me and I will be stuck using Mapbox 5.5.0 until this issue is resolved.

@pacomacman
Copy link

pacomacman commented Jun 21, 2018

I can confirm that Mapbox 5.5.0 also seems to have this issue, however it took quite a few attempts to make it fail. Once it fails, and the cluster markers stop appearing it stays this way launch after launch. However once I delete the offline map, everything starts to work as normal, so this issue is most definately linked to have one or more offline maps installed.

@pacomacman
Copy link

I attempted to go back as far as 4.0.0 and the problems are still there. My feeling is that offline maps have never worked with clustering. It's as if an attempt to fetch http data is not being treated as an error and invalid data is overwritting good data since it can often work fine for some time before issues arrise.

@pacomacman
Copy link

pacomacman commented Jun 23, 2018

I think I might have stumbled on something relevent to this offline maps with clustering issue. At the point everything goes wrong if I comment out the following lines the clusters re-appear but without number overlays:

//Add the cluster count labels
SymbolLayer count = new SymbolLayer("icon-count", "marker_source");
count.setProperties(
textField("{point_count}"),
textSize(12f),
textColor(Color.BLACK)
);
mMapboxMap.addLayer(count);

As soon as I put this code back in it all stops again so something to do with numeric textField overlays is causing this problem.

@jeffm19
Copy link

jeffm19 commented Jun 25, 2018

pacomacman, this is the exact scenario that I am dealing with. Except I have clusters working fine with offline maps using 5.5.0. The numbers appear in the clusters with no problems. But as soon as I updated to 6.x.x, I see the exact behavior that you described. Something with adding the SymbolLayer with {point_count} screws it up.

@pacomacman
Copy link

jeffmiyamoto, I can only assume your usage case is slightly different from mine because I've revisited 5.5.0 and for me it still doesn't work. I use stops to have multiple cluster icons etc. I'm pretty sure it's got something to do with a missing font, but I would have thought it would be easy to track down given the amount of information we now have on this issue. Use a fallback font if the specified one isn't available for instance.

@captainbarbosa
Copy link
Contributor Author

I think I might have stumbled on something relevent to this offline maps with clustering issue. At the point everything goes wrong if I comment out the following lines the clusters re-appear but without number overlays

I'm also seeing this. For some reason this symbol layer containing the cluster labels is triggering the absence of the cluster layer completely 🤔

@ChrisLoer
Copy link
Contributor

I got @captainbarbosa's test case running in the debugger, and could confirm that this was just a problem with the harbor-15 icon -- since it's not downloaded in the offline pack, any future offline requests for it will basically just hang. The clustering is actually working fine, but all three of those layers live on the same "GeoJSON tile", and the tile as a whole never finishes layout and sends its results to the main thread because it's waiting for that icon to be downloaded.

I'm not sure the best way to do it, but ideally we'd have some way of really aggressively surfacing errors for missing resources in some kind of debug mode to help people putting together offline packs at least figure out what's going wrong. Right now all you get is this cryptic message that looks like all the other errors you see when you're running offline:

[Style]: Failed to load sprite: The Internet connection appears to be offline.

We could also consider some kind of "fallback to partially rendered tiles" behavior when some resource dependencies can't be satisfied -- in this case the behavior would have been easier to understand if the harbor icons had just disappeared. But in general, I think we want to avoid getting in the business of displaying potentially "broken" tiles -- it introduces too many confusing states between "not loaded" and "loaded".

Interestingly, this doesn't happen if the source is added as an unclustered style layer.

I don't have an explanation for this... maybe there is still some clustering related problem I'm missing that's hidden behind the icon dependency? Or it may be that it's just really easy to get the harbor icon into your cache and once you do that you won't see the bug manifest any more?

@jeffm19
Copy link

jeffm19 commented Jun 27, 2018

@ChrisLoer, did you happen to find out any reason why adding a SymbolLayer with the "textField" property would cause all the clusters to disappear in offline mode?

This is the other issue that all three of us have found in common. As stated above, in @pacomacman comments, if we add this code to add a count value to the clusters, it disappears.

//Add the cluster count labels
SymbolLayer count = new SymbolLayer("icon-count", "marker_source");
count.setProperties(
textField("{point_count}"),
textSize(12f),
textColor(Color.BLACK)
);
mMapboxMap.addLayer(count);

If we simply remove the line, textField("{point_count}"), then the you will see empty clusters appear on the map with no count value. But if you leave the textField property in there, they disappear.

@ChrisLoer
Copy link
Contributor

@jeffmiyamoto the probable reason is that the text field is using a font stack that hasn't otherwise been downloaded by the style at the time the offline pack was generated. An unsatisfied font dependency would cause pretty much exactly the same behavior as the unsatisfied harbor-15 dependency in the initial example (this is what @1ec5 mentioned in #12167 (comment)). You can get around this by explicitly setting the text-font property to match a font-stack already used in the base style.

@1ec5
Copy link
Contributor

1ec5 commented Jun 28, 2018

I'm not sure the best way to do it, but ideally we'd have some way of really aggressively surfacing errors for missing resources in some kind of debug mode to help people putting together offline packs at least figure out what's going wrong.

I think a typical iOS application would expect that -[MGLMapViewDelegate mapViewDidFailLoadingMap:withError:] would get called, perhaps with MGLErrorCodeLoadStyleFailed, if -mapView:didFinishLoadingMap: never gets called.

@pacomacman
Copy link

@jeffmiyamoto the probable reason is that the text field is using a font stack that hasn't otherwise been downloaded by the style at the time the offline pack was generated. An unsatisfied font dependency would cause pretty much exactly the same behavior as the unsatisfied harbor-15 dependency in the initial example (this is what @1ec5 mentioned in #12167 (comment)). You can get around this by explicitly setting the text-font property to match a font-stack already used in the base style.

I think you are correct but why wouldn't the font be included in the offline map if we specify a style at the time? My belief is that the font is getting downloaded but somehow gets overwritten or corrupted as sometimes after program restart everything works fine for a couple of runs, and sometime after things stop working.

@jeffmiyamoto How to we explicitely set the text-font propery for the base style?

@jeffm19
Copy link

jeffm19 commented Jun 28, 2018

@pacomacman I found a sample on how to set the "textFont" property here
https://github.com/mapbox/mapbox-gl-native/blob/master/platform/android/MapboxGLAndroidSDKTestApp/src/main/java/com/mapbox/mapboxsdk/testapp/activity/style/SymbolLayerActivity.java

I did a quick test and copied the example into my project and observed that the number label did appear inside the clusters in offline mode, however when I turned the device online, the app crashed. Most likely due to not using the correct font-stack.
layer.setProperties(textFont(new String[] {"DIN Offc Pro Medium", "Arial Unicode MS Regular"}));

I'm still trying to figure out how to set the font-stack to my project's base style. I have a property set in my styles.xml (fonts/AvenirLTStd.otf). I tried passing in the string of my fontPath into the textFont function but that didn't work.

@ChrisLoer Do you have an example of how to set the font-stack to the apps base style?

@1ec5
Copy link
Contributor

1ec5 commented Jun 28, 2018

@pacomacman, if you’re setting a symbol layer’s font property at runtime (textFontNames on iOS, textFont on Android) and that array of fonts appears in exactly the same order somewhere in the style JSON at design time, then indeed the fonts should be included in the offline pack. However, if that array of fonts isn’t in the style JSON, then that’s when the hang @ChrisLoer describes might occur.

Since #11055 – which went into iOS SDK v4.0.0 and Android SDK v6.0.0 – the hang also occurs if you don’t set the symbol layer’s font, because the “default default” is Open Sans Regular and Arial Unicode MS Regular, which may not match the style as it was originally designed. For example, Mapbox Light v9 never specifies Open Sans Regular anywhere, so you’d effectively have to set textFontNames or textFont whenever you create a new symbol layer at runtime.

Do you have an example of how to set the font-stack to the apps base style?

A style doesn’t specify a “base” set of fonts. Instead, the fonts are specified layer by layer. So you’d pick a layer you want to match and copy over its textFont property. In Swift, that looks something like this:

let townLayer = style.layer(withIdentifier: "place-town") as! MGLSymbolStyleLayer
numberLayer.textFontNames = townLayer.textFontNames

Or, to be more generic, you could look for a style layer that uses a particular layer in the Mapbox Streets source:

let placeLayer = style.layers.first { ($0 as? MGLSymbolStyleLayer).sourceLayer == "place_label" } as! MGLSymbolStyleLayer
numberLayer.textFontNames = placeLayer.textFontNames

@captainbarbosa
Copy link
Contributor Author

captainbarbosa commented Jun 28, 2018

I also confirmed that including the font stack of another layer should to the trick and get the clusters to appear again while offline. For those following along, here are the steps I took to workaround this issue:

  1. Lookup fonts used in layers by viewing the style JSON directly, using this request to our Styles API: https://www.mapbox.com/api-documentation/#the-style-object

Once you have that, find a symbol style layer with the fonts you'd want to use in your cluster layer. In the case of the default Mapbox Light style one of the layers (waterway-label) has a font stack like this:

text-font": [
  "DIN Offc Pro Italic",
  "Arial Unicode MS Regular"
]
  1. Now, the symbol layer that represents the clusters needs to be explicitly set with this font stack. I did this in my test case included in the original comment of this issue as follows:
numbersLayer.textFontNames = NSExpression(forConstantValue: ["DIN Offc Pro Italic",
            "Arial Unicode MS Regular"])

⚠️ You must include the whole font stack here, and not just a singular font!

This is a really interesting problem. I'm glad there is a way around it, but I would imagine there is definitely room for improvement in how we communicate this font mismatch for users working with offline maps.

@pacomacman
Copy link

pacomacman commented Jun 28, 2018

The problem is I'm not using custom styles, I'm switching between default styles:

mapView.setStyleUrl(Style.OUTDOORS);
and
mapView.setStyleUrl(Style.SATELLITE);

Why would these default styles not contain everything that is required to render an offline map in this style?

I can confirm however hardwiring the font-stack does work, I just don't understand why it is really neccesary for a user to even consider this.

@1ec5
Copy link
Contributor

1ec5 commented Jun 29, 2018

Lookup fonts used in layers by viewing the style JSON directly, using this request to our Styles API

The map SDK has already gone through the trouble of requesting the style from the Styles API, so this is all that’s needed to match an existing layer’s fonts:

let waterwayLabelLayer = style.layer(withIdentifier: "waterway-label") as! MGLSymbolStyleLayer
numberLayer.textFontNames = waterwayLabelLayer.textFontNames

Why would these default styles not contain everything that is required to render an offline map in this style?

I can confirm however hardwiring the font-stack does work, I just don't understand why it is really neccesary for a user to even consider this.

The offline map does contain everything needed to render the original style offline, but the offline API is unable to account for any dependencies added at runtime via the runtime styling API. In the past, the default set of fonts would be included with every offline pack, whether the style used it or not. But that added significant heft to every offline pack, so #11055 removed that behavior.

That said, I agree that it’s both counterintuitive and inconvenient that you have to explicitly match a new layer’s fonts to existing layers. To me, this is a good argument for the map SDK using local or system fonts (#7862), at least for the default fonts. Currently, it uses system fonts for CJK characters only: #10522.

@tobrun tobrun added Core The cross-platform C++ core, aka mbgl and removed Android Mapbox Maps SDK for Android iOS Mapbox Maps SDK for iOS labels Sep 20, 2018
@stale stale bot added the archived Archived because of inactivity label Mar 19, 2019
@stale
Copy link

stale bot commented Mar 20, 2019

This issue has been automatically detected as stale because it has not had recent activity and will be archived. Thank you for your contributions.

@stale stale bot closed this as completed Mar 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
archived Archived because of inactivity bug clustering Core The cross-platform C++ core, aka mbgl
Projects
None yet
Development

No branches or pull requests

6 participants