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

Unnecessary Resampling leading to poor pixel positioning #30

Open
staffordsmith83 opened this issue Jun 27, 2024 · 9 comments
Open

Unnecessary Resampling leading to poor pixel positioning #30

staffordsmith83 opened this issue Jun 27, 2024 · 9 comments

Comments

@staffordsmith83
Copy link

staffordsmith83 commented Jun 27, 2024

Dear Hongfaqiou,

there seems to be an issue with how TIFFImageryProvider maps the tiff values to the canvas. This occurs even when re-projection is not needed, e.g. for cogs in EPSG:4326 and EPSG:3857.

Using this Climate Data cog as an example (it is in EPSG:4326): https://nex-gddp-cmip6-cog.s3-us-west-2.amazonaws.com/daily/GFDL-CM4/ssp585/r1i1p1f1/pr/pr_day_GFDL-CM4_ssp585_r1i1p1f1_gr1_2015_01_25.tif

The data should look like this:
image

But TIFFImageryProvider renders it like this:
image

Note the following:

  • The pixels are no longer square and appear to be resampled
  • It does not retain the data integrity - we need users to be able to see the value of this scientific data at a single point on the globe.

Note in a more zoomed in example, how the pixels are clearly not square. This is TIFFImageryProvider in TerriaJS:
image

This is TIFFImageryProvider in your online demo:
image

Whereas the tru data is square pixels (shown in QGIS):
image

Please note, to be able to see what is happening at the pixel level we have changed the Cesium settings for _defaultMagnificationFilter and _defaultMinificationFilter to remove the blurring:
https://github.com/TerriaJS/terriajs/blob/13fa4cfaabecdc82dff749836a1dcef602b0a1ae/lib/Models/Catalog/CatalogItems/CogCatalogItem.ts#L140-L141

If you like, we have put more details on the perceived error in out PR here: TerriaJS/terriajs#7209

@hongfaqiu
Copy link
Owner

hongfaqiu commented Jun 27, 2024

@staffordsmith83 I'm glad to see you continue to support TIFF in Terriajs. This issue is the same as #18 #22 , and I've been quite busy lately. I'll try to fix this issue next week, which seems to be related to resampling

@staffordsmith83
Copy link
Author

staffordsmith83 commented Jun 27, 2024

Thats great, thanks @hongfaqiu, @nf-s watch this ticket

@hongfaqiu
Copy link
Owner

@staffordsmith83 I spent a day solving this problem, and now:

  1. Scaling no longer causes distortion
  2. The values are accurately represented from the original TIFF

I will submit the improved code in the next two days.
tiffImageryProvider

@staffordsmith83
Copy link
Author

That's great news thanks for your hard work on this, I'll let the Terria team know!

hongfaqiu added a commit that referenced this issue Jul 9, 2024
@hongfaqiu
Copy link
Owner

@staffordsmith83
I have released the corrected version v2.11.0.
The reason for this issue is as follows:

  1. For tiles exceeding the maximum level of the COG (Cloud Optimized GeoTIFF), the accuracy is lost when calculating the sampling window using latitude and longitude.
  2. The default resampling method of GeoTIFF changes the width and height structure of the acquired TIFF grid.
  3. When rendering the data resampled with a small window using GeoTIFF, the grid boundaries of the original TIFF cannot be aligned.

The implementation approach is as follows:

  1. No longer set the maximum level to maximumLevel by default for COG.
  2. Use the tiles of the last level of the COG at higher levels.
  3. Calculate the window size of the current tile on the clipped tiles.
  4. Resample to the target resolution.

The current issues are:

  1. Although WebWorker is used to accelerate, the speed seems to be average.
  2. When fetching TIFF data at higher levels, there are cases of repeated calculations, especially poor performance when using reprojection.

The optimization plan is:

  1. Use WebGL for resampling, pass the original data as a texture, and resample to the target width and height.
@hongfaqiu
Copy link
Owner

hongfaqiu commented Jul 17, 2024

@staffordsmith83 @nf-s, a huge thanks to the wonderful new contributor @TheMrCam for offering the resampling method for bilinear.

Thank you so much for your hard work, and the functionality of this library has become even more splendid. How delightful it is!

But there are still some problems (unusual)

  • I found in some tiffs that when zooming to a larger level, the position of the grid is shifted overall.
    resample2
  • Bilinear interpolation produces unnecessary segmentation rather than sharpening when scaling to larger levels
    resample

The test tiff is here #33 (comment)

@hongfaqiu
Copy link
Owner

@staffordsmith83 I have released the corrected version v2.11.0. The reason for this issue is as follows:

  1. For tiles exceeding the maximum level of the COG (Cloud Optimized GeoTIFF), the accuracy is lost when calculating the sampling window using latitude and longitude.
  2. The default resampling method of GeoTIFF changes the width and height structure of the acquired TIFF grid.
  3. When rendering the data resampled with a small window using GeoTIFF, the grid boundaries of the original TIFF cannot be aligned.

The implementation approach is as follows:

  1. No longer set the maximum level to maximumLevel by default for COG.
  2. Use the tiles of the last level of the COG at higher levels.
  3. Calculate the window size of the current tile on the clipped tiles.
  4. Resample to the target resolution.

The current issues are:

  1. Although WebWorker is used to accelerate, the speed seems to be average.
  2. When fetching TIFF data at higher levels, there are cases of repeated calculations, especially poor performance when using reprojection.

The optimization plan is:

  1. Use WebGL for resampling, pass the original data as a texture, and resample to the target width and height.

GPU based resampling supported, please check the latest version @staffordsmith83

@staffordsmith83
Copy link
Author

Thanks @hongfaqiu this is great work! Just to let you know, the latest terria release incorporates your TIFFImageryProvider, we had to fork the code to make some modifications to suit our custom fork of cesium, but it is essentially your code that powers our COG support. Ive now left the project, but want to thank you very much for your contribution! I will alert the existing team to this new version

@hongfaqiu
Copy link
Owner

@staffordsmith83 I have noticed that terriajs seems to have opened a rather different branch. The main branch fixed some obvious bugs some time ago. Therefore, I would like to remind you.

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