Fixed lingering bugs with image rendering related to exact half display pixels#31313
Fixed lingering bugs with image rendering related to exact half display pixels#31313QuLogic merged 6 commits intomatplotlib:mainfrom
Conversation
92de70f to
51745bf
Compare
|
This PR is now ready for review! I'm annoyed that I didn't catch these subtleties as part of #31021, but fixing them changes only four existing baseline images – only one of those in a meaningful way – because the bugged criteria are rather specific (but, at the same time, not overly unusual to accidentally stumble across). A couple of comments:
|
| # All values in this test are chosen carefully so that many display pixels are | ||
| # aligned with an edge or a corner of an input pixel |
There was a problem hiding this comment.
I'm not sure how exactly (maybe checking ax.get_window_extent()?), but if this test is very dependent on specific locations of Axes, then it might be a good idea to assert that they are in the places you expect, in case layout somehow changes in the future?
Or, if possible, maybe switch to figure-level artists, to remove some level of indirection (but I'm not sure if those exercise what you want)?
There was a problem hiding this comment.
Rather than testing specific values, I added asserts for the precise height/width for each axes and the precise anchoring (whether on whole pixels or half pixels), which are the critical requirements. Translations, if they were to happen for some reason, would be "okay" as far as this test is concerned.
Maybe you can mention that in the commit message? |
tacaswell
left a comment
There was a problem hiding this comment.
Adding asserts that the axes a really where they should be via get_window_extent is good but I would be happy to merge without it.
The interpolator was previously written to minimize bias with each step, effectively equivalent to rounding half even. However, rendering alignment requires there to be bias equivalent to rounding half up.
The Agg renderer internally has the vertical axis flipped, so the rounding of the bounds in the vertical direction needs to be round half down instead of round half up.
The code previously performed the flipping in output space, but the flipping needs to be in input space because that is where the rounding to nearest input pixel takes place. Also, flipping in the vertical direction already happens if origin='lower', so flip vertically only if origin='upper'.
There are multiple instances where a value that would ideally be exactly at a half display pixel might instead be one floating-point tick away in the wrong direction. Here fudge amounts are added so that the rounding occurs in the desired direction. An analogous situation occurs for truncation to a whole pixel. The amount of 1e-8 is chosen because it covers 1e-13 relative error for 100,000 pixels.
Now included |
PR summary
This is a follow-on to #31021 to fix image-rendering bugs when a display pixel is exactly aligned with the edge between two image pixels (or with the edge of the image). An attempt at a fix was part of #31021, but the tests there had not been carefully crafted to verify that the bugs were comprehensively fixed.
Before this PR
After this PR
Generating code
AI Disclosure
N/A
PR checklist