• The filtering weight at one extreme is not being calculated correctly. In the case of linear interpolation, this makes the one weight that should be zero be instead nonzero. This then results in a pixels of constant values manifesting a small blip after linear interpolation if the center of an input pixel exactly coincides with the center of a output pixel (within the agg backend, its pixels may not be exactly aligned with the true pixels).

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?

This killed the log-scale images?

Indeed, I need to track down why.

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?

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.

I fixed the issue withlog_scale_image, so this PR is now ready for review. The final count is 65 baseline images that need to be updated. Maybe a third of those images have (subtle) changes in the rendering of content, e.g.:

Before this PR:

After this PR:

Difference:

On the other hand, most of the updated images have solely very slight changes in the appearance of text. As@jklymak suggested, it might make more sense in those cases to simply increase the tolerance instead for the comparison instead of updating the baseline image, so let me know if I should do that instead.

Are you sure you have the right set of images?lib/matplotlib/tests/baseline_images/test_axes/formatter_ticker_004.png for example doesn't have any images to be resampled.

OK, I see thatRendererAgg::draw_text_image usesimage_filter_spline36 when rotating text, so it only affects tests with rotated text. So for any tests that are purely rotated text, we should just increase the tolerance and make a note of them, then revert the tolerance changes in#30161 when regenerating the images. Or re-target this PR to thetext-overhaul branch without test image changes (but then they'd all be regenerated much later.)

Since you're deep into this code right now, I wonder if you have any ideas about#14143?

Since you're deep into this code right now, I wonder if you have any ideas about#14143?

Thanks for pointing out that issue! (and#1441 before it) It's all tied to the subpixel architecture of Agg, so some level of discrepancy is unavoidable given that architecture. I'll look into whether there's any bug that's making the discrepancies larger than they need to be.

(by using the algorithm I mentioned above, making sure to always update both sides, and then nudging the center filter value up by one or down by one if needed)

I think you missed the fact that it's not the whole weight array that is normalized, but rather each set of corresponding subpixels. That is, there are 256 normalization constraints.

For example, a weight arrayw for a filter radius of 2 has 2 * 2 * 256 = 1024 elements, and the pivot is at index 511. For symmetry,w[510] should equalw[512]. However, those two weights are part of two separate normalization constraints:w[254] + w[510] + w[766] + w[1022] = 16384 andw[0] + w[256] + w[512] + w[768] = 16384. So, requiring symmetry after nudgingw[510] up to fix one normalization would mean also nudging upw[512], which could potentially mean having to nudge down a third weight (w[0],w[256], orw[768]). Due to symmetry, that would mean nudging down a fourth weight, which then couples to a third normalization constraint, and so on.

The system of equations is very annoying, and there may not be an efficient way to get to an acceptable solution even when one exists. Also, there'd be some question of whether negative weight values are allowed if the original weight array does not have any.

Given the above, I feel that it is justifiable for matplotlib to avoid this headache by accepting a tiny bit of asymmetry in the weight array.

I've done a deep dive into the subpixel architecture of Agg, and it's a bit of an inconsistent mess. There are 256 subpixels per pixel. Most of the Agg code treats the subpixels as if they are "center-aligned" (by this, I mean that the center of subpixel 128 is coincident with the center of the original pixel), but some of the Agg code – specific to affine transformations – treats the subpixels as if they are "edge-aligned" (by this, I mean that left edge of subpixel 0 is coincident with the left edge of the original pixel).

• For nearest-neighbor interpolation, one wants edge-aligned subpixels. That way, the edges as defined by subpixels looks exactly the same as the edges defined by the original pixels.
• For all other types of interpolation, one typically wants center-aligned subpixels. That way, there are interpolation points at the center of original pixels and at the edges between pixels.

For nonaffine transformations, matplotlib provides a distortion table to Agg, . So,72d759a now aligns the subpixels of the distortion table as appropriate for the interpolation type. With this change, the nonaffine analogues to#1441 and#14143 (which use nearest-neighbor interpolation) look correct, while maintaining the better behavior earlier in this PR for other types of interpolation.

For affine transformations, the mix of center-aligned code and edge-aligned code in Agg can be fixed, but it requires a bunch of modifications and API changes. Even with those changes, plots like#1441 and#14143 still do not come out correct because of the rounding-to-subpixel code that I described above (#30184 (comment)). That is, there will always be the potential for misalignments up to a subpixel (1/256 of a pixel) with affine transformations.

So, rather than making all of those changes to Agg, I decided that the more sensible approach for affine transformations is to first check how much the transformation scales up the image (46e0171) prior to using Agg. If the scale factor is at least 256 / 2 = 128, then there can be subpixel-related artifacts in the output image. In that case, this PR redirects the transformation down the code path for nonaffine transformations and passes it to Agg as such. With this change,#1441 and#14143 are now fixed:

I'm finally getting back to this PR. This PR had broken two tests related toSegmentedBivarColormap becauseSegmentedBivarColormap calls the Agg-based resampler to generate its lookup table (LUT), and of course the resampler now gives slightly different results. In my opinion,SegmentedBivarColormap should not be calling the Agg-based resampler because the Agg's internal use of subpixels means that LUT values are not quite correct (again, in my opinion). That said, I think that any change to the LUT calculation methodology is out of the scope of this PR. Instead, I have simply added a subpixel shift to the LUT calculation so that theSegmentedBivarColormap LUT is the same before and after this PR.

Let me know if there's anything I can do to facilitate reviews in order to get this PR merged in.

I likely won't be able to review properly the changes since Oct. 8 before the holiday break. Sorry about that, this looks like great work.

Thanks for reviewing this!

We should merge#30824 first because they both update the bivar cmap image.

I agree that#30824 should be merged first, and then71c7f4e in this PR will be removed as no longer applicable (becauseSegmentedBivarColormap no longer calls the Agg resampler)