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

Poorly encoded multipoints #144

Closed
springmeyer opened this issue Aug 7, 2015 · 3 comments
Closed

Poorly encoded multipoints #144

springmeyer opened this issue Aug 7, 2015 · 3 comments
Milestone

Comments

@springmeyer
Copy link
Contributor

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): #119

@springmeyer springmeyer added this to the v0.9.1 milestone Aug 7, 2015
@springmeyer
Copy link
Contributor Author

The bug is right here:

for (auto const& pt : geom)
{
if (first)
{
first = false;
backend_.start_tile_feature(feature_);
backend_.current_feature_->set_type(vector_tile::Tile_GeomType_POINT);
}
path_count += backend_.add_path(pt);
}
. Instead of encoding each point of a multipoint separately the encoder should be encoding the multipoint itself.

@springmeyer
Copy link
Contributor Author

fixed in 4a3017c

@springmeyer
Copy link
Contributor Author

landed in master (and future v0.9.2) in 2f71ea9

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

No branches or pull requests

1 participant