fonttools / pyclipper

Cython wrapper for the C++ translation of the Angus Johnson's Clipper library (ver. 6.4.2)
MIT License
233 stars 42 forks source link

New Clipper version (6.4.2) #10

Closed athos-ribeiro closed 6 years ago

athos-ribeiro commented 7 years ago

Hello,

I am preparing this package to include it in Project Fedora repositories. I would like to know if you are willing to update Clipper to the latest version here (I patched the project to use system libraries and will provide an option for that if you would like it).

greginvm commented 7 years ago

Hi @athos-ribeiro ,

will check it out in the following week, I'll let you know the status of this in a few days. For the patched version - can you link the fork here?

athos-ribeiro commented 7 years ago

Thank you for the repy, Gregor.

This [1] is all I have so far. It is just a patch I am applying to the RPM package to use the system library instead of the bundled one. I was thinking of adding some option or lock file to trigger that behaviour, any comments on that?

[1] https://github.com/athos-ribeiro/pyclipper/commit/01527c0c122c5c52632ef937b4affbed15f99b34

QuLogic commented 7 years ago

Probably would be nicer to use the pkgconfig file to get the arguments to compile against the system library.

anthrotype commented 7 years ago

I was thinking of adding some option or lock file to trigger that behaviour, any comments on that?

The easiest way would be to export an environment variable, e.g. "PYCLIPPER_USE_SYS_LIBRARY=1" or similar, and read that via os.environ from setup.py and pyclipper.pyx. If that environment variable is defined and non-empty, then in setup.py we pass the custom extra_link_args to link with the polyclipping system library, and also define some additional macro that we can #ifdef from the "pyclipper/extra_defines.hpp" to include the correct header.

athos-ribeiro commented 7 years ago

It seems that there is one single change required to make the current wrapper compatible with the latest version of Clipper (6.4.2):

Clipper::ExecuteInternal() was rewritten and returns false after running Clear(). Although the assertion on the test_clear test case seems to be correct, the python implementation of Execute raises an exception, leading the test case to fail. Should the test case capture the exception?

Here is a diff between the current clipper.hpp and version 6.4.2