Rework/fix Text layout cache.

anntzer · anntzer · commit 412cabc67347 · 2022-01-21T08:10:51.000+01:00
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.
diff --git a/lib/matplotlib/tests/test_text.py b/lib/matplotlib/tests/test_text.py
@@ -764,3 +764,26 @@ def test_pdf_chars_beyond_bmp():
     plt.rcParams['mathtext.fontset'] = 'stixsans'
     plt.figure()
     plt.figtext(0.1, 0.5, "Mass $m$ \U00010308", size=30)
+
+
+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
@@ -273,30 +273,29 @@ 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):
         """
         Return the extent (bbox) of the text together with
         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):
         """
