Skip to content
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

New approach to preventing image overflow in LaTeX (fixes #9660) #9666

Merged
merged 1 commit into from
Jun 23, 2024
Merged

Conversation

jgm
Copy link
Owner

@jgm jgm commented Apr 18, 2024

Instead of relying on graphicx internals, we now define a macro that gets used with images when explicit size information is not provided. This macro prevents the image from overflowing vertically or horizontally.

Templates will have to be updated to include the new macro.

Closes #9660.

The macro should be credited to @mrpiggi. See mrpiggi/svg#60 for discussion.

@tarleb
Copy link
Collaborator

tarleb commented May 10, 2024

An approach like this might make it necessary to reconsider #8249. Or maybe to publish a pandoc CTAN package, as the growing number of custom commands makes it increasingly difficult to maintain custom LaTeX templates.

@jgm
Copy link
Owner Author

jgm commented May 11, 2024

@tarleb I think I'd rather not publish a pandoc CTAN package; that's one more maintenance step required in every release. Big changes like this are relatively rare, but in general one does have to keep an eye on template changes if one uses custom templates.

I don't remember why I haven't merged this yet. I suppose it's because I'm worried about people who will use custom templates with the new pandoc and get errors about undefined \pandocbounded. Do you think we should hold off or merge it? It does solve a real problem.

#8249 is still a cool idea and would largely avoid issues like this; I just haven't gotten around to exploring it.

@tarleb
Copy link
Collaborator

tarleb commented May 11, 2024

I'm torn. On one hand I worry about how much this would impact the usefulness of LaTeX snippets, and how the snippets are becoming more pandoc-specific. On the other hand, I fully agree that this solves a real problem, and I'm aware that some people, all of whom know much more about LaTeX than I do, have been complaining about the current image scaling method.

Adding a plain flag will certainly lead to discussions about what exactly clean LaTeX output looks like, so that's a risky step, too. But I think it would currently be my favored approach.

@jgm
Copy link
Owner Author

jgm commented May 11, 2024

We're already using some pandoc-specific things in our output, e.g. \tightlist.

@jgm
Copy link
Owner Author

jgm commented May 11, 2024

In principle, we could omit the \pandocbounded when writerTemplate is Nothing.
This would make fragments more "plain," at the cost of introducing a potentially confusing mismatch between fragment and standalone output.

@jgm
Copy link
Owner Author

jgm commented Jun 3, 2024

It's also a possibility to patch \includegraphics to add \pandocbounded implicitly, so that we can emit LaTeX that would at least work without special macro definitions. (Against this see the comment mrpiggi/svg#60 (comment) .) This would still require a sizable bit of code in the default template, but it would improve backwards-compatibility with older templates. It's still a bit of a mixed bag, though. With a patched \includegraphics, pandoc would still work with older templates, but we'd lose the ability to entirely defeat the size upper bound by specifying sizes explicitly (unless we made the new definition very complicated and actually parsed options).

Maybe it's okay to do something slightly painful and start using \pandocbounded. I'm just not sure.

@CGMossa
Copy link

CGMossa commented Jun 20, 2024

I've just been facing issues with the pandoc and SVG files, so this would be a welcome change. Thanks for the hardwork on this!

@jgm
Copy link
Owner Author

jgm commented Jun 23, 2024

Note that up to 2014 we did redefine includegraphics in the template: see commit a3cce8d

@jgm jgm force-pushed the issue9660 branch 2 times, most recently from b48c90c to ab875ea Compare June 23, 2024 18:12
@tarleb
Copy link
Collaborator

tarleb commented Jun 23, 2024

I agree that this is preferable over the current system. I can open a new issue to discuss something like a plain extension for LaTeX.

Previously we relied on graphicx internals and made global
changes to Gin to force images to be resized if they exceed
textwidth.  This approach is brittle and caused problems
with `\includesvg` (see #9660).

The new approach uses a new macro `\pandocbounded` that is
now defined in the LaTeX template. (Thanks here to Falk Hanisch in
mrpiggi/svg#60.)

The LaTeX writer has been changed to enclose `\includegraphics`
and `\includesvg` commands in this macro when they don't explicitly
specify a width or height.

In addition, the writer now adds `keepaspectratio` to the
`\includegraphics` or `\includesvg` options if `height` is specified
without width, or vice versa. Previously, this was set in the preamble
as a global option.

Compatibility issues:

- If custom templates are used with the new LaTeX writer, they will have
  to be updated to include the new `\pandocbounded` macro, or an error
  will be raised because of the undefined macro.

- Documents that specify explicit dimensions for an image may render
  differently, if the dimensions are greater than the line width or
  page height. Previously pandoc would shrink these images to fit,
  but the new behavior takes the specified dimensions literally.
  In addition, pandoc previously always enforced `keepaspectratio`,
  even when width and height were both specified, so images with
  width and height specified that do not conform to their intrinsic
  aspect ratio will appear differently.

Closes #9660.
@jgm jgm merged commit 26b25a4 into main Jun 23, 2024
18 of 21 checks passed
@tarleb tarleb deleted the issue9660 branch June 24, 2024 04:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Large SVGs are rendered incorrectly in latex output
3 participants