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 non-sprite images #563

Open
cns-solutions-admin opened this issue May 18, 2022 · 7 comments
Open

Support non-sprite images #563

cns-solutions-admin opened this issue May 18, 2022 · 7 comments

Comments

@cns-solutions-admin
Copy link
Contributor

In Mapbox you can add images to the map (see https://docs.mapbox.com/mapbox-gl-js/api/map/#instance-members-images) either statically by loading them before use or dynamically as response to a "styleimagemissing" event.

We used this

  • to simplify setup: just throw the image into the images folder and reference it in the style
  • to dynamically create images, e.g. an image name of "COLORIZE:myicon:FF0000" would load (the white) myicon and colorize it red

Even with OpenLayers we prefer to configure our layers with mapbox style JSONs and would like to add this possibility to the stylefunction, e.g.

  • {{export default function (..., getFonts, getImage)}} with getImage optional and of type {{function(Layer,string):HTMLImageElement|HTMLCanvasElement|undefined}}
  • if getImage is defined and returns an image, it is used, otherwise the default find-image-in-sprite path is used
  • if getImage is lazily creating the image it will return undefined first and when the image is loaded, call layer.changed() and return the image on the next request.
@ahocevar
Copy link
Member

Generating sprites from individual images is an easy process, e.g. using spritezero-cli. And sprite sheets are more efficient than individual images. So I'd recommend looking into that instead.

@cns-solutions-admin
Copy link
Contributor Author

For static images that might be ok.

However, we need to support dynamic environments and (relatively) unskilled administrators:

  • you want another icon for your POI - or it should be bigger, have another color? just replace the png file in that directory!
  • there is a new status for your resource? just drop a png file with the status name in that directory!

As opposed to

  • take all the png files we delivered (and hope they are in sync with the existing sprite)
  • replace or add the relevant png
  • feed them to this sprite generator (which should work with pngs, but I suppose there might be one somewhere)
  • copy the generated json and png to that folder
  • keep the updated pngs for the next time use (yeah, sure)

And of course a sprite is useless, if images are dynamically generated based on the data in the features.

If there was a way to just wrap the style function without duplicating the logic, that might be better, but I don't see a way.

@ahocevar
Copy link
Member

ahocevar commented May 18, 2022

In my workflow, the user puts the desired image in a directory and runs

npm run sprite

Then the uploaded image will be available in a 1x and 2x resolution sprite sheet, available under the name of the file that the user uploaded (e.g. myimage for myimage.png).

But like I proposed in #561 (comment), a style modification hook would also allow you to bypass sprites and provide images in a different way.

@cns-solutions-admin
Copy link
Contributor Author

@ahocevar: add the style modification hook: where (which class) would this be?

@ahocevar
Copy link
Member

ahocevar commented May 19, 2022

The options can be passed to any of the API functions, and are carried on all the way down to styleFunction internally. @cns-solutions-admin I'm probably going to implement it this weekend, so no need for you to worry about it.

@ahocevar
Copy link
Member

@cns-solutions-admin Digging a bit more into this, I think an API like mapbox-gl-js's would be nice:

import { addImage } from 'ol-mapbox-style';

addImage('myimage', 'path/to/myimage.png', {pixelRatio: 1});
addImage('myimage', 'path/to/myimage@2x.png', {pixelRatio: 2});

Then this image would be available as if myimage was defined in the sprite file. The options passed as 3rd argument are equivalent to those provided in a sprite json, with defaults for width and height being the image width and height, and x and y being 0.

@cns-solutions-admin
Copy link
Contributor Author

Yes, as described above, with mapbox we used addImage, but also the "styleimagemissing" event:

  • addImage to add images (we didn't use sprites)
  • respond to "styleimagemissing" events and load/create dynamic images (e.g. colorize an image, convert it to gray, etc.)

However, for use in the stylefunction this would add to much complexity, so I just added a function getImage like getFont that can return images not found in the sprite:

  • if it is already available, just return it
  • it it has to be loaded/created, return null first and when it is loaded/created, call layer.changed() and return it the next time

I think that the mapbox styling could use a radical refactoring, e.g.

  • convert to a class
  • split into multiple functions (esp. by layer type)
  • probably move mapbox tile layer from ol to remove circular dependency

By having a class, it can be extended, pre-/postprocessing can be easily added into the functions, ...
This requires some thoughts and a better design.

But until that time I still think, my approach with the additional optional getImage function is the easiest and best.

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

No branches or pull requests

2 participants