search

valhalla / valhalla

Open Source Routing Engine for OpenStreetMap
https://valhalla.github.io/valhalla/
Other
4.52k stars 682 forks source link

Error encoded, decode #4251

Open xyzhieu opened 1 year ago

xyzhieu commented 1 year ago

I need help. When decode and encode the following test case i don't get the desired result.

//vCoords_Case != vCoords_Test [0] = (107.3749770, 11.5833730) [1] = (107.3739679, 11.5830050) [2] = (107.3729499, 11.5825950) [3] = (107.3727600, 11.5825169)

//vCoords_Case == vCoords_Test [0] = (107.3727600, 11.5825169) [1] = (107.3729499, 11.5825950) [2] = (107.3739679, 11.5830050) [3] = (107.3749770, 11.5833730)

std::string shapeText = valhalla::midgard::encode(vCoords_Case ,1e7); auto vCoords_Test = valhalla::midgard::decode<std::vector<std::pair<double, double>>>(shapeText ,1e-7);

kevinkreiser commented 1 year ago

they are the same but reverse order?

xyzhieu commented 1 year ago

yes, I found out when encode with 7 digits, the above case number overflows

kevinkreiser commented 1 year ago

yeah because we use 32bits internally per coordinate. this is not a bug really it wasn't meant to support arbitrary precision it was really for switching between 5 and 6 digits

kevinkreiser commented 1 year ago

I suppose if there is no performance penalty we could use 64bit integers. we'll have to test it

nilsnolde commented 1 year ago

We could also test vendoring this one: https://github.com/vahancho/polylineencoder.

kevinkreiser commented 1 year ago

i would vote against vendoring:

  1. we already have an annoying amount of dependencies to manage, i prefer the devil i know :smile:
  2. our varint implementation supports using up to 7 bits of the byte, it would be stupid for us to rip out the canonical 5bit one for the sake of saving 100 lines of code when we'd have to keep the 7bit version for the sake of smaller tiles and backwards compatibility in the data