FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Simplify glyph stream logic in ps backend · Issue #23965 · matplotlib/matplotlib · GitHub

Repository navigation

Simplify glyph stream logic in ps backend  #23965

Closed

Description

Seems good, but perhaps simpler to use a groupby here? Something like (untested)

diff --git a/lib/matplotlib/backends/backend_ps.py b/lib/matplotlib/backends/backend_ps.py
index f209e811f1..4d31fd881a 100644
--- a/lib/matplotlib/backends/backend_ps.py
+++ b/lib/matplotlib/backends/backend_ps.py
@@ -7,6 +7,7 @@ import datetime
 from enum import Enum
 import functools
 from io import StringIO
+import itertools
 import logging
 import os
 import pathlib
@@ -626,10 +627,13 @@ grestore
         if ismath:
             return self.draw_mathtext(gc, x, y, s, prop, angle)
 
+        stream = []  # list of (ps_name, x, char_name)
+
         if mpl.rcParams['ps.useafm']:
             font = self._get_font_afm(prop)
+            ps_name = (font.postscript_name
+                       .encode("ascii", "replace").decode("ascii"))
             scale = 0.001 * prop.get_size_in_points()
-            stream = []
             thisx = 0
             last_name = None  # kerns returns 0 for None.
             xs_names = []
@@ -643,37 +647,24 @@ grestore
                 kern = font.get_kern_dist_from_name(last_name, name)
                 last_name = name
                 thisx += kern * scale
-                xs_names.append((thisx, name))
+                xs_names.append((ps_name, thisx, name))
                 thisx += width * scale
-            ps_name = (font.postscript_name
-                       .encode("ascii", "replace").decode("ascii"))
-            stream.append((ps_name, xs_names))
 
         else:
             font = self._get_font_ttf(prop)
             self._character_tracker.track(font, s)
-            stream = []
-            prev_font = curr_stream = None
             for item in _text_helpers.layout(s, font):
                 ps_name = (item.ft_object.postscript_name
                            .encode("ascii", "replace").decode("ascii"))
-                if item.ft_object is not prev_font:
-                    if curr_stream:
-                        stream.append(curr_stream)
-                    prev_font = item.ft_object
-                    curr_stream = [ps_name, []]
-                curr_stream[1].append(
-                    (item.x, item.ft_object.get_glyph_name(item.glyph_idx))
-                )
-            # append the last entry
-            stream.append(curr_stream)
+                glyph_name = item.ft_object.get_glyph_name(item.glyph_idx)
+                stream.append((ps_name, item.x, glyph_name))
 
         self.set_color(*gc.get_rgb())
 
-        for ps_name, xs_names in stream:
+        for ps_name, group in itertools.groupby(stream, lambda entry: entry[0]):
             self.set_font(ps_name, prop.get_size_in_points(), False)
             thetext = "\n".join(f"{x:g} 0 m /{name:s} glyphshow"
-                                for x, name in xs_names)
+                                for _, x, name in group)
             self._pswriter.write(f"""\
 gsave
 {self._get_clip_cmd(gc)}

Edit: In fact set_font itself already checks if the font has already been set previously and skips redundant setfont calls, so we could just entirely get rid of the groupby logic too.

Originally posted by @anntzer in #23910 (comment)

Activity

  1. added this to the v3.7.0 milestone on Sep 20, 2022
  2. added
    🌱 Good first issueOpen a pull request against these issues if there are no active ones!
    Difficulty: Mediumhttps://matplotlib.org/devdocs/devel/contribute.html#good-first-issues
    on Sep 20, 2022
  3. tacaswell commented on Sep 20, 2022

    MemberAuthor

    Labeling this as a good first issue because it is an refactor of a limited portion of the codebase and there is a proposed patch.

    Labeling as medium because the code that needs to be refactored is a complex and care will need to be taken to ensure the refactor behaves identically to the existing code.

  4. story645 commented on Sep 22, 2022

    Member

    xref #23964 since that fixes the issue that spawned this one and therefore any PR here should possibly be aware of that one

  5. 404-P4rziv4L commented on Oct 27, 2022

    Contributor

    If this is issue is still open, I would like to try working on this. Could I be assigned?

  6. oscargus commented on Oct 27, 2022

    Member

    We do not* assign issues, but you are very welcome to work on it!

    • If you find any issues assigned, it is either a core developer assigning themselves as a reminder or assigning another core developer to fix things they have broken earlier.
  7. oscargus commented on Nov 3, 2022

    Member

    Closed in #24287

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Difficulty: Mediumhttps://matplotlib.org/devdocs/devel/contribute.html#good-first-issues🌱 Good first issueOpen a pull request against these issues if there are no active ones!

    Type

    No type

    Projects

    No projects

      Milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions


      Back | FazBrowse Home | New Git URL