Skip to content

FIX: only try to update blit caches if the canvas we expect #25085

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

Merged
merged 3 commits into from
Feb 9, 2023

Conversation

tacaswell
Copy link
Member

Because we may switch the canvas to a different type (that may not support blitting) when saving, check it _clear that the current canvas is the canvas we expect when we update the blitting caches on the draw_event callback.

Closes #25075

@tacaswell tacaswell added this to the v3.7.0 milestone Jan 25, 2023
@tacaswell tacaswell force-pushed the fix/widget_pdf_save branch from eaf59b5 to 9cec214 Compare January 25, 2023 22:16
@@ -1088,7 +1088,7 @@ def __init__(self, ax, labels, actives=None, *, useblit=True,

def _clear(self, event):
"""Internal event handler to clear the buttons."""
if self.ignore(event):
if self.ignore(event) or self.canvas is not self.ax.figure.canvas:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if you want to wrap this in a small method with an explanation like your explanation below as the doc string:

def _not_orig_canvas():
    """
    Explain
    """"
    return self.canvas is not self.ax.figure.canvas

Perhaps thats too much cruft, but otherwise this is a bit of a mysterious clause.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done.

@ksunden ksunden added the Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. label Feb 3, 2023
Because we may switch the canvas to a different type (that may not
support blitting) when saving, check it `_clear` that the current canvas
is the canvas we expect when we update the blitting caches on the
draw_event callback.

Closes matplotlib#25075
@ksunden
Copy link
Member

ksunden commented Feb 8, 2023

Did the rebase as it was the result of conflicts with undeprecating clear in widgets

@ksunden ksunden force-pushed the fix/widget_pdf_save branch from b88acdb to faa4816 Compare February 8, 2023 23:47
@jklymak
Copy link
Member

jklymak commented Feb 9, 2023

Anyone should merge when Azure finally times out ;-)

@ksunden
Copy link
Member

ksunden commented Feb 9, 2023

I'm pretty sure the failed Azure test was just a known somewhat flaky test... but it was in fact test_blit, so going to make sure by re-running.

@tacaswell tacaswell merged commit 887f39f into matplotlib:main Feb 9, 2023
@tacaswell tacaswell deleted the fix/widget_pdf_save branch February 9, 2023 23:14
meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request Feb 9, 2023
ksunden added a commit that referenced this pull request Feb 10, 2023
…085-on-v3.7.x

Backport PR #25085 on branch v3.7.x (FIX: only try to update blit caches if the canvas we expect)
@ksunden ksunden mentioned this pull request Feb 20, 2023
6 tasks
@tacaswell tacaswell mentioned this pull request Jun 26, 2024
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Widget blitting broken when saving as PDF
3 participants