Repository navigation
Speed up RGBA-stage image resampling - #32275
Conversation
Premultiplying and unpremultiplying around the resample step both operated on the strided A[..., :3] view, and the unpremultiply additionally used the `where=` form of np.divide, which is several times slower than the unmasked form. Operate on the whole contiguous array and restore the alpha channel instead, and skip premultiplication altogether when alpha is uniformly 1. The rendered output is unchanged.
|
Thank you for opening your first PR into Matplotlib! If you have not heard from us in a week or so, please leave a new comment below and that should bring it to our attention. Most of our reviewers are volunteers and sometimes things fall through the cracks. We also ask that you please finish addressing any review comments on this PR and wait for it to be merged (or closed) before opening a new one, as it can be a valuable learning experience to go through the review process. You can also join us on discourse chat for real-time discussion. For details on testing, writing docs, and our review process, please see the developer guide. We strive to be a welcoming and open project. Please follow our Code of Conduct. |
ayshih
left a comment
There was a problem hiding this comment.
Interesting! I wouldn't have expected there to be such a degree of speed-up by (incorrectly) multiplying the alpha channel by itself and then restoring it.
My primary comment is please change "unpremultiply" to "demultiply" in comments and test name. Aside from that, all of my comments are about cleaning up the writing of your unit test.
| axes_image.make_image(None)[0] | ||
|
|
||
|
|
||
| def test_rgba_stage_unpremultiplies_resampled_alpha(): |
There was a problem hiding this comment.
Rewrite this test using pytest.mark.parametrize() instead of the internal function
| fig, ax = plt.subplots() | ||
| axes_image = ax.imshow(data, interpolation='nearest', | ||
| interpolation_stage='rgba') | ||
| with np.errstate(divide='raise', invalid='raise'): |
There was a problem hiding this comment.
I think the RuntimeWarning would already get converted to an error by pytest, right?
There was a problem hiding this comment.
That's true, fixed
| # Trim the partially covered pixels at the edges of the image. | ||
| return image[2:-2, 2:-2] |
There was a problem hiding this comment.
I think your test as written will use nearest-neighbor interpolation, so I don't think trimming the edge is even necessary
There was a problem hiding this comment.
Thank you, fixed!
| opaque = render(1) | ||
| for alpha in (0.25, 0.5, 0.75): | ||
| partial = render(alpha) | ||
| assert_array_equal(partial[..., :3], opaque[..., :3]) |
There was a problem hiding this comment.
Since your arrays (both input and output) are constant in value, you can just check against the expected output value
Use Agg's "demultiply" terminology, matching src/_image_resample.h, rather than the invented "unpremultiply". Rewrite the test with pytest.mark.parametrize, and drop the np.errstate guard and the edge trimming. The guard is redundant because the test suite already turns the RuntimeWarning into an error, and nearest-neighbour interpolation makes the output a single flat colour, so the edges need no special handling and the expected value can be asserted directly.
|
@ayshih Thank you for the review! All comments have been addressed. |
ayshih
left a comment
There was a problem hiding this comment.
The PR looks good to me! I've checked it out on my Windows machine, but it'd be prudent for another reviewer to explicitly confirm the speed-up on a non-Windows x86 processor.
|
macOS 14 on an M2 Ultra (arm64): mainThis PRThe numbers look good, I'll review the code later today! |
iccir
left a comment
There was a problem hiding this comment.
This looks good!
Long term, I think that we should investigate resampling in premultiplied-space (per the "TODO:" comment). This solution makes complete sense and works in the meantime.
Congrats on your first PR to Matplotlib, @zekebarger! We hope to hear from you again! 🥳🎉
|
This is great. I wonder if the speed tests could find its way into mpl_bench ?? |
…275-on-v3.11.x Backport PR #32275 on branch v3.11.x (Speed up RGBA-stage image resampling)
PR summary
Closes #32240.
Redrawing an
imshowimage got about 2x slower between 3.10.9 and 3.11. @ayshih traced this to #29776, which moved resampling into premultiplied alpha space. The premultiply and demultiply are both full-array NumPy passes, but in their current form, they can't use NumPy's fast paths.Demultiply is the more expensive one:
This takes ~16 ms per draw in the code I used to reproduce the issue in #32240. Two reasons it's slow:
out=res[..., :3]is a stride-4 view (so contiguous inner loops can't be used), and passing an array towhere=is itself expensive. Running it on a 462x1560x4 float64 array takes 16.3 ms as written, 8.3 ms withoutwhere=, and only 2.8 ms operating on the whole contiguous array. An all-True mask array still costs 2.3-2.8x depending on whether the destination is strided, so the cost just comes from taking the masked path itself.For both the demultiply and premultiply steps, this PR implements contiguous whole-array operations that save the alpha channel and then restore it.
where=form, and while dividing alpha by itself involves more operations, the fact that it runs contiguously means it's much faster.One positive side effect is that the line
A = A.copy()is no longer needed. It existed to stopA[..., :3] *= A[..., 3:]from writing into the caller's array, but nowAis not modified in place. When alpha is all 1 it's left untouched, and otherwise the multiplication just builds a new array.Timings
Median of 15 repeats, same code from #32240 (macOS arm64, NumPy 2.5.2)
interpolation='none'interpolation='nearest'interpolation='none'interpolation='nearest'Verification
A new test,
test_rgba_stage_demultiplies_resampled_alpha, checks that RGB values survive the premultiply/demultiply round trip for several alpha values and that a fully transparent image never divides by zero.To double-check that rendered output is unchanged, I rendered a large set of configurations and compared the RGBA canvas buffers against
mainand against the 3.11.1 wheel. No differences were observed.Script used for the comparison
Run under each build, then compare:
AI Disclosure
I used Claude Code in several places and thoroughly reviewed its output. It profiled 3.10.9 against 3.11.1 to locate the two affected lines, benchmarked the approach implemented here, wrote the patch and the test, and ran the verification. I wrote or revised all prose, reviewed all code, and re-ran the verification script.
PR quality check