Skip to content

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Correctly embed fonts in generated pdf #95

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 14 additions & 11 deletions sphinxcontrib/plantuml.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,7 @@ def _split_cmdargs(args):

_ARGS_BY_FILEFORMAT = {
'eps': ['-teps'],
'eps:text': ['-teps:text'],
'png': [],
'svg': ['-tsvg'],
'txt': ['-ttxt'],
Expand All @@ -228,12 +229,14 @@ def generate_plantuml_args(self, node, fileformat):
return args


def render_plantuml(self, node, fileformat):
def render_plantuml(self, node, fileformat, plantuml_type=None):
if not plantuml_type:
plantuml_type = fileformat
Copy link
Collaborator

@yuja yuja Aug 7, 2024 •
edited
Loading

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.

Copy link
Collaborator

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.

Copy link
Author

@albertored albertored Aug 7, 2024 •
edited
Loading

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"

Copy link
Author

Choose a reason for hiding this comment

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

If -teps:text doesn'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

Copy link
Collaborator

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.

Copy link
Author

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

Copy link
Collaborator

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.

refname, outfname = generate_name(self, node, fileformat)
if os.path.exists(outfname):
return refname, outfname # don't regenerate

cachefname = self.builder.plantuml_builder.render(node, fileformat)
cachefname = self.builder.plantuml_builder.render(node, fileformat, plantuml_type)
ensuredir(os.path.dirname(outfname))
# TODO: optionally do symlink/link
shutil.copyfile(cachefname, outfname)
Expand Down Expand Up @@ -285,7 +288,7 @@ def __init__(self, builder):
elif builder.format == 'latex':
fmt = builder.config.plantuml_latex_output_format
if fmt != 'none':
fileformat, _postproc = _lookup_latex_format(fmt)
fileformat, _postproc, _plantuml_type = _lookup_latex_format(fmt)
self.image_formats = [fileformat]

self._known_keys = set()
Expand Down Expand Up @@ -366,7 +369,7 @@ def _render_files(self, keys, fileformat):
else:
raise PlantUmlError('error while running plantuml\n\n%s' % serr)

def render(self, node, fileformat):
def render(self, node, fileformat, plantuml_type):
key = hash_plantuml_node(node)
outdir = os.path.join(self.cache_dir, key[:2])
basename = '%s.%s' % (key, fileformat)
Expand All @@ -382,7 +385,7 @@ def render(self, node, fileformat):
) as f:
try:
p = subprocess.Popen(
generate_plantuml_args(self, node, fileformat),
generate_plantuml_args(self, node, plantuml_type),
stdout=f,
stdin=subprocess.PIPE,
stderr=subprocess.PIPE,
Expand Down Expand Up @@ -615,10 +618,10 @@ def _convert_eps_to_pdf(self, refname, fname):


_KNOWN_LATEX_FORMATS = {
'eps': ('eps', lambda self, refname, fname: (refname, fname)),
'pdf': ('eps', _convert_eps_to_pdf),
'png': ('png', lambda self, refname, fname: (refname, fname)),
'tikz': ('latex', lambda self, refname, fname: (refname, fname)),
'eps': ('eps', 'eps', lambda self, refname, fname: (refname, fname)),
'pdf': ('eps', 'eps:text', _convert_eps_to_pdf),
'png': ('png','png', lambda self, refname, fname: (refname, fname)),
'tikz': ('latex', 'latex', lambda self, refname, fname: (refname, fname)),
}


Expand Down Expand Up @@ -678,8 +681,8 @@ def latex_visit_plantuml(self, node):
if fmt == 'none':
raise nodes.SkipNode
try:
fileformat, postproc = _lookup_latex_format(fmt)
refname, outfname = render_plantuml(self, node, fileformat)
fileformat, plantuml_type, postproc = _lookup_latex_format(fmt)
refname, outfname = render_plantuml(self, node, fileformat, plantuml_type)
refname, outfname = postproc(self, refname, outfname)
except PlantUmlError as err:
logger.warning(str(err), location=node, type='plantuml')
Expand Down
Loading