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

New MvtTileDecoder to allow parsing mvt pbf tiles #481

Closed
wants to merge 4 commits into from

Conversation

boldtrn
Copy link

@boldtrn boldtrn commented Jan 11, 2018

This PR adds a new MvtTileDecoder, using this nice project. The above mentioned project apparantly does not work with older Android version, that's why I used a fork (which will be hopefully merged soon) that supports older Android versions as well (the issue is that the project used some JDK8 methods, see here). I tried the new decoder with Mapzen and OMT, both basically work.

There is an issue with decoding Polygons. Probably I made a mistake when creating the new decoder or maybe the MvtReader does something odd? Maybe someone can have a look? I think there might be something messed up with holes and the outer shell, but I don't know what.

Screenshots from OMT:
omt-pbf-munich
omt-pbf-central-europe
omt-pbf-world

Screenshots from Mapzen:
mapzen-pbf-munich
mapzen-pbf-central-europe
mapzen-pbf-world

@boldtrn
Copy link
Author

boldtrn commented Jan 12, 2018

BTW I also received an issue when running:

./gradlew clean build

Execution failed for task ':vtm-android-example:transformDexArchiveWithExternalLibsDexMergerForDebug'.
> java.lang.RuntimeException: java.lang.RuntimeException: com.android.builder.dexing.DexArchiveMergerException: Unable to merge dex

@boldtrn
Copy link
Author

boldtrn commented Jan 12, 2018

BTW: An alternative library to use could be this one. I haven't compared them in detail, this library hasn't been updated since mid 2016, but has more stars. Both libraries are under Apache2 and use JTS at its core.

@boldtrn
Copy link
Author

boldtrn commented Jan 12, 2018

I did some more digging regarding the Polygon issue and saw that the reason is probably in the used library and create an issue.

@boldtrn
Copy link
Author

boldtrn commented Jan 12, 2018

Thanks to @ShibaBandit I was able to resolve the Polygon issue, the maps look perfect (or at least similar to our current geojson endpoint) now from what I can see. Performance feels good, didn't run any detailed performance comparisons.

OMT:
omt-mvt-munich
omt-mvt-europe
omt-mvt-world

Mapzen:
mapzen-mvt-munich
mapzen-mvt-europe
mapzen-mvt-world

@boldtrn
Copy link
Author

boldtrn commented Jan 12, 2018

I guess a good next step would be to move the new code into it's own package to avoid additional dependencies in the core?

@devemux86
Copy link
Collaborator

Thanks, will check it!

It's nice that there are still people willing to share work with the community. 🙂

@devemux86
Copy link
Collaborator

I guess a good next step would be to move the new code into it's own package to avoid additional dependencies in the core?

Definitely, vtm module should remain free from third-party dependencies.
I can do it while reviewing and before merging.

@boldtrn
Copy link
Author

boldtrn commented Jan 12, 2018

I can do it while reviewing and before merging.

Sure, please feel free to go ahead :)

@devemux86
Copy link
Collaborator

Execution failed for task ':vtm-android-example:transformDexArchiveWithExternalLibsDexMergerForDebug'.
> java.lang.RuntimeException: java.lang.RuntimeException: com.android.builder.dexing.DexArchiveMergerException: Unable to merge dex

That could be due to many vtm-android-example dependencies.
I had fixed that when Gradle 4 was instroduced via enabling minify, but latest Android plugin seemed to solve the situation and I reverted the change in 9b14865 to regain performance in debugging.
Will see with the PR new dependencies and if needed can enable again that fix.

};
File f = new File("vtm-tests/resources/mvt-test.pbf");
System.out.println(f.getAbsolutePath());
decoder.decode(tile, sink, new FileInputStream(f));
Copy link
Collaborator

@devemux86 devemux86 Jan 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For Gradle build also to pass, better use here:

decoder.decode(tile, sink, getClass().getResourceAsStream("/mvt-test.pbf"));

I'll include it in merge.

@devemux86 devemux86 added this to the 0.10.0 milestone Jan 12, 2018
@devemux86
Copy link
Collaborator

Thanks for the very nice work!
Merged via d049d37.

I introduced vtm-mvt module to host MVT tile decoder and sources.
The polygons fix requires the latest mapbox-vector-tile and so Android minSdkVersion 26, will see how can improve it after their changes.

@devemux86 devemux86 closed this Jan 12, 2018
@boldtrn boldtrn deleted the mvt_decoder branch January 12, 2018 23:27
devemux86 added a commit that referenced this pull request Jan 22, 2018
devemux86 added a commit that referenced this pull request Jan 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants