mapbox / mapbox-gl-native

Interactive, thoroughly customizable maps in native Android, iOS, macOS, Node.js, and Qt applications, powered by vector tiles and OpenGL
https://mapbox.com/mobile
Other
4.37k stars 1.33k forks source link

Data-driven styling should support arbitrary functions #7860

Closed ericrwolfe closed 7 years ago

ericrwolfe commented 7 years ago

It should be possible to use D3.js-style blocks/callbacks for representing functions in data-driven styling.

I imagine this might be less performant than static functions since core would have to call back to platform code for every feature rendered, but would be more powerful for defining arbitrary interpolation functions as well as make custom mappings and data joins much easier.

A example in Swift: MGLStyleFunction(withBlock: (feature: MGLFeature, zoomLevel: Double) -> MGLStyleValue)

let highlightedFeatures = Set<String>()
circleLayer.circleColor = MGLStyleFunction(withBlock: { (feature, zoomLevel) in
    let attr = feature.attributes
    if highlightedFeatures.contains(attr["osm_id"]) {
        return MGLStyleValue(rawValue: UIColor.gold) 
    }
    if attr["category"] == "restaurant" {
        return MGLStyleValue(rawValue: UIColor.green)
    }
    return MGLStyleValue(rawValue: UIColor.blue)
})
symbolLayer.textFontSize = MGLStyleFunction(withBlock: { (feature, zoomLevel) in
    let size = round(9.0 + 2 * log10(feature.attributes["score"] as! Double + 1))
    return MGLStyleValue(rawValue: size)
})

This has been discussed before for JS in the following tickets: https://github.com/mapbox/mapbox-gl-style-spec/issues/47, https://github.com/mapbox/mapbox-gl-function/issues/28

Arbitrary function callbacks aren't possible in JS per @lucaswoj because modified styles need to be serialized to JSON so that they can be transferred to worker threads.

In gl-native, we shouldn't have this limitation.

/cc @jfirebaugh @boundsj @1ec5

jfirebaugh commented 7 years ago

In gl-native, we shouldn't have this limitation.

We kind of do, actually. The style function is sometimes evaluated on a worker thread. An API like this would have to come with the caveat that the user must supply a function that either does not access shared state (unlikely -- the primary use of this API would be to do exactly that), or ensures that all such accesses are made thread safe in some manner. Such caveats are frequently problematic, as they are easily overlooked or ignored, leading to concurrency bugs that are difficult to diagnose.

ericrwolfe commented 7 years ago

In gl-native, we shouldn't have this limitation.

Whoops, I meant to say the limitation of needing to serialize back to JSON.

Working across threads is a given on the native platforms. The rule of thumb on iOS has always been to do all work that touches anything UIKit, MapKit, etc on the main thread. We could either follow similar conventions (i.e. note the caveats and leave the responsibility to the developer) or force the callbacks to happen on the main thread at the SDK level.

anandthakker commented 7 years ago

defining arbitrary interpolation functions as well as make custom mappings and data joins much easier

These do seem really valuable, but I'm not convinced that the difficulties of supporting arbitrary native/platform DDS functions would be worth it. Instead, seems to me that:

1ec5 commented 7 years ago

Along these lines, because the iOS and macOS SDKs expose filters as NSPredicates (#5973), one of the first questions a developer might ask is whether we support block-based predicates, which you can think of as callback-based filters. (It isn’t unheard of for a predicate backend to lack support for blocks, but it is annoying.)

Just as a JavaScript developer would often find it convenient to be able to filter using a JavaScript function, an Objective-C or Swift developer would gravitate toward block-based predicates. Case in point: the standard way to filter an array in Objective-C is to pass a predicate into -filteredArrayUsingPredicate:. In the old days, you had to create the predicate using format strings, just like creating a style layer predicate in our iOS SDK. But ever since block-based predicates became available, block-based predicates have been the more popular approach because you don’t have to learn a new language.

Despite the ergonomic and expressive advantage of callbacks for filters and style functions, it’s unfortunately the case that evaluating filters and property values on a worker thread – or even in a shader – turn callbacks into a trap. No matter how much we document the fact that these callbacks get called on a background thread, even an experienced developer is susceptible to accidentally capturing non-thread-safe state into the callback, making deadlocks and race conditions more likely. This is the same concern I had around the background-thread callbacks proposed in https://github.com/mapbox/mapbox-gl-native/pull/7485#issuecomment-270400565.

Would it be feasible to create a “slow” path on the main thread for evaluating callback-based filters and style functions, keeping declarative filters and style functions on worker threads, or would a parallel path introduce too much complexity to be worth it?

defining arbitrary interpolation functions would be nicely handled by something like the proposal being developed in mapbox/mapbox-gl-style-spec#47

mapbox/mapbox-gl-style-spec#47 would expand our declarative programming capabilities, but accommodating every use case declaratively would ultimately force us to reimplement a programming language. Imperative features like custom vector sources (#6940), low-level OpenGL layers (#7250 #5830), high-level custom-drawn layers (#7823), and this proposal for callback-driven style functions all allow the developer to take better advantage of the language and their existing model objects without forcing us to bloat our API surface area. (The irony isn’t lost on me that I just listed a bunch of features that increase our API surface area dramatically in the short term.)

jfirebaugh commented 7 years ago

Closing for now -- as detailed above, there are substantial difficulties to this approach that rule it out for the time being.