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

Explore DHI-GRAS/terracotta and bring change over rio-tiler #56

Closed
vincentsarago opened this issue Nov 28, 2018 · 14 comments
Closed

Explore DHI-GRAS/terracotta and bring change over rio-tiler #56

vincentsarago opened this issue Nov 28, 2018 · 14 comments

Comments

@vincentsarago
Copy link
Member

https://github.com/DHI-GRAS/terracotta is a new lib inspired by rio-tiler made by @dionhaefner et all.
This lib has plenty of nice tricks which could be useful in rio-tiler
eg:

@dionhaefner
Copy link

prevent interpolation artefact

Maybe I can give you some background on this one. I found that rio-tiler has some trouble with visible tile edges at high zoom levels if resampling was set to anything else than nearest.

If I understand the problem correctly, this occurs because the VRT ends right at the tile boundary, so the entity reading from it cannot know what lies beyond, and how it should interpolate. Padding the VRT with 2 pixels in each direction fixes the problem (if you're careful with roundoff errors). It has to be two pixels because cubic interpolation has a bandwidth of 2, I guess.

@dionhaefner
Copy link

dionhaefner commented Nov 28, 2018

You might also like this re-implementation of rasterio.warp.calculate_default_transform:

https://github.com/DHI-GRAS/terracotta/blob/7aa0751a105d1a804f0c5389e0abaab4399ffdee/terracotta/drivers/raster_base.py#L302

I found it to perform unconditionally better (i.e., more accurate) when round-tripping reprojections, especially at high latitudes (we had some imagery from northern Greenland). The problem is that GDAL's default transform enforces square pixels in the target CRS, which destroys information if the image gets elongated along one dimension.

@dionhaefner
Copy link

By the way, thank you for building rio-tiler! It was a huge inspiration and I had no idea something like Terracotta could be possible before playing with it.

@vincentsarago
Copy link
Member Author

@dionhaefner

If I understand the problem correctly, this occurs because the VRT ends right at the tile boundary, so the entity reading from it cannot know what lies beyond, and how it should interpolate. Padding the VRT with 2 pixels in each direction fixes the problem (if you're careful with roundoff errors). It has to be two pixels because cubic interpolation has a bandwidth of 2, I guess.

I see, but if I understand this correctly, you'll need to fetch more data than actually needed ? This could be worrying in case when we have a file which is aligned with mercator grid, and it might result in fetching more internal tiles.

@dionhaefner
Copy link

That is correct. I haven't even tried to implement any shortcuts for rasters that are already aligned yet because part of the point of Terracotta is that you don't need to do any fancy preprocessing to your data (but it's definitely on my list, especially with the recent support in rio-cogeo).

I would love to find a more elegant solution for the problem, but I don't see how that would look. The reader has to be able to "look ahead" somehow when using higher-order interpolation if you want accurate results.

On a side note, you can reduce the padding to 1 for linear interpolation and to 0 for nearest. Nearest should be accurate for rasters that are aligned until you're at sub-pixel level, so maybe we could exploit that.

@rowanwins
Copy link
Contributor

Hey @vincentsarago & @dionhaefner

I've just been having a play with the idea of reducing tile edge effects. I basically took the ideas contained within terracotta here and applied them to rio-tiler by introducing an tile_padding parameter.

My work is available in this fork which I'm happy to submit as a pull request if you'd like.

Below you can see some examples of two tiles next to each other rendered rendered with bilinear sampling but with different tile_padding settings.

  • Prior is what rio-tiler currently produces
  • with tile-padding=0 you are seeing my changes, and the images are exactly the same as what you currently get
  • with tile-padding=2 you start seeing the improvements
  • with tile-padding=4 I'm pretty sure you get the same as tile-padding=2 (presumably that's a reflection on how bilinear sampling works...)
    Slice

From a library user perspective adding a flag like tile_padding (which I'm happy to change the name of) at least allows me to specify whether I want to make the tradeoff of retrieving extra data vs the quality of the tile edges that are returned.

Anyway let me know what you think.

Cheers
Rowan

@vincentsarago
Copy link
Member Author

Nice @rowanwins please feel free to submit a PR.
I wonder if we should use tile_padding=2 as default and then overwrite it to 0 if we don't need resampling (if the internal tiles match the requested bounds)

@vincentsarago
Copy link
Member Author

I also wonder if this can fix #95

@rowanwins
Copy link
Contributor

rowanwins commented May 3, 2019

Cool, will submit a pr in the next few days. Let me know if you think off a better name for the parameter.
I also want to bolster the tests a smidge too make sure it's only modifying pixels within a certain range of the tile edge.

The only other thought i had was if there was a python performance library where we could test the impact of these changes... I mostly come from js world where we use a library called benchmark.js to measure the time impact of changes, i presume there is an equivalent in python world...

@vincentsarago
Copy link
Member Author

@rowanwins

Cool, will submit a pr in the next few days. Let me know if you think off a better name for the parameter.

Name is ok I think. Maybe tile_edges_padding is more obvious ?

I also want to bolster the tests a smidge too make sure it's only modifying pixels within a certain range of the tile edge.

👍

The only other thought i had was if there was a python performance library where we could test the impact of these changes... I mostly come from js world where we use a library called benchmark.js to measure the time impact of changes, i presume there is an equivalent in python world...

Well I don't think it will impact the performance much, will just use a bit more data thus memory but nothing too impactful IMO.

@DanSchoppe
Copy link
Contributor

I'm excited to hear about this development. The padded tiles look amazing. Thanks @rowanwins!

@vincentsarago I'm curious if this feature is antithetical to the --web optimized COGs from rio-cogeo. Just curious how requests for padded 260x260 tiles will work when the COG is optimized for 256x256 boundaries. If this is a reasonable concern, obviously this conversation would be best moved to the rio-cogeo repo.

@vincentsarago
Copy link
Member Author

I'm curious if this feature is antithetical to the --web optimized COGs from rio-cogeo. Just curious how requests for padded 260x260 tiles will work when the COG is optimized for 256x256 boundaries. If this is a reasonable concern, obviously this conversation would be best moved to the rio-cogeo repo.

Yeah basically this is my only concern, if the tile bounds fits the internal tile (no need of resampling) we should overwrite tile_padding to 0. But right now I don't know how to handle that

@DanSchoppe
Copy link
Contributor

Would it be reasonable to add configuration to cogeo --web for padding? This would allow a user to generate web+padding optimized COGs when they anticipate it will be accessed from rio-tiler with tile_edges_padding enabled.

@vincentsarago
Copy link
Member Author

@DanSchoppe my first though is that we don't want tiles that are not a 64px multiple (not sure if it's possible anyway) and IMO we should always consider rio-cogeo and rio-tiler as separate entity (so don't change things in it to accommodate the other).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants