Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Filter images in premultiplied alpha mode.#29776
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
lib/matplotlib/image.py Outdated
| # Temporarily increase bitdepth to avoid precision loss. | ||
| A = A.astype(np.uint16) | ||
| A[..., :3] *= A[..., 3:] | ||
| A[..., 3] *= np.uint16(0x100) |
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.
Overall this looks good, but..
given that 0x100 is a semi-arbitrary number, why are you representing it as hex? I had to look this up.
Is this really necessary? Should it have happened before now? Why do we want to preserve uint8 precision 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.
This arises because agg back-clips the rgb channels to no more than the alpha channel
matplotlib/extern/agg24-svn/include/agg_span_image_filter_rgba.h
Lines 742 to 745 inc887ecb
| if(fg[order_type::A] >color_type::full_value()) fg[order_type::A] =color_type::full_value(); | |
| if(fg[order_type::R] > fg[order_type::A]) fg[order_type::R] = fg[order_type::A]; | |
| if(fg[order_type::G] > fg[order_type::A]) fg[order_type::G] = fg[order_type::A]; | |
| if(fg[order_type::B] > fg[order_type::A]) fg[order_type::B] = fg[order_type::A]; |
i.e. it forbids emitting but non-occluding pixels (see description athttps://en.wikipedia.org/wiki/Alpha_compositing#Straight_versus_premultiplied).
To convert from straight to premultiplied, I just multiply the RGB (uint8) values by the alpha (uint8) value, which results naturally in an uint16 value. In order to avoid agg's back-clipping behavior, I thus need to also upscale the alpha values so that they remain bigger than the RGB values, and the normal way to do this is just to scale them by the highest uint8 RGB value (or one more than it), i.e. 0x100 = 256. I guess the even more natural way to write this would be as a 2-byte bitshift (
A[..., 3] <<= 16) but the symmetry breaks down when scaling back to uint8 because I think I need to divide by 0x100 andround rather than truncate (both to best repro the old behavior and because rounding makes more sense, I believe...).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.
Sure, but I think my point is that we are mangling the incoming data anyhow, why do song and dance versus just converting to float64?
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.
Fair point, that's quite a bit simpler.
Slightly tweaked the implementation to use rounding instead of truncation when back-casting to uint8, this gets rid of nearly all remaining test failures except test_image_composite where I think there are other issues with the compositing anyways (#8847) so I just put in a tiny bit of tolerance. |
Test e.g. with```pythonimport matplotlib.pyplot as pltimport numpy as npfig = plt.figure(figsize=(6, 6))img = np.zeros((5, 5, 4))img[..., 1] = 1.img[..., 3] = np.tril(np.ones((5, 5)))fig.add_subplot(221).imshow(img, interpolation="bilinear")fig.add_subplot(222).imshow((img * 0xff).astype("u1"), interpolation="bilinear")fig.add_subplot(212).imshow( np.where( np.mgrid[:100, :200][0] > np.random.randint(100, size=200), np.arange(100)[:, None], np.nan), cmap="Greens", interpolation="antialiased", interpolation_stage="rgba")plt.show()```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 makes sense to me, and I think is a good improvement.
| else: | ||
| if A.ndim == 2: # interpolation_stage = 'rgba' | ||
| self.norm.autoscale_None(A) | ||
| A = self.to_rgba(A) | ||
| if A.dtype == np.uint8: | ||
| # uint8 is too imprecise for premultiplied alpha roundtrips. | ||
| A = np.divide(A, 0xff, dtype=np.float32) |
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.
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 rendering without premultiplying in the uint8 case would then be different compared to the float case, which would be really bad.
If memory is an issue I suspect we should write a C++ variant of span_image_resample_rgba_foo::generate that does the premultiplication at that stage, it seems doable...
I suspect this PR would supersede#29453 as it should remove the problematic parts targeted by it?
2d319fe intomatplotlib:mainUh oh!
There was an error while loading.Please reload this page.
Test e.g. with
Implements#29711 (comment) which is part of#29711 (but there is another part to that issue as well).
Some test failures due to tiny rendering differences remain, and I would like to investigate whether they can be suppressed by tweaking the conversion to/from premultiplied alpha (in particular, should the alpha channel be multiplied by 0xff or 0x100?).
The change in downsampling_speckle.py is clearly for the better, it suppresses a tiny gray band at the boundary between data and no data (visible with "swipe" diff) similar to the one shown in#29711 (comment).
See also#11316 and#12062, which should be subsumed by this fix.
PR summary
PR checklist