Skip to content
This repository has been archived by the owner on Apr 10, 2018. It is now read-only.

Remove "fill-outline-color" #240

Closed
jfirebaugh opened this issue Jan 26, 2015 · 6 comments
Closed

Remove "fill-outline-color" #240

jfirebaugh opened this issue Jan 26, 2015 · 6 comments

Comments

@jfirebaugh
Copy link
Contributor

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?

@jfirebaugh jfirebaugh mentioned this issue Jan 26, 2015
Merged
@peterqliu
Copy link
Contributor

👍 causes no breaking changes for me.

Looking forward to polygons

@nickidlugash
Copy link
Contributor

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

@jfirebaugh
Copy link
Contributor Author

jfirebaugh commented Oct 7, 2016

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

@nickidlugash
Copy link
Contributor

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

@mourner
Copy link
Member

mourner commented Nov 7, 2016

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.

@lucaswoj
Copy link
Contributor

lucaswoj commented Feb 1, 2017

This issue was moved to mapbox/mapbox-gl-js#4088

@lucaswoj lucaswoj closed this as completed Feb 1, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants