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

feature request: a way to conditionally provide icon images #7587

Closed
ansis opened this issue Nov 13, 2018 · 3 comments · Fixed by #7987
Closed

feature request: a way to conditionally provide icon images #7587

ansis opened this issue Nov 13, 2018 · 3 comments · Fixed by #7987

Comments

@ansis
Copy link
Contributor

ansis commented Nov 13, 2018

Motivation

There are currently two ways to provide images for icons and patterns: the sprite and addImage. These work well when you have a limited number of images or when you know ahead of time which images you need. These approaches do not work when there are many possible images and you don't know which you need, for example user icons.

We have a way to push images. We need a way to pull images.

Example:
You have a vector tile source that contains user locations. You want to set a unique icon image for each user depending on a userid data property. This is currently not possible there are too many icons to add in the sprite and because you can't use addImage because you don't know which icons are needed.

A solution to this problem could be also useful for rendering custom icons based on feature properties.

Design Alternatives

A) Do nothing

Force users to load the data separately as geojson and have them manually iterate through features and add the correct icons before setting the data.

Cons:

  • does not work at all for vector tile sources
  • adds complexity to geojson setting and updating, especially on -native where json isn't as simple to use

B) Support loading images from urls

"icon-image": ["load-image", ["concat", "http://example.com/", ["get", "userid"], ".png"]]

The transformRequest map option could be used by users to transform custom uri schemes to regular urls. For example, my-scheme://images/123 to http://example.com/images/.

Cons:

  • doesn't allow for local icon rendering
  • can't use external code (that handles auth, etc) for loading images
  • lets people use separate images everywhere in the style which could let people hurt their load performance too easily

C) event for missing images

An event that gets fired after noticing we need an icon but before actually trying to use it. This lets the user add an image within the callback.

"icon-image": ["load-image", ["concat", "my-user-image", ["get", "userid"]]
map.on('imagemissing', function(name) {
    if (name.indexOf('my-user-image') === 0) {
        const img = document.createElement('img');
        const userid = name.replace('my-user-image', '');
        img.src = 'http://example.com/image/' + userid + '.png';
        map.addImage(name, image);
    }
});
map.on('imagemissing', function(name) {
    if (name.indexOf('my-user-image') === 0) {
        const data = new Uint8Array(...);
        // render icon into `data`
        map.addImage(name, { width: ..., height: ..., data: data });
    }
});

We would need some way of cleaning up images after they aren't needed anymore. One possible way is to add a third addImage parameter that lets the map drop it when it isn't used anymore: addImage(name, img, dropWhenUnused).

We would also need support for async images.

Cons:

  • the developer needs to encode all data into a string and then parse it back out of a string. This gets more complicated if your icon depends on multiple properties

D) add image rendering callback with structured data

"icon-image": ["render-image", "mynamespace", ["get", "userid"], ["feature-state", "hover"]]
map.addImageRenderer("mynamespace", function(userid, hover) {
    ...
    return img;
});

Cons:

  • new expression, new api method
  • typing could be complicated on -native

E) user-implemented expressions

D) allows for user code to be called by expressions indirectly. We could take this further and allow users to register their own expressions.

map.registerExpression("generate-user-image": {
    evaluate: function(arguments, evalContext) {
        // return image
    }
});
"icon-image": ["generate-user-image", ["get", "userid"]]

Cons:

  • a much broader impact and riskier than the other options
  • a larger interface
  • tricky to run user code in workers without breaking csp compliance

Design

I think we should implement C) event for missing images. It does what we need with the least amount of changes. It seems like something similar could be added on iOS (cc @1ec5) with a delegate and on Android with listener. There is already precedent for this on iOS with -mapView:imageForAnnotation:.

Options D and E feel over designed. B works only in some cases and the possible performance trap it would introduce worries me.

Concepts

C) also has the least amount of new concepts introduced. We would teach this design by adding an example that fetches a custom image based on data and an example that renders an image locally. No new terminology would need to be introduced.

Implementation

ImageManager#getImages would fire the event when images are missing. ImageManager would also check if the image has loaded, and if not, delay until it has. The implementation in -native should be very similar.


@ChrisLoer does C) sound good?

@ansis ansis changed the title feature request: a way of conditionally providing icon images feature request: a way to conditionally provide icon images Nov 13, 2018
@ryanhamley
Copy link
Contributor

@ansis I don't know if you saw this ticket already but we had a request for something just like option C last week #7533 I also began laying some ground work for this sort of thing in #6831 by logging a warning when a requested image doesn't exist or the sprite sheet isn't present.

@joewoodhouse
Copy link
Contributor

Any update on this? I have just written almost verbatim your code in example B) under the assumption it would "just work" and then found this issue. I'm using a vector tile source.

@1ec5
Copy link
Contributor

1ec5 commented Feb 25, 2019

Some previous discussion along these lines: mapbox/mapbox-gl-style-spec#485, #5261.

ansis added a commit that referenced this issue Mar 4, 2019
The event gets fired when a layer needs an image that the map doesn't
have. The developer has a chance to listen for this event and add an
image within the callback in order to have it included.

This event can be used to dynamically generate icons based on feature
properties.

fix #7587
ansis added a commit that referenced this issue Mar 4, 2019
The event gets fired when a layer needs an image that the map doesn't
have. The developer has a chance to listen for this event and add an
image within the callback in order to have it included.

This event can be used to dynamically generate icons based on feature
properties.

fix #7587
ansis added a commit that referenced this issue Mar 4, 2019
The event gets fired when a layer needs an image that the map doesn't
have. The developer has a chance to listen for this event and add an
image within the callback in order to have it included.

This event can be used to dynamically generate icons based on feature
properties.

fix #7587
ansis added a commit that referenced this issue Mar 6, 2019
The event gets fired when a layer needs an image that the map doesn't
have. The developer has a chance to listen for this event and add an
image within the callback in order to have it included.

This event can be used to dynamically generate icons based on feature
properties.

fix #7587
ansis added a commit that referenced this issue Mar 6, 2019
The event gets fired when a layer needs an image that the map doesn't
have. The developer has a chance to listen for this event and add an
image within the callback in order to have it included.

This event can be used to dynamically generate icons based on feature
properties.

fix #7587
ansis added a commit that referenced this issue Mar 6, 2019
The event gets fired when a layer needs an image that the map doesn't
have. The developer has a chance to listen for this event and add an
image within the callback in order to have it included.

This event can be used to dynamically generate icons based on feature
properties.

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

Successfully merging a pull request may close this issue.

4 participants