Skip to content

Comments

Warper: Fixes related to mode resampling#11661

Merged
rouault merged 2 commits intoOSGeo:masterfrom
dbaston:warp-mode-fixes
Jan 16, 2025
Merged

Warper: Fixes related to mode resampling#11661
rouault merged 2 commits intoOSGeo:masterfrom
dbaston:warp-mode-fixes

Conversation

@dbaston
Copy link
Member

@dbaston dbaston commented Jan 15, 2025

What does this PR do?

On top of #11649, renames several variables as suggested in #11649 (comment)

While editing the code, I noticed that

  • unlike "sum" and "average", mode resampling was not considering partial pixel coverage
  • a logic error prevented the value of -1 from being selected as the mode

This PR addresses both of these issues.

psWO->hSrcDS = hWrkSrcDS;
psWO->hDstDS = hDstDS;

if (const char *pszTieStrategy =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this should be done in GDALWarpOperation::Initialize() and GDALWarpOperation::ValidateOptions(). Otherwise, as it, I don't think that doing "gdalwarp -r mode -wo MODE_TILES=xxxx -of VRT in.tif tmp.vrt" + "gdal_translate tmp.vrt out.tif" is going to work properly

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, it does not work properly. Fixed according to your suggestion

@coveralls
Copy link
Collaborator

Coverage Status

coverage: 70.062% (+0.009%) from 70.053%
when pulling bf4f0f2 on dbaston:warp-mode-fixes
into 4f6fde6 on OSGeo:master.

@rouault rouault merged commit b68bf74 into OSGeo:master Jan 16, 2025
38 checks passed
@rouault rouault added this to the 3.11.0 milestone Jan 16, 2025
@schwehr
Copy link
Member

schwehr commented Jan 16, 2025

rasterio/rasterio#3293 to handle a new value for mode in test_reproject_resampling

@dbaston dbaston added the bug label Oct 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants