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

Readme & decode demo updates #47

Open
wants to merge 3 commits into
base: 2.x
Choose a base branch
from
Open

Readme & decode demo updates #47

wants to merge 3 commits into from

Conversation

mapsam
Copy link
Contributor

@mapsam mapsam commented May 31, 2018

This fixes the /demo/decode.cpp to work with the 2.x branch. It's a little clumsy, and still relies on the branch of geometry.hpp that hasn't been merged yet. I had trouble writing a proper geometry type reader, but we could theoretically piggy-back on the work over at mapbox/geometry.hpp#56

Updates build instructions as well in the README so others can replicate. #34

cc @mapbox/core-tech @joto

@mapsam
Copy link
Contributor Author

mapsam commented May 31, 2018

Also noting that the vtzero decode demo is pretty solid, and is likely more reliable than this one. Curious if the demo is necessary? Right now it feels a little redundant, and doesn't really highlight the benefits of using lib vector-tile.

@springmeyer
Copy link
Contributor

Right now it feels a little redundant, and doesn't really highlight the benefits of using lib vector-tile.

Yes, the demo code pre-dates vtzero and the 2.x branch of this repo. It is not critical to keep around from my perspective as the original author.

@mapsam
Copy link
Contributor Author

mapsam commented Jun 5, 2018

Sounds good @springmeyer - it'll be good to have some demo/example code around, but right now the vector-tile decoder feels almost more complex than decoding with vtzero, since we've removed the helpful methods from the 1.x branch. Not sure what the long-term plan is here, but I'm sure @flippmoke has an idea! I'm going to remove the demo code from this branch now, so we can start fresh with 2.x

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