Mapbox-gl-js: Fix feature-state dependent styles for *-pattern properties

Created on 28 Aug 2018  路  12Comments  路  Source: mapbox/mapbox-gl-js

per https://github.com/mapbox/mapbox-gl-js/pull/6289#issuecomment-416363685

In the recent addition of data-driven styling support for *-pattern properties, feature-state dependent expressions do not work. The removal of the possibleOutputs state (https://github.com/mapbox/mapbox-gl-js/pull/6289#discussion_r210777581) makes it possible that the icons needed forfeature-state related paint array updates may not be available in the tile's ImageAtlas at update time.

We haven't run into this before because the other data-driven properties that require glyph/icon assets are layout properties which don't support feature-state expressions yet.

I see a couple options on how to fix this:

  • request new images in the CrossFadedCompositeBinder#updatePaintArrays method that runs on the main thread when feature states are updated/changed and update the tile's ImageAtlas

    • 馃憥 binder would need access to ImageManager, binder would have to send the new ImageAtlas to the tile somehow

  • pass available feature-states to the buckets at parse time so the expressions can be evaluated with feature state and all needed images can be requested

    • 馃憥 additional payload on worker thread transfer, doesn't cover the case where no features have the state value that is used in the expression when it is set as a paint property

  • reintroduce possibleOutputs state and require that feature-state dependent expressions for cross-faded-data-driven properties be contained as literals

    • 馃憥 lots of special-case logic and behavior, might be confusing/unexpected to users

  • not support feature-state expressions for cross-faded-data-driven properties

    • 馃憥 this is the current behavior, but it fails silently

leaning towards the first option here, but curious if anyone has a different preference or another idea.
cc @asheemmamoowala @jfirebaugh

bug needs discussion medium

Most helpful comment

Any updates on this?

All 12 comments

I lean toward the first option, although it sounds like we have a few other issues also pushing us towards the second one.

Overall, it feels to me like we might have to think about rearchitecting how we partition work to the worker.

  • Divide it up much more granularly -- per bucket.
  • Use SharedArrayBuffer to share feature data?
  • Have the option of doing certain buckets synchronously on the main thread where necessary. Like #2275 but maybe for other cases too, like when a property uses feature-state or when setLayoutProperty is used.

Any update on this @mollymerp?

I would love some attention on this from the team, it's holding me on a fork since the days of #6289.

@charandas currently there are no plans to implement this because it requires a lot of effort to implement, and our team is focused on higher priority issues at the moment.

Would it please be possible to explicitly state in the docs which style specs are supported by feature-state operations?

@songololo Thank you for the suggestion. It turns out that we already have the relevant infromation to update the docs. You can follow the docs update in https://github.com/mapbox/mapbox-gl-js-docs/pull/171

Is there any chance this is going to get worked on? @jmalanen says the native PR is on hold? This is a critical feature for us.

My team also has a deep shared interest in this piece of work. Any updates on when we might possibly see some traction on this? Or any ways we can help catalyze that?

Any updates on this?

I'm still stuck on a fork because of this. It's really frustrating.

Hi @asheemmamoowala I've got some customers that aren't able to enjoy the benefits of v2 due to reliance on feature state to show multiple data overlays. Do we have any updates in v2 that might be a solve here?

Was this page helpful?
0 / 5 - 0 ratings