Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: support javascript functions for the data driven styles, versus just the style expression DSL #10893

Closed
cdaringe opened this issue Jul 25, 2021 · 6 comments

Comments

@cdaringe
Copy link

mapbox-gl-js version: 2.3.x

Question

Can mapbox-gl-js support styles via JS functions, vs the style expression DSL?

I.e (feature) => style vs [ExprType, ...expr-node]

Upon studying the documentation & attempting to style a feature, I encountered the documentation for style expressions & data-driven styling.

Candidly, I am (personally) not delighted by the status-quo API.

Why not? Because I have the power of my current programming language to do various computations using the same knowledge and tooling of that programming language, which is now removed and replaced by a new DSL. This product specific DSL that is a whole new mini-language for expression/functions, and isn't valuable anywhere except mapbox products.

I understand that there may be benefits in such a DSL, in that its declarative nature ports easily across platform, but I just want functions. The DSL surely eventually compiles into functional equivalent expression evaluations by the mapbox internals. I care to skip the middleman, receive my feature, and do my own style computation on my own. In the projects I work on, I do not require x-platform functionality, and would prefer that I do my computation in the computation environment I already have domain expertise in, versus coercing my computation into a DSL and sending it into mapbox. The DSL translation is a non-value add activity for my project, takes some cognitive effort and removes power from me (e.g. ease of unit testing, etc).

Thanks for the consideration.

I did search the issue tracker for related discussions, but found none. Please close if this is a duplicate :)

Links to related documentation

See above

@rreusser
Copy link
Contributor

rreusser commented Jul 26, 2021

Thanks for writing up this proposal, @cdaringe! 🙇

I definitely sympathize with the challenge of wrapping one's head around expressions. For a start, if you find particular aspects of the documentation lacking, please do leave feedback on the "Was this page helpful?" widget. It may feel like it goes into the void, but the feedback is received and we take it seriously.

One reason why this does not currently exist is the fact that while the behavior of a map may be affected by event callbacks, for example, Mapbox goes to great lengths to ensure that the appearance of a map is platform-independent. Many users design these expressions in Studio and don't directly write the expression syntax themselves (I know I've clicked the </> Edit JSON button and copy-pasted expressions generated by Studio so that I can modify them to suit my needs). I realize this isn't a very satisfying answer if your concern is strictly JS, and custom layers are an example of one platform-specific escape hatch.

Apart from portability, a specific consideration with providing callbacks is that web workers would require that your callback can be stringified, passed to a worker, and eval'd for execution. The function as well as any external data could be serialized and passed around in this manner, but normal JS scoping rules would not apply.

Additionally, GL JS performs type checking of expressions and does some static analysis so that it can, for example, evaluate expressions at integer zoom levels, place the result in per-feature attribute buffers, and interpolate exponentially between them to minimize evaluation during rendering. Other properties are handled in other very specific ways, as required by their particular quirks. I think allowing arbitrary functions would require expensive worst-case assumptions about what can be evaluated when, in addition to exposing more of the internal implementation machinery to the developer.

None of these are strictly blockers, and frankly I'd certainly find it pretty convenient to use this syntax, but I fear that it would significantly impact performance, add rigidity by exposing some of the internal abstractions, and would diverge from the consistency and portability that Mapbox tries hard to offer.

A similar proposal for extending expressions has been discussed here: #9462 That ticket instead proposes a slightly different approach of leaning on expressions but allowing custom implementation of what particular functions do.

I hope that clarifies some of the motivation for the current design. Please don't hesitate to continue the discussion, and no matter the end result, feedback like this is very helpful and appreciated! 🙏

@cdaringe
Copy link
Author

Thanks for the thoughtful response.

For a start, if you find particular aspects of the documentation lacking

Nah, I was able to find everything I need. It was the mere thought that I have existing, valid code to do the mapping, of which I had to re-express in a different format to achieve the same result.

