search

fonttools / skia-pathops

Python bindings for the Skia library's Path Ops
https://skia.org/docs/dev/present/pathops/
BSD 3-Clause "New" or "Revised" License
47 stars 14 forks source link

Update to latest skia library #20

Closed anthrotype closed 4 years ago

anthrotype commented 4 years ago

This updates the embedded skia library to the latest revision from git mirror's master branch. Since currently skia requires a C++14 compiler (and it's 2019), I had to remove support for the 32-bit builds and also for the Windows 2.7 build. I'm still building python 2.7 for Linux/Mac, we can keep that until cython drops supports.

To build for the manylinux1 platform, I needed to use a more recent gcc compiler than the one in the default Docker image from PyPA. I used one that has gcc-9.1.0, and also passed -static-libstdc++ flag.

For Windows, I need to use Visual Studio 2017, which is the miniumum one that supports C++14.

For macOS, specifically only for the Python 3.6 build (not sure why that, and not also for 2.7 or 3.7...), I needed to patch one of skia headers to rename a symbol that was clashing with another macro defined in sys/termios.h system header. This is the patch: https://github.com/google/skia/compare/master...fonttools:fix-B0-macro?expand=1 The src/cpp/skia git submodule is configured to point to our skia fork at fonttools/skia. I will file a bug report upstream sometime later.

There's also one test case that changed output. I haven't had the chance to look at it visually to confirm that it's doing the right thing. I was lazy and simply updated the expected test results.

madig commented 4 years ago

The overwritten test case is suspicious, need to visualize.

madig commented 4 years ago

Old: image

New: image

anthrotype commented 4 years ago

Thanks for visualizing it! What do the original paths before overlap-removal look like?

madig commented 4 years ago

image The top stroke goes off into the far distance.

anthrotype commented 4 years ago

Ok. Well, very weird test contours we have. I'm inclined to merge this anyway.

typemytype commented 4 years ago

Does this skia update fix any of the issues where additional inflection points are added or other issues reported on fonttools path ops?

anthrotype commented 4 years ago

unfortunately no..

anthrotype commented 4 years ago

btw it looks like Cary Clark has left Google since the time we got in touch about these issues. The major issue I see currently is #11. We should probably send a feature request to https://bugs.chromium.org/p/skia/issues asking if it would be possible to re-join these complex curves that had been split for the sake of finding intersections so that we don't end up with unnecessary additional oncurve points. @typemytype fancy doing that?