-
Notifications
You must be signed in to change notification settings - Fork 44
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
Web-Optimized COG: COGEO optimized for dynamic tiling #62
Conversation
This PR is not ready. I'll need help to finish this PR |
Alright I think b06ad79 fixes the problem with latitude correction. I'll ask for some user to test it before merging it |
README.rst
Outdated
--overview-resampling [nearest|bilinear|cubic|cubic_spline|lanczos|average|mode|gauss] Resampling algorithm. | ||
--overview-blocksize TEXT Overview's internal tile size (default defined by GDAL_TIFF_OVR_BLOCKSIZE env or 128) | ||
-w, --web-optimized Create COGEO optimized for Web. | ||
--latitude-correction Apply latitude correction to ensure max zoom equality for dataset accross different latitudes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vincentsarago it's a little bit unclear to me which choice latitude correction refers to:
- uncorrection (== global maxzoom regardless of latitude)
- Maxzoom based on actual on-the-ground resolution
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes this was also unclear to me, when passing --latitude-correction
it will adjust the resolution to match the resolution at equator (so global maxzoom regardless of latitude).
I'm not sure what default it should be then, uncorrection
seems to be a better choice but this is a personal opinion
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤔 rather than correct / not correct (which comes with problems of semantic interpretation) what about --latitude-adjustment
where True
indicates that it adjusts for latitude, and False
indicates that it doesn't and simply uses mercator meters.
Or, it could be --mercator-meters|--src-crs-meters
which is clear (use mercator meters for calculation) but possibly obscured to those that aren't familiar with the fact that a mercator meter only ~= an on the ground meter near the equator?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
the --mercator-meters|--src-crs-meters
seems a bit complex even if I understand it better.
I wonder if we could do something like --latitude-adjustment|--global-maxzoom
,
--latitude-adjustement
will be the default, meaning the zoom level will be defined by the dataset mercator resolution at the given latitude (like gdal2tiles does)--global-maxzoom
will apply a correction factor to ensure a max_zoomcorrected
for global dataset
I'm not really happy with this semantic either tbh
tests/test_web.py
Outdated
# High resolution | ||
# Top Left tile | ||
mime_type, tile = cog.get_tile(0, 0, 0) | ||
tile_lenght = 256 * 256 * 3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this a spelling mistake? Should it be tile_length
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes it is, thanks @dwsilk
Alright this is ready. I've updated the phrasing for the @dionhaefner do you want to test this before merging it ? |
Please go ahead, I'd assume that testing this with Terracotta would take quite some time (which I don't have at the moment). |
rio_cogeo/cogeo.py
Outdated
transform=vrt_transform, | ||
width=vrt_width, | ||
height=vrt_height, | ||
resampling=Resampling[overview_resampling], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well in fact this is not ready!
I added a little shortcut for the resampling method but this should be a proper options! (🎉 one more options)
Closes #10
This pr adds a
--web-optimized
options to create COG aligned with Web-Mercator grid.