-
Notifications
You must be signed in to change notification settings - Fork 59
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
Runtime POI sprite colors #776
Runtime POI sprite colors #776
Conversation
"poi\nsprite=poi_military_plane\ncolor=" + Color.poi.transport, | ||
"poi\nsprite=poi_plane\ncolor=" + Color.poi.transport, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other than the hospital and medical facility icons, I think these icons are now equivalent to SDFs. Instead of sending POI icons through the missing images handler, we should leverage the built-in recoloring properties to achieve the same effect more performantly.
When sprites.js generates the spritesheet JSON file, it should set sdf
to true
for these POI icons. Then this layer can use the real image names but set the icon-color
and icon-halo-color
properties.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I experimented with SDFs in the past for icons and I had major problems making them work. I believe they don't anti-alias but I could be wrong on that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It depends how you generate them. mapbox/mapbox-gl-js#1594 (comment) has some tips on generating a real SDF. A monocolored PNG with alpha transparency is technically not an SDF but it’s close enough to pass for one. You can probably get around the antialiasing issue by upscaling the PNG and using icon-size
to bring it back down to a normal size.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm skeptical that the SDF format can support an alpha channel - can you confirm or deny that? The discussion in that mapbox link doesn't sound promising. If it doesn't, we may be better off adding support in maplibre to implement icon-color
for re-coloring non-SDF images.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As GL JS understands it, an SDF is a PNG that consists of all black in the RGB channels and a silhouette in the alpha channel – in other words, the inverse of a mask. If by “an alpha channel” you mean the ability to make part of the icon translucent while the rest is transparent or opaque, then no, I don’t think GL JS supports that, but would we need that for POI icons?
The main advantage of using SDFs is that both the icon and its halo can scale without getting pixelated, but you’ll get better results by downscaling than by upscaling. The main limitation of SDFs is that GL JS only supports single-color SDFs, not multicolor SDFs.
A more general-purpose icon-color
would be interesting, but I don’t think it should block this PR, nor would I discount SDFs without giving it a try. Another alternative, if this recolored-monocolor aesthetic is desired, would be to use a custom font, but that’s a bit overkill for just a few fonts. (That demo takes a custom font and converts it into SDFs.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If by “an alpha channel” you mean the ability to make part of the icon translucent while the rest is transparent or opaque, then no, I don’t think GL JS supports that, but would we need that for POI icons?
My thinking, and I could be totally wrong here, is that an icon with diagonal lines will necessarily create pixels that are an alpha blend between the icon and whatever is behind it. And so an SDF which is pixel-on or pixel-off would make the icon look worse.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An alpha channel can contain pixels that are between 0% and 100%; that much is supported. What probably isn’t supported is to have a whole region of the image be filled with 50% transparency in the alpha channel.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yes that sounds workable then. I was concerned about making sure icons could be properly anti-aliased.
So something like this?
https://github.com/dy/svg-path-sdf
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That could work, though it’s also possible to do this manually. The SVGs currently in this PR branch would be a fine starting point:
- Stick a white background in the SVGs and delete the halos.
- In sprites.js, do this to each image’s data buffer before writing it out to a file.
- Set
icon-halo-*
properties on the symbol layer to restore the halos that had been in the SVG.
(Now these halos would be recolorable independently of the rest of the icon, so we could even implement a sometimes-halo, sometimes-knockout like on some text labels.)
# Conflicts: # src/js/shield.js
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My main concerns about this event-based approach were poorer performance than a built-in sprite approach, coupled with making the format for sprite names even more complex. However, the implementation in this PR has the advantage of compatibility with all the POI icons, including the medical icons that currently require two colors (or, seen another way, have an abnormally thick halo in the middle).
I still think the SDF approach is worth exploring sooner or later, definitely before we branch out to things like a Dark Mode style or start varying the icon colors by zoom level. But at least the current workflow is familiar to enough of this project’s contributors.
@1ec5 Thanks for outlining the SDF option, I wasn't aware of that GL JS functionality. I agree that this PR is a bit hacky—although it's consistent with the shield-rendering code—and it'd be great to try implementing a built-in solution at some point. I'm particularly interested in run-time icon halos. |
I believe we need an update in the contributor's guide with this change. Currently it instructs POI contributors to color their SVG icons, whereas now they need to make black and white icons but assign them to one of the POI categories. |
Done. Let me know if anything needs clarity. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great!
With #764 merged, we can now set the color of monochrome icons at runtime without a loss in quality. This PR enables such functionality for POI icons. This comes with a number of advantages as we build out the POI layer:
I was able to generalize some code from the shield-drawing for this. I'm definitely open to suggestions around the implementation, I'm not very picky about what code lives where.