search

drufat / triangle

Python bindings to the triangle library
GNU Lesser General Public License v3.0
224 stars 53 forks source link

Crashing in Windows with Python 3.5 and 3.6 #17

Closed cedh4c closed 7 years ago

cedh4c commented 7 years ago

Tried to install via pip, easy_install, and using the binaries on the gohlke page with py35 and py36 (Anaconda3). In all cases, compiling/install goes fine, I can import triangle in python but then get a crash with any call to a function (for example convexhull on the example). Event logger reports an access violation from the core.xx.pyd library. If I install with py27, works fine. Is this another VC version problem?

cgohlke commented 7 years ago

triangle.c uses C long type to store pointer addresses. On Windows, that will work for 32 bit binaries, but it is not correct for 64-bit. With the old VS2008 CRT you might get away with that depending on the application/use, but not with the new UCRT used by Python >=3.5.

drufat commented 7 years ago

I am not using Windows so there is no way for me to test this, but if you would like to submit a pull request which fixes the issue for Windows users, I will gladly merge it.

cgohlke commented 7 years ago

Without unit tests I don't feel confident to submit a PR.

There's a patch at https://github.com/wo80/Triangle/commit/6a87bb006c3a0de22615c041b89ed7a1f8013bfb. That project also contains other improvements/changes to the triangle library.

cedh4c commented 7 years ago

I hate the new UCRT so much... I'm currently tied to Windows but will switch my project to Linux as soon as I can. I've tried to do mods to make it work with UCRT (py35 and py36) but I've already wasted enough time on UCRT and will leave it for later. If I make progress on this, I'll come back to report.

I can confirm this repo works with py34 on Windows, as is.

I'm a bit confused regarding this wo80 repository. There seem to be an "original" Triangle static library, which differs a lot from what is used in this present repo (drufat's). Then, there is the dynamic API, which sounds quite appealing although it will take some work to get the python bindings done.

By the way, cgohlke, I must thank you for your fantastic binary library page for Python. It's been (and continues to be) extremely useful. Similarly, drufat, thanks a lot for these bindings.

cgohlke commented 7 years ago

@cedh: please try the new builds at http://www.lfd.uci.edu/~gohlke/pythonlibs/#triangle. They are using unsigned long long instead of unsigned long for handling pointers on 64-bit. I'll submit a PR if this fixes your issues.

cedh4c commented 7 years ago

@cgohlke I just tried and it looks like it works both in py35 and py36. Thanks so much!

I did try to replace the unsigned long but made the mistake of using ULONG_PTR that I found somewhere in msdn... It caused me problems of undefined symbol because cl couldn't find the right headers for some reason (go figure, the path to the correct include dir was there). It was the last straw after a week end of dealing with other libraries breaking in py35/6 because of the new UCRT. But I learned something today regarding the unsigned long long, so not a complete waste!

cgohlke commented 7 years ago

Should be fixed by PR #18