Skip to content

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 29, 2025
Merged

Conversation

anntzer
Copy link
Contributor

@anntzer anntzer commented Mar 18, 2025

Test e.g. with

import matplotlib.pyplot as plt
import numpy as np

fig = 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()

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

# Temporarily increase bitdepth to avoid precision loss.
A = A.astype(np.uint16)
A[..., :3] *= A[..., 3:]
A[..., 3] *= np.uint16(0x100)
Copy link
Member

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?

Copy link
Contributor Author

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

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 at https://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 and round rather than truncate (both to best repro the old behavior and because rounding makes more sense, I believe...).

Copy link
Member

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?

Copy link
Contributor Author

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.

@anntzer
Copy link
Contributor Author

anntzer commented Mar 19, 2025

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

```python
import matplotlib.pyplot as plt
import numpy as np

fig = 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()
```
Copy link
Member

@jklymak jklymak left a 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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to convert here and pay the additional memory cost, or rather be defensive in the change and just not premultiply uint8 in this case?

Ping @QuLogic who looked deeper into memory usage in #29453.

Copy link
Contributor Author

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?

@anntzer anntzer mentioned this pull request Mar 23, 2025
2 tasks
@timhoffm timhoffm added this to the v3.11.0 milestone Mar 29, 2025
@timhoffm timhoffm merged commit 2d319fe into matplotlib:main Mar 29, 2025
41 checks passed
@anntzer anntzer deleted the ifpa branch March 29, 2025 08:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants