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

Resolve units of measure in the Z direction #3385

Closed
peterqliu opened this issue Oct 14, 2016 · 13 comments
Closed

Resolve units of measure in the Z direction #3385

peterqliu opened this issue Oct 14, 2016 · 13 comments
Labels
breaking change ⚠️ Requires a backwards-incompatible change to the API feature 🍏 needs discussion 💬

Comments

@peterqliu
Copy link
Contributor

peterqliu commented Oct 14, 2016

As we flesh out our offerings in 3D, it becomes necessary to render multiple layers in the same 3D space: labels atop extrusions, lofted lines alongside extrusions, etc. For stuff to line up properly, we need all layers to speak the same (or easily convertible) measurement language(s).

Currently, there are two things that "pop up" off the map:

  • Extrusions. Relevant properties are fill-extrude-height and fill-extrude-base, which are specified in a nameless, but real-world unit that scales automatically with zoom.
  • Point symbols, mediated by text/symbol-translate. This is specified in pixels, which are independent of zoom.

What should be our paradigm, moving forward?

@lucaswoj
Copy link
Contributor

lucaswoj commented Oct 14, 2016

We should measure extrusions in pixels.

Everything else in GL is measured in pixels or ems.

The need for a pixel -> meter conversion is a cross-cutting concern (i.e. is needed on lots of other properties in lots of other situations). That suggests a separate feature implemented at the function level.

@lucaswoj lucaswoj added this to the Frankfurt milestone Oct 14, 2016
@lucaswoj lucaswoj added feature 🍏 breaking change ⚠️ Requires a backwards-incompatible change to the API labels Oct 14, 2016
@lbud
Copy link
Contributor

lbud commented Oct 20, 2016

Extrusions in pixels is a pretty weird concept to me because you'll actually never see the number of pixels specified, since we don't pitch the map to 90º. While presumably for any other properties measured in pixels you see variable sizes based on pitch, you would see the specified value for an unpitched (default) map, whereas in the case of extrusions on an unpitched map, while centered on an object you would actually see no vertical lines, and for features around it you'll see variable-length vertical lines depending on how far they are from the center of a viewport… 🤔

@ansis
Copy link
Contributor

ansis commented Oct 28, 2016

Extrusions in pixels is a pretty weird concept to me

I agree, pixels seem weird. I think some real-world units (meters) makes more sense as a default here.

The need for a pixel -> meter conversion is a cross-cutting concern (i.e. is needed on lots of other properties in lots of other situations). That suggests a separate feature implemented at the function level.

I agree but I think we can still make meter the default here.

@peterqliu
Copy link
Contributor Author

peterqliu commented Oct 28, 2016

Real-world units are convenient when we think about buildings, but introduce some weird problems:

  • It binds gl-js to Web Mercator, in order to make the meters-to-pixels conversion. This is less an issue of "let's support all projections!", than "gl-js has stayed agnostic to specific implementations, and this is a step in the opposite direction."
  • m-to-px conversion is dependent on latitude. Should we calculate latitude separately for every vertex of the polygon, or just the centroid, or some other way? The former would necessitate pitched roofs (amount of pitch is trivial on the building scale, but significant on the country scale), while the latter would make for crummy approximations at large scales.

For gl-js to be a generic, versatile renderer for all visualization needs, I'm leaning toward pixels.

@ansis
Copy link
Contributor

ansis commented Oct 28, 2016

It binds gl-js to Web Mercator, in order to make the meters-to-pixels conversion.

We're already heavily tied to web mercator. Compared to all the other things we'd need to implement to support other projections I don't think this would be a huge challenge.

m-to-px conversion is dependent on latitude.

Yep, we need to handle this otherwise the basic use case of rendering buildings with actual heights is impossible. If we don't implement this internally then buildings closer to the poles will be rendered shorter and it won't be possible for users to work around this.

And it's not just buildings. In most visualizations, I think you'd want the height of a bar to be proportional to real-world distance. I don't think you'd want the visualizations to look squished at some latitudes.

Should we calculate latitude separately for every vertex of the polygon, or just the centroid, or some other way? The former would necessitate pitched roofs (amount of pitch is trivial on the building scale, but significant on the country scale), while the latter would make for crummy approximations at large scales.

I'm not sure. Each approach has it's tradeoffs but I think both could be ok in practice. I'm not too worried about crummy approximations at low zooms because web mercator is already pretty bad at those scales. All the approaches would look identical at higher zoom levels.

I'm leaning toward pixels.

Do you have an example of a fill-extrusion you'd rather style with pixels? I think all the examples I've seen so far (buildings, block level population density, etc) would work better with units proportional to real-world space. I'm fine with supporting pixels but I think meters is more important for the examples I've seen.

@peterqliu
Copy link
Contributor Author

peterqliu commented Oct 29, 2016

In a nutshell, it's difficult to reason about three-dimensional space when two dimensions are in one unit, and the third is another, untranslatable unit. For an example, how do I extrude a thick line such that the resultant shape's cross-section is square, without reconciling line-width with line-extrude-height?

If we were to do real world units, some kind of map.pxPerMeter(latitude, zoom) method would be helpful to make the conversion less painful.

@lbud
Copy link
Contributor

lbud commented Oct 29, 2016

Based on the above + after voicing with @ansis and @jfirebaugh I'm going to do this in meters. Eventually if this causes problems at low zooms we may consider adding something like a fill-extrusion-units:{meters,pixels} enum.

@ansis
Copy link
Contributor

ansis commented Oct 29, 2016

In a nutshell, it's difficult to reason about three-dimensional space when two dimensions are in one unit, and the third is another, untranslatable unit.

Yeah, it sounds like we should use a different default for lines where the other two dimensions are pixels. The difficulties when the third dimension mismatches the other two is why the default for fills should be meters. The other two dimensions for fills are always real-world.

Supporting multiple units for all of these sounds good to me.

@lbud
Copy link
Contributor

lbud commented Oct 29, 2016

Oh @peterqliu sorry — this page hadn't refreshed after you commented/before I did.

The converse of your question is how do you extrude a building to its actual height, given that you have its geographic footprint and physical height, if you have to use pixels? Right now extruding buildings is a common use case easily solved with an identity function; with zoom-dependent pixels, this is…not so simple.

@peterqliu
Copy link
Contributor Author

peterqliu commented Oct 29, 2016

The converse of your question is how do you extrude a building to its actual height, given that you have its geographic footprint and physical height, if you have to use pixels?

@lbud it would be a speculative, zoom-and-double-property (latitude + actual height) DDS function. Certainly not as pithy as an identity fx, but makes for a consistent way to measure and reason about the 3D world.

@jfirebaugh
Copy link
Contributor

jfirebaugh commented Oct 29, 2016

a speculative, zoom-and-double-property

Given that we don't have such a feature currently, I'm comfortable sticking with meters for now, with extrusion-units: {meters,pixels} or real-world units for line-extrude-width as future directions to support the (relatively-less common) use cases that don't fall in the buildings/block level population density category.

@1ec5
Copy link
Contributor

1ec5 commented Nov 1, 2016

If we were to do real world units, some kind of map.pxPerMeter(latitude, zoom) method would be helpful to make the conversion less painful.

We have this feature in the iOS and macOS SDKs. It would be useful regardless of the outcome of this issue. Time for a separate ticket?

lbud pushed a commit that referenced this issue Nov 1, 2016
* Resolve Z units to meters (and move z scaling to transform for use in future other 3D types)
Fixes #3385.
lbud pushed a commit that referenced this issue Nov 1, 2016
* Resolve Z units to meters (and move z scaling to transform for use in future other 3D types)
Fixes #3385.
lbud pushed a commit that referenced this issue Nov 1, 2016
* Resolve Z units to meters (and move z scaling to transform for use in future other 3D types)
Fixes #3385.
lbud pushed a commit that referenced this issue Nov 2, 2016
* Resolve Z units to meters (and move z scaling to transform for use in future other 3D types)
Fixes #3385.
@lbud
Copy link
Contributor

lbud commented Nov 2, 2016

Fixed in #3487.

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 feature 🍏 needs discussion 💬
Projects
None yet
Development

No branches or pull requests

6 participants