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

Add kerning to single-byte strings in PDFs by sauerburger · Pull Request #19582 · matplotlib/matplotlib · GitHub

Repository navigation

Add kerning to single-byte strings in PDFs - #19582

Merged
QuLogic merged 2 commits into
matplotlib:masterfrom
sauerburger:pdf-ascii-string-kerning
Mar 4, 2021
Merged

QuLogic merged 2 commits into
matplotlib:masterfrom
sauerburger:pdf-ascii-string-kerning

Conversation

sauerburger commented Feb 25, 2021 •
edited
Loading

Copy link
Copy Markdown
Member

PR Summary

PR #18517 adds kerning to strings in PDFs if the string contains multi-byte chars. In case of strings consisting only of single-byte chars, the PDF output does not contains kerning (PDF operator Tj). This PR adds kerning to the single-byte case and adds the string using the TJ operator. See also #19581.

PR Checklist

  • Has pytest style unit tests (and pytest passes).
  • Is Flake 8 compliant (run flake8 on changed files to check).
  • New features are documented, with examples if plot related.
  • Documentation is sphinx and numpydoc compliant (the docs should build without error).
  • Conforms to Matplotlib style conventions (install flake8-docstrings and run flake8 --docstring-convention=all).
  • New features have an entry in doc/users/next_whats_new/ (follow instructions in README.rst there).
  • API changes documented in doc/api/next_api_changes/ (follow instructions in README.rst there).

jklymak commented Feb 26, 2021

Copy link
Copy Markdown
Member

Thanks for this! This changes 10 PDF tests, so those images will have to be updated. There is also a flake8 error.

sauerburger force-pushed the pdf-ascii-string-kerning branch 2 times, most recently from b30d609 to 08f03c1 Compare March 1, 2021 16:06

Copy link
Copy Markdown
Member Author

The failing PDF tests were actually a string encoding issue on my side. I've fixed this now, and improved the code style.

Comment thread lib/matplotlib/tests/test_text.py Outdated
Comment thread lib/matplotlib/backends/backend_pdf.py Outdated
sauerburger force-pushed the pdf-ascii-string-kerning branch from 08f03c1 to f179cce Compare March 1, 2021 17:25
sauerburger force-pushed the pdf-ascii-string-kerning branch from f179cce to b555611 Compare March 1, 2021 19:42
QuLogic added this to the v3.4.0 milestone Mar 2, 2021

QuLogic commented Mar 2, 2021

Copy link
Copy Markdown
Member

Don't forget to mark this ready for review.

sauerburger marked this pull request as ready for review March 2, 2021 08:27
Comment thread lib/matplotlib/backends/backend_pdf.py Outdated
Comment thread lib/matplotlib/tests/test_text.py Outdated

tacaswell left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I agree with @QuLogic that we should use the new style for the new images.

Anyone can clear this.

sauerburger force-pushed the pdf-ascii-string-kerning branch from b555611 to 4881758 Compare March 3, 2021 21:39
sauerburger force-pushed the pdf-ascii-string-kerning branch from 4881758 to 1251563 Compare March 3, 2021 22:22
sauerburger requested a review from tacaswell March 3, 2021 22:25
QuLogic dismissed tacaswell’s stale review March 4, 2021 00:37

Fixed test image

QuLogic merged commit 451e329 into matplotlib:master Mar 4, 2021
meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request Mar 4, 2021
tacaswell added a commit that referenced this pull request Mar 4, 2021
…582-on-v3.4.x

Backport PR #19582 on branch v3.4.x (Add kerning to single-byte strings in PDFs)
QuLogic mentioned this pull request Mar 18, 2021
5 tasks done
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants


Back | FazBrowse Home | New Git URL