-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
ENH: anti-alias down-sampled images #13724
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
ENH: anti-alias down-sampled images #13724
Conversation
Also please note that of course I don't think people should save at 30dpi. The point here is that even at 300 dpi, if someone tries to imshow 10k x 10k matrix they will get aliasing and we really should anti-alias by default unless they explicitly tell us not to ( |
lib/matplotlib/image.py
Outdated
if the image will down-sample the data (i.e. if the there is | ||
more data than pixels). If *True*, and the data is down-sampled, | ||
any user supplied filters are ignored, and a Lanczos filter is | ||
applied with radius `2 * M_data / M_image`. |
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.
2 or 3? (it's 3 below...)
That is really nice. |
The agg filters are written in c and as far as I can tell are just simple convolutions. So it’s not clear if a different shaped filter would be more efficient. 2d convolutions are slow. We could try w scipy.conv2d which should also be pretty optimized. I think the algorithm is NxMxrxr where r is the filter radius. |
lib/matplotlib/axes/_axes.py
Outdated
@@ -5435,7 +5435,8 @@ def get_interp_point(ind): | |||
def imshow(self, X, cmap=None, norm=None, aspect=None, | |||
interpolation=None, alpha=None, vmin=None, vmax=None, | |||
origin=None, extent=None, shape=None, filternorm=1, | |||
filterrad=4.0, imlim=None, resample=None, url=None, **kwargs): | |||
filterrad=4.0, imlim=None, resample=None, | |||
antialiased=True, url=None, **kwargs): |
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 needs to go at the end (or be kwarg only) to not change the API.
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.
ooops. Also, I should have made a tentative PR....
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.
Fixed...
My guess from some quick playing around w/ |
lib/matplotlib/image.py
Outdated
@@ -422,6 +441,10 @@ def _make_image(self, A, in_bbox, out_bbox, clip_bbox, magnification=1.0, | |||
A_scaled += 0.1 | |||
A_resampled = np.zeros((out_height, out_width), | |||
dtype=A_scaled.dtype) | |||
if self.get_antialiased() and A.shape[0] > out_height: | |||
self.set_interpolation('lanczos') |
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.
can we use a simpler filter?
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.
I don't think lanczos is very complicated. https://en.wikipedia.org/wiki/Lanczos_resampling
The other filters (i.e. Gaussian) don't have a radius associated with them, for some reason I can't fathom, so I didn't really play with them, but will do...
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.
Simpler filter seems to work great. See update above:
I am not sure it is actually implemented as a convolution, I think it is
where the |
@anntzer this doesn't do what you are after, which would be to essentially add more anti-aliasing filters to the agg code. I don't think that would be outrageously hard, but this PR is much more modest in what it is trying to do... |
a2a3db2
to
af9b9a7
Compare
Can you also add the timings with antialiasing disabled? |
@anntzer updated above: filt nearest dpi 210 time 0.241 |
lib/matplotlib/image.py
Outdated
def get_antialiased(self): | ||
""" | ||
Get whether down-sampled images are low-pass filtered before | ||
down-sampling (i.e. when there is more data than pixels). |
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.
docstring needs update
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.
Yes, I've not done anything to properly document this yet
So less than 2x slowdown. Given the "esthetic" quality improvement, this is really good. |
This PR proposes to make it on by default. |
I'm happy with that, but I guess some will complain :) |
Maybe, but I don't think they should - the [sampling theorem]((https://en.wikipedia.org/wiki/Nyquist–Shannon_sampling_theorem) is pretty inescapable. I don't think we should show aliased images to our users unless they specifically ask for them. I actually think we should just leave Gaussian on by default: even when upsampling it helps. This image is upsampled by 2.05: |
OK, the problem with all this is that manually zooming doesn't re-call |
When upsampling by a large amount, quite often (for some definition of "quite often") one does (I do, at least) expect to see sharp pixel boundaries and the blurring out by the filter is just annoying. Obviously there's no sharp transition between "we need an antialiasing filter" and "we don't need one", but we need to set it somewhere (if we agree we don't want it when the pixels are large) and putting it where the sampling theorem says to put it sounds like a reasonable default. |
For sure when zooming you want the blur to be un-noticable. OTOH, we use the transform stack to keep track of what part of the image will be drawn makes it challenging to see how zoomed in we are, so its hard to know how many data pixels are geing mapped to how many canvas pixels a-priori... (well or my ignorance of how to get such things out of an arbitrary transform needs to be overcome). |
New version works w/ zooming. I found it easiest to drag-resize this code to see the changes... import matplotlib.pyplot as plt
import numpy as np
import scipy.signal as signal
import timeit
a = plt.imread('rings_sm_orig.gif')
print(np.shape(a))
for dpi in [200]:
fig, axs = plt.subplots(1, 2, figsize=(4.1, 2.1), num=1, sharex=True, sharey=True)
ax = axs[0]
ax.imshow(a, cmap='gray', antialiased=True)
ax.set_position([0, 0, 0.5, 1])
if 1:
ax = axs[1]
ax.imshow(a, cmap='gray', antialiased=False)
ax.set_position([0.5, 0, 0.5, 1])
ax.axis('off')
ax.text(100, 190, dpi, fontsize=40)
plt.show() |
Before working on tests and docs I'd like some consensus that this is the way to go.... |
I'm definitely +1 on providing this functionality, undecided but leaning towards +1 on having this on by default. |
981818c
to
04fad9d
Compare
lib/matplotlib/image.py
Outdated
# compare the number of displayed pixels of the image to the number of | ||
# the data pixels. | ||
disppixels = int(transform.transform([data.shape[1], 0])[0]) | ||
if (disppixels < 3 * data.shape[1] and disppixels != data.shape[1]): |
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.
compare in both x and y and antialias if at least one of them is below nyquist?
perhaps also disable antialiasing if exactly 1x or 2x zoom? (in which case it's more likely the user exactly adjusted the values for pixel-precise resampling...)
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 filters are isotropic, so I don't know what we should do if the up/down sampling isn't isotropic.
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.
then at least the condition should either be "if both are <3x" or "if one is <3x", not "if the x-axis is <3x".
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.
But thats what I mean - the x re-sample rate should be the same as the y, unless someone is using a funky transform?
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.
like, its just extra code, so happy to include it if you like...
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.
One could just be doing imshow([[0, 1], [2, 3]], aspect="auto")
for example.
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.
OK, did the y case as well, and added the check for 2x upsample
lib/matplotlib/image.py
Outdated
@@ -354,6 +372,20 @@ def _make_image(self, A, in_bbox, out_bbox, clip_bbox, magnification=1.0, | |||
out_height = int(out_height_base) | |||
out_shape = (out_height, out_width) | |||
|
|||
# decide if we are going to do anti-aliasing or not on the image |
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.
do you also need to do the check here? isn't it redundant with the one in _resample above?
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.
No, didn't catch this. You refactored this, so I now have that logic where you refactored.
762dc0b
to
2d1c785
Compare
lib/matplotlib/tests/test_axes.py
Outdated
@@ -3764,21 +3764,24 @@ def test_specgram_freqs(): | |||
ax23 = fig2.add_subplot(3, 1, 3) | |||
|
|||
ax11.specgram(y, NFFT=NFFT, Fs=Fs, noverlap=noverlap, | |||
pad_to=pad_to, sides='default') |
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.
just set rcParams["image.interpolation"] = "nearest" above in the function and be done once for all? we reset the rcparams at the end of each test anyways.
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.
Ooops, that would have saved some typing. OTOH, I've already done it, can't hurt to leave it in explicitly..
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 adds a bit of noise to the code :(
You could just do git checkout HEAD~ lib/matplotlib/tests/test_axes.py
to undo these changes, but it's up to you.
2d1c785
to
5924668
Compare
65ae08a
to
08a98be
Compare
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 is great :)
I actually do not understand the test failure. Its saying "antialiased" is not a valid value for "interpolation", but all the other tests work fine. And I can't reproduce locally, ie. the failing test runs fine locally. |
ae46fb3
to
37410a6
Compare
------------------------------------------------------------- | ||
|
||
Images displayed in Matplotlib previously used nearest-neighbor | ||
interpolation, leading to aliasing effects for non-integer upscaling, and |
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.
Do you mean "downsampling" here?
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.
Changing to
Images displayed in Matplotlib previously used nearest-neighbor
interpolation, leading to aliasing effects for downscaling and non-integer
upscaling.
37410a6
to
d7fa884
Compare
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 is great. Just some optional suggestions for the descriptions in the example.
@@ -915,6 +915,7 @@ def test_nonfinite_limits(): | |||
|
|||
@image_comparison(['imshow', 'imshow'], remove_text=True, style='mpl20') | |||
def test_imshow(): | |||
matplotlib.rcParams['image.interpolation'] = 'nearest' |
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 would make sense to add a comment
# use former defaults to match existing baseline image
on every rcParams use that is just there to prevent the creation of new baseline images.
If we have to regenerate the baseline images all at some point anyway, we can easily search for that comment and remove the artificial non-default.
image file. When data that makes up the image has a different resolution | ||
than its representation on the screen we will see aliasing effects. | ||
|
||
By default Matplotlib has a mild anti-aliasing filter turned on for images |
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.
Maybe:
The default image interpolation in Matplotlib is 'antialiased'. This uses a hanning interpolation for reduced aliasing in most situations. Only when there is upsampling by a factor of 2 or >=3 a simple nearest neighbor interpolation is used.
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.
Changed to
The default image interpolation in Matplotlib is 'antialiased'. This uses a
hanning interpolation for reduced aliasing in most situations. Only when there
is upsampling by a factor of 1, 2 or >=3 is 'nearest' neighbor interpolation
used.
Note that factor of 1 sampling is also special cased (not strictly upsampling, but...)
representation has fewer pixels than the data) or mild up-sampling, to a | ||
limit of three-times upsampling. | ||
|
||
If a different anti-aliasing filter is desired (or no filter), it can be |
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.
Simpler:
Other anti-aliasing filters can be specified ...
|
||
|
||
############################################################################### | ||
# Now plot. Note that these images are subsampled, with the images |
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 images are subsampled from 1000 data pixels to 604 rendered pixels.
plt.show() | ||
|
||
############################################################################### | ||
# Note that even up-sampling an image will lead to Moire patterns unless |
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.
# Note that even up-sampling an image will lead to Moire patterns unless | |
# Even up-sampling an image will lead to Moire patterns unless |
# The Moire patterns in the "nearest" interpolation are caused by the | ||
# high-frequency data being subsampled. The "antialiased" image | ||
# still has some Moire patterns as well, but they are greatly reduced. | ||
# We can reduce them more, as in the examples below, but better filters |
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 'better filter + compultationally expensive' information should be moved to the section where it's actually rendered. I'd remove this sentence here completely (If you really want something here, it would just be "See below for even better filters.").
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.
Thanks @timhoffm made the suggested changes as noted above.
d7fa884
to
7e0efe5
Compare
PR Summary
Closes #5602
See new example documenting the change: https://23268-1385122-gh.circle-artifacts.com/0/home/circleci/project/doc/build/html/gallery/images_contours_and_fields/image_antialiasing.html
UPDATED: 16 April 2019
The API is now
with
'antialiased'
as the default, and'nearest'
giving the old behaviour. This API was discussed on the early-April development call, and strongly suggested by @efiring .Q: a) should this be the default? and b) should the 12 or so failing tests be changed to non-default so the images don't have to change?
OLD: UPDATED:
Below is the logic for turning on hanning-window smoothing:
anti-alias
No anti-aliasing:
Hanning:
Gaussian
Lanczos
Sinc
Importance of anti-aliasing even if not "downsampling":
Note that we really need the anti-aliasing filter unless we upsample by more than a factor of two, so this PR proposes doing that: