Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Rewrite of image infrastructure#5718
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
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Away from my computer at the moment. Will pushing all of this stuff down
|
Our image interpolation is already done using the The change here (and I admit it's confusing and I wasn't very clear) is that the |
Ah, that is much clearer! Sometimes I forget the distinction between the
|
f677ac8 to615c0dcCompareThis is now passing on Travis, has some "What's New" entries, and is ready for final review. @tacaswell: Thanks to the pointers to related issues. That was helpful for verifying some things outside of the existing test suite. |
I'll give the C++ code a thorough read through tomorrow.
|
master...matplotlib:6659dd704336b7575d8234b83a789531abd721ff are links to the diffs excluding the svg files |
examples/api/demo_affine_image.py picked up the execute bit for all, was that intentional? |
in backend_svg it looks like the alpha value is only applied if the transform is not |
docstring of class ScalarMappable.to_rgba needs some updating. |
L1251 is |
Would it be better to use |
I have a some what pathological example where colors not from the color map end up in the final image. importnumpyasnpimportmatplotlib.pyplotaspltX,Y=np.ogrid[-1000:1000,-1000:100000]R=np.sin((X**2+Y**2)/1000)fig,ax=plt.subplots()ax.imshow(R,cmap='viridis',interpolation='nearest')ax.set_aspect('auto')ax.set_xlim((28860.457192461989,31279.898576044176))ax.set_ylim((1044.2529963498005,906.46842593439703))ax.set_xscale('log')plt.show()
To be fair, this is a couple layers of stacked Moire patterns as out here the grid isway under sampling the underlying function to begin with and then it is being way down sampled. I also found that the initial draw is faster under the new code, but panning is snappier under the old code. |
Nope. Will fix. My emacs is set up to helpfully apply the executable bit if there's is
Yes. If there is a transform passed, it gets exactly the original data, so the alpha needs to be applied by the renderer. If there is no transform passed, the alpha has already been applied by
done.
Not sure I follow. Isn't that already the case? Compositing happens if there's
It looks like it's the same in both places. (Could be refactored into a shared function, I suppose...)
I forgot about that... That's certainly more readable. |
an update on my pathological case, if you zoom way in in the png it looks correct, the red is coming from something outside of mpl doing blending in RGB space. Re L1251 Sorry, I was reading the first half of the diff and that was changed in the second half. |
What do you think that would be? -- "we control all the pixels" all the way down (at least for PNG). As far as I can tell, the "pinky" color is some sort of perceptual trick of the eyes when you have two very contrasting colors of only one pixel right next to each other (and personally, my display is 220dpi, so the pixels are quite small). If I zoom way in on the screenshot in the GIMP and use the eyedropper tool, however, I don't actually see any colors from outside the colormap. All this makes a very good stress test, though. |
An optical illusions makes more sense. The display software (web browser / gimp) might be down-sampling to display, but Qt should not be. If I take a screen shot of this page and zoom in on the (downsized) png gimp does show some red. |
I see. As you imply, I don't think there's anything we can do about that. As long as that doesn't happen from |
edf0078 to74723faCompareYou can download the image failures from the artifact tab of each not passing appveyor run. They include a nice html file which has the failing tests at the top. ->https://ci.appveyor.com/project/mdboom/matplotlib/build/1.0.832/job/cnc0brawn35inqpw/artifacts |
1f4e9eb tofff1d66CompareI think this is working again, though Appveyor seems stopped up. Feel free to merge once passing. |
Time for#6004... |
This "is the test running too slow" test seems a bit finicky. AppVeyor is passing otherwise. |
Rewrite of image infrastructure
Merged despite the too-slow failure. |
Rewrite of image infrastructureConflicts:lib/matplotlib/tests/test_axes.py - do not back-port appveyor spceific limitslib/matplotlib/tests/test_image.py - do not backport the jpeg_alpha testsetupext.py - do not include windows/appveyor related changes
backported to v2.x as6595139 |
\o/ |
This began as an attempt tofix#5490, but it turned out to be impossible to fix that without addressing a number of other issues.
There's a number of things this does. Let's start with fixing#5490.
The order of operations has changed from:
to
I had experimented with doing the normalizationafter the resize, but Agg is designed for data in range (0, 1). It might actually be more mathematically/perceptually correct to do the normalization before the resizing (which does linear interpolation modulo some filter) so that if a non-linear norm is being applied, the interpolation takes that into account.
BEFORE:
AFTER:
You can see the images no longer have the "red" that doesn't belong to the color map.
In addition:
image.pywas almost completely rewritten. There was a lot of copy-and-pasted code in there that has now been factored out by having all the image classes inherit from the same base class.svg.image_noscalercParam is not longer supported. One can get the same behavior by settinginterpolation=Noneon images orimage.interpolation: NonercParam globally._image_skew_coordinatehas been removed. One can set the transform on the image object or its parent Axes to get the same behavior, and this will be understood by other artists as well so I think is much preferred.image.resampletoTrue. The difference there isresample: Falseonly applies interpolation when upsampling.resample: Trueapplies it in both directions. And it has really odd behavior when one dimension is upsampled and the other is downsampled. I believe the history of this is thatresample: Falseis the old behavior before our Agg backend had full-on resampling in it, and the flag was maintained to allow for backward-compatible performance. I think it's high-time to use the more modern approach by default, especially wrt the upcoming 2.0 release._imageextension and the agg backend, apparently so that the Agg backend could apply an affine transform to the image like some of our vector backends. To reduce the number of complex code paths, the Agg backend has been reverted to being a "dumb" backend that can't resize an image and all resampling happens before it hits the backend. This doesn't break theinterpolation='none'feature where an exact copy of the original data is stored in a vector file.draw_imagebackend API has been simplified. It now simply takes an optional affine transform if the backend can handle transforming the image itself (this is true of all vector backends, and none of the others).