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: Updating the appearance of individual features #6020

Closed
ansis opened this issue Jan 18, 2018 · 14 comments
Closed

RFC: Updating the appearance of individual features #6020

ansis opened this issue Jan 18, 2018 · 14 comments

Comments

@ansis
Copy link
Contributor

ansis commented Jan 18, 2018

Motivation

What problem are we trying to solve?

Interaction often triggers changes to appearance of features. For example:

  • changing the colour of a selected polygon
  • changing the halo of label on hover
  • hiding labels based on text input

These changes are currently both slow and hard to implement. We need to make it possible to update the paint properties of of a reasonable number of features instantly.

This issue focuses on how to respond to these interaction events, not how to detect them.

Implementation Design Alternatives

Do nothing is not an option.

Option 1: Making style updates cheap

When you want to change an appearance of a feature, change the style:

map.setPaintProperty("countries", "fill-color",
    ["case",
        ["==", ["id"], 123],
        "#ff0000",
        "#000000"]);

// ...

map.setPaintProperty("countries", "fill-color", "#000000");

// ...

map.setPaintProperty("countries", "fill-color",
    ["case",
        ["==", ["id"], 555],
        "#ff0000",
        "#000000"]);
map.setFilter("countries", ["==", "$id", 123]);

These examples are close to the current approach to highlighting features. If we can make paint property changes fast enough we can use this approach to respond to interaction.

Changing these properties currently involves recreating the entire bucket in the worker. To make it fast enough we'd need to a) only recreate the relevant paint buffers, and b) do it on the main thread. Since we can't tell which features were affected by the change we have to recalculate the paint values for all of them. There might also be challenges with transitioning changes and with switching between data-driven and literal expressions.

setFilter would be harder to optimize. We would need to either:

  • completely recreate the buckets when this changes
  • include all unfiltered features in the bucket, and filter on-the-fly by rewriting the element index buffer
    Since neither of these are great options, we would encourage users use data-driven opacity instead.

Option 2: Making data updates cheap

Instead of constantly updating the style, specify the possible style variations as a data-driven expression that is set once and then update the data:

"fill-color": ["case",
    ["get": "isSelected"],
    "#ff0000",
    "#000000"]

When creating the buckets keep track of which paint array vertices correspond to which feature ids:

vertexPositions[feature.id] =  { start: 34, end: 39 };

There would be some kind of way to update the data of individual features. See the mockups for possible external APIs.

When the data of a feature changes, recalculate any paint properties that depend on the data that changed and write the values to the buffer using the previously stored indices. Do this within a render frame.

updatePaintArray(paintArray, vertexPositions[feature.id], expression, feature);

Upload any buffers that changed.

Implementation Design

I think we should use Option 2.

The advantages are:

  • Since we can tell which features changed, we can update individual features instead of all features. This could be significantly faster than recalculating the property for all features.
  • It keeps state data and styling more separate, which seems like the data-driven way to do it
  • It is easier to handle transitions
  • It avoids the problem of smoothly switching between data-driven and literal expressions

The main disadvantage is:

  • This would introduce the concept of updating feature properties
  • This would require a new API method for updating the properties of feature

Mockups

Option 2 would require some kind of API for updating data. Here are some rough possibilities. I could see a combination of them being useful.

Option 2A: Setting data updates internally

We could keep track of hover state, activation and selection internally. These states would be exposed within expressions using something like ["state", "hover"]. This could be very useful for basic cases but not complex enough to handle all the possible approaches to selection, etc.

Option 2B: setters

map.setDataProperty("source", feature, "hover", true);

Option 2C: feature objects

Queries and events could return feature objects that could be manipulated directly.

map.on('mouseenter', 'layerid', featureId, (e) => {
    e.feature.properties.hover = true;
    e.feature.update();
});

Concepts

What existing precedents support the new concepts?
Where do the concepts set new precedents?
This would introduce the concept of updating feature appearance by updating feature data. This builds on the concept of data-driven styling (as used in Mapbox and in d3) but takes it a step further with live updates.

How will we teach this design?
With examples showing how to use this approach to interactivity.

What terminology will work best for the new concepts introduced by this design?
I'm not sure. "feature data updating"?

Next steps

The main question here is:

  • should we fix this problem by making style updates fast or by making data updates fast?

@kkaefer @asheemmamoowala @mollymerp @anandthakker @mourner @lucaswoj

@asheemmamoowala
Copy link
Contributor

When creating the buckets keep track of which paint array vertices correspond to which feature ids:

@ansis with the above and Option 2C, does exposing the feature object also allow updating the geometry?
Will we try to restrict the API such that buckets don't have to be recreated to 'update the appearance.' For example, is it OK to grow the size of a circle or change symbol/text on hover?

@ansis
Copy link
Contributor Author

ansis commented Jan 19, 2018

with the above and Option 2C, does exposing the feature object also allow updating the geometry?

I'm not sure. The mockups are just rough sketches but I think it might make sense to allow geometry updates for GeoJSON sources using a similar approach. This could be a nice way to implement things like point dragging?

Will we try to restrict the API such that buckets don't have to be recreated to 'update the appearance.' For example, is it OK to grow the size of a circle or change symbol/text on hover?

Good question. I'm not sure. One option would be to allow it but to have different performance expectations for changing layout properties vs paint properties.

@ansis
Copy link
Contributor Author

ansis commented Jan 19, 2018

