-
Notifications
You must be signed in to change notification settings - Fork 45
Correctly embed fonts in generated pdf #95
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
base: master
Are you sure you want to change the base?
Conversation
yuja
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
| def render_plantuml(self, node, fileformat): | ||
| def render_plantuml(self, node, fileformat, plantuml_type=None): | ||
| if not plantuml_type: | ||
| plantuml_type = fileformat |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of adding separate fileformat, plantuml_type parameters, maybe eps:text can be mapped to <filename>.text.eps (i.e. 'text.eps': ['-teps:text'])?
Suppose -teps and -teps:text generate different contents, the output filenames should be unique.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, but it might break batch rendering where the output file names are controlled by plantuml.
If -teps:text doesn't break existing users, maybe it's okay to always use -teps:text.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be an option available on config? Like plantuml_latex_output_format = "eps:text"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If
-teps:textdoesn't break existing users, maybe it's okay to always use-teps:text.
Depends on the definition of "breaking". For sure it can lead to a different output
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that seems good if we need to support both eps formats.
If eps:text is strictly better in latex rendering, I think it's good to use it by default. eps format wouldn't be used for the other purposes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In alternative I can add a new option, something like plantuml_latex_pdf_embed_fonts = True (default to False as today) that if given uses -teps:text instead of -teps
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have no preference over adding option vs changing the default. If we add an option, it will be something like plantuml_eps_embed_fonts so that all <hash>.eps files of the same name are rendered with the same configuration.
With this change the eps generated as a middle step to have pdf files as output are generated with the
-teps:textflag on plantuml. See https://plantuml.com/eps for reference.In this way text is not rendered as outlines in eps file but it keeps the font indication allowing epstopdf to correctly embed font in the final pdf