Calculate distance metrics for line features

[mourner]

If lineMetrics: true option is specified, geojson-vt slices lines into tiles while keeping track of which part of the line each slice represents. This may allow us to do some new line rendering tricks in Mapbox GL in future (e.g. line gradients).

Line features get the following special properties:

• clip_start marks the start of the line relative to its full length; 0 if it matches the start of the original, unclipped line
• clip_end marks the end of the line relative to its full length; 1 if it matches the end of the original line

The option also explodes MultiLineString features into multiple LineString ones to be able to add different properties to each.

One big caveat is that metrics are calculated before final tile simplification (because they happen during the clipping stage). This means that actual distances on screen for these lines won't be perfectly proportional to those in the calculated properties, so if the tiles have buffers, rendering that relies on these properties may have visible seams. I'm not sure how bad this would be in practice, but let's test it out. This caveat is also the reason I opted for relative metrics rather than absolute distances (e.g. distance_total) — this way, if the buffers are 0, rendering will be seamless, even if slightly non-uniform.

• add unit tests
• maybe find better names for the properties?
• maybe force zero buffer when lineMetrics is true? seems to work OK with bigger buffers
• test it out on some proof of concept in GL JS

cc @lbud @kkaefer

[lbud]

🎉!!! It looks like there may be a few bugs in this (or this may be related to the caveat you mentioned) — for example, with this feature at z12, the lower tile (where the feature starts) has {start: 0, end: 1}, and the upper tile has {start: 0.14595801184858997, end: 1}:

If I turn clipping off I can see that the full line is actually contained within the buffered lower tile, so that makes sense; it doesn't seem like it would make sense for the upper tile to start at 0.14, though, and if it were something more like ~0.6 or 0.7 it looks like the gradient would come in at the right place on the clipped tile.

Here's a debug page with that route — https://bl.ocks.org/lbud/fc5790798b951516ad0d01965a9f282f#13.9/38.88334/-77.03918
If you zoom across different zoom thresholds you can see especially the tile containing the end of the line doesn't seem to be tracking metrics correctly. (I left stats logging on in the console in this build for convenience — start and end are coming from geojson-vt.)

I'm not totally sure I understand the caveat you mentioned in practice, but the other consequence of using relative distances rather than any absolute distance is that in the line bucket we have to iterate over the entire feature one extra time in order to precalculate the total distance in tile units of the sliced feature before creating any vertices, so that we can calculate the scale factor of a tile unit on this tile and the correctly scale absolute tile distances to relative units.

Here's the commit (branched off routeline) that integrates this branch (did not update package.json, just linked locally): mapbox/mapbox-gl-js@380e1b6

[jingsam]

🎉🎉🎉 Note that this would also probably solve this issue. I recommend a paper maybe helpful about this: http://journals.plos.org/plosone/article?id=10.1371/journal.pone.0176387.

[mourner]

@lbud oops, there was indeed a couple of bugs there. Thanks for the quick turnaround and setting up the debug page — this allowed me to track them down quickly. Looks legit now (even with buffers! 🎉):

@jingsam great point! We should definitely look into this too. One issue is that we'd have to add the line metrics feature to our server-side stack to be able to explore a proper fix for the core styles, which should be much more inolved that in geojson-vt. Another concern is that if we try to incorporate metrics into the Mapbox Streets tile source, this may be a big hit to vector tile sizes. But finding a fix for just GeoJSON sources would probably be a good start.

[mourner]

I'm not totally sure I understand the caveat you mentioned in practice

Here's my clunky shot at an illustration:

So basically both partial and total distances change after simplification, but since we calculate those before it, this can lead to a mismatch across tiles in case of a non-uniformly detailed line. E.g. in this case, if we use absolute distances, we'll render the left line part up till e.g. ~35m but the second tile will start at 70m.

we have to iterate over the entire feature one extra time in order to precalculate the total distance in tile units of the sliced feature before creating any vertices

I think this is fine — we can still avoid that for non-lineMetrics sources, right?

[anandthakker]

🎉 looks great to me!

Some potential clarifications that helped me understand clipLines better: #94

[anandthakker]

Declare t at top of function?

[mourner]

@lbud this is ready to merge. The only thing I'm worried about is the choice of properties — $distance_start and $distance_end sound a bit weird, especially given that they're relative. Any ideas? Maybe just $start and $end?

[lbud]

Capturing from chat: $distance_start + $distance_end do sound a bit weird, but $start and $end might be too simple as to cause collisions with user tileset properties. I wonder if we should use something a bit more specific like $start_segment or $start_ratio or something? The other options would be to

• document that $start and $end are reserved properties
• declare that all $-prefixed properties are reserved and/or liable to collide with core features (this feels too broad

@1ec5 also suggested we prefix with something more specific:

Or maybe prefix w/ com.mapbox., as we did for the IDs of the layers and source that power annotations?

[mourner]

@lbud How do you like clip_start and clip_end? I'd go with those. Removed $ so that they're not confused with special keys in the expression system (which refer to things that are not in feature properties) and for consistency with e.g. supercluster-generated properties (e.g. cluster, point_coint), and the new name seems to reflect the purpose better while being unlikely to collide.

[lbud]

@mourner 👌 sounds good to me!

[lbud]

Actually, sorry, I want to push back on that one more time — we should probably at least try to run an analysis on mapbox.com user tilesets before going with clip_start and clip_end: I assume supercluster-related properties only trigger different paths if the source has a cluster: true property, but line buckets treat features differently if they include the relevant keys regardless of style properties, so we should either be extra sure no one has ever used clip_start and clip_end, or go back to using $ prefixes to denote specialness, or maybe use a gvt_ prefix or something more obscure 🤔

[anandthakker]

I'm with @lbud: I think we should prefix these properties (and I'd lean towards something like gvt_ or even mapboxgl_ over just $).

[mourner]

line buckets treat features differently if they include the relevant keys regardless of style properties

Could we change the logic so that it only hits in when lineMetrics is true on the source?

[jingsam]

I don't like the prefix gvt_, it seems these fields are geojson-vt specific. When tippecanoe or other tools generate vector tiles, it is wired.

I think fields like$start and $end are extra information to help mapbox-gl to be able to better render dashed or gradient lines. If the $start and $end are not existed, mapbox-gl should think $start=0 and $end=1, otherwise use the existed one. So I think lineMetrics in source is not necessary.

[e-n-f]

Please let me know if you need this in Tippecanoe too.

I do hope that we can establish first-class support soon in the vector tile format for rejoining geometries across tile boundaries instead of relying on magic attributes like this.

[jingsam]

I have a idea that put start and end in the root-level of feature instead of properties, just as same as id:

{
  "type": "LineString",
  "id": 1,
  "start": 0.2,
  "end": 1,
  "geometry": {},
  "properties": {}
}

These two fields can be referenced with $start and $end.This could avoid name conflicting with users' properties. The defect is that we need add these two field to vector-style-spec. But I think it would be a compatible change.

[mourner]

The defect is that we need add these two field to vector-style-spec.

Not only to vector-style-spec — this would need to be added to vector-tile-spec as well to be included in encoding by implementations. So I think we should keep the values as properties.

[jingsam]

I miss typed vector-style-spec, which should be vector-tile-spec. mapbox-gl-style-spec would not be affected by this feature. As start and end are optional fields, we can add a compatible change to vector_tile.proto:

message Feature {
                optional uint64 id = 1 [ default = 0 ];

                // Tags of this feature are encoded as repeated pairs of
                // integers.
                // A detailed description of tags is located in sections
                // 4.2 and 4.4 of the specification
                repeated uint32 tags = 2 [ packed = true ];

                // The type of geometry stored in this feature.
                optional GeomType type = 3 [ default = UNKNOWN ];

                // Contains a stream of commands and parameters (vertices).
                // A detailed description on geometry encoding is located in 
                // section 4.3 of the specification.
                repeated uint32 geometry = 4 [ packed = true ];

               optional float start = 5;
               optional float end = 6;
        }

So existing vector tile encoding tools would not be affected. I think this way is more elegant than the way to add magic fields and pollute user's properties .

[mourner]

@jingsam I'm strongly against this approach. For consistency, would we add any potential calculated properties across our tools (see mapbox/vector-tile-spec#102) to the scheme, and update all our vector tile encoders accordingly? Would we have to do this every time we add a new calculated property? This would place a lot of maintenance overhead and pollute the spec with tons of noise. Given that those properties are behind a flag, I see no harm in having them as user properties, especially given how unlikely collision is with these names.

[jingsam]

@mourner I think start and end are special and different from these tools-generated properties such as point_count、 sqrt_point_count、tippecanoe_feature_density. Why they are special? start and end would be used to help rendering engine to better rendering dashed or gradient lines. So start and end are special as they will change the behavior of rendering engine. These two fields should be not be visible to users as there are no use cases that use start and end in paint properties.

However, Tools-generated properties such as point_count、 sqrt_point_count、tippecanoe_feature_density are not special properties. They should be visible to users because users need to use these properties to style map, such as using point_count as circle-radius or symbol text-field.

So, start and end are special as they are not intended to be referenced in map styles. Tools-generated properties act as ordinary properties as they can be referenced in map styles.

[mourner]

@jingsam if you think those should be in the vector-tile-spec, you should submit a proposal to the vector-tile-spec repo. However, releasing a new version of the spec and getting all VT producers up to date will take a very long time. Since we want to ship this experimental feature now, it makes more sense to do it within the limits of the existing spec — I see no disadvantages to this approach since those properties are only used internally, and we can always change it after a VT spec revision.

[jingsam]

@mourner I agree with you. So let's make this feature work at first, and improve it later.

[lbud]

Great, so I think we should prefix these properties regardless to indicate that they're special generated properties. Maybe we go with mapboxgl_ as @anandthakker suggested, in case we eventually add generated properties from a different tile generator.

[mourner]

For the sake of moving this forward, I chose mapbox_clip_start and mapbox_clip_end as the property names, and merging this now.

[mourner]

@lbud released as v3.1.0.

[lbud]

🙌 🎉 🎂 thanks @mourner!