Repository navigation
[MNT]: Replace BufferRegion/restore_region by (x, y, array) tuples and draw_image? #23882
Description
Summary
Based on the discussion at https://discourse.matplotlib.org/t/method-to-add-transparency-to-a-saved-bufferregion/23121 (thanks @raphaelquast), I realized it may be possible to get rid of the internal BufferRegion type and restore_region, in favor of copy_from_bbox returning a tpl = (x, y, rgba_array) tuple and using canvas.get_renderer().draw_image(gc, x, y, rgba_array) to restore the region. draw_image could specifically add support for passing gc = None (i.e. create a throwaway gc just for this call), and then the restore call would be canvas.get_renderer().draw_image(None, *tpl) which seems just fine. Or we can just keep restore_region as a canvas method (on FigureCanvasBase, without need of being overridden by backends) which does the right draw_image call on the renderer, only getting rid of Renderer.restore_region.
Currently, restore_region just blindly copies the BufferRegion contents, without doing any alpha compositing; the new approach would instead alpha-composite against whatever was here before. However, most uses of copy_from_bbox/restore_region suppose opaque regions anyways, and callers can also modify the rgba_array's alpha channel as desired before calling draw_image. Hopefully the extra alpha compositing wouldn't be too much of a slowdown, or we could always extend draw_image to support rgb (not rgba) inputs and detect that in this case it can go through a fast path without alpha compositing.
The advantage of such a change would be to simplify the backend API (e.g. for third party backends), including removing the undocumented BufferRegion type. (Probably the correct transition path would be to first make the BufferRegion type fully opaque, by deprecating all its public methods.)
attn @tacaswell as I believe you have written quite a bit of docs about blitting.
Proposed fix
No response
Activity
tacaswell commentedon Sep 13, 2022
Interesting. Have only put a little thought into this, but my initial reactions
BufferRegionas a one-liner would be gooddraw_imageto do this sort of thing would be good as it is conceptually more consistent with everything elsepelson commentedon Mar 14, 2024
I agree that
copy_from_bboxshould return a well defined object (for a raster renderer at least), and that it shouldn't be an Agg-internal, inaccessible type. It is surprising that we don't have a decent Image representation (from my memory) which would be a good fit for the return type - a tuple of(x0, y0, img)is a poor-man's image type here 😉.As for removing
restore_region- I'm not convinced that we should bother, the base renderer class can implement thedraw_imagepass through, and other renderers can specialise (I doubt they would though). I think the removal ofbboxandxyfrom therestore_regionmethod would be reasonable (I see from the linked discussion that this would break at least one workflow) - there is no use of those arguments in themplcodebase. IMO the use case served by those kwargs would be better placed in a non-poor-man's representation of an image + domain, where subsetting by bbox would be non renderer specific.pelson commentedon Mar 14, 2024
I'm reminded that I wish the Image artist classes didn't do the subsetting/clipping for the renderer, and instead just passed the complete image (plus some way to know that the image itself hasn't changed). This would allow us to optimise the clipping using whatever tools are at our disposal in the renderer. There is no reason that the renderer couldn't call back to the image to do a software based clipping if the renderer doesn't want to implement anything itself (a reasonable default).
I appreciate that this is off-topic for the issue, but just wanted to share this thought somewhere 💡
QuLogic commentedon Jun 21, 2025
See also #19095?
anntzer commentedon Jun 22, 2025
Somewhat related, but not directly identical?