Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Fixed several accuracy bugs with image resampling#30184
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Conversation
} | ||
unsigned end = (diameter() << image_subpixel_shift) - 1; | ||
m_weight_array[0] = m_weight_array[end]; |
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.
This line is one of the bugs.0
andend
are different distances frompivot
, so the corresponding weights typically should not be the same.
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.
Please put the new version under a macro guard, as was done in#28122, so that we keep a track of what's the original Agg and what we changed on top of it.
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.
Makes sense, done
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.
It turns out that I can't add a preprocessor definition in_image_resample.h
a la#28122. There is buggy code inagg_image_filters.cpp
that needs to be skipped, but since it is a CPP file rather than a H file, it gets compiled separately with no awareness of_image_resample.h
. I have put the preprocessor definition (MPL_FIX_IMAGE_FILTER_LUT_BUGS
) here at the top ofagg_image_filters.h
, which is lower level than would be nice, but at least still makes it clear what is custom code.
ayshih commentedJun 18, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
You can see such a blip in the example above, but it's easy to overlook. Here's a starker example: an array of two pixels with the same value (0.1) is resampled to an array of ten pixels. Before this PR, two of the pixels in the output array have values slightly greater than 0.1. >>>importnumpyasnp>>>frommatplotlib.transformsimportAffine2D>>>frommatplotlib.imageimportresample,BILINEAR>>>>>>in_data=np.array([[0.1,0.1]])>>>in_shape=in_data.shape>>>>>>out_shape= (1,10)>>>>>># Create a simple affine transform for scaling the input array>>>affine=Affine2D().scale(sx=out_shape[1]/in_shape[1],sy=1)>>>>>>affine_data=np.empty(out_shape)>>>resample(in_data,affine_data,affine,interpolation=BILINEAR)>>>>>>print(affine_data)[[0.10.100012210.10.10.10.10.100012210.10.10.1 ]] After this PR, the output is the expected: >>>print(affine_data)[[0.10.10.10.10.10.10.10.10.10.1]] |
This killed the log-scale images? Also this changed abunch of image tests, but I can't see any difference by eye. Is it worth just relaxing the tolerance on those tests a bit and at the same time adding new tests for these fixes? |
Indeed, I need to track down why.
Yes, the other 17 image tests are mostly subtle differences, but merely relaxing the tolerance doesn't make sense to me. The updated baseline images do represent what the output should be. |
Fair enough - however, it would be good if there were some tests devised where it showed a visual effect of these inaccuracies. |
e198af6
to717a221
Compare@@ -88,6 +88,7 @@ namespace agg | |||
} | |||
} | |||
#ifndef MPL_FIX_IMAGE_FILTER_LUT_BUGS |
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 following code block is removed because it not only has the same bugs as the code inagg_image_filters.h
, but also it shouldn't even exist because it modifies the weight array after it has been normalized, thus potentially destroying the normalization.
Uh oh!
There was an error while loading.Please reload this page.
PR summary
This PR fixes several accuracy bugs with image resampling:
Below are illustrative plots and the generating script for linear resampling of a 2-pixel array. There are fundamental limitations of the accuracy due to the subpixel approach of the agg backend, but the motivation here is for the average behavior to be accurate (i.e., eliminate bias).
Before this PR
After this PR
Script
PR checklist