Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Fix baseline alignment when using usetex.#16476
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
698df42 to497738cCompare| def test_usetex(): | ||
| mpl.rcParams['text.usetex'] = True | ||
| fig = plt.figure() | ||
| ax = fig.add_subplot(111) | ||
| ax.text(0.1, 0.2, | ||
| kwargs = {"verticalalignment": "baseline", "size": 24, |
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.
This test is significantly better 👍
(I'll combine this with#16410 as a single PR for the usual baseline image changes reason.) |
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.
Feel free to ignore my comments, but this is full of so much state....
| # push <= here, v is the baseline position. | ||
| # etc. | ||
| # (dviasm is useful to explore this structure.) | ||
| self._baseline_v = None |
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.
do we want to put this in__init__ as well? Are we sure that_output will never be called without_read being called first?
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.
we'd already have errored out earlier anyways, because self.text wouldn't have been defined either.
Uh oh!
There was an error while loading.Please reload this page.
b1c5b53 toa494553Compare@tacaswell can you repro the failure locally? |
I can reproduce locally, though it's a slightly different RMS. |
Can you try regen'ing the baseline image locally and force-push? probably something about the version of freetype that dvipng is using... |
Problem is, now it's also doing: |
Strange, I can't repro that failure locally. Tried rebasing on master, and also deleting ~/.cache/mpl/test_cache, still passing... |
I guess an additional diff --git i/lib/matplotlib/dviread.py w/lib/matplotlib/dviread.pyindex bb14480e4..dcaba415a 100644--- i/lib/matplotlib/dviread.py+++ w/lib/matplotlib/dviread.py@@ -278,6 +278,9 @@ class Dvi: maxy_pure = self._baseline_v # This should normally be the case. self._baseline_v = None+ if not self.text and not self.boxes: # Avoid infs/nans from inf+/-inf.+ return Page(text=[], boxes=[], width=0, height=0, descent=0)+ if self.dpi is None: # special case for ease of debugging: output raw dvi coordinates return Page(text=self.text, boxes=self.boxes, may help, but I still don't know why I don't have an issue locally. numpy version, perhaps? |
My environment has lowest NumPy supported, 1.15.0. |
Does the proposed patch help? |
Yes, it works. |
Looks like this still failed on the image diff. Do you know if we can download the test artefacts from e.g. azure to check the image and possibly just dump it in the baselines directory? |
But then it would just fail for everyone else off CI? #17011 for Azure artifacts, if it works. |
At least I want to know if it's just a freetype version problem (in which case we can just bump the tolerance) or something more serious. |
Looks like freetype-version related, will work on that... |
Looks like forcing freetype-less dvipng mode fixes the issue. It's a bit annoying to expose testing bits in texmanager, but I'm not really sure I want to expose that more because I have a further plan to just get rid of the dependency on dvipng altogether (for raster output we can actually just use the parsed dvi (as we already do for vector output) to extract the glyphs and draw them ourselves using our own freetype renderer -- that's what's done in mplcairo for example). This will skip a subprocess call (always good on Windows in particular), and of the FreeType version problem (we don't control the FreeType version used by dvipng, and FreeType-less mode is broken in dvipng 1.16 -- fixed in the most recent 1.17 and was good before 1.16 too, but version bounds on dvipng for tests only may be a bit painful...). Or we can first wait forthat feature to go in... |
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Unfortunately at least some dvipngs appear to be statically linked toFreeType so we can't play tricks with LD_PRELOAD/LD_LIBRARY_PATH.
Currently, `figtext(.5, .5, "%foo", usetex=True)` results in an obscureexception (`FileNotFoundError: No such file or directory: blah.dvi`).Fix that by forcing tex to always generate a page, by inserting at leastan empty \hbox{}.Previously we inferred the baseline of usetex strings with the baselineof the character with the lowest baseline, which is wrong in thepresence of indices (see usetex_baseline_test.py). There was an optionto use the "preview" latex package to fix that, but that was never madethe default and likely cannot as the package is GPL.Instead I can infer the baseline position by "reverse-engineering"the starting instructions of the dvi stream (this was done withoutconsulting the approach in "preview"). The results can be checkedusing usetex_baseline_test.py (preview=False now looks the same aspreview=True; likewise, the output of test_usetex was changed.The text.latex.preview rc can now be deprecated (in a future PR).The test was also updated to include the strings of theusetex_baseline_test example (which may go away once text.latex.previewis removed).
Fix baseline alignment when using usetex.Conflicts:lib/matplotlib/tests/test_usetex.py - import np and formatting issues on decorator
Uh oh!
There was an error while loading.Please reload this page.
Previously we inferred the baseline of usetex strings with the baseline
of the character with the lowest baseline, which is wrong in the
presence of indices (see usetex_baseline_test.py). There was an option
to use the "preview" latex package to fix that, but that was never made
the default and likely cannot as the package is GPL.
Instead I can infer the baseline position by "reverse-engineering"
the starting instructions of the dvi stream (this was done without
consulting the approach in "preview"). The results can be checked
using usetex_baseline_test.py (preview=False now looks the same as
preview=True; likewise, the output of test_usetex was changed.
(see#16373)
The text.latex.preview rc can now be deprecated (in a future PR).
The test was also updated to include the strings of the
usetex_baseline_test example (which may go away once text.latex.preview
is removed). (The baseline image was already changing, in any case...)
I think the test is "spuriously" failing because my local version of latex tools are not compiled against the same freetype version as on CI :/ Basically the baseline image will need to be regen'd by someone who can otherwise already pass test_usetex.py locally.
Edit: Now also includes#16410 as first commit (that PR has been closed), socloses#16409.
... but still can't repro the failure locally.
PR Summary
PR Checklist