I pushed a proof of concept Option 2: Making data updates cheap to data-update-proof-of-concept with an example http://localhost:9966/debug/highlight.html

@andrewharvey
Copy link
Collaborator

I like Option 2, it's provides a very clear separation of Style and Data. As features are hovered over the Style remains constant from Studio and the client side app just updates each features data with its state.

Agreed, 2A is simpler but ultimately something generic allowing user defined states would be better. To retain the simplicity of 2A when implementing 2B/2C we could use convention in Studio and examples to set things like "hover", "active" etc.

@mollymerp
Copy link
Contributor

awesome write-up @ansis! I agree that option 2 seems like a logical extension of paradigms we've already introduced with expressions. The POC demo is 🔥 ⚡️
The API design in 2B seems most akin to our existing APIs for runtime styling (i.e. setPaintProperty etc), but 2C could also be simplified to something like e.feature.update({"hover": true, "visited": true});

@kkaefer
Copy link
Contributor

kkaefer commented Jan 23, 2018

/cc @mapbox/studio

@ansis
Copy link
Contributor Author

ansis commented Feb 7, 2018

Last we talked about this and settled on option 2. There was some concern that fast geometry updates would make the POC approach unnecessary but I think we still think that even if the geometry updates are fast for a small number of features this POC approach could be faster for more and still useful. Also, this provides a clear path to make property updates fast now.

@asheemmamoowala will be working on implementing this

@lucaswoj
Copy link
Contributor

lucaswoj commented Feb 8, 2018

What does it mean if a user updates properties on a vector tile feature? Do those updates persist across zoom levels?

Will we require all custom source types to allow feature property updates?

@ansis
Copy link
Contributor Author

ansis commented Feb 8, 2018

What does it mean if a user updates properties on a vector tile feature? Do those updates persist across zoom levels?

Yep, it would be updated for all tiles across all zoom levels and would persist even if the tile was unloaded and reloaded. In order to do this we need some way of telling what feature is the same feature: #6019

Will we require all custom source types to allow feature property updates?

Not sure, good question

@asheemmamoowala
Copy link
Contributor

The ongoing discussion in #6021 is converging on the idea of separating state from feature properties.

Setting state

State should be tracked independently of feature data and assigned per source. This would look similar to 2B

//Set a feature_id to a named state
map.setState("source", "state", feature_id);

//Clear state
map.setState("source", "state");

//Set one or more feature Ids to a named state
map.setState("source", "state", [feature_id]);

States are tracked per source in the SourceCache where they can be applied to tiles when preparing them for upload before every frame.

Question: Is there a need for an API to un-set a named state on a single feature or set of features , while preserving other features in that state?

Using state

State can be referenced in expressions as part of layer paint properties

// Increase opacity for features in the 'highlight' state
"fill-opacity": [ "case", ["state", "highlight"], ["number", 0.9], ["number", 0.5]] 

or queried through an API:

map.getState("state");
// Where the returned object looks like:
{ 
 "source_A" : { "highlight": [1234, ...], "dragging" : [...], ... } ,
 "source_B": { "visited" : ["foo", "bar"] }
}

Reacting to changed state

For expressions, state is considered a per-feature datum, and as an expression itself, it can only be used with paint properties that support data driven styling.

To quickly update a features paint properties, the implementation needs to:

  • Enable dynamic updates of buffers for buckets whose layers reference state expressions.
  • On state changes, mark the map as needing re-render or re-render immediately.
  • Determine the sources and layers whose buckets need to be updated and re-uploaded
  • Propagate state change to affected buckets
  • Re-evaluate the affected paint property expressions.
  • In each affected bucket, update the vertex buffers (and index buffers?) with new values

@ansis' proof of concept branch shows most of the the plumbing needed to update paint arrays.

@lucaswoj
Copy link
Contributor

I very much like the direction of this API 😄

One could imagine allowing other animation parameters to be stored in state as well for smoother animations.

One consideration: If I were coming to Mapbox with fresh eyes and saw an API called Map#setState, I would probably assume it set the state of the Map (camera position, styling, etc). A name like Map#setFeatureState would more likely give me the right impression.

@stevage
Copy link
Contributor

stevage commented Mar 1, 2018

Really glad to see this happening. Several times I've implemented a "highlight polygon on mouseover" in a map, only to remove it because it wasn't responsive enough.

Couple of questions:

"map.setState("source", "state", feature_id);"

What is feature_id here? Currently features don't necessarily have user-exposed id's, do they? (Ensuring each feature has some kind of unique ID I can use to filter the styling with is sometimes annoying, so if this proposal fixes that, great...)

In any case, 2B looks right to me. In my most recent app, there is a "multi select" for polygons, which would be easy to implement with 2B, and probably not catered for with 2A.

Question: Is there a need for an API to un-set a named state on a single feature or set of features , while preserving other features in that state?

Like, user selects 5 polygons, then unselects 1? Absolutely.

Or do you mean, a polygon is selected and hover, then we want to make it just hover? Probably, but less commonly. (Eg, user selects and hovers over it, then something else happens that removes the selection - like data changes in the background.)

@asheemmamoowala
Copy link
Contributor

@stevage Thanks for the feedback!

Setting a state value to undefined will be the equivalent of un-set.

What is feature_id here?

feature_id is a unique identifier for each feature in the source data. It is not required by GeoJSON or vector tile sources in general, but it would be required to enable interactive states.

@asheemmamoowala
Copy link
Contributor

Implemented by #6263

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

8 participants