Skip to content

Unset the canvas manager when saving the figure. #10292

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.

Sign up for GitHub

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 1 commit into from
Jan 24, 2018
+11 −1
Merged

Unset the canvas manager when saving the figure. #10292

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion lib/matplotlib/backend_bases.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2149,6 +2149,15 @@ def print_figure(self, filename, dpi=None, facecolor=None, edgecolor=None,

"""
self._is_saving = True
# Remove the figure manager, if any, to avoid resizing the GUI widget.
Copy link
Member

Choose a reason for hiding this comment

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

Maybe # Remove the figure manager, currently set in some backeneds (i.e. nbagg).... I was looking at this and not understanding when self.manager ever got set. Similarly above when you instantiate it, maybe a comment on what a manager is and where one is set?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is a FigureManagerBase class defined in the same file, and if you look at its init you'll see it creates the manager attribute on the canvas. I think it's better to have a single doc that describes the interaction between all classes in backend_bases rather than document this in a disjoint, one-comment-at-a-time way... (but it is also true that that single doc doesn't exist yet)

Copy link
Member

Choose a reason for hiding this comment

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

I'm still confused what backend I need to use to test this works, and how. I assume I need to trigger a manual save. This works fine on Qt5Agg, but did not check if manager is set in Qt5Agg.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Set the savefig.dpi rcparam to some large number (say, three times your figure.dpi, just to be safe with hidpi stuff...) and then interactively trigger a save, while the window has a relatively small size on your desktop. Depending on yourdesktop environment (specifically, how it handles fast window resize events), you may see the window flicker in size, first to, well, three times its current size, and then back.
This resize is handled by the canvas manager. This PR disconnects the manager for the duration of the save, thus suppressing the flicker.

Copy link
Member

@jklymak jklymak Jan 24, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Hmmm, I guess my machine is too fast - I don't get a flicker w/ Qt5Agg on Master or any apparent resizing. The PR doesn't make things any better or worse on Qt5Agg for me, so if it helps other folks its fine by me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can you try on linux? It may well be that OSX just detects and suppresses fast resizes.
May also help if you make the plot relatively slow to draw (e.g., N=100000; scatter(rand(N), rand(N), s=rand(N))), which may make the effect more apparent.

Copy link
Member

Choose a reason for hiding this comment

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

No, I don't have a linux machine (set up for this). I guess OSX is just clever - making the plot more ornate doesn't change anything.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I assume @tacaswell checked that this works for him too?

# Having *no* manager and a *None* manager are currently different (see
# Figure.show); should probably be normalized to None at some point.
_no_manager = object()
if hasattr(self, 'manager'):
manager = self.manager
del self.manager
else:
manager = _no_manager

if format is None:
# get format from filename, or from backend's default filetype
Expand Down Expand Up @@ -2267,8 +2276,9 @@ def print_figure(self, filename, dpi=None, facecolor=None, edgecolor=None,
self.figure.set_facecolor(origfacecolor)
self.figure.set_edgecolor(origedgecolor)
self.figure.set_canvas(self)
if manager is not _no_manager:
self.manager = manager
self._is_saving = False
#self.figure.canvas.draw() ## seems superfluous
return result

@classmethod
Expand Down