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

Fix misaligned tiles #33

Merged
merged 5 commits into from
Oct 5, 2024
Merged

Fix misaligned tiles #33

merged 5 commits into from
Oct 5, 2024

Conversation

ConnectedSystems
Copy link
Collaborator

@ConnectedSystems ConnectedSystems commented Oct 2, 2024

Previously, when loading tiles for display, the far north and south of each region were misaligned when sufficiently zoomed out.
This is because the method previously applied was incorrectly handling curvature of the earth.

The issue would go away when sufficiently zoomed in because an alternate, faster, approach was being applied at that zoom level when no adjustment for earth's curvature was needed (a detail I had forgotten).

I've now replaced the previous approach with a more accurate one, and also replaced the "fast" approach with an image resizing method already implemented in a package dependency (Images.jl), which turns out to be much faster than my own implementation.

Rough comparison for the same area:

Before:

image

After:

image

The further user is zoomed in, the less need there is to perform additional calculations to account for the curvature of the Earth, so can do a direct resampling.
Copy link
Contributor

@arlowhite arlowhite left a comment

Choose a reason for hiding this comment

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

Looks good, though the math is a bit beyond me without taking a lot more time to study it.

I'm surprised there isn't more we can leverage in GDAL for this.

return resampled
end
# Helper functions for Web Mercator projection
_lat_to_y(lat::Float64) = log(tan(π / 4 + lat * π / 360))
Copy link
Contributor

Choose a reason for hiding this comment

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

these don't already exist in a geo library?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I was interacting with GDAL via Rasters.jl and the performance hit was very significant (talking minutes).

I could potentially call out to GDAL.jl directly, but I think it would be doing extra things we don't need for our immediate use case.

Mark as future improvement?

Copy link
Contributor

Choose a reason for hiding this comment

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

Would this still be needed if we switched to web-mercator for all the data in the future?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yep!

@ConnectedSystems
Copy link
Collaborator Author

This PR requires changes to the project so please don't forget to update and/or resolve as necessary.

@ConnectedSystems ConnectedSystems merged commit 2e5b172 into main Oct 5, 2024
2 checks passed
@ConnectedSystems ConnectedSystems deleted the tile-fixes branch October 5, 2024 23:53
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.

3 participants