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

[BREAKING] Remove fill-extrude-height and fill-extrude-base; move to a separate fill-extrusion type #554

Closed
7 of 9 tasks
lbud opened this issue Oct 28, 2016 · 2 comments
Closed
7 of 9 tasks
Assignees

Comments

@lbud
Copy link
Contributor

lbud commented Oct 28, 2016

We're going to remove the fill-extrude-height and fill-extrude-base properties and instead create a separate fill-extrusion type.

Unfortunately this is a breaking change, but it's the right way to maintain flexibility for future 3D types in the spec and to keep certain logical patterns in our renderers.

As it stands, fill layers and extruded fill layers have the following differences:

  • fill layers render with per-feature opacity; extruded fill layers render with per-layer opacity
  • extruded fill layers do not support fill-outline-color
  • extruded fill layers utilize a different bucket type, and switching between those causes problems (Unable apply runtime styles for extrusion properties mapbox-gl-js#3386); maintaining a 1:1 spec type - bucket type - render type is much cleaner
  • flat fill layers are rendered at a specified color, whereas the tops of extruded fill layers, even if they are flat features, are colored depending on light settings, and don't match fill-color exactly

Moreover, creating separate layer types will leave room for flexibility in deciding how to handle extruded texture compositing.

This will be a reversion to the way we originally designed extrusions in #456. (In happy news, this actually will make it easier to implement extrusions in gl-native once it supports property functions, since I more or less did it this way this summer…)

This change will go into effect ASAP so as to minimize the damage. fill-extrude-height and fill-extrude-base were introduced in Mapbox GL JS v0.26.0; they will be removed in favor of a fill-extrusion type in Mapbox GL JS v0.27.0, to be released next week. Layers created with these properties will render the same but will just require a bit of layer property renaming <- rendering will change due to mapbox/mapbox-gl-js#3385 (extrusion heights will now be specified in meters instead of magicnumbers).

cc @mapbox/gl @mapbox/labs @mapbox/cartography-cats @mapbox/support

🙏

To do:

* There are some Mapbox demos floating around (embedded in blog posts, etc) that use fill-extrude-height/fill-extrude-base and link to either a prior dev build or v0.26.0; I think it's fine if those stay as they are.

@jfirebaugh
Copy link
Contributor

☝️ That's a lot of linked issues! Thanks for hitting this @lbud.

@ryanbaumann
Copy link

ryanbaumann commented Oct 29, 2016

@lbud this is awesome fix before extrusions are heavily adopted. Thank you!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants