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

Support loading icons asynchronously on styleimagemissing #8335

Open
joewoodhouse opened this issue Jun 10, 2019 · 16 comments
Open

Support loading icons asynchronously on styleimagemissing #8335

joewoodhouse opened this issue Jun 10, 2019 · 16 comments

Comments

@joewoodhouse
Copy link
Contributor

joewoodhouse commented Jun 10, 2019

Update: Design proposals that close this issue are in #9018.


mapbox-gl-js version: 1.0.0

browser: Chrome 75.0.3770.80

Steps to Trigger Behavior

  1. Load a map with a style containing a layer that has a missing icon-image
  2. Attach to the styleimagemissing event and load an image for the missing icon-image

Link to Demonstration

This codepen demonstrates the behaviour: https://codepen.io/anon/pen/OeLYqW

It does the following:

  • Loads the streets-v11 style JSON (via the API)
  • Appends a symbol layer to the style's layers. This new layer re-uses an existing source layer (composite -> roads) but the icon-image is set to an non-existent value
  • It also attaches to the 'styleimagemissing' event and loads up a valid image when it fires.
  • Once the map has loaded, it waits 1.5 seconds then outputs how many features have been rendered from the test layer.

When you reload the codepen (reload the entire browser tab) you should see the number of features rendered is basically random. The correct answer (which you can see by changing the code-pen and setting the 'icon-image' property to e.g. beer-11 which is an image from the sprite) is 21, but I saw results like 0, 4, 23,24,25 (not sure how there is more!)

Expected Behavior

Features should render properly at initial zoom and all other zooms

Actual Behavior

On initial load, the features with the missing icon render seemingly at random.

@joewoodhouse
Copy link
Contributor Author

@mourner any updates on this one? I sort of built quite a large feature based off this new styleimagemissing functionality, and this bug has now made it unusable. I'd be happy to have a go at fixing it myself, if you could provide any sort of starting point as to where/what you think the issue might be.

@ghost
Copy link

ghost commented Jun 23, 2019

Even I am facing the same issue, any update on this

@mourner
Copy link
Member

mourner commented Jun 24, 2019

Sorry for a late response! This seems to be working as designed — here's a relevant bit of the docs:

The missing image can be added with Map#addImage within this event listener callback to prevent the image from being skipped.

This means you have to synchronously call map.addImage within the styleimagemissing callback. So to fix your example, you would load the fallback image first, then add the layer that contains missing images (so that the image data is already there inside imagemissing).

@mourner mourner closed this as completed Jun 24, 2019
@joewoodhouse
Copy link
Contributor Author

@mourner I don't agree with closing this issue. Surely your suggestion totally defeats the purpose of the feature itself? To quote the original PR/commit (fd32cc3)

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

If you have to pre-add all potential images that might be missing before adding the layer, I wouldn't describe that as dynamically generating them. It also requires you to load images that potentially may never be used.

Also my codepen example I put together clearly shows something is broken, and it's an issue I'm facing in my production code. Please could you reconsider this?

@mourner
Copy link
Member

mourner commented Jun 24, 2019

@joewoodhouse the codepen shows the API being misused so it's expected to be broken. The purpose of the feature was to allow generating icons on demand, which has to be synchronous (a technical limitation in the way the renderer handles icons), as opposed to loading icons on demand. You can see the intent demonstrated in the official example.

Perhaps we could add another, asynchronous API to handle your use case, but I'm not sure if that's technically viable — @ansis any thoughts on this? I'll reopen the issue to discuss.

@mourner mourner reopened this Jun 24, 2019
@mourner mourner changed the title Adding images via styleimagemissing produces unpredicatble results on initital load Support loading icons asynchronously on styleimagemissing Jun 24, 2019
@chloekraw
Copy link
Contributor

chloekraw commented Jul 2, 2019

@joewoodhouse @rhushikeshSahaj, do you know the size of the images you want to add through styleimagemissing in advance? if so, you can synchronously add a blank image in the same size as your actual image, and then use the updateImage method to replace the blank image with your actual image after you've fetched it asynchronously.

@joewoodhouse
Copy link
Contributor Author

joewoodhouse commented Jul 2, 2019

@chloekraw @mourner @ansis nice idea, but no I think the use-case I'm looking for here is that you don't know anything about the images (what size, how many, etc) before you load the map. Imagine a map where you're loading some 3rd party GeoJSON, and you want to style the icon-image based on a particular property of the features. You don't know in advance what values that property might have. Or you might want to create the icon-image based on some computation of a number of properties in the record. That's the use-case I'm imagining for this feature.

The feature itself is good, it's purely the restriction that images have to be loaded synchronously. I can't really imagine any real-world scenarios which would match the examples and tests, where an image is loaded by creating the image data manually in e.g. a Uint8Array. Surely 95% of real-world use-cases are going to be loading an image from a URL.

@chloekraw
Copy link
Contributor

Thanks for the feedback, @joewoodhouse! We will look into technical implementation paths for supporting synchronous image loading in our next iteration of the styleimagemissing feature, but this work isn't currently planned.

@asheemmamoowala asheemmamoowala added this to the release-s milestone Aug 28, 2019
@chloekraw chloekraw modified the milestones: release-sangria, release-t Oct 1, 2019
@asheemmamoowala asheemmamoowala removed this from the release-t milestone Nov 5, 2019
@joedjc
Copy link

joedjc commented Nov 20, 2019

We've encountered this issue. The workaround we've found is to hide and show the layer after the image has loaded which seems to trigger it to display. Would be great if there was a better solution though

@zacharyliu
Copy link

I was able to work around this issue by reloading the tile cache after calling addImage with the real data, although this does involve a bunch of hidden APIs.

/**
 * Reload tile cache of all non-raster sources in the map.
 * Used when adding new images, since Mapbox doesn't play well with image sizes changing.
 */
function reloadTileCache(map: mapboxgl.Map) {
  for (const cache of Object.values((map as any).style.sourceCaches)) {
    // Skip raster sources, like satellite tiles.
    if ((cache as any)._source.type === 'raster') {
      continue;
    }
    (cache as any).reload();
  }
}

@cammanderson
Copy link
Contributor

cammanderson commented Feb 14, 2020

Hi Guys,

Any update of this? We have a scenario where we want to be able to create an image through the canvas and return the data, but there is no way around async. Even if we have Base64 data and want to use the Image/Canvas object, we have to wait for an onload on image.src = BASE64.

Any chance of a review of this priority? Even if we call "addImage" and we can identify which sources need refresh?

Thanks
Cam

@chloekraw
Copy link
Contributor

@cammanderson we have provided design proposals for fixing this issue here: #9018

If you all have feedback for the API design, it would be helpful to provide it on that ticket.

cc @zacharyliu @joedjc @joewoodhouse @rhushikeshSahaj

@florianPOLARSTEPS
Copy link

@chloekraw Since we are also encountering the same limitation of not being able to offload any work in the onstyleimagemissing callback to a background thread, I created a little demo project (for the Android SDK) that demonstrates our use-case.
https://github.com/florianPOLARSTEPS/mapbox-image-loading
I hope that helps for understanding on how exactly this would be a very useful feature to have.

Thanks!

@stefanrybacki
Copy link

stefanrybacki commented Feb 22, 2021

Here is my workaround:

    this.map.on('styleimagemissing', (event) => {
        const id = event.id;
        this.map.addImage(id, {width: 0, height: 0, data: new Uint8Array()});
        this.map.loadImage(someUrl+id, (error, image) => {
          if (!error) {
            this.map.removeImage(id);
            this.map.addImage(id, image);
          } else {
            ...
          }
        });
      }
    });

So basically I add a 0x0 image with the id, and after asynchronously loading the actual image I remove the previous image and add the loaded one. It seems to work in my case with 1.13.1

@djibarian
Copy link

@stefanrybacki A repaint is needed in that case.

@blq
Copy link

blq commented Jul 7, 2022

I handle it using a "pending images" dictionary that blocks further calls for a specific image-id during the async load. (I.e similar to @stefanrybacki but using an external DS)
Seems to work quite fine so far, no need to explicitly repaint. Is this approach correct or am I missing something subtle that could fail?

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