-
-
Notifications
You must be signed in to change notification settings - Fork 8.1k
FIX: Make widget blitting compatible with swapped canvas #30591
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
base: main
Are you sure you want to change the base?
Conversation
890404a
to
48346de
Compare
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. |
b418b60
to
f7ca7cc
Compare
Drat, I did not re-look at this PR before I opened #30605 |
Rebased. |
@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:
|
"""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.
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?
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.
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
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()
andload_blit_background()
toAxesWidgets
. 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
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.