GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — FIX: Make widget blitting compatible with swapped canvas by timhoffm · Pull Request #30591 · matplotlib/matplotlib

Conversation

@timhoffm

@timhoffm timhoffm commented Sep 20, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

First step towards improving blitting - towards #30503 and fixing #30575.

This PR proposes a minimal low-level API to store background information on the canvas.
It also adds higher-level methods save_blit_background() and load_blit_background() to AxesWidgets. Child classes can execrise managed background storage though this API.

I specifically would like to have feedback on the solution idea and API design. Note: This is not intended to be a solution for everything right away, but I would like to learn more on blitting through the case of widgets.

Open topics

  • Check whether widgets act reasonably if the background is pulled under them. - As implemented now, they won't crash, but we must ensure that they can recover the background. It may be (to be checked) that they just loose the background.
  • We still need to refactor useblit from statically including canvas blitting capability to dynamic handling.

Edit: Tested the following examples all with blitting enabled, and they work as expected (after closing the figure, plt.figure(fig); plt.show() recrates the figure and the widgets continue to work.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@github-actions github-actions Bot added topic: widgets/UI topic: canvas and figure manager labels Sep 20, 2025
@timhoffm
timhoffm force-pushed the safe-blit branch 2 times, most recently from 890404a to 48346de Compare September 21, 2025 15:37
@timhoffm timhoffm mentioned this pull request Sep 26, 2025
Make Widget.useblit robust against underlying canvas changes #30503
Open
8 of 13 tasks
@timhoffm

timhoffm commented Sep 26, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member Author

Recent update/commit: Support blitting with changing canvas in some widgets

Approach depends a bit on the widget, but generally:

  • self._useblit only stores the flag passed to __init__
  • canvas.supports_blit is checked dynamically when needed
  • we sometimes have the property sef.useblit, which is the effective
    value of flag and capability
  • if animated artists are used, that state is set during
    __init__ based on the flag. This relies on the fact that
    the flag cannot be changed later, and that the value of
    "animated" does not play a role for drawing canvases without blit support.

Supported:

  • Button
  • _SelectorWidget
  • SpanSelector
  • ToolLineHandles
  • ToolHandles
  • RectangleSelector
  • LassoSelector
  • PolygonSelector

Not supported yet:

  • CheckButtons
  • RadioButtons
  • Cursor
  • MultiCursor
  • Lasso

This should already fix #30575 as Selector widgets are covered.

Note: It's probably best to review the commits individually.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@timhoffm
timhoffm force-pushed the safe-blit branch 2 times, most recently from b418b60 to f7ca7cc Compare September 26, 2025 10:21
@timhoffm
timhoffm marked this pull request as ready for review September 26, 2025 10:46
@tacaswell

tacaswell commented Sep 26, 2025

Copy link
Copy Markdown
Member

Drat, I did not re-look at this PR before I opened #30605

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@github-actions github-actions Bot added the status: needs rebase label Sep 30, 2025
@timhoffm
timhoffm force-pushed the safe-blit branch from f7ca7cc to 80b08c2 Compare October 3, 2025 08:39
@github-actions github-actions Bot removed the status: needs rebase label Oct 3, 2025
@timhoffm

timhoffm commented Oct 3, 2025

Copy link
Copy Markdown
Member Author

Rebased.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@timhoffm
timhoffm force-pushed the safe-blit branch from 80b08c2 to 47e1d44 Compare October 3, 2025 08:44
@timhoffm

timhoffm commented Oct 3, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member Author

@tacaswell How do we want to continue here? I consider this PR complete for the handled widgets, and suggest that we review and merge it. The remaining widgets should be handled in a follow-up PR to keep this PR more manageable.

Here's a summary of this PR approach:

  • First commit: Create an infrastructure that safely handles background images by storing them on the canvas and defining an API in a way that there is no guarantee a background image is available. Consumers can try to obtain a background but must gracefully handle the case that no background is available.
    This is the basis to be able to reasonably handle blit-backgrounds while swapping canvas. Apply this to widgets that store backgrounds.
  • Second commit: Fix dynamic changes in useblit on the individual Widgets case-by-case. Since logic slightly differs between widgets, we have to think them through individually. The list of currently fixed widgets is in FIX: Make widget blitting compatible with swapped canvas #30591 (comment).
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@timhoffm timhoffm mentioned this pull request Oct 3, 2025
MNT: Make useblit a property in Widget classes #30605
Closed
1 of 5 tasks
tacaswell
tacaswell reviewed Oct 3, 2025
View reviewed changes
Comment thread lib/matplotlib/widgets.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

tacaswell
tacaswell reviewed Oct 3, 2025
View reviewed changes
Comment thread lib/matplotlib/widgets.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

tacaswell
tacaswell reviewed Oct 3, 2025
View reviewed changes
Comment thread lib/matplotlib/widgets.py
"""Update the canvas cursor."""
self.ax.get_figure(root=True).canvas.set_cursor(cursor)

def _save_blit_background(self, background):

@tacaswell tacaswell Oct 3, 2025

Copy link
Copy Markdown
Member

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.

Do we want to pass the bounding box in instead?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@timhoffm timhoffm Oct 3, 2025

Copy link
Copy Markdown
Member Author

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.

For symmetry reasons and to keep the changes within the PR minimal, I would leave the complete handling to the individual widgets for now.

They do

self._save_blit_background(self.canvas.copy_from_bbox(self.ax.bbox))

and

background = self._load_blit_background()
if background is not None:
    self.canvas.restore_region(background)

It may be reasonable to refactor in a follow-up and move that logic to the base class.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
tacaswell
tacaswell reviewed Oct 3, 2025
View reviewed changes
Comment thread lib/matplotlib/widgets.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

@timhoffm
timhoffm force-pushed the safe-blit branch 2 times, most recently from 30ca550 to 87fa6db Compare October 3, 2025 18:15
@tacaswell tacaswell added this to the v3.11.0 milestone Oct 3, 2025
@tacaswell tacaswell added the Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. label Oct 3, 2025
@timhoffm
timhoffm force-pushed the safe-blit branch from 87fa6db to 747155e Compare October 3, 2025 18:28
timhoffm added 2 commits October 3, 2025 20:34
@timhoffm
FIX: Make widget blitting compatible with swapped canvas
3edc6c7
@timhoffm
Support blitting with changing canvas in some widgets
bec8b2b
Approach depends a bit on the widget, but generally:
- `self._useblit` only stores the flag passed to __init__
- canvas.supports_blit is checked dynamically when needed
- we sometimes have the property `sef.useblit`, which is the effective
value of flag and capability
- if animated artists are needed that state is set during
  __init__ based on the flag. This relies on the fact that
  the flag cannot be changed later, and that the value of
  "animated" does not play a role for drawing canvases without blit support.

Supported:
- Button
- _SelectorWidget
- SpanSelector
- ToolLineHandles
- ToolHandles
- RectangleSelector
- LassoSelector
- PolygonSelector

Not supported yet:
- CheckButtons
- RadioButtons
- Cursor
- MultiCursor
- Lasso
@timhoffm
timhoffm force-pushed the safe-blit branch from 747155e to bec8b2b Compare October 3, 2025 18:34
@timhoffm timhoffm added the status: needs review label Oct 5, 2025
@tacaswell tacaswell linked an issue Oct 30, 2025 that may be closed by this pull request
[Bug]: Regression in widget behavior #30575
Closed
greglucas
greglucas reviewed Dec 18, 2025
View reviewed changes
Comment thread lib/matplotlib/widgets.py

self._useblit = useblit and self.canvas.supports_blit
self._background = None
self._useblit = useblit and self.canvas.supports_blit # TODO: make dynamic

@greglucas greglucas Dec 18, 2025

Copy link
Copy Markdown
Contributor

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.

@timhoffm are you planning on addressing this TODO in this PR or is it a follow-up?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@timhoffm timhoffm Dec 18, 2025

Copy link
Copy Markdown
Member Author

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 is a follow-up and can land in 3.11 or later.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
QuLogic
QuLogic reviewed Dec 18, 2025
View reviewed changes

@QuLogic QuLogic left a comment

Copy link
Copy Markdown
Member

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 API is semi-private, so it's open to change a bit, but I think it looks good regardless.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread lib/matplotlib/widgets.py

self._useblit = useblit and self.canvas.supports_blit
self._background = None
self._useblit = useblit and self.canvas.supports_blit # TODO: make dynamic

@QuLogic QuLogic Dec 18, 2025

Copy link
Copy Markdown
Member

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.

Is this TODO necessary to complete before 3.11?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@timhoffm timhoffm Dec 18, 2025

Copy link
Copy Markdown
Member Author

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 is a follow-up and can land in 3.11 or later.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
greglucas
greglucas approved these changes Dec 18, 2025
View reviewed changes

@greglucas greglucas left a comment

Copy link
Copy Markdown
Contributor

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.

I think this is good to go with the current implementation and the TODOs can be addressed in follow-up PRs, but will leave that up to Tim whether he wants to get it in now.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@timhoffm

timhoffm commented Dec 18, 2025

Copy link
Copy Markdown
Member Author

First and foremost. This covers the regression #30575. That's prerequisite for 3.11.

We can live with not supporting canvas swapping on all widgets for 3.11. So it's ok to leave the TODOs and #30503 open.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@tacaswell
tacaswell merged commit 15697ea into matplotlib:main Dec 23, 2025
3 checks passed

Uh oh!

There was an error while loading. Please reload this page.

@timhoffm
timhoffm deleted the safe-blit branch December 23, 2025 21:08
@larsoner larsoner mentioned this pull request Dec 29, 2025
MAINT: Remove workaround for mpl mne-tools/mne-python#13569
Closed
@larrybradley larrybradley mentioned this pull request Jan 6, 2026
FIX: Handle AxesWidget cleanup after failed init #30935
Merged
5 tasks
@QuLogic QuLogic removed the status: needs review label Jan 24, 2026
andreas16700 added a commit to andreas16700/matplotlib that referenced this pull request Jan 30, 2026
@andreas16700 @claude
Run 74: Match upstream 15697ea (PR matplotlib#30591) - Safe blit for …
163703d
…widgets

Co-Authored-By: Claude Opus 4.5 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tacaswell tacaswell tacaswell left review comments
@QuLogic QuLogic QuLogic left review comments
@greglucas greglucas greglucas approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. topic: canvas and figure manager topic: widgets/UI

Projects

None yet

Milestone

v3.11.0

Development

Successfully merging this pull request may close these issues.

[Bug]: Regression in widget behavior

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@timhoffm @tacaswell @QuLogic @greglucas
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.