mapbox / mapbox-gl-js

Interactive, thoroughly customizable maps in the browser, powered by vector tiles and WebGL
https://docs.mapbox.com/mapbox-gl-js/
Other
11.23k stars 2.23k forks source link

Remove "fill-outline-color" #4088

Open lucaswoj opened 7 years ago

lucaswoj commented 7 years ago

From @jfirebaugh on January 26, 2015 19:30

As discussed in #199 and #223.

fill-outline-color is currently used in the bright and outdoors styles. @nickidlugash, @peterqliu can you comment on the importance of the property in those styles and in general? Would it be ok to remove for v7 without an immediate replacement via a "polygon" type or "multiple types" feature?

Copied from original issue: mapbox/mapbox-gl-style-spec#240

lucaswoj commented 7 years ago

From @peterqliu on January 26, 2015 19:35

:+1: causes no breaking changes for me.

Looking forward to polygons

lucaswoj commented 7 years ago

From @nickidlugash on January 26, 2015 20:18

@jfirebaugh For these styles (and styles based on Mapbox Streets in general), the two features that depend the most on fill-outline-color are water and buildings. For these features, having an outline option is pretty important.

Would it be ok to remove for v7 without an immediate replacement via a "polygon" type or "multiple types" feature?

How long do we think it'll take to implement either a replacement (or optimize the performance of drawing lines enough that creating a second GL layer for the outlines won't kill performance)? If this is something that we're definitely doing soon, we can remove outlines for v7. I will want to manually update the v7 styles though, so I can optimize the way they look sans outline.

lucaswoj commented 7 years ago

From @jfirebaugh on October 7, 2016 0:37

We really should do this. fill-outline-color adds complex special cases to the renderer for very little value.

lucaswoj commented 7 years ago

From @nickidlugash on October 7, 2016 14:38

@jfirebaugh we use fill-outline-color in our private styles and public styles for buildings and road polygons. If we decide to remove it, we'll need to coordinate a styles update with the spec update.

lucaswoj commented 7 years ago

From @mourner on November 7, 2016 22:59

I was initially worried about performance implications of this, but it looks like the only massive layer we use it for is buildings, which only appear on z15+, which is very easy to render compared to heavier zooms like z11-13. So I'm đź‘Ť on this.

1ec5 commented 7 years ago

fill-outline-color is also used by the shape annotation API in the native SDKs, for functional parity with MapKit’s MKOverlayPathRenderer.strokeColor and the Google Maps SDK’s GMSPolygon.strokeColor. mapbox/mapbox-gl-native#6181 could end up replacing the core annotation API implementation with a style layer–based implementation, but there would be some inconvenience in the fact that a feature collection would be required to represent each polygon overlay.

Beyond the native SDKs, fill-outline-color would also come in handy for https://github.com/mapbox/simplespec-to-gl-style/pull/25#issuecomment-285767989.

lucaswoj commented 7 years ago

@1ec5 Our intention after removing this property is to recommend that users use an additional line layer for strokes (or provide some syntactic sugar that effectively creates this line layer).

The existing fill-outline-color is a hack and limited to 1px width.

1ec5 commented 7 years ago

I think the fact that many (most?) higher-level APIs provide options analogous to fill-stroke-color and fill-stroke-width suggests that we should seriously consider providing built-in support for them as well. But I understand that the current implementation is limited by the OpenGL facility it’s hooking into.

or provide some syntactic sugar that effectively creates this line layer

That sounds good. That’s basically the proposal (at a higher level) in https://github.com/mapbox/mapbox-gl-native/issues/6181#issuecomment-285788856.

jfirebaugh commented 7 years ago

I think the fact that many (most?) higher-level APIs provide options analogous to fill-stroke-color and fill-stroke-width suggests that we should seriously consider providing built-in support for them as well.

See #4087.

gabimoncha commented 5 years ago

There could also be cases where the fill-stroke isn't needed. For example when showing population density and the zoom level is really small, it can be a bit annoying during zoom transition.

screenshot 2019-02-21 at 16 52 05

For example in this case it's needed just when the zoom level is increased.

screenshot 2019-02-21 at 16 48 41