GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||||||||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||||||||||||||||||||||||||||||||||
GeezerVison is showing github.com — FIX: Make widget blitting compatible with swapped canvas by timhoffm · Pull Request #30591 · matplotlib/matplotlib Conversation
|
|
Recent update/commit: Support blitting with changing canvas in some widgets Approach depends a bit on the widget, but generally:
Supported:
Not supported yet:
This should already fix #30575 as Selector widgets are covered. Note: It's probably best to review the commits individually. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
b418b60 to
f7ca7cc
Compare
|
Drat, I did not re-look at this PR before I opened #30605 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Rebased. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
@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:
|
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
| """Update the canvas cursor.""" | ||
| self.ax.get_figure(root=True).canvas.set_cursor(cursor) | ||
|
|
||
| def _save_blit_background(self, background): |
There was a problem 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.
There was an error while loading. Please reload this page.
There was a problem 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.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
30ca550 to
87fa6db
Compare
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
|
|
||
| self._useblit = useblit and self.canvas.supports_blit | ||
| self._background = None | ||
| self._useblit = useblit and self.canvas.supports_blit # TODO: make dynamic |
There was a problem 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.
There was an error while loading. Please reload this page.
There was a problem 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.
There was an error while loading. Please reload this page.
There was a problem 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.
There was an error while loading. Please reload this page.
|
|
||
| self._useblit = useblit and self.canvas.supports_blit | ||
| self._background = None | ||
| self._useblit = useblit and self.canvas.supports_blit # TODO: make dynamic |
There was a problem 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.
There was an error while loading. Please reload this page.
There was a problem 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.
There was an error while loading. Please reload this page.
There was a problem 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.
There was an error while loading. Please reload this page.
|
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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
…widgets Co-Authored-By: Claude Opus 4.5 <[email protected]>
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
[Bug]: Regression in widget behavior
There was an error while loading. Please reload this page.