Don't change Figure DPI if value unchanged by dopplershift · Pull Request #16971 · matplotlib/matplotlib

dopplershift · 2020-03-31T00:45:16Z

So while digging into #15131, I noticed a change in how much we call resize. Here is the sample code I'm running:

import matplotlib
matplotlib.use('macosx')
import matplotlib.pyplot as plt
fig1, ax = plt.subplots()
plt.show()

When I run that 3.2.1, I see the following set of print outs of calls of various resize-related methods:

_macosx.View.windowDidResize
FigureCanvasMac.resize: 640 480 1.0
Figure.set_size_inches: False [6.4 4.8] 100.0
Figure._set_dpi 200.0 True
Figure.set_size_inches: True [6.4 4.8] 200.0
FigureManagerBase.resize

That looks reasonable. This is what the same code does on current master:

_macosx.View.windowDidResize
FigureCanvasMac.resize: 640 480 1.0
Figure.set_size_inches: False [6.4 4.8] 100.0
Figure._set_dpi 200.0 True
Figure.set_size_inches: True [6.4 4.8] 200.0
FigureManagerBase.resize
Figure._set_dpi 200.0 True
Figure.set_size_inches: True [6.4 4.8] 200.0
FigureManagerBase.resize
Figure._set_dpi 200.0 True
Figure.set_size_inches: True [6.4 4.8] 200.0
FigureManagerBase.resize
...[repeat last 3 lines 50 more times]

It looks like this change came in #16828. Prevously when Text.get_window_extent got a DPI of None, the function did not set figure.dpi. Now:

matplotlib/lib/matplotlib/text.py

Lines 894 to 913 in e702edd

    
           if dpi is None: 
        
               dpi = self.figure.dpi 
        
           if self.get_text() == '': 
        
               with cbook._setattr_cm(self.figure, dpi=dpi): 
        
                   tx, ty = self._get_xy_display() 
        
                   return Bbox.from_bounds(tx, ty, 0, 0) 
        
           if renderer is not None: 
        
               self._renderer = renderer 
        
           if self._renderer is None: 
        
               self._renderer = self.figure._cachedRenderer 
        
           if self._renderer is None: 
        
               raise RuntimeError('Cannot get window extent w/o renderer') 
        
           with cbook._setattr_cm(self.figure, dpi=dpi): 
        
               bbox, info, descent = self._get_layout(self._renderer) 
        
               x, y = self.get_unitless_position() 
        
               x, y = self.get_transform().transform((x, y)) 
        
               bbox = bbox.translated(x, y) 
        
               return bbox

If DPI comes in as None, the current DPI is grabbed and then reset on Figure. The change I've implemented here is to make setting Figure.dpi a no-op if the value isn't different. This gets us back to:

_macosx.View.windowDidResize
FigureCanvasMac.resize: 640 480 1.0
Figure.set_size_inches: False [6.4 4.8] 100.0
Figure._set_dpi 200.0 True
Figure.set_size_inches: True [6.4 4.8] 200.0
FigureManagerBase.resize

Check that the value of dpi is different before going through all the machinery to actually flush the change of value.

QuLogic · 2020-03-31T02:38:12Z

Oh, this is the same conclusion I ended up at #16972, though slightly different implementation.

I don't mind rebasing on this one, but perhaps include a similar comment (or stuff from commit message).

tacaswell · 2020-03-31T04:48:59Z

As pure style, I have a slight preference for the early return (less indentation, less code churn), but 🤷‍♀️.

tacaswell

Either this or the implementation in #16972

dopplershift · 2020-03-31T04:51:57Z

If #16972 needs to go in anyway for the gtk3 fixes, we can just merge that.

anntzer · 2020-04-04T09:41:16Z

Superseded by #16972.

Don't change Figure DPI if value unchanged

b7e881f

Check that the value of dpi is different before going through all the machinery to actually flush the change of value.

tacaswell approved these changes Mar 31, 2020

View reviewed changes

tacaswell added this to the v3.3.0 milestone Mar 31, 2020

dopplershift force-pushed the figure-dpi branch from aa02e4c to b7e881f Compare April 1, 2020 07:27

dopplershift mentioned this pull request Apr 1, 2020

Implement FigureManager.resize for macosx backend #16992

Merged

1 of 6 tasks

anntzer added the status: duplicate label Apr 4, 2020

anntzer closed this Apr 4, 2020

dopplershift deleted the figure-dpi branch April 12, 2020 00:30

Navigation Menu

Repository navigation

Don't change Figure DPI if value unchanged - #16971

Don't change Figure DPI if value unchanged#16971
dopplershift wants to merge 1 commit into
matplotlib:masterfrom
dopplershift:figure-dpi

dopplershift commented Mar 31, 2020

Uh oh!

QuLogic commented Mar 31, 2020 •

edited

Loading

Uh oh!

tacaswell commented Mar 31, 2020

Uh oh!

tacaswell left a comment

Uh oh!

dopplershift commented Mar 31, 2020

Uh oh!

anntzer commented Apr 4, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

	if dpi is None:
	dpi = self.figure.dpi
	if self.get_text() == '':
	with cbook._setattr_cm(self.figure, dpi=dpi):
	tx, ty = self._get_xy_display()
	return Bbox.from_bounds(tx, ty, 0, 0)

	if renderer is not None:
	self._renderer = renderer
	if self._renderer is None:
	self._renderer = self.figure._cachedRenderer
	if self._renderer is None:
	raise RuntimeError('Cannot get window extent w/o renderer')

	with cbook._setattr_cm(self.figure, dpi=dpi):
	bbox, info, descent = self._get_layout(self._renderer)
	x, y = self.get_unitless_position()
	x, y = self.get_transform().transform((x, y))
	bbox = bbox.translated(x, y)
	return bbox

Conversation

dopplershift commented Mar 31, 2020

Uh oh!

QuLogic commented Mar 31, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tacaswell commented Mar 31, 2020

Uh oh!

tacaswell left a comment

Choose a reason for hiding this comment

Uh oh!

dopplershift commented Mar 31, 2020

Uh oh!

anntzer commented Apr 4, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

QuLogic commented Mar 31, 2020 •

edited

Loading