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

Add entry for .notdef to CharStrings for type 42 fonts in eps files. … by selasley · Pull Request #9059 · matplotlib/matplotlib · GitHub

  1. matplotlib
  2. matplotlib

Repository navigation

Add entry for .notdef to CharStrings for type 42 fonts in eps files. … - #9059

Merged
efiring merged 1 commit into
matplotlib:masterfrom
selasley:ps-type42-fix
Aug 21, 2017
ConversationCommits (1)ChecksFiles changed
Merged

Add entry for .notdef to CharStrings for type 42 fonts in eps files. …#9059
efiring merged 1 commit into
matplotlib:masterfrom
selasley:ps-type42-fix

Conversation

selasley commented Aug 18, 2017

Copy link
Copy Markdown
Contributor

… See issue #9044

PR Summary

According to the Postscript Language Ref CharStrings dictionaries for Type 42 fonts "must have an entry whose key is .notdef". The lack of this entry caused several macOS apps to fail to open eps files created with ps.fonttype = 42. This PR modifies ppdrvr_tt.cpp to add an entry for .notdef to the CharStrings dict.

#9044

PR Checklist

  • Has Pytest style unit tests
  • Code is PEP 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

dopplershift added this to the 2.1 (next point release) milestone Aug 18, 2017
dopplershift added the Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. label Aug 18, 2017
tacaswell reviewed Aug 20, 2017
View reviewed changes
Comment thread extern/ttconv/pprdrv_tt.cpp Outdated
/* Emmit the start of the PostScript code to define the dictionary. */
stream.printf("/CharStrings %d dict dup begin\n", glyph_ids.size());
stream.printf("/CharStrings %d dict dup begin\n", glyph_ids.size()+1);
/* PS Lang Ref says ChatStrings must contain an entry for .notdef */

tacaswell Aug 20, 2017

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

Should this be 'CharStrings'?

tacaswell Aug 20, 2017

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

It is section 5.8.2 table 5.7 that has this requirement.

selasley Aug 20, 2017

Copy link
Copy Markdown
Contributor Author

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

Yes, thanks for catching that and for the section / table suggestion. I tried fixing/squashing/pushing. I haven't done many PRs, so if I made a mistake and you can fix it please do. Thanks for matplotlib. I use it and pandas at work instead of IDL and it makes my work easier to do

tacaswell commented Aug 20, 2017

Copy link
Copy Markdown
Member

@selasley You are a hero for tracking this down! I just left 2 small doc-related comments (a typo and can you add the section/table information). I can also take care of this and push to your branch if you want.

tacaswell requested changes Aug 20, 2017
View reviewed changes

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

typo in docs + reference to PS spec

tacaswell commented Aug 20, 2017

Copy link
Copy Markdown
Member

@mdboom Is there an upstream we should push this to?

efiring merged commit 21bbf8c into matplotlib:master Aug 21, 2017

tacaswell commented Aug 21, 2017

Copy link
Copy Markdown
Member

@selasley I think this is your first matplotlib contribution (sorry if I am wrong). Congratulations 🎉

Hopefully we will hear from you again!

Thank you again for tracking this down.

durack1 commented Aug 23, 2017

Copy link
Copy Markdown

@tacaswell @efiring when will these changes make their way into a conda release - do you guys mint nightlies?

efiring commented Aug 24, 2017

Copy link
Copy Markdown
Member

No nightlies. 2.1rc1 is expected by the end of the month, though.

dstansby mentioned this pull request Aug 25, 2017
Closed
selasley deleted the ps-type42-fix branch August 25, 2017 15:38
tacaswell mentioned this pull request Sep 1, 2017
Closed
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
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

tacaswell tacaswell approved these changes

Assignees

No one assigned

Labels

Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions.

Projects

None yet

Milestone

v2.1

Development

Successfully merging this pull request may close these issues.

5 participants

Back | FazBrowse Home | New Git URL