mapbox / simplespec-to-gl-style

Converts GeoJSON styled with simplestyle-spec to a GL Style
ISC License
31 stars 6 forks source link

Account for lines with fill properties. Also move to cuid #25

Closed bsudekum closed 7 years ago

bsudekum commented 7 years ago

This PR accounts for lines that have fill properties and also moves off of hat from cuid.

/cc @1ec5

bsudekum commented 7 years ago

@1ec5 refactored this a bit as the main issue with the previous commit is that we were attempting to render a layer with type fill with a source of type LineString. This produced all sorts of strange rendering issues, most notably, not full filled polygons.

Now:

  1. all features enter and are checked to see if are linestrings with a fill properties
  2. if they do have fill properties, mutate this feature to be a polygon
  3. everytime we add a polygon, we also add a linestring already https://github.com/mapbox/simplespec-to-gl-style/blob/1a3f56a7b3f0a2dba6ecf23ca983a9a4aa75ca6e/index.js#L34-L39
1ec5 commented 7 years ago

everytime we add a polygon, we also add a linestring already

Instead of representing the stroke as a separate feature, could we set fill-outline-color to a value that accounts for both the strokecolor and strokeopacity options and represent the fill as a single feature?

bsudekum commented 7 years ago

@1ec5 yep, didn't know that value was possible.

1ec5 commented 7 years ago

mapbox/mapbox-gl-js#4088 proposes to remove the fill-outline-color property altogether, but that won’t happen until the v9 style specification anyways.

bsudekum commented 7 years ago

Per discussion since there not a fill-outline-opacity or fill-outline-width, fill-outline-color will not work.

1ec5 commented 7 years ago

To clarify, fill-outline-opacity isn’t the problem, since fill-outline-width accepts an RGBA value. The problem is that fill-outline-width doesn’t exist.