Fix bbox_to_xyz #1070
Fix bbox_to_xyz #1070
Conversation
martin-tile-utils/src/lib.rs
Outdated
| pub fn wgs84_to_webmercator(lon: f64, lat: f64) -> (f64, f64) { | ||
| let x = EARTH_CIRCUMFERENCE / 360.0 * lon; | ||
| let y = ((90.0 + lat) * PI / 360.0).tan().ln() / (PI / 180.0); | ||
| let y = EARTH_CIRCUMFERENCE / 360.0 * y; |
There was a problem hiding this comment.
i merged the two suggestions here, but as the result, it no longer matches the expected result due to rounding (?). I suspect that you would need to call .min((1<<zoom) -1) somewhere
There was a problem hiding this comment.
oh, and another suspicious thing - are you sure y should cover 360? maybe it does, i don't know. Make sure you read https://wiki.openstreetmap.org/wiki/Slippy_map_tilenames
There was a problem hiding this comment.
This code is from our private server. TBH I don't remember the source....
I've adopted some code from esri. Sadly i'm bad at math😔,any help?
|
I made a few minor cleanups and rebased this on main. You may have to reset your local repo though, sorry about that. At least now i see the "more correct" values (I do not know if they are perfectly ok, or just "somewhat" ok). If tile_grid has issues like this, we probably should setup a new project for this to have it cleaned up |
| fn test_tile_index() { | ||
| assert_eq!((0, 0), tile_index(-180.0, 85.0511, 0)); | ||
| fn test_tile_colrow() { | ||
| assert_eq!((0, 0), tile_colrow(-180.0, 85.0511, 0)); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_xyz_to_bbox() { |
There was a problem hiding this comment.
Due to the inherent precision limitations of floating point numbers, performing back-and-forth transformations between XYZ and bounding box coordinates within a single unit test seems not a good idea?
I removed the transform back test code. And actually if you add them back, you can see some failed. Any idea? I can't see logic error in these methods. Is it our const not accurate. My mind get lost now.
There was a problem hiding this comment.
Or maybe we should reserve tile-grid in this?
| @@ -248,13 +251,31 @@ pub fn webmercator_to_wgs84(x: f64, y: f64) -> (f64, f64) { | |||
| (lng, lat) | |||
| } | |||
|
|
|||
| /// transform WGS84 to WebMercator | |||
| // from https://github.com/Esri/arcgis-osm-editor/blob/e4b9905c264aa22f8eeb657efd52b12cdebea69a/src/OSMWeb10_1/Utils/WebMercator.cs | |||
There was a problem hiding this comment.
It's from esri.
There was a problem hiding this comment.
I think this code is actually from the OSM wiki that has all this math
There was a problem hiding this comment.
The extra tests are great! I think we keep using old names (index/x/y) as they are more expected. I think this is almost ready to go. It is totally ok to not have a "perfect roundtrip" - as long as they are close enough, esp if we reduce the lat/lng by some tiny percentage, we are good to go
| @@ -248,13 +251,31 @@ pub fn webmercator_to_wgs84(x: f64, y: f64) -> (f64, f64) { | |||
| (lng, lat) | |||
| } | |||
|
|
|||
| /// transform WGS84 to WebMercator | |||
| // from https://github.com/Esri/arcgis-osm-editor/blob/e4b9905c264aa22f8eeb657efd52b12cdebea69a/src/OSMWeb10_1/Utils/WebMercator.cs | |||
There was a problem hiding this comment.
I think this code is actually from the OSM wiki that has all this math
Figure out and fix the bug with
tile-gridcrate or remove the crate and implement it ourself.