Repository navigation
Fix/elevation tile cache corruption - #832
Merged
Merged
Conversation
Restore file size validation for cached tiles (removed in 6ef8169) and swap the write/validate order in download_tile_once so invalid image data is never persisted to disk. When users interrupt the app mid-download (Ctrl+C, closing window), std::fs::write can leave truncated files on disk. On subsequent runs these files pass the exists() check but decode as all-black pixels (RGB 0,0,0 = -32768m), poisoning the height grid and collapsing elevation range to zero — producing completely flat terrain. Deleting arnis-tile-cache was the only workaround. Changes: - Validate image via load_from_memory before writing to cache, so invalid data never gets persisted - Re-add <1000 byte file size check to detect and remove truncated cached tiles on load
Store tile cache in the platform-standard cache directory (AppData/Local on Windows, ~/.cache on Linux, ~/Library/Caches on macOS) via dirs::cache_dir() instead of ./arnis-tile-cache relative to the current working directory. The CWD-relative path caused silent elevation failures when: - The working directory lacked write permissions (e.g. Run as Admin sets CWD to System32, or Windows Controlled Folder Access blocks writes) - The CWD was unexpected (shortcuts, file managers, terminal launch from a different directory) In these cases, create_dir_all failed, the error propagated to ground.rs which silently fell back to flat terrain — with no indication to GUI users since errors only went to stderr. Also adds migration logic to remove the old CWD-relative cache directory on startup.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR aims to prevent elevation tile cache corruption and make tile caching more robust across different working directories by moving the cache to an OS-standard location and validating tiles before persisting them.
Changes:
- Switch elevation tile cache path from a CWD-relative directory to an OS cache directory (with fallback).
- Prevent caching invalid downloads by validating image bytes before writing to disk, and add a truncated-file guard when loading cached tiles.
- Minor refactor in HTTP status-to-message mapping for download errors.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/retrieve_data.rs | Simplifies match arm formatting for server error status messaging. |
| src/elevation_data.rs | Moves tile cache to OS cache dir, adds cache validation/truncation handling, and adds legacy cache migration logic. |
Comments suppressed due to low confidence (1)
src/elevation_data.rs:65
cleanup_old_cached_tiles()returns early when the new cache directory doesn’t exist, which prevents the legacy-cache migration block later in the function from ever running on first startup. This means an existing./arnis-tile-cachefrom previous versions will not be removed/migrated unless the OS cache dir already exists. Consider running the legacy migration before the early return (or splitting migration into a separate pre-check) so it executes even when the new cache dir hasn’t been created yet.
pub fn cleanup_old_cached_tiles() {
let tile_cache_dir = get_tile_cache_dir();
if !tile_cache_dir.exists() || !tile_cache_dir.is_dir() {
return; // Nothing to clean up
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Move the legacy ./arnis-tile-cache removal before the early return that checks if the new OS cache directory exists. Otherwise on first startup after upgrade, the new cache dir doesn't exist yet, the function returns early, and the old CWD-relative cache is never cleaned up.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The old ./arnis-tile-cache directory is harmless if left in place. Users who want to reclaim the space can delete it manually. The new OS cache directory is now used exclusively for all new downloads.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.