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

Patched Mapbox GL Spec #10

Open
lukasmartinelli opened this issue Jan 10, 2017 · 12 comments
Open

Patched Mapbox GL Spec #10

lukasmartinelli opened this issue Jan 10, 2017 · 12 comments

Comments

@lukasmartinelli
Copy link

@ahocevar Would it make sense to have a patched Mapbox GL spec that only contains the properties and layer types that are supported by ol-mapbox-style?

I would take care of it. It could be in this repo or in another one.

For https://github.com/maputnik/editor this would allow me to only display properties for a OL3 style that can be supported.
And in general it would help to specify and document the subset of the Mapbox GL spec that is supported by ol-mapbox-style.

As always - I love this plugin.

@ahocevar
Copy link
Member

Are you talking about e.g. https://github.com/mapbox/mapbox-gl-style-spec/blob/mb-pages/reference/v8.json? In general, I like the idea. I'm just wondering what would be the best way to implement it. Ideally, unsupported features would just be commented out, but JSON does not allow comments. On the other hand, if unsupported features are removed from the spec, it will be harder to add them back later. Do you have any thoughts on this?

@lukasmartinelli
Copy link
Author

Are you talking about e.g. https://github.com/mapbox/mapbox-gl-style-spec/blob/mb-pages/reference/v8.json?

Yes.

In general, I like the idea. I'm just wondering what would be the best way to implement it. Ideally, unsupported features would just be commented out, but JSON does not allow comments. On the other hand, if unsupported features are removed from the spec, it will be harder to add them back later. Do you have any thoughts on this?

Perhaps fork and add ol-mapbox-style the sdk-support field. This way we only annotate properties that are supported and do not delete/add anything to the spec.

          "sdk-support": {
            "basic functionality": {
              "js": "0.10.0",
              "android": "2.0.1",
              "ios": "2.0.0",
              "macos": "0.1.0"
            }

@ahocevar
Copy link
Member

ahocevar commented Jan 10, 2017

So you mean something like

      "sdk-support": {
        "basic functionality": {
          "js": "0.10.0",
          "android": "2.0.1",
          "ios": "2.0.0",
          "macos": "0.1.0",
          "ol-mapbox-style: "1.0.0"
        }
      }

I agree that doing this is the cleanest way. If you agree, before creating a fork, I'd create an upstream ticket with the suggestion - maybe it can even be added there.

@lukasmartinelli
Copy link
Author

I agree that doing this is the cleanest way. If you agree, before creating a fork, I'd create an upstream ticket with the suggestion - maybe it can even be added there.

Good idea, try that. I think the chances are not so big but upstream would be the best way.

@ahocevar
Copy link
Member

@lukasmartinelli I think you can create a pull request against https://github.com/mapbox/mapbox-gl-style-spec. I'll take care of the formal requirements once you have submitted it. Just reference the pull request here.

@orangemug
Copy link
Contributor

Hey @ahocevar did you get anywhere with this? I noticed in mapbox/mapbox-gl-js#4170 (comment) you were planning to work on it.

Also congrats on the improvements to ol-mapbox-style I've been updating the dependency in https://github.com/maputnik/editor over the months and the map styles are starting to look really good.

@ahocevar
Copy link
Member

@orangemug I still didn't find time to do this, but it's still high on my list.

@orangemug
Copy link
Contributor

Thanks for the response @ahocevar. I really want to get OpenLayers properly supported in Maputnik so anything I can do to help just shout. We've still got a little bit of work to do Maputnik side also, see https://github.com/maputnik/editor/issues?q=is%3Aissue+is%3Aopen+label%3Aopenlayers

Note: I had a look into this over the weekend, I was thinking visual regression type tests might be quite nice. Basically some GeoJSON + styling for each style rule that's in the Mapbox GL style spec.

@orangemug
Copy link
Contributor

Note: I had a look into this over the weekend, I was thinking visual regression type tests might be quite nice. Basically some GeoJSON + styling for each style rule that's in the Mapbox GL style spec.

@ahocevar I'm going to give this a go, see where I can get to. I've started over at https://maparatus.github.io/ol-mapbox-style-spec/ I'm going to build a bunch of test styles to see the visual differences between ol-mapbox-style/maplibre-gl later on maybe we could build this into some automated visual regression tests.

@orangemug
Copy link
Contributor

There are some interesting quirks, for example here I'd argue that ol/ol-mapbox-style does a better job (OpenLayers is on the right)

Screenshot from 2023-12-11 16-59-37

I'm currently unsure if the style-spec come with a caveat for OpenLayers

@orangemug
Copy link
Contributor

I've update https://maparatus.github.io/ol-mapbox-style-spec/ with a bunch more examples. Some notable differences

  • background - we currently set CSS on the background <div/> however the background should move with the camera. This isn't noticeable at the moment because we only set background-color (I tried adding background-pattern and noticed this).
  • *-pitch-* - OpenLayers doesn't have pitch so we can exclude these
  • raster-* - most of these should be possible with the approach outlined here Added raster-(hue-rotate/contrast/saturation/opacity/brightness-*/) as operations #1055

@orangemug
Copy link
Contributor

orangemug commented Dec 14, 2023

Here is how the maplibre compatibility support is going so far, results are in the ol-mapbox-style-spec repo and the spec is at ./src/spec. That page also contains PR and issue links to this repo for various features missing/buggy.

Any feedback welcome (and encouraged)

Key for the results:

  • ❌ Not supported: not yet supported
  • 🏚️ Fallback: fill-extrusion emulates the 2D part of that spec for example
  • ✅ Supported: has same behaviour as maplibre to a large enough extent to considered supported

Results:

  • ❌ source_raster_dem.redFactor
  • ❌ source_raster_dem.blueFactor
  • ❌ source_raster_dem.greenFactor
  • ❌ source_raster_dem.baseShift
  • ✅ layer.type.values.fill
  • ✅ layer.type.values.line
  • ✅ layer.type.values.symbol
  • ✅ layer.type.values.circle
  • ❌ layer.type.values.heatmap
  • 🏚️ layer.type.values.fill-extrusion
  • ✅ layer.type.values.raster
  • ✅ layer.type.values.hillshade
  • ✅ layer.type.values.background
  • ✅ layout_background.visibility
  • ✅ layout_fill.fill-sort-key
  • ✅ layout_fill.visibility
  • ✅ layout_circle.circle-sort-key
  • ✅ layout_circle.visibility
  • ❌ layout_heatmap.visibility
  • 🏚️ layout_fill-extrusion.visibility
  • ✅ layout_line.line-cap
  • ✅ layout_line.line-join
  • ✅ layout_line.line-miter-limit
  • ❌ layout_line.line-round-limit
  • ✅ layout_line.line-sort-key
  • ✅ layout_line.visibility
  • ❌ layout_symbol.symbol-placement
  • ❌ layout_symbol.symbol-spacing
  • ❌ layout_symbol.symbol-avoid-edges
  • ✅ layout_symbol.symbol-sort-key
  • ❌ layout_symbol.symbol-z-order
  • ✅ layout_symbol.icon-allow-overlap
  • ❌ layout_symbol.icon-overlap
  • ❌ layout_symbol.icon-ignore-placement
  • ❌ layout_symbol.icon-optional
  • ✅ layout_symbol.icon-rotation-alignment
  • ✅ layout_symbol.icon-size
  • ❌ layout_symbol.icon-text-fit
  • ❌ layout_symbol.icon-text-fit-padding
  • ✅ layout_symbol.icon-image
  • ✅ layout_symbol.icon-rotate
  • ❌ layout_symbol.icon-padding
  • ❌ layout_symbol.icon-keep-upright
  • ❌ layout_symbol.icon-offset
  • ✅ layout_symbol.icon-anchor
  • ❌ layout_symbol.icon-pitch-alignment
  • ❌ layout_symbol.text-pitch-alignment
  • ✅ layout_symbol.text-rotation-alignment
  • ✅ layout_symbol.text-field
  • ✅ layout_symbol.text-font
  • ✅ layout_symbol.text-size
  • ✅ layout_symbol.text-max-width
  • ✅ layout_symbol.text-line-height
  • ✅ layout_symbol.text-letter-spacing
  • ✅ layout_symbol.text-justify
  • ❌ layout_symbol.text-radial-offset
  • ❌ layout_symbol.text-variable-anchor
  • ❌ layout_symbol.text-variable-anchor-offset
  • ✅ layout_symbol.text-anchor
  • ✅ layout_symbol.text-max-angle
  • ❌ layout_symbol.text-writing-mode
  • ✅ layout_symbol.text-rotate
  • ✅ layout_symbol.text-padding
  • ❌ layout_symbol.text-keep-upright
  • ✅ layout_symbol.text-transform
  • ✅ layout_symbol.text-offset
  • ❌ layout_symbol.text-allow-overlap
  • ❌ layout_symbol.text-overlap
  • ❌ layout_symbol.text-ignore-placement
  • ❌ layout_symbol.text-optional
  • ✅ layout_symbol.visibility
  • ✅ layout_raster.visibility
  • ✅ layout_hillshade.visibility
  • ❌ light.anchor
  • ❌ light.position
  • ❌ light.color
  • ❌ light.intensity
  • ❌ terrain.source
  • ❌ terrain.exaggeration
  • ❌ paint_fill.fill-antialias
  • ✅ paint_fill.fill-opacity
  • ✅ paint_fill.fill-color
  • ✅ paint_fill.fill-outline-color
  • ❌ paint_fill.fill-translate
  • ❌ paint_fill.fill-translate-anchor
  • ✅ paint_fill.fill-pattern
  • 🏚️ paint_fill-extrusion.fill-extrusion-opacity
  • 🏚️ paint_fill-extrusion.fill-extrusion-color
  • ❌ paint_fill-extrusion.fill-extrusion-translate
  • ❌ paint_fill-extrusion.fill-extrusion-translate-anchor
  • 🏚️ paint_fill-extrusion.fill-extrusion-pattern
  • ❌ paint_fill-extrusion.fill-extrusion-height
  • ❌ paint_fill-extrusion.fill-extrusion-base
  • ❌ paint_fill-extrusion.fill-extrusion-vertical-gradient
  • ✅ paint_line.line-opacity
  • ✅ paint_line.line-color
  • ❌ paint_line.line-translate
  • ❌ paint_line.line-translate-anchor
  • ✅ paint_line.line-width
  • ❌ paint_line.line-gap-width
  • ❌ paint_line.line-offset
  • ❌ paint_line.line-blur
  • ✅ paint_line.line-dasharray
  • ❌ paint_line.line-pattern
  • ❌ paint_line.line-gradient
  • ✅ paint_circle.circle-radius
  • ✅ paint_circle.circle-color
  • ✅ paint_circle.circle-blur
  • ✅ paint_circle.circle-opacity
  • ✅ paint_circle.circle-translate
  • ❌ paint_circle.circle-translate-anchor
  • ❌ paint_circle.circle-pitch-scale
  • ❌ paint_circle.circle-pitch-alignment
  • ✅ paint_circle.circle-stroke-width
  • ✅ paint_circle.circle-stroke-color
  • ✅ paint_circle.circle-stroke-opacity
  • ❌ paint_heatmap.heatmap-radius
  • ❌ paint_heatmap.heatmap-weight
  • ❌ paint_heatmap.heatmap-intensity
  • ❌ paint_heatmap.heatmap-color
  • ❌ paint_heatmap.heatmap-opacity
  • ✅ paint_symbol.icon-opacity
  • ❌ paint_symbol.icon-color
  • ❌ paint_symbol.icon-halo-color
  • ❌ paint_symbol.icon-halo-width
  • ❌ paint_symbol.icon-halo-blur
  • ❌ paint_symbol.icon-translate
  • ❌ paint_symbol.icon-translate-anchor
  • ✅ paint_symbol.text-opacity
  • ✅ paint_symbol.text-color
  • ✅ paint_symbol.text-halo-color
  • ✅ paint_symbol.text-halo-width
  • ✅ paint_symbol.text-halo-blur
  • ✅ paint_symbol.text-translate
  • ❌ paint_symbol.text-translate-anchor
  • ✅ paint_raster.raster-opacity
  • ✅ paint_raster.raster-hue-rotate
  • ❌ paint_raster.raster-brightness-min
  • ❌ paint_raster.raster-brightness-max
  • ✅ paint_raster.raster-saturation
  • ❌ paint_raster.raster-contrast
  • ❌ paint_raster.raster-resampling
  • ❌ paint_raster.raster-fade-duration
  • ✅ paint_hillshade.hillshade-illumination-direction
  • ❌ paint_hillshade.hillshade-illumination-anchor
  • ✅ paint_hillshade.hillshade-exaggeration
  • ✅ paint_hillshade.hillshade-shadow-color
  • ✅ paint_hillshade.hillshade-highlight-color
  • ✅ paint_hillshade.hillshade-accent-color
  • ✅ paint_background.background-color
  • ❌ paint_background.background-pattern
  • ✅ paint_background.background-opacity

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

3 participants