search

mapbox / mapnik-vector-tile

Mapnik implemention of Mapbox Vector Tile specification
BSD 3-Clause "New" or "Revised" License
553 stars 117 forks source link

Poorly encoded multipoints #144

Closed springmeyer closed 9 years ago

springmeyer commented 9 years ago

Currently in v0.9.x multipoint geometries are not encoded correctly. Multipoints are currently being encoded as:

directive,dx,dy,directive,dx,dy,directive,dx,dy....

Where directive is the command that stores the command (e.g. move_to in this case) and the length of repeated commands (number of move_to commands). The problem is that the directive is always encoding cmd=move_to and length=1 which it not ideal.

Instead multipoints should be encoded as:

directive,dx,dy,dx,dy,dx,dy,dx,dy.

Where directive is cmd=move_to and length=4 (or more).

This is likely a regression around the time of this merge.

While compliant decoders should (and do) handle this type of encoding, it is incorrect in the sense that it does not take advantage of the ability of the directive to tell the decoder about the # of repeated commands. This bug therefore makes it impossible to take advantage of this obvious optimization (for multipoints): https://github.com/mapbox/mapnik-vector-tile/issues/119

springmeyer commented 9 years ago

The bug is right here: https://github.com/mapbox/mapnik-vector-tile/blob/37f42a27934b3901be482892d4d5b01e2b2e906c/src/vector_tile_processor.ipp#L928-L937. Instead of encoding each point of a multipoint separately the encoder should be encoding the multipoint itself.

springmeyer commented 9 years ago

fixed in 4a3017c

springmeyer commented 9 years ago

landed in master (and future v0.9.2) in https://github.com/mapbox/mapnik-vector-tile/commit/2f71ea9a0318c0fa2c32ed29bb074018ee8909ec