-
Notifications
You must be signed in to change notification settings - Fork 211
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
Proposed representations of version 3 extensions #115
Conversation
// in delta encoding: the delta of the elevation after | ||
// a NaN is relative to the last finite elevation. | ||
// Elevations are in meters above or below the WGS84 ellipsoid. | ||
repeated double node_elevations = 8 [ packed = true ]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A double here would take up quite a bit of space as it could not be delta encoded and would not use integers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the elevations are integers, the normalized doubles for those integers should still work as deltas and compress well, although not as well as plain integers.
What would you prefer to do instead? Integer references into the values
? Only allowing integer elevations?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do agree there that is an exact-representation problem with doubles, and that the sum of the deltas might drift too much from the true elevation by the time you got to the end of a complex feature.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could do a custom byte stream that has separate type markers for integer deltas, floating deltas, and resets to absolute values periodically to address drift, but that is getting very complicated unless there is an existing spec for this kind of thing that we could point to.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that we should likely have an configuration to change the precision much like extent, delta encode the values as integers and apply an offset to the values. This means that you could likely highly compress almost set of z values. I think standardizing on distance from the ellipsoid is probably ideal distance from our offset, but could be very challenging if we are mixing 2d and 3d data as the 2d data is likely on the surface.
Additionally, do you feel that layers should support both 2d and 3d features?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What unspecified elevation means
I guess my feeling was that 2D and 3D features could coexist, but that 2D features all implicitly had an elevation of 0. But that contracts GeoJSON, which says that coordinates are WGS84 but also that "In the absence of elevation values, applications sensitive to height or depth SHOULD interpret positions as being at local ground or sea level."
Even if 2D and 3D can't be in the same layer, it has to be possible to render them together, so they have to be reconciled somehow. A tile-level declaration of local ground level relative to the ellipsoid could work, but just moves the responsibility of knowing the local ground level everywhere in the world to the tile creator.
Maybe we could hedge and say that null/unspecified elevations are extruded to the highest intersecting point in any other features they are being rendered together with, so if there is a terrain layer, they inherit from it, but if there isn't, they are on the ellipsoid?
Data type
The precision scale on integer elevations sounds like it would work (aside from worries above about intermixing known and unknown elevations) and would keep the math exact.
I do think it should be possible to have both specified and unspecified elevations within the same feature, because if you don't, you end up with data sets assigning magic values like "-999" to unknown elevations. So I think we should provide our own official magic value for explicit nulls within otherwise-3D features.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would unreasonable to expect that encoders and decoders know the actual elevation of the ground they are currently on in order to calculate the position of a point in 3d space. This would violate the concept of VTs being a single source of data if they had to reference another set of data to function.
Additionally, I am still not completely convinced that elevation data should be different from the 2D data and could be all encoded in the same geometry. Your design here does enable easier backwards compatibility but also worries me about writing an efficient encoder/decoder and also would result in a slightly less compressed binary I would think.
Putting data into the 2D case could also be a problem for purely vertical lines in 3d for example. In this 2d case the X,Y would be a line-string that never changed and therefore would be violating the guarantees of linestrings as this would resolve to simply a point. Granted this is not a big concern but should be noted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What unspecified elevation means
I agree that it's unreasonable to expect that encoders or decoders will know the ground elevation. But as far as I can tell, if no one knows what it is, the alternatives are:
- 0 and null both mean the ellipsoid, and 3-D features will tower above them
- 0 and null both mean the ground plane, and rendering 3-D geometries is impossible because the renderer also doesn't know the shape of the ground
Of these choices, I think the first is better, as unfortunate as it is, because I think the second is completely unworkable.
One object or two
The main argument I can make in favor of a separate object for the third dimension is that we are contemplating at least one more pseudo-dimension (arbitrary attributes), and there has been a request for another (the M-dimension from shapefile). If all of these dimensions live in the same object, then we either need 6 different versions of each operator or a bitfield that specifies all 6 ways to read each coordinate "pair".
- lineto (2-D)
- lineto (3-D)
- lineto (4-D)
- lineto (2-D with attributes)
- lineto (3-D with attributes)
- lineto (4-D with attributes)
My opinion is that giving each dimension its own object is both more compatible with the past (existing encoders and decoders do not need to change) and more future-proof because we can add whatever other parallel arrays may be necessary in the future as we think of other things that need to be encoded. Putting everything in the same object is neither compatible nor straightforwardly extensible.
You are right that we currently forbid 0-length movetos and linetos. Does anything actually enforce this or require it, or is it in the spec only as encouragement for people not to bloat their tiles with do-nothing geometry operations?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right that we currently forbid 0-length movetos and linetos. Does anything actually enforce this or require it, or is it in the spec only as encouragement for people not to bloat their tiles with do-nothing geometry operations?
It is more a reason to not bloat, but it is enforced in the tile creation operations currently.
The main argument I can make in favor of a separate object for the third dimension is that we are contemplating at least one more pseudo-dimension (arbitrary attributes)
I think we probably should keep attributes out of the geometry encoding section, but the 3d data encoding there might make sense. Just as a different way I solved this in another example, I put a dimensions
field https://github.com/mapbox/vector-tile-base/blob/master/vector_tile.proto#L77
LINESTRING = 2; | ||
POLYGON = 3; | ||
CURVE = 4; // like a LineString, but a spline | ||
AREA = 5; // like a Polygon, but a spline |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this would require that the spline reconnects with itself?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the AREA
case? That's what I was thinking I meant, but I don't know enough about splines. Would it not work? In PostScript you can have spline-outlined areas like this.
To enable discussions here to be separated out better I am splitting these proposed solutions into pieces, the first I have done is #117. Additionally this is off an existing |
@ericfischer I split out the rest except for Elevations/3d and per node attributes. I wasn't sure if you wanted those as a single PR or two. |
Addresses:
Explicitly does not address:
cc @flippmoke