Skip to content

Autodetect whether pgf can use \includegraphics[interpolate]. #15150

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 4, 2019

Conversation

anntzer
Copy link
Contributor

@anntzer anntzer commented Aug 28, 2019

Closes #10963 (see discussion there); replaces #10973.

This is done by reusing the same LatexManager instance as the one used
for measuring text extents.

As an aside, this exposes an awkwardness in the implementation of
LatexManager -- after an error occurs, the instance needs to be
discarded (ideally, the instance would auto-create a new subprocess).
This is why _get_image_inclusion_command manually discards the cached
instance in such a case.

For recent setups where \includegraphics[interpolate=true] is available,
the cost of this PR is essentially just an additional
\usepackage{graphicx} executed once; for older setups, an additional
instantiation of a latex subprocess (only once).

This can't be tested on CI because Travis' version of latex is too old,
but can be manually tested by inspecting a resulting pgf output, and
possibly changing interpolate=true by a nonexistent option (e.g.
foobar and checking that there's fallback to \pgfimage in that case.

PR Summary

PR Checklist

  • Has Pytest style unit tests
  • Code is Flake 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

This is done by reusing the same LatexManager instance as the one used
for measuring text extents.

As an aside, this exposes an awkwardness in the implementation of
LatexManager -- after an error occurs, the instance needs to be
discarded (ideally, the instance would auto-create a new subprocess).
This is why _get_image_inclusion_command manually discards the cached
instance in such a case.

For recent setups where \includegraphics[interpolate=true] is available,
the cost of this PR is essentially just an additional
\usepackage{graphicx} executed once; for older setups, an additional
instantiation of a latex subprocess (only once).

This can't be tested on CI because Travis' version of latex is too old,
but can be manually tested by inspecting a resulting pgf output, and
possibly changing `interpolate=true` by a nonexistent option (e.g.
`foobar` and checking that there's fallback to \pgfimage in that case.
@tacaswell tacaswell requested a review from pwuertz August 28, 2019 11:37
@tacaswell tacaswell added this to the v3.2.0 milestone Aug 28, 2019
@pwuertz
Copy link
Contributor

pwuertz commented Aug 28, 2019

Ubuntu Bionic seems to be available on Travis now. Is there still reason to support those very old TeX distributions where \includegraphics fails?

@anntzer
Copy link
Contributor Author

anntzer commented Aug 28, 2019

Either way works for me, if you can make travis pass with includegraphics.

@tacaswell
Copy link
Member

I would also be 👍 on documenting the minimum versions of latex required for PGF, skipping the test if we can detect a too-old latex so long as at least 1 CI runs the test.

@tacaswell
Copy link
Member

On a bit more consideration, the complexity to support both old and new versions of latex is not great so we might as well. I have a slight preference for this PR over #10973.

@jklymak jklymak merged commit 2cb6bc7 into matplotlib:master Sep 4, 2019
@anntzer anntzer deleted the pgfincludegraphics branch September 4, 2019 17:49
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.

Replace \pgfimage by \includegraphics in PGF backend
4 participants