Skip to content

MNT: Remove cached renderer from figure #23202

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
Aug 12, 2022
Merged

MNT: Remove cached renderer from figure #23202

Show file tree
Hide file tree
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
5 changes: 5 additions & 0 deletions doc/api/next_api_changes/deprecations/23202-GL.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
``Axes.get_renderer_cache``
~~~~~~~~~~~~~~~~~~~~~~~~~~~

The canvas now takes care of the renderer and whether to cache it
or not. The alternative is to call ``axes.figure.canvas.get_renderer()``.
19 changes: 4 additions & 15 deletions lib/matplotlib/axes/_base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3068,33 +3068,22 @@ def draw(self, renderer):
def draw_artist(self, a):
"""
Efficiently redraw a single artist.

This method can only be used after an initial draw of the figure,
because that creates and caches the renderer needed here.
"""
if self.figure._cachedRenderer is None:
Copy link
Member

Choose a reason for hiding this comment

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

On one hand, this call making a new renderer would be odd, my understanding of why this exists is a helper specifcally for managing blitting where you can rely on the render not only existing, but also having already had everything else of interest rasterized. By caching the renderer on the figure we are saying "yes, this figure was rendered with this renderer so we are sure that it is holding the state of everything else, also you can use it to draw the given artist". If we cache / create the renderer on the canvas we only get "this renderer will work".

However, that is a narrow case and while I can think of stories of how this would break someone ("catch the AttirbuteError to know we need to re-render the whole figure"....but for that to make sense we would also have to be sure that we were properly invalidating the cache (by setting it to None) when we resize the Figure).

Which is a very long way of saying that I am 👍🏻 on this change (despite some risk), will need an API change note.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I agree with all of that. I was a bit surprised that there wasn't more code churn in this PR which does make me a bit hesitant about what the edge-cases I'm missing are...

One thing I thought about is that we have a get_renderer() method on Agg/Cairo Canvas' now, but not on PDF, but the PDF requires a file handle so it isn't as straightforward to just point to a renderer. I was curious if that was perhaps the idea behind the figure caching in the first place... But, surprisingly it didn't affect any of the PDF tests.

Copy link
Member

Choose a reason for hiding this comment

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

The pdf and svg renderer's work in a streaming mode so going back to one only makes limited sense in general and makes no sense in this case.

raise AttributeError("draw_artist can only be used after an "
"initial draw which caches the renderer")
a.draw(self.figure._cachedRenderer)
a.draw(self.figure.canvas.get_renderer())

def redraw_in_frame(self):
"""
Efficiently redraw Axes data, but not axis ticks, labels, etc.

This method can only be used after an initial draw which caches the
renderer.
"""
if self.figure._cachedRenderer is None:
raise AttributeError("redraw_in_frame can only be used after an "
"initial draw which caches the renderer")
with ExitStack() as stack:
for artist in [*self._axis_map.values(),
self.title, self._left_title, self._right_title]:
stack.enter_context(artist._cm_set(visible=False))
self.draw(self.figure._cachedRenderer)
self.draw(self.figure.canvas.get_renderer())

@_api.deprecated("3.6", alternative="Axes.figure.canvas.get_renderer()")
def get_renderer_cache(self):
return self.figure._cachedRenderer
return self.figure.canvas.get_renderer()

# Axes rectangle characteristics

Expand Down
5 changes: 2 additions & 3 deletions lib/matplotlib/backend_bases.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1535,8 +1535,7 @@ def _mouse_handler(event):

def _get_renderer(figure, print_method=None):
"""
Get the renderer that would be used to save a `.Figure`, and cache it on
the figure.
Get the renderer that would be used to save a `.Figure`.

If you need a renderer without any active draw methods use
renderer._draw_disabled to temporary patch them out at your call site.
Expand All @@ -1559,7 +1558,7 @@ def _draw(renderer): raise Done(renderer)
try:
print_method(io.BytesIO())
except Done as exc:
renderer, = figure._cachedRenderer, = exc.args
renderer, = exc.args
return renderer
else:
raise RuntimeError(f"{print_method} did not call Figure.draw, so "
Expand Down
3 changes: 3 additions & 0 deletions lib/matplotlib/backends/backend_cairo.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,9 @@ def _renderer(self):
self._cached_renderer = RendererCairo(self.figure.dpi)
return self._cached_renderer

def get_renderer(self):
return self._renderer

def copy_from_bbox(self, bbox):
surface = self._renderer.gc.ctx.get_target()
if not isinstance(surface, cairo.ImageSurface):
Expand Down
2 changes: 1 addition & 1 deletion lib/matplotlib/backends/backend_wx.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -620,7 +620,7 @@ def gui_repaint(self, drawDC=None):
# DC (see GraphicsContextWx._cache).
bmp = (self.bitmap.ConvertToImage().ConvertToBitmap()
if wx.Platform == '__WXMSW__'
and isinstance(self.figure._cachedRenderer, RendererWx)
and isinstance(self.figure.canvas.get_renderer(), RendererWx)
else self.bitmap)
drawDC.DrawBitmap(bmp, 0, 0)
if self._rubberband_rect is not None:
Expand Down
18 changes: 2 additions & 16 deletions lib/matplotlib/figure.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2253,7 +2253,6 @@ def axes(self):

def draw(self, renderer):
# docstring inherited
self._cachedRenderer = renderer

# draw the figure bounding box, perhaps none for white figure
if not self.get_visible():
Expand Down Expand Up @@ -2493,7 +2492,6 @@ def __init__(self,

self._axstack = _AxesStack() # track all figure axes and current axes
self.clear()
self._cachedRenderer = None

# list of child gridspecs for this figure
self._gridspecs = []
Expand Down Expand Up @@ -2655,9 +2653,7 @@ def axes(self):
get_axes = axes.fget

def _get_renderer(self):
if self._cachedRenderer is not None:
return self._cachedRenderer
elif hasattr(self.canvas, 'get_renderer'):
if hasattr(self.canvas, 'get_renderer'):
return self.canvas.get_renderer()
else:
return _get_renderer(self)
Expand Down Expand Up @@ -3051,7 +3047,6 @@ def clear(self, keep_observers=False):
@allow_rasterization
def draw(self, renderer):
# docstring inherited
self._cachedRenderer = renderer

# draw the figure bounding box, perhaps none for white figure
if not self.get_visible():
Expand Down Expand Up @@ -3092,14 +3087,8 @@ def draw_without_rendering(self):
def draw_artist(self, a):
"""
Draw `.Artist` *a* only.

This method can only be used after an initial draw of the figure,
because that creates and caches the renderer needed here.
"""
if self._cachedRenderer is None:
raise AttributeError("draw_artist can only be used after an "
"initial draw which caches the renderer")
a.draw(self._cachedRenderer)
a.draw(self.canvas.get_renderer())

def __getstate__(self):
state = super().__getstate__()
Expand All @@ -3109,9 +3098,6 @@ def __getstate__(self):
# re-attached to another.
state.pop("canvas")

# Set cached renderer to None -- it can't be pickled.
state["_cachedRenderer"] = None

# discard any changes to the dpi due to pixel ratio changes
state["_dpi"] = state.get('_original_dpi', state['_dpi'])

Expand Down
4 changes: 2 additions & 2 deletions lib/matplotlib/tests/test_backend_macosx.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,11 @@ def test_cached_renderer():
# a fig.canvas.draw() call
fig = plt.figure(1)
fig.canvas.draw()
assert fig._cachedRenderer is not None
assert fig.canvas.get_renderer()._renderer is not None

fig = plt.figure(2)
fig.draw_without_rendering()
assert fig._cachedRenderer is not None
assert fig.canvas.get_renderer()._renderer is not None


@pytest.mark.backend('macosx')
Expand Down
2 changes: 1 addition & 1 deletion lib/matplotlib/tests/test_image.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -984,7 +984,7 @@ def test_empty_imshow(make_norm):
fig.canvas.draw()

with pytest.raises(RuntimeError):
im.make_image(fig._cachedRenderer)
im.make_image(fig.canvas.get_renderer())


def test_imshow_float16():
Expand Down
2 changes: 1 addition & 1 deletion lib/mpl_toolkits/tests/test_axes_grid1.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -552,7 +552,7 @@ def test_auto_adjustable():
pad = 0.1
make_axes_area_auto_adjustable(ax, pad=pad)
fig.canvas.draw()
tbb = ax.get_tightbbox(fig._cachedRenderer)
tbb = ax.get_tightbbox()
assert tbb.x0 == pytest.approx(pad * fig.dpi)
assert tbb.x1 == pytest.approx(fig.bbox.width - pad * fig.dpi)
assert tbb.y0 == pytest.approx(pad * fig.dpi)
Expand Down