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

Provide "fill-stroke-*" properties and rename "fill" to "polygon" #4087

Open
lucaswoj opened this issue Feb 1, 2017 · 23 comments
Open

Provide "fill-stroke-*" properties and rename "fill" to "polygon" #4087

lucaswoj opened this issue Feb 1, 2017 · 23 comments
Labels
breaking change ⚠️ Requires a backwards-incompatible change to the API cross-platform 📺 Requires coordination with Mapbox GL Native (style specification, rendering tests, etc.) feature 🍏

Comments

@lucaswoj
Copy link
Contributor

lucaswoj commented Feb 1, 2017

From @jfirebaugh on November 21, 2014 3:46

Pros:

Cons:

  • Performance concerns?

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

@lucaswoj
Copy link
Contributor Author

lucaswoj commented Feb 1, 2017

From @ljbade on November 21, 2014 7:42

Can't you just do two passes for this layer? Just reuse fill and line code, but do two passes.

@lucaswoj lucaswoj added the breaking change ⚠️ Requires a backwards-incompatible change to the API label Feb 1, 2017
@lucaswoj
Copy link
Contributor Author

lucaswoj commented Feb 1, 2017

From @mourner on November 21, 2014 15:1

Performance concerns?

Yes, this may lead to people inadvertently producing extremely sluggish styles. E.g. adding a line building layer in addition to fill layer — the performance hit is noticeable right away.

@lucaswoj
Copy link
Contributor Author

lucaswoj commented Feb 1, 2017

From @mourner on December 24, 2014 7:57

would implementing this be as much of a performance issues as doing two separate layers now for fill and line?

@nickidlugash yes, as long as we don't differentiate between outlines that are always 1px and never change, and lines with bigger widths — the first is rendered in the same pass/shader as fill without any additional processing or geometry data, while the second needs much more data to render and a different pass, leading to worse framerate and slower tile loading.

@lucaswoj
Copy link
Contributor Author

lucaswoj commented Feb 1, 2017

From @ansis on January 23, 2015 4:8

