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

Add bilinear resample method & supporting TypeScript types (plus bugfixes) #34

Merged
merged 3 commits into from
Jul 17, 2024

Conversation

TheMrCam
Copy link
Contributor

@TheMrCam TheMrCam commented Jul 16, 2024

Overview

For accurately displaying elevation model rasters (DEM), we'd prefer bilinear resampling over the current nearest neighbor implementation. This PR adds the option to use bilinear resampling with the resampleMethod: 'bilinear' constructor option.

To help own understanding of what was going on, I removed a lot of the TypeScript ambiguity to the resample and reprojection areas of the code. I removed (most) casts to any, mostly with geotiff.js's TypedArray and variations. I also fixed a few bugs while I was investigating the codebase.

Primary changes

  • New resampleBilinear method, based on the geotiff.js implementation
  • Added resampleMethod to TIFFImageryProviderOptions
  • Improved TypeScript types & annotations
  • Uses geotiff.js's copyNewSize utility to maintain TypedArray type throughout transformations

Bugfixes

  • Swapped sourceWidth & targetWidth in reprojection()
    • My colleague pointed out they seemed incorrectly flipped, but I noticed the values are typically the same anyway, so I'm not sure the significance of this bug in the implementation.
  • stringColorToRgba() used green for blue, potentially missing a whole band of data.
  • No more any (in the code I was looking at)
  • Fixed typo of ResampleDataOptions

TheMrCam added 2 commits July 12, 2024 08:07
Replaced the built-in resample method with a copy of geotiff.js's
bilinear resample function, to see if it would improve accuracy.

It does not.

Also added TypeScript annotations to requestImage and supporting code so
it'd be easier to understand what was going on.
Added optional resampleMethod prop to constructor options, to choose
between existing resampleData function and the new geotiff.js based
bilinear resampling.

Added BBox type to reduce duplicated typings of [num,num,num,num].

Also found and fixed a few bugs, namely:
- Missing blue in stringColorToRgba()
- Filled in nodata value in reprojection()
- Swapped sourceWidth & targetWidth in reprojection()
  - In the last commit, but I forgot to mention it
Copy link

vercel bot commented Jul 16, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
tiff-imagery-provider ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 16, 2024 9:26pm

@hongfaqiu hongfaqiu merged commit 082d6ae into hongfaqiu:main Jul 17, 2024
2 checks passed
@hongfaqiu
Copy link
Owner

@TheMrCam What a great update! Thank you for your contribution!
I've reviewed your code, and it all looks correct, including the fixes for a few bugs.
The addition of linear interpolation is fantastic!

@hongfaqiu
Copy link
Owner

Dear @TheMrCam

I've added a buffer parameter for the bilinear interpolation resampling method, which can ensure continuous changes at the tile seams. However, the buffer parameter has not yet been adjusted in the reprojection method, and the rendering performance doesn't seem to have changed much. I'll need to address this problem when I have some time.

Additionally, I've moved the resampleMethod parameter into the renderOptions.

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.

2 participants