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

Add support for downloading remote images for icon-image handling #65

Closed
wants to merge 3 commits into from

Conversation

rodrigoscc
Copy link

@rodrigoscc rodrigoscc commented Oct 9, 2020

We are using mapbox-gl-renderer and it's great. But had an issue when trying to use icon-image, it doesn't work because the image is not available and there is not a way to specify an url there, just an image name.

This PR is to solve that use case. I added a images parameter to the server so that it downloads all the images first.

I am not that experienced with javascript so I will be very happy to get some feedback :)

@brendan-ward
Copy link
Collaborator

@rstcruzo thank you for putting this together!

A few questions first:

  1. Can you please provide a small example style JSON that demonstrates icon-image as used here? We can then use that to create unit tests.

  2. Do you think it would be possible to use images stored locally on the server, and simply reference them with an appropriate URL in the style JSON, and possibly some extra mechanics to load them from disk? See Provide local assets directory #51; we are thinking of a local assets folder, and it seems like this could also be used to store images.

I haven't done an exhaustive read of the docs for icon-image, but it looks like it is possible host sprites containing those images, and refer to them with icon-image. However, that would mean creating sprites through some other process beforehand; we don't want to create them in mbgl-renderer. Is it possible for you to create image sprites for your images, and then use those sprite URLs in your style JSON to load them?

Adding support to use map.addImage for locally-available images seems reasonable, and we could use your images parameter to provide a mapping between image name used in the style file and image filename in a local assets directory.

I would like to avoid adding image download capabilities to mbgl-renderer.

I think we could use sharp to read the images from disk into buffers instead of adding a new dependency on pngjs, but let's first solve where to get the images from.

@rodrigoscc
Copy link
Author

Let's say the style is this one:

{
  "layers": [
    {
      "layout": {
        "icon-image": "festival.png",
        "icon-allow-overlap": true,
        "icon-size": 1
      },
      "source": "source_1",
      "paint": {
        "icon-opacity": 1
      },
      "minzoom": 0,
      "maxzoom": 23,
      "type": "symbol"
    }
  ],
  "sources": {
    "source_1": {
      "maxzoom": 23,
      "tiles": [
        "..."
      ],
      "type": "vector",
      "minzoom": 0
    }
  }
}

We need festival.png to be available to the renderer, so we use the images parameter to tell the renderer to download the image first.

"images": {"festival.png": "http://image.com/festival.png"}

@brendan-ward I agree that allowing locally-available images would be a nice feature too. But it doesn't accommodate to our use case.

We are using images uploaded by users. Those images are hosted on our server. We would need to send an image to the mbgl-renderer each time a user uploads one (and a way to send images to the renderer). That's why a download url is better for us. Also we would like to avoid building sprites on our end.

Let me know your thoughts :)

@brendan-ward
Copy link
Collaborator

@rstcruzo are the images small enough that they could be base64 encoded instead of passed around as image files?

Then the images parameter could take as input a data structure that is either a lookup to locations on disk or strings of base64 encoded data.

Are the requests to mbgl-renderer originating from users, or from your server?

We should probably look into how mapbox.com handles user-uploaded icon images, to see if that is inspirational here. It is possible that the platform creates sprites for user-uploaded images and hosts them alongside the other style assets and sidesteps need to load the images.

@rodrigoscc
Copy link
Author

rodrigoscc commented Oct 9, 2020

@brendan-ward images could be of any size. Requests to mbgl-renderer are originated from server.

I didn't think downloading an image would be undesirable since the renderer is already downloading sprites, image sources, etc. with getRemoteAsset. The procedure is pretty much the same.

Hope this PR helps someone else anyways :)

@brendan-ward
Copy link
Collaborator

Superseded by #83

@rstcruzo I spliced your changes here in with the ability to load base64 encoded images (we needed for our use case); thank you so much for the good start on this, and apologies it took so long to work these changes in!

Note: the final API is a little different than what you proposed here. images is now an object with keys for image IDs, and properties for url and other things that we pass in to Mapbox.

{
    "id": {
        "url": "<required: base64 or remote URL>",
        "pixelRatio": <optional, defaults to 1, only used for icon-image not patterns>,
        "sdf": <optional, defaults to false, converts image to SDF format for recoloring in Mapbox GL>
    },
    ....
}

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

Successfully merging this pull request may close these issues.

2 participants