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

mplcairo-0.1 #6

Closed
cgohlke opened this issue Jul 23, 2018 · 27 comments
Closed

mplcairo-0.1 #6

cgohlke opened this issue Jul 23, 2018 · 27 comments

Comments

@cgohlke
Copy link

cgohlke commented Jul 23, 2018

I noticed a few issues with mplcairo-0.1:

  1. the Windows wheel is missing freetype.dll.

  2. the file mplcairo.pth seems wrong, containing literal "\n" instead of line breaks.

  3. on Windows, I get a segfault when closing the Python interpreter window after executing import mplcairo. I am unable to get a stacktrace.

  4. the documentation claims that the cairo binaries at https://www.lfd.uci.edu/~gohlke/pythonlibs/ are not built with freetype support, however I am sure they are.

@anntzer
Copy link
Collaborator

anntzer commented Jul 23, 2018

the Windows wheel is missing freetype.dll.

I think "preshing's cairo.dll" (https://github.com/preshing/cairo-windows/releases) statically links it. See also (3).

the file mplcairo.pth seems wrong, containing literal "\n" instead of line breaks.

This is intentional (the pth trick, see e.g. https://www.reddit.com/r/Python/comments/1g94vc/are_pth_file_hacks_elegant_or_evil/; I use exec("some literal\nstring with newlines") to be able to exec, well, multiple lines).

on Windows, I get a segfault when closing the Python interpreter window after executing import mplcairo. I am unable to get a stacktrace.

The following works for me:
Create a clean conda env with just Py3.6, run pip install mplcairo (pulling in matplotlib as dependency), then:

set MPLBACKEND=module://mplcairo.tk
python -c "from pylab import *; plot(); show()"

(one can print(gcf().canvas) to check from the canvas type that mplcairo is indeed active); this also confirms that the freetype symbols are available. Similarly, qt works (pyqt needs to be installed from conda -- the PyPI wheels have never been compatible with conda's python in my experience.

One can also do

set MPLBACKEND=tkagg
set MPLCAIRO_PATCH_AGG=1
python -c "from pylab import *; print(gcf().canvas.renderer)"

to check that the pth file is working.

the documentation claims that the cairo binaries at lfd.uci.edu/~gohlke/pythonlibs are not built with freetype support, however I am sure they are.

Indeed. Did this change recently? That part of the docs was written a few months ago and I am happy to fix it (not that this changes anything to the build strategy I think(?), as I still cannot "steal" the symbols from _cairo.cpXX-win_amd64.pyd if I understand Windows symbol resolution correctly).

@cgohlke
Copy link
Author

cgohlke commented Jul 23, 2018

the Windows wheel is missing freetype.dll.

I think "preshing's cairo.dll"
(https://github.com/preshing/cairo-windows/releases) statically links
it. See also (3).

The _mplcairo C extension is linked to freetype.dll

the file mplcairo.pth seems wrong, containing literal "\n" instead
of line breaks.

This is intentional (the pth trick, see e.g.
https://www.reddit.com/r/Python/comments/1g94vc/are_pth_file_hacks_elegant_or_evil/;
I use |exec("some literal\nstring with newlines")| to be able to exec,
well, multiple lines).

I see. I missed that the multi-line statements were in an exec().

on Windows, I get a segfault when closing the Python interpreter
window after executing import mplcairo. I am unable to get a stacktrace.

The following works for me:

I was not reporting that mplcairo does not function (it seems to works well!) but that simply importing mplcairo leads to segfaults when closing the Python interpreter window without quitting the interpreter first (at least it does on my computer).

the documentation claims that the cairo binaries at
lfd.uci.edu/~gohlke/pythonlibs are not built with freetype support,
however I am sure they are.

Indeed. Did this change recently? That part of the docs was written a
few months ago and I am happy to fix it (not that this changes anything
to the build strategy I think(?), as I still cannot "steal" the symbols
from |_cairo.cpXX-win_amd64.pyd| if I understand Windows symbol
resolution correctly).

No, I did not change this recently. I was just wondering if there was a specific reason why those cairo binaries should not work. Anyway, it's probably best to leave it to the distributor of binaries which freetype enabled cairo library to link/ship.

@anntzer
Copy link
Collaborator

anntzer commented Jul 24, 2018

The _mplcairo C extension is linked to freetype.dll

So overall, is there a problem or something that needs to be fixed? I don't think I follow your point.

I was not reporting that mplcairo does not function (it seems to works well!) but that simply importing mplcairo leads to segfaults when closing the Python interpreter window without quitting the interpreter first (at least it does on my computer).

Oh, I see, I misread the original message. I can reproduce this (on Windows only), or rather I can get the "Python stopped working" window pop up. Likely some stuff that gets torn down in the wrong order? I don't have experience with debugging on Windows so any insights would be welcome here.

Edit: I figured out that this happens during deallocation of the MathTextParser class (tstate is NULL at line 1135 of cpython 3.6.6). I guess the class is deallocated too late in the Python shutdown so other machinery has already been torn down, but it's not clear why.

No, I did not change this recently. I was just wondering if there was a specific reason why those cairo binaries should not work. Anyway, it's probably best to leave it to the distributor of binaries which freetype enabled cairo library to link/ship.

pycairo binaries are not useful on Windows anyways because I can't steal the symbols from them (unlike on POSIX systems, where after importing (at the Python level) pycairo, the cairo symbols become available to everyone). So in a sense the whole discussion in the README was indeed written in a somewhat confusing way. What I should have referred to is anaconda and conda-forge's cairo (not pycairo), which on Windows is built without FreeType support and thus cannot be used. AFAICT you do not distribute cairo (not pycairo) binaries so I didn't need to mention you for that specific point. I have updated the README, does it look clearer now?

You can point to your cairo of choice by setting the LINK environment variable, as documented in https://github.com/anntzer/mplcairo#windows.


As a side note, if you have some time to spare and are interested in looking into it, generating wheels with Raqm support (for complex text layout, e.g. Arabic and Hebrew) would be greatly appreciated (likely relevant code in _raqm.cpp and _os.cpp, I think just dumping raqm.dll next to every other file should be sufficient).

@cgohlke
Copy link
Author

cgohlke commented Jul 24, 2018

the Windows wheel is missing freetype.dll.

The _mplcairo C extension is linked to freetype.dll

So overall, is there a problem or something that needs to be fixed? I don't think I follow your point.

freetype.dll needs to be included in the official wheels. Or linked statically.

@anntzer
Copy link
Collaborator

anntzer commented Jul 24, 2018

Why? The official wheels include cairo.dll, which statically link freetype.
If a downstream redistributor (e.g. yourself) wants to ship their own build of cairo.dll which does not statically link freetype, then they should start from the sdist and it is their job to also include freetype.dll in the wheel -- I think? (If you need to add an optional copy of freetype.dll at https://github.com/anntzer/mplcairo/blob/master/setup.py#L156, then I can do it of course.)

@cgohlke
Copy link
Author

cgohlke commented Jul 24, 2018

Why?

Because the _mplcairo.cp36-win_amd64.pyd C extension itself is linked to freetype.dll (in addition to cairo.dll) and does not load without freetype.dll

>py -3.6 -m pip install mplcairo
Collecting mplcairo
  Downloading https://files.pythonhosted.org/packages/6a/70/45f26f57cfd23f4bfaa63c24562cd523144071f3fcea11f7de85d3122a5b/mplcairo-0.1-cp36-cp36m-win_amd64.whl (1.3MB)
    100% |████████████████████████████████| 1.3MB 6.6MB/s
Requirement already satisfied: matplotlib>=2.2 in x:\python36\lib\site-packages (from mplcairo) (2.2.2)
Requirement already satisfied: pybind11>=2.2.3 in x:\python36\lib\site-packages (from mplcairo) (2.2.3)
Requirement already satisfied: python-dateutil>=2.1 in x:\python36\lib\site-packages (from matplotlib>=2.2->mplcairo) (2.7.3)
Requirement already satisfied: pyparsing!=2.0.4,!=2.1.2,!=2.1.6,>=2.0.1 in x:\python36\lib\site-packages (from matplotlib>=2.2->mplcairo) (2.2.0)
Requirement already satisfied: kiwisolver>=1.0.1 in x:\python36\lib\site-packages (from matplotlib>=2.2->mplcairo) (1.0.1)
Requirement already satisfied: pytz in x:\python36\lib\site-packages (from matplotlib>=2.2->mplcairo) (2018.5)
Requirement already satisfied: six>=1.10 in x:\python36\lib\site-packages (from matplotlib>=2.2->mplcairo) (1.11.0)
Requirement already satisfied: numpy>=1.7.1 in x:\python36\lib\site-packages (from matplotlib>=2.2->mplcairo) (1.14.5+mkl)
Requirement already satisfied: cycler>=0.10 in x:\python36\lib\site-packages (from matplotlib>=2.2->mplcairo) (0.10.0)
Requirement already satisfied: setuptools in x:\python36\lib\site-packages (from kiwisolver>=1.0.1->matplotlib>=2.2->mplcairo) (39.2.0)
Installing collected packages: mplcairo
Successfully installed mplcairo-0.1

>py -3.6 -c"import mplcairo"
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "X:\Python36\lib\site-packages\mplcairo\__init__.py", line 24, in <module>
    from ._mplcairo import antialias_t, operator_t, get_options, set_options
ImportError: DLL load failed: The specified module could not be found.

@anntzer
Copy link
Collaborator

anntzer commented Jul 24, 2018

conda create -yn tmpenv & activate tmpenv & pip install mplcairo & python -c "import mplcairo; print(mplcairo)" works for me...
Should I try a non-conda install?

@anntzer
Copy link
Collaborator

anntzer commented Jul 24, 2018

A clean non-conda install (from python.org) also works.

@cgohlke
Copy link
Author

cgohlke commented Jul 24, 2018

A clean non-conda install (from python.org) also works.

You probably have freetype.dll somewhere in the DLL search path. Try where freetype.dll.

@anntzer
Copy link
Collaborator

anntzer commented Jul 24, 2018

Ah, I see, I was implicitly relying on freetype.dll being provided by conda's matplotlib package (or rather its dependency freetype) that was installed in the root environment. Thanks for pointing that out!

I plan to just grab freetype.dll from conda and put it next to cairo.dll. Sounds good?

@anntzer
Copy link
Collaborator

anntzer commented Jul 24, 2018

How does the wheel at https://ci.appveyor.com/project/anntzer/mplcairo/build/artifacts look for you?

@anntzer
Copy link
Collaborator

anntzer commented Aug 13, 2018

Interestingly, on Linux, I can get the same segfault on Py3.7 (not Py3.6) by importing both mplcairo and pandas into the same IPython (not plain python), and then exiting IPython (again, tstate is null when deallocating MathTextParser).

@anntzer
Copy link
Collaborator

anntzer commented Aug 14, 2018

@cgohlke Can you check whether the following patch fixes the issue re: segfault at shutdown for you? It does fix the issue I described in the previous comment, and the suggested cause (see comment) at least seems plausible...

diff --git i/src/_mplcairo.cpp w/src/_mplcairo.cpp
index 8fbc1ef..bc90c6c 100644
--- i/src/_mplcairo.cpp
+++ w/src/_mplcairo.cpp
@@ -1514,19 +1514,22 @@ PYBIND11_MODULE(_mplcairo, m)
     py::module::import("matplotlib.mathtext")
     .attr("MathTextParser")("mplcairo");
 
-  auto cleanup = py::cpp_function{
-    [&](py::handle weakref) -> void {
-      FT_Done_FreeType(detail::ft_library);
-
-      // Make sure that these objects don't outlive the Python interpreter.
-      detail::UNIT_CIRCLE = {};
-      detail::PIXEL_MARKER = {};
-      GraphicsContextRenderer::mathtext_parser_ = {};
-
-      weakref.dec_ref();
+  py::module::import("atexit").attr("register")(
+    py::cpp_function{
+      [&]() -> void {
+        FT_Done_FreeType(detail::ft_library);
+        // Make sure that these objects don't outlive the Python interpreter.
+        // (It appears that sometimes, a weakref callback to the module doesn't
+        // get called at shutdown, so if we rely on that approach instead of
+        // using `atexit.register`, we may get a segfault when Python tries to
+        // deallocate the type objects (via a decref by the C++ destructor) too
+        // late in the shutdown sequence.)
+        detail::UNIT_CIRCLE = {};
+        detail::PIXEL_MARKER = {};
+        GraphicsContextRenderer::mathtext_parser_ = {};
+      }
     }
-  };
-  py::weakref(m, cleanup).release();
+  );
 
   // Export symbols.
 

@cgohlke
Copy link
Author

cgohlke commented Aug 14, 2018

Can you check whether the following patch fixes the issue re: segfault at shutdown for you?

Unfortunately not. I just tried mplcairo-0.1.post6+g5a4ec01.d20180814 with your patch on Python 3.7.

@anntzer
Copy link
Collaborator

anntzer commented Aug 16, 2018

Reported the issue upstream to pybind11 (I can repro it independently of mplcairo), let's see if something comes out of it. If not, I can probably make the MathTextParser instance live at Python-level and refetch it everytime (the cost is negligible compared to the actual parse), which may also help...

@anntzer
Copy link
Collaborator

anntzer commented Aug 17, 2018

Actually I just wrote d7390dd, which should fix (work around) the issue. Can you give it a try?

@anntzer
Copy link
Collaborator

anntzer commented Sep 10, 2018

I think all issues are fixed now (on master), feel free to request a reopen if not.

@anntzer anntzer closed this as completed Sep 10, 2018
@anntzer
Copy link
Collaborator

anntzer commented Oct 31, 2018

@cgohlke I realized that in the 0.1 release, I was not loading the function pointers for vector output (matplotlib/matplotlib#12636); upon fixing this (469de4d), it appears that the cairo.dll build I'm using (from https://github.com/preshing/cairo-windows) gives blank output when rendering pdfs (already reported to the packager: https://preshing.com/20170529/heres-a-standalone-cairo-dll-for-windows/#IDComment1047546463).
If you have a chance, can you check whether your build (using mplcairo master) renders pdfs correctly? (Any trivial plot, e.g. plt.plot(); plt.savefig("foo.pdf") would do.) If it does, I'll make a new release.

@cgohlke
Copy link
Author

cgohlke commented Oct 31, 2018

check whether your build (using mplcairo master) renders pdfs correctly?

This works for me (is that what you meant?):

import matplotlib, mplcairo
matplotlib.use('cairo')
from matplotlib import pyplot as plt
plt.plot(); plt.savefig("foo.pdf")

@anntzer
Copy link
Collaborator

anntzer commented Oct 31, 2018

Does the pdf contain a plot, or is it blank? (I get a blank pdf with my build.)

If this works, would you mind if I put the wheels you provide on PyPI? I will acknowledge you, of course.

@nyanpasu64
Copy link

Windows 10 x64, Python 3.6.6 x64, matplotlib 3.0.2, mplcairo 0.1.

poetry add mplcairo

    from ._mplcairo import antialias_t, operator_t, get_options, set_options
ImportError: DLL load failed: The specified module could not be found.

@anntzer
Copy link
Collaborator

anntzer commented Jan 2, 2019

As noted in this issue the Windows 0.1 wheel is actually broken unless you are using conda's matplotlib.
I need to fix a few things before making a new release, which will take a bit of time. In the meantime I would suggest using the wheels kindly provided by @cgohlke at https://www.lfd.uci.edu/~gohlke/pythonlibs/#mplcairo, looks like python-poetry/poetry#76 (for example) documents how to do this in poetry.

@nyanpasu64
Copy link

Can you repackage those wheels and upload them to PyPi as 0.1.1-ish for Windows only?

https://www.lfd.uci.edu/~gohlke/pythonlibs/#mplcairo isn't a PyPi index (and uses Javascript obfuscation to prevent programmatic downloading), and I don't want to package 4 different wheels and select one at install time (and I don't know if Poetry understands that either).

(On Linux, I have to apt-get install libcairo2-dev. Is this a hard requirement for mplcairo, or an artifact of pycairo/swig (don't remember if needed?)

@anntzer
Copy link
Collaborator

anntzer commented Jan 5, 2019

Can you check whether mplcairo-0.1-1-cp36-cp36m-win_amd64.whl at https://github.com/anntzer/mplcairo/releases/tag/v0.1 works for you? If yes it'll go on PyPI.

mplcairo naturally depends on cairo. On Unix systems, I decided to push the "how to find cairo" problem to pycairo (mostly because downstream packagers would rip out libcairo.so if I bundled it in anyways). Right now there's no pycairo manylinux wheels, so you need to get the dependencies from apt (for example). If you want to change that, you should comment on the pycairo tracker.

@nyanpasu64
Copy link

the 64-bit wheel, I think cp37, works for me. (will you upload all 4?)

It seems that mplcairo _util.h needs Cairo headers.

@anntzer
Copy link
Collaborator

anntzer commented Jan 5, 2019

I don't want to upload wheels I cannot build myself (or via CI).
The wheel on github right now is a 64bit py36 wheel, I'll build a 64bit py37 wheel next week, but I don't have access to a 32bit machine so for these you'll have to use Gohlke's wheels.

You may be able to use a local wheel index (https://stackoverflow.com/questions/51873465/pip-install-from-wheel-and-repository), not sure if poetry knows how to handle that but that belongs to their tracker.

If you're building from source then you'll need the cairo headers for sure.

@anntzer
Copy link
Collaborator

anntzer commented Jan 7, 2019

Added a py37 wheel to https://github.com/anntzer/mplcairo/releases/tag/v0.1. Please confirm it works for you.

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

No branches or pull requests

3 participants