I'd like to learn more about the worker remarks. Are these values always computed from within a worker? I suppose that could make sense given the computational complexity of maps, generally. I find this to be a compelling argument. It likely does not make sense to send data back and forth over a bridge for derivation with sufficiently large datasets. If the data derivation happened in the main thread in batched RAFs, then was streamed into the worker, then I think my suggestion would be worth further thought.

I fear that it would significantly impact performance

Yes, footgunning could be abound. with freedom comes responsibility. I personally believe that developer friendly APIs are of greater importance in this specific scenario. But that's my own biases :)

add rigidity by exposing some of the internal abstractions

Agreed. Counter-point, versioning schemes more or less negate that risk, and pragmatically one would not expect too much thrash here, as the essential datamodels of interest (warning, naivety ahead) seem to largely be GeoJSON models. When GeoJSON schemas thrash, I imagine consumers are already bound to some-sort-of-code-migration. I.o.w. (feature: GeoJSON.Feature{v1,v2,v3,...,vn}) => string is probably a low API surface area, low-trash interface to have deep concern over.

would diverge from the consistency and portability that Mapbox tries hard to offer

Yes, it would diverge. This is certainly a gut feel kind of decision. A decision of art, wisdom, and preference. If the goal is to create a mapping from value => value, intrinsically, a function is precisely that. For the sake of cross-platform consistently (certainly relevant for many users) creating a DSL to declaratively model a function can be beneficial. I would argue, however, that it makes mapbox cumbersome on a core feature, thus less delightful to use. In no case would I advocate for removing the status-quo functionality--your argument is completely valid. However, for those of us for which the x-platform interface provides little value and adds friction, I personally would support both mechanisms just to apply grease to the wheels for scenarios where developers observe unnecessary friction.

@mourner
Copy link
Member

mourner commented Jul 27, 2021

The most important reason for keeping expressions pure JSON is that we have a single style language that works on both the web and mobile / native platforms, where there's a significant cost to adding a JS engine to a C++ codebase to handle such functions. We also wanted to strictly limit the scope of what's possible with expressions so that we can have full control over what happens when they are executed, so that we don't have to deal with security and other unexpected issues we didn't account for or have no capacity to support from GL JS side.

There have been conversations about possibly introducing a small subset of JS for this purpose, but this will be an enormously big lift if you consider not just GL JS but the ecosystem as a whole (Studio, Android/iOS/macOS/etc. SDKs, etc.).

@cdaringe
Copy link
Author

You don’t need to add it to the other platforms though. I’m suggesting a JavaScript feature for the JavaScript environment. Intentional, opt in divergence.

The cost to the lib, naively disregarding the worker remarks above, is migrating from value = evalDslExpr(field)(feature) to value: typeof field === function ? field(feature) : evalDslExpr(field)(feature).

Low cost, high user impact. Heck, we could go full react-style and make the field ` dangerouslyDeriveField’ or the likes.

@mourner
Copy link
Member

mourner commented Jul 30, 2021

@cdaringe not mentioning the technical complexity of implementing this, the huge compatibility risks here far outweigh potential benefits in my opinion. The whole point of the style specification is that if you write a Mapbox style, it will work pretty much everywhere. Adding platform-specific concepts to it will add a lot of complexity to the ecosystem. So if we ever add this kind of feature, we will add it simultaneously for all platforms. Meanwhile, an alternative approach to explore would be some kind of parser for JS functions that turns them into expressions where possible. Similar to this: https://github.com/mapbox/expression-jamsession

@arindam1993
Copy link
Contributor

arindam1993 commented Jul 30, 2021

There's also the fact that for paint props expressions effectively compile down to shader code. That would not be possible with arbitrary JS functions.
The only way to implement that would be to run the supplied JS function for every feature every frame. We're pretty much saturated for what we can do on the main CPU thread, there's almost no budget left really. An O(n) loop over all visible features for every frame will put it well over.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants