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

Merges compatible Point features into MultiPoint #725

Merged
merged 1 commit into from
May 28, 2024

Conversation

daniel-j-h
Copy link
Contributor

For #719: This changeset merges multiple compatible points into a single multipoint.

Motivation: while looking into the MVT spec and how it's implemented in Everything You Wanted to Know About Vector Tiles (But Were Afraid to Ask) I learned about inefficiencies in encoding single points in tiles and how multipoints help there. Turns out tilemaker did not emit multipoints - let's change that!

This changeset combines multiple point features into a single multipoint so that the tiles will be smaller, there is less to encode/decode, and the frontend clients only have to decode a single multipoint feature with multiple MoveTo commands.

How much this will help in practice depends on the configs and areas used and the points have to be compatible (attributes).

@systemed I would appreciate a review here since my C++ is a bit rusty nowadays! Also how do we best test this change? I ran it manually on a few pbf files but otherwise how do we make sure this is working as intended?

More context in


If you run with the --verbose flag you get to see the debug output

Merging 20 points into a multipoint
Merging 5 points into a multipoint
Merging 4 points into a multipoint
Merging 2 points into a multipoint
Merging 4 points into a multipoint
Merging 4 points into a multipoint
Merging 4 points into a multipoint
Merging 4 points into a multipoint
Merging 4 points into a multipoint
Merging 5 points into a multipoint
..

@systemed
Copy link
Owner

This looks really good - thank you!

I ran a quick test with a local pbf and then running the tile through vt2geojson, and the output is what you'd expect:

    {
      "type": "Feature",
      "geometry": {
        "type": "MultiPoint",
        "coordinates": [
          [
            -1.4897257089614868,
            51.87246698977762
          ],
          [
            -1.489870548248291,
            51.872523294874156
          ],
          [
            -1.4903587102890015,
            51.87297373310824
          ]
        ]
      },
      "properties": {
        "urban": 0,
        "type": "bicycle_parking",
        "vt_layer": "pois"
      }
    },

and they render properly:

Screenshot 2024-05-28 at 15 54 52

so I think this is good to go.

@systemed systemed merged commit d5ded3b into systemed:master May 28, 2024
6 of 7 checks passed
@daniel-j-h daniel-j-h deleted the issues/719 branch May 28, 2024 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants