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

Parsing OSM tiles on WatchOS fails due to feature id overflow #45

Closed
JeffLutzenberger opened this issue Mar 15, 2024 · 10 comments
Closed
Assignees
Labels
bug Something isn't working enhancement New feature or request

Comments

@JeffLutzenberger
Copy link

JeffLutzenberger commented Mar 15, 2024

It looks like integer feature ids should be explicitly cast to UInt64 on WatchOS. I have a branch with tests for this.

Gist for discussion: https://gist.github.com/JeffLutzenberger/369d3062dafb5925d9641119eef41edd

(MVT tile spec: mapbox/vector-tile-spec#94)

@trasch trasch added bug Something isn't working enhancement New feature or request labels Mar 18, 2024
@trasch
Copy link
Contributor

trasch commented Mar 18, 2024

@JeffLutzenberger Thank you for bringing this up.

There are two conflicting definitions here:

GeoJSON:
o If a Feature has a commonly used identifier, that identifier
SHOULD be included as a member of the Feature object with the name
"id", and the value of this member is either a JSON string or
number.

Meaning, a GeoJSON id could be any Int, positive or negative.

MVT Spec:
var id: UInt64 {...}

So an Unsigned Integer in vector tiles...

In the end, we need a solution that works with both:

  • GeoJSON FeatureId needs to support Int and UInt64
  • The MVT decoder needs a change

Do you have an example URL for a OSM tile with an overflowing id?

@trasch
Copy link
Contributor

trasch commented Mar 18, 2024

... and I have to admit that it was rather shortsighted of me to not handle this correctly right from the beginning... 🙄

@JeffLutzenberger
Copy link
Author

JeffLutzenberger commented Mar 18, 2024

it seems subtle though. I'm not 100% sure, but this might be a result of arm64_32 on the watch. Regardless, maybe just explicitly parsing Int64 would suffice.

I'll see if I can get you a tile, or branch with a tile on disk that demonstrates the issue

@JeffLutzenberger
Copy link
Author

Here's one that fails. Parse with Ultra 2 simulator or device:

osm-8-41-101.pbf.zip

@trasch
Copy link
Contributor

trasch commented Mar 19, 2024

@JeffLutzenberger

I think it should work now with Outdooractive/mvt-tools#18 and #46, but I will test it later

@trasch
Copy link
Contributor

trasch commented Mar 19, 2024

@JeffLutzenberger The new uint64Value property on Feature.Identifier should give you consistent access to a UInt64 id for all OSM features, on all platforms. I hope this solves your problem... (?)

@JeffLutzenberger
Copy link
Author

Testing now. Looks good so far!

@trasch
Copy link
Contributor

trasch commented Mar 26, 2024

@JeffLutzenberger How is it going? Everything good so far?

@JeffLutzenberger
Copy link
Author

So far, so good! Thanks for the quick response and engagement on this.

@trasch
Copy link
Contributor

trasch commented Apr 5, 2024

I will merge this next week if everything is working now, so that this can be closed... :-)

trasch added a commit that referenced this issue Apr 8, 2024
@trasch trasch closed this as completed Apr 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants