From a5efd24b486e342ad83b1c00626efcd4f394a6ee Mon Sep 17 00:00:00 2001 From: Antony Lee Date: Thu, 20 Jan 2022 01:18:43 +0100 Subject: [PATCH] Rework/fix Text layout cache. Instead of caching the text layout based on a bunch of properties, only cache the computation of the text's metrics, which 1) should be the most expensive part (everything else in _get_layout is relatively simple iteration and arithmetic) and 2) depends on fewer implicit parameters. In fact, the old cache key was insufficient in that it would conflate usetex and non-usetex strings together, even though they have different metrics; e.g. with the (extremely artificial) example ```python figtext(.1, .5, "foo\nbar", size=32) # (0) figtext(.1, .5, "foo\nbar", usetex=True, size=32, c="r", alpha=.5) # (1) figtext(.3, .5, "foo\nbar", usetex=True, size=32, c="r", alpha=.5) # (2) ``` the linespacing of the first usetex string (1) would be "wrong": it is bigger that the one of the second usetex string (2), because it instead reuses the layout computed for the non-usetex string (0). The motivation is also to in the future let the renderer have better control on cache invalidation (with a yet-to-be-added renderer method), e.g. multiple instances of the same renderer cache could share the same layout info. --- lib/matplotlib/tests/test_text.py | 24 +++++++++++++++++ lib/matplotlib/text.py | 43 ++++++++++++++----------------- 2 files changed, 44 insertions(+), 23 deletions(-) diff --git a/lib/matplotlib/tests/test_text.py b/lib/matplotlib/tests/test_text.py index aced39bc1afe..ac029b6deddb 100644 --- a/lib/matplotlib/tests/test_text.py +++ b/lib/matplotlib/tests/test_text.py @@ -764,3 +764,27 @@ def test_pdf_chars_beyond_bmp(): plt.rcParams['mathtext.fontset'] = 'stixsans' plt.figure() plt.figtext(0.1, 0.5, "Mass $m$ \U00010308", size=30) + + +@needs_usetex +def test_metrics_cache(): + fig = plt.figure() + fig.text(.3, .5, "foo\nbar") + fig.text(.3, .5, "foo\nbar", usetex=True) + fig.text(.5, .5, "foo\nbar", usetex=True) + fig.canvas.draw() + renderer = fig._cachedRenderer + ys = {} # mapping of strings to where they were drawn in y with draw_tex. + + def call(*args, **kwargs): + renderer, x, y, s, *_ = args + ys.setdefault(s, set()).add(y) + + renderer.draw_tex = call + fig.canvas.draw() + assert [*ys] == ["foo", "bar"] + # Check that both TeX strings were drawn with the same y-position for both + # single-line substrings. Previously, there used to be an incorrect cache + # collision with the non-TeX string (drawn first here) whose metrics would + # get incorrectly reused by the first TeX string. + assert len(ys["foo"]) == len(ys["bar"]) == 1 diff --git a/lib/matplotlib/text.py b/lib/matplotlib/text.py index 4dcf40648d35..fc25391d27ef 100644 --- a/lib/matplotlib/text.py +++ b/lib/matplotlib/text.py @@ -273,19 +273,22 @@ def update_from(self, other): self._linespacing = other._linespacing self.stale = True - def _get_layout_cache_key(self, renderer): - """ - Return a hashable tuple of properties that lets `_get_layout` know - whether a previously computed layout can be reused. - """ - return ( - self.get_unitless_position(), self.get_text(), - hash(self._fontproperties), - self._verticalalignment, self._horizontalalignment, - self._linespacing, - self._rotation, self._rotation_mode, self._transform_rotates_text, - self.figure.dpi, weakref.ref(renderer), + def _get_text_metrics_with_cache( + self, renderer, text, fontproperties, ismath): + """ + Call ``renderer.get_text_width_height_descent``, caching the results. + """ + cache_key = ( + weakref.ref(renderer), + text, + hash(fontproperties), + ismath, + self.figure.dpi, ) + if cache_key not in self._cached: + self._cached[cache_key] = renderer.get_text_width_height_descent( + text, fontproperties, ismath) + return self._cached[cache_key] def _get_layout(self, renderer): """ @@ -293,10 +296,6 @@ def _get_layout(self, renderer): multiple-alignment information. Note that it returns an extent of a rotated text when necessary. """ - key = self._get_layout_cache_key(renderer=renderer) - if key in self._cached: - return self._cached[key] - thisx, thisy = 0.0, 0.0 lines = self.get_text().split("\n") # Ensures lines is not empty. @@ -306,16 +305,16 @@ def _get_layout(self, renderer): ys = [] # Full vertical extent of font, including ascenders and descenders: - _, lp_h, lp_d = renderer.get_text_width_height_descent( - "lp", self._fontproperties, + _, lp_h, lp_d = self._get_text_metrics_with_cache( + renderer, "lp", self._fontproperties, ismath="TeX" if self.get_usetex() else False) min_dy = (lp_h - lp_d) * self._linespacing for i, line in enumerate(lines): clean_line, ismath = self._preprocess_math(line) if clean_line: - w, h, d = renderer.get_text_width_height_descent( - clean_line, self._fontproperties, ismath=ismath) + w, h, d = self._get_text_metrics_with_cache( + renderer, clean_line, self._fontproperties, ismath) else: w = h = d = 0 @@ -439,9 +438,7 @@ def _get_layout(self, renderer): # now rotate the positions around the first (x, y) position xys = M.transform(offset_layout) - (offsetx, offsety) - ret = bbox, list(zip(lines, zip(ws, hs), *xys.T)), descent - self._cached[key] = ret - return ret + return bbox, list(zip(lines, zip(ws, hs), *xys.T)), descent def set_bbox(self, rectprops): """