-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Round pixel snapping to actual next pixel #7465
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
Conversation
I don't understand why there's a + 1 here, but it rounds up to the next-next pixel instead of the next pixel. This makes images extend past the end of the frame. Fixes matplotlib#6773. Fixes matplotlib#7350.
The pixel snapping in these images moved them outside the Axes frame, or in such a way that doesn't quite align with the ticks.
I can't say for certain, but they don't look any worse. Probably the last row/column was overlapping the Axes frame and was invisible before now.
bb05c12
to
c8191a8
Compare
c8191a8
to
d0a40c9
Compare
Since this includes specgram tests that @tacaswell simplified greatly, ping @matthew-brett in case there are any issues with them on 32-bit systems. |
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.
Looking at the changes to the images, I certainly think the new ones look aligned/centered better than before.
The old code taking the ceiling then adding 1 also certainly seems suspect, so I'm good with this.
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 revised version looks correct to me.
This is hopefully a simpler alternative to #6776 (@mdboom). I don't fully understand why the rounding-to-next-pixel uses
int(ceil(width + 1))
; switching toint(ceil(width))
works just as well, though unlike #6776 (which only special-cases non-standard Axes), it does have an effect on our test images. An unfortunately large number of our test images are changed, though several of them for the better.I split up the test images updates into definitely-good (images were originally outside the Axes by one pixel), probably-good (images were probably on the Axes frame, but aren't any longer) and I-have-no-idea (basically all the mlab plots, which do something really strange.)