ol-mapbox-style icon indicating copy to clipboard operation
ol-mapbox-style copied to clipboard

Patched Mapbox GL Spec

Open lukasmartinelli opened this issue 8 years ago • 12 comments
trafficstars

@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.

lukasmartinelli avatar Jan 10 '17 13:01 lukasmartinelli

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?

ahocevar avatar Jan 10 '17 14:01 ahocevar

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"
            }

lukasmartinelli avatar Jan 10 '17 14:01 lukasmartinelli

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.

ahocevar avatar Jan 10 '17 15:01 ahocevar

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.

lukasmartinelli avatar Jan 10 '17 15:01 lukasmartinelli

@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.

ahocevar avatar Jan 12 '17 13:01 ahocevar

Hey @ahocevar did you get anywhere with this? I noticed in https://github.com/mapbox/mapbox-gl-js/issues/4170#issuecomment-297946609 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.

orangemug avatar Apr 09 '18 10:04 orangemug

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

ahocevar avatar Apr 23 '18 15:04 ahocevar

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 avatar Apr 24 '18 12:04 orangemug

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 avatar Dec 11 '23 16:12 orangemug

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 avatar Dec 11 '23 17:12 orangemug

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 https://github.com/openlayers/ol-mapbox-style/pull/1055

orangemug avatar Dec 12 '23 16:12 orangemug

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

orangemug avatar Dec 14 '23 16:12 orangemug