I agree that we should kill fill-outline-color (#199). It's confusing. We can make up the performance by optimizing line tessellation more, or if that still isn't fast enough we could draw lines with "line-join": "butt" the same way the antialiasing edge for fills is drawn.

I'm not convinced that we should add a polygon type though.

  • all the line-* properties would need to be duplicated as stroke-* properties
  • the performance cost of create a stroke would be less clear than when it is a separate layer
  • if strokes are kept separate it is easier to re-arrange the z order
  • having a ton of new properties for fill layers could make UIs more cluttered

It is tedious to create a second layer for the stroke with the same data as the fill layer. A polygon type would fix this. But maybe it could be fixed by adding a way to reference other layer's data, or maybe by letting UIs handle it with an "add line" button for fill layers.

If we add a SVG-like polygon, would we add SVG-like lines with strokes and fills too?

This reminds me of the idea to allow a single layer to have multiple types, which could feel a bit more like carto. For example, "type": ["fill", "line", "symbol"] would let you use properties for all three. I suggested this way back in #244, but maybe for the wrong reasons. @yhahn talked me out of thinking it was a good idea. It came up again in #356 and mapbox/mapbox-gl-style-spec#9 and mapbox/mapbox-gl-style-spec#27

@lucaswoj
Copy link
Contributor Author

lucaswoj commented Feb 1, 2017

From @nickidlugash on June 10, 2015 17:40

@ansis @jfirebaugh @mourner Can we address this for v8?

@lucaswoj
Copy link
Contributor Author

lucaswoj commented Feb 1, 2017

From @jfirebaugh on June 10, 2015 18:0

This is out of scope for v8 -- we don't have a good lead on implementing it and it's lower priority than other features that are not currently planned for v8 like data-driven styles.

@lucaswoj
Copy link
Contributor Author

lucaswoj commented Feb 1, 2017

From @nickidlugash on June 10, 2015 18:36

Ok, but I'm pretty hesitant for us to launch the app with the fill-outline-color still implemented.

@lucaswoj
Copy link
Contributor Author

lucaswoj commented Feb 1, 2017

From @incanus on July 4, 2015 16:27

mapbox/mapbox-gl-native#1737

@lucaswoj
Copy link
Contributor Author

lucaswoj commented Feb 1, 2017

@mourner Is this easier to do with earcut?

@lucaswoj
Copy link
Contributor Author

lucaswoj commented Feb 1, 2017

From @mourner on November 13, 2015 18:42

@lucaswoj probably not related much.

@1ec5 1ec5 added the cross-platform 📺 Requires coordination with Mapbox GL Native (style specification, rendering tests, etc.) label Feb 3, 2017
@brunoabinader
Copy link
Member

As part of Qt integration, providing fill-stroke-width would enable Qt's MapRectangle border width property conversion.

@z0d14c
Copy link

z0d14c commented Feb 27, 2017

+1 for outline stroke width. the ability to have a thick border outline with no fill, for example, seems quite important.

@jdeboer-geoplan
Copy link

I'd be happy with a fill-stroke that was covered by other polygons in the same layer. I understand that if I do a line layer for the edge it'll be on top of all the polygons but I'm not sure why the fill-outline-colour does. Basically want to avoid this kind of mess
image

@mb12
Copy link

mb12 commented May 19, 2017

@jdeboer-geoplan it's a bug in the geometries you are sending over to GL JS. Your geometries are overlapping. You must create geometries with holes to make them non overlapping to model what you are trying to do.

@jdeboer-geoplan
Copy link

Hi @mb12 yeah I could cut up my geometries to remove overlap for the opaque case but if the polygons are semi-transparent I can't do that. I think I'd expect progressively occluded polygon edges. Which should look something like this
image

@DannyDelott
Copy link
Contributor

DannyDelott commented Jan 16, 2019

+1 for outline stroke width. the ability to have a thick border outline with no fill, for example, seems quite important.

I think this is a great idea. We've run into a limitation where we want our polygon layers to have thicker strokes, but doing so requires applying a line style layer on top of our fill.

The result is that our polygon with "strokes" don't preserve the actual area of the polygon:

image

Perhaps a fill-stroke-thickness property would be applied to the inside edge of the polygon, making it a better approach than using two (fill and line) style layers.

@cs09g
Copy link
Contributor

cs09g commented Jan 17, 2019

I guess fill-stroke-* something would be matter with WebGL spec.
Refs: #3018, https://developer.mozilla.org/en-US/docs/Web/API/WebGLRenderingContext/lineWidth

@ryanhamley
Copy link
Contributor

We removed all instances of gl.lineWidth() with #5541 for exactly the reasons MDN notes (there's more discussion in the PR). Chrome has marked the issue of gl.lineWidth having no effect as "Won't Fix" for this reason.

@arthur-clifford
Copy link

Rather than stroke properties why not adapt the line styles to apply to polygon typeas stroke styles?

Under the mapboxgl hood you could use the same source multiple times one pass for the fill using fill paint properties and and a pass for each polygon ring if line styles are defined for a polygon source (so if you have a donut the outer ring and empty inner ring would have the same line style).

If you don't like doing this for the "fill" type, maybe add a "stroked-fill" type.

@3zzy
Copy link

3zzy commented Jan 21, 2020

@DannyDelott Out of interest, how did you end up fixing the issue with your stroke layers.

I have a similar issue where the stroke of say 10px is rendered at the "center", so 5px of the stroke is inside the actual area, and 5px outside.

@marhas
Copy link

marhas commented Oct 22, 2020

Just wanted to upvote this, as we have a very similar requirement; a semi-transparent polygon with an opaque border.

Our current workaround is to use two layers, one fill and one line. This works but is a bit inconvenient since we want to toggle their visibility programatically and thus have to handle more layers in code.

Also a little confusing as Mapbox Studio makes it look like this should be possible to do with just one layer.

@dhana6795
Copy link

dhana6795 commented Jul 5, 2021

+1 for outline stroke width. the ability to have a thick border outline with no fill, for example, seems quite important.

I think this is a great idea. We've run into a limitation where we want our polygon layers to have thicker strokes, but doing so requires applying a line style layer on top of our fill.

The result is that our polygon with "strokes" don't preserve the actual area of the polygon:

image

Perhaps a fill-stroke-thickness property would be applied to the inside edge of the polygon, making it a better approach than using two (fill and line) style layers.

Hi Danny, i'm trying to get transparent polygons just like your screenshots, but my entire map is not visible if i put deck gl layer on top of mapbox, can you tell me how to achieve this opacity, like the words on the maps visible on the polygon
image

@tjukanovt
Copy link
Contributor

+1 for this although the issue has been open for quite a while.

I would really like to be able to use fill-stroke-width and fill-stroke-color. Would give a lot of flexibility for map design.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change ⚠️ Requires a backwards-incompatible change to the API cross-platform 📺 Requires coordination with Mapbox GL Native (style specification, rendering tests, etc.) feature 🍏
Projects
None yet
Development

No branches or pull requests