Skip to content

Correctly position text with nonzero descent with afm fonts / ps output. #18519

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.

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
Sep 18, 2020

Conversation

anntzer
Copy link
Contributor

@anntzer anntzer commented Sep 18, 2020

See e.g.

rcParams["ps.useafm"] = True
text(.5, .5, "qk")
axhline(.5)
savefig("/tmp/test.ps")

Previously the bottom of the tip of the "q" would be on the baseline;
now the baseline is correctly positioned.

(I picked a relatively generic name for the test because we could later
update the baseline image "in place" for more afm tests.)

PR Summary

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 pydocstyle<4 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).

See e.g.
```
rcParams["ps.useafm"] = True
text(.5, .5, "qk")
axhline(.5)
savefig("/tmp/test.ps")
```
Previously the bottom of the tip of the "q" would be on the baseline;
now the baseline is correctly positioned.

(I picked a relatively generic name for the test because we could later
update the baseline image "in place" for more afm tests.)
@tacaswell
Copy link
Member

This line came in via 839434c with the comment that it is fixing an aligment issues, but it is not clear what issues. The code around this has also been changed quite a bit as we used to have draw_text and draw_unicode and now just draw_text (they got merged in f9a68c1).

I agree that the proposed change is correct and it is not worth doing further digging to sort out what was being fixed previously.

@jklymak jklymak merged commit 8349677 into matplotlib:master Sep 18, 2020
@anntzer anntzer deleted the afmdescent branch September 18, 2020 17:29
@@ -0,0 +1,66 @@
%!PS-Adobe-3.0 EPSF-3.0
%%Title: /home/antony/src/extern/matplotlib/result_images/test_backend_ps/useafm.eps
Copy link
Member

Choose a reason for hiding this comment

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

This seems a bit leaky; I wonder if we should trim the title to just the filename?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As a general matter (i.e. not just for tests), I would say yes.

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.

4 participants