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 of Mapbox Style Specification out-of-the-box #121

Closed
claustres opened this issue Jan 16, 2024 · 10 comments
Closed

Support of Mapbox Style Specification out-of-the-box #121

claustres opened this issue Jan 16, 2024 · 10 comments

Comments

@claustres
Copy link

claustres commented Jan 16, 2024

Hi and thanks for the awesome work done by protomaps.

We are coming from a background where we use PlaneTiler to generate MBTiles based on a OpenMaptiles profile then visualize it using TileServer GL with custom MapBox Styles derived from OpenMapTiles styles.

We would be happy to now use PMTiles but as we rely on Leaflet and not MapLibre a work has to be done to convert our styles from Mapbox Style Specification to protomaps-leaflet rules/symbolizers. Indeed, as far as we can see in the documentation, there is no out-of-the-box support for this.

However, we discovered what seems to be an undocumented function named json_style in the source code that seems to exactly do what we'd like to. So far we tried to use this function on one of our style and it gave some encouraging results as you can see in this video. However, we have been required to fix the style a little bit so that parsing went fine. Moreover, some colors do not seem to be correctly taken into account (e.g. waterway, background), we have to investigate to understand why. Last but not least, we are stucked with fonts and sprites as we don't really know how to handle it.

As a consequence, we wonder if someone in the community could give us some insights about the style conversion function, notably how fonts/sprites are expected to work. Also does this feature be relevent to be hardened, enhanced, documented ? We would be glad to help and make some PRs if required.

@bdon
Copy link
Member

bdon commented Jan 16, 2024

Most likely I'm going to remove json_style completely because the MapLibre JSON specification is too difficult to maintain a complete third-party implementation of (see https://github.com/openlayers/ol-mapbox-style). Also, the general way of constructing MapLibre styles with possible dozens of drawing layers for roads results in a poor user experience with protomaps-leaflet where the UI blocks when zooming and panning.

Fonts work by loading normal web (woff2) fonts as shown here: https://protomaps.github.io/protomaps-leaflet/examples/fonts.html#2/0.0/0.0

There isn't any sprite support yet, but it can be accomplished by writing a custom Symbolizer and sampling a sprite atlas loaded into a canvas element.

@claustres
Copy link
Author

claustres commented Jan 16, 2024

Thanks for your answer @bdon, so what you suggest is that we create our own module to support this if we'd like to as it will be shortly removed from this module ?

By the way, based on your experience/knowledge do you know what parts of the spec specifically need to be enhanced or added ? In case of missing features is it only at the style parsing level or because current rules/symbolizers cannot support it ?

@bdon
Copy link
Member

bdon commented Jan 16, 2024

Yes, I'd suggest building your own module for this or making a package that can be installed separately for reading MapLibre JSON and generating Symbolizers. You don't have to rely on only the Symbolizers defined in this project (and it would be more flexible to write your own).

I'd guess <20% of the spec is implemented, and only the easy parts; the difficult parts are anything related to fonts and dynamic label layout.

@claustres
Copy link
Author

claustres commented Jan 17, 2024

I guess that supporting OpenMapTiles-like styles is not a high step w.r.t. supporting the whole standard. Indeed, I made some adjustements and it already gives a better result: https://drive.google.com/file/d/1s6VK7EWdgepwuDSBRXohon1dnRG_p6zP/view. Here are the details:

  • text-fields rewritten in style from eg "text-field": "{name:latin} {name:nonlatin}" to "text-field": "name:latin" (interpolation and/or multiple fields to be implemented)
  • stops not supported on all numbers, replaced eg opacity: layer.paint["fill-opacity") by opacity: numberOrFn(layer.paint["fill-opacity"]) in code
  • circle/line opacity not supported, add eg opacity: numberOrFn(layer.paint["line-opacity"]) in code
  • background layer type not supported so I converted to fill type in style but it does not seem to work correctly
  • not sure if hsla colors were supported so I converted to hex and added opacity in style

For now font management only relies on the default font as I did not take time to try to load new fonts. For sure sprite support needs to be considered. However I think that two simple things could make result far more better:

  • support background layer type to (I guess) clear the canvas with it prior anything else
  • support stops on colors properties

Any help is welcome, notable code pointers to where to insert better support or fix issues.

I have probably noticed some issues to be confirmed:

  • some elements like labels seem to appear at a smaller z level than with tileservergl
  • sometimes polygon rendering seems to be wrongly done eg see video at 00:49, maybe it's related to specific geometries like inner/outer rings or multipolygons or polygon wrongly drawn on top of others elements

@bdon
Copy link
Member

bdon commented Jan 17, 2024

Nice work!

some elements like labels seem to appear at a smaller z level than with tileservergl

Yes, right now it will always be off by 1 zoom level, you can change the parameter levelDiff when creating the layer from the default (2) to 1 and the data at a given zoom level should be identical to maplibre/tileserverGL. This will negatively impact the performance, since it needs to load and draw ~4x the data for a given viewport. my goal to alleviate this would be to move the entire library to OffscreenCanvas but that may not be supported by the browsers you're targeting

sometimes polygon rendering seems to be wrongly done eg see video at 00:49

can you link to an example of this? it may depend on invalid geometry in the tileset you're using

@claustres
Copy link
Author

I will try to use levelDiff, my current PMTiles is pretty large (whole world), I will try to perform an extract on a zone exhibiting the problem.

About supporting background color could you point me in the right direction in the code as not taking it into account currently breaks the whole color rendering.

Thanks.

@bdon
Copy link
Member

bdon commented Jan 19, 2024

the backgroundColor is implemented as part of the leaflet frontend and not the JSON style right now, since there should only ever be one of these a workaround may be to ignore the JSON style and use this: https://github.com/protomaps/protomaps-leaflet/blob/main/src/frontends/leaflet.ts#L206

bdon added a commit that referenced this issue Jan 24, 2024
* Remove compat/json_style for MapLibre JSON styling
* Remove extra basemap styles
* Remove color shading feature of default style
* Remove user interaction, inspect and x-ray features

See CHANGELOG for explanations and alternatives.
@bdon
Copy link
Member

bdon commented Jan 24, 2024

#129 is an overview of the many feature removals planned for 2.0. It should still be possible to use the json style as a module as part of your own code.

bdon added a commit that referenced this issue Jan 25, 2024
* Remove compat/json_style for MapLibre JSON styling
* Remove extra basemap styles
* Remove color shading feature of default style
* Remove user interaction, inspect and x-ray features

See CHANGELOG for explanations and alternatives.
bdon added a commit that referenced this issue Jan 27, 2024
* Remove features prior to major version 2.0, make parameter names consistent case. [#112, #121, #71, 55]

* Remove compat/json_style for MapLibre JSON styling
* Remove extra basemap styles
* Remove color shading feature of default style
* Remove user interaction, inspect and x-ray features

See CHANGELOG for explanations and alternatives.

* update CHANGELOG with 2.0 feature removal details [#122]
* Unify naming conventions [#112]
* change all internal and external-facing property and variable names to camelCase instead of snake_case.
@bdon
Copy link
Member

bdon commented Jan 27, 2024

This has been removed from main as I prep for a 2.0 release, but it should be possible to use the original code at https://github.com/protomaps/protomaps-leaflet/blob/a08304417ef36fef03679976cd3e5a971fec19a2/src/compat/json_style.ts or even make a separate NPM package that pulls in protomaps-leaflet as a dependency.

@bdon bdon closed this as completed Jan 27, 2024
@claustres
Copy link
Author

Just for information we started this, it's a work in progress but in any case anyone want to help you are welcome.

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