[ios, macos] fixes #5962 added geojson options to support clustering

[frederoni]

wip #5962

- [ ] Reuse default values from core

• Write documentation
• Add unit tests

- [ ] Documentation for mobile environment (will happen in #6236)

👀 @boundsj

[1ec5]

Thinking ahead to when we document these options: buffer and clusterRadius will unfortunately be difficult APIs for developers to reason about. They’re expressed in units of “⅟₅₁₂ of a tile”, which don’t correspond to any fixed real-world or screen distances, yet we don’t offer any way for the developer to find out the size of the tile at a given zoom level.

Fortunately, I don’t think this needs to be one of those “guess-and-check” APIs that we try to avoid exposing in the SDK. Rather than the formulation used in the style specification documentation, I think we should talk about the buffer and cluster radius being expressed in screen points (which is true at integral zoom levels), with a caveat that they may be scaled up to 2× between integral zoom levels. (The offset threshold implemented in #1843 is only for raster and video sources, so it won’t affect clustering.)

@mourner, a couple questions for you:

1. The style specification talks about “⅟₅₁₂ of a tile” units. I assume that means a tile is 512 of these units tall and 512 of these units wide, not that there are 512 to a tile, correct? If so, that should be an easy documentation fix.
2. On Retina displays, do these ⅟₅₁₂-of-a-tile units remain ⅟₅₁₂-of-a-tile units, or do they become ⅟₁₀₂₄-of-a-tile units?
3. The style specification describes tolerance as a “Douglas-Peucker simplification tolerance”, but in this case that means it’s also measured in ⅟₅₁₂-of-a-tile units, right? Most descriptions of the Douglas–Peucker algorithm discuss curve simplification specifically, so it’ll be unclear to the developer how the tolerance relates to clustering. We can probably clarify the documentation a bit by talking about a minimum distance between points.

[1ec5]

It’s ironic for me to say this in light of #5970, but I think we could get away with a simple NSDictionary and some global string constants instead of a full-blown options object. That’s the way most Cocoa APIs accept options, especially when not all of the options are required (for example, NSAttributedString). Moving to a loosely-typed dictionary also frees us from having to hard-code default values.

[boundsj]

I actually already started this in another branch. I noticed that when I try to combine similar changes in this commit with the changes in the 6049 branch and filter on point_count (like this example from the Android demo app) no data is rendered. I'm going to investigate that more now

[boundsj]

2495143 implements this with an NSDictionary as suggested above.

Regarding #6217 (comment) and the discussion in mapbox/mapbox-gl-js#2985; this PR has minimum viable documentation and I'd like to land it sooner for an alpha release. I'll create another ticket to capture the work to make the documentation better.

cc @frederoni @1ec5

[boundsj]

Tail work ticket for documentation opened: #6236

[1ec5]

Nit: our convention is currently to use Objective-C Boolean values, so YES and NO (in backticks) rather than true and false.

[boundsj]

Great points @1ec5. Thanks for the help. I've simplified this and will land it later today if it looks ok.

[incanus]

we don’t offer any way for the developer to find out the size of the tile at a given zoom level.

AFAICT, isn't this just FractionalZoom * 512?

[1ec5]

we don’t offer any way for the developer to find out the size of the tile at a given zoom level.

AFAICT, isn't this just FractionalZoom * 512?

Not quite: zoom levels are logarithmic, not linear. But we don’t have an API for it anyways, so I don’t think we should hand-wave about it in documentation.

[incanus]

zoom levels are logarithmic, not linear

Ah, right. But then: log2(FractionalZoom) * 512?

But, true. This is a side point anyway.