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

implement private SkConic.computeQuadPOW2 in cython #34

Closed anthrotype closed 3 years ago

anthrotype commented 3 years ago

Fixes #33

behdad commented 3 years ago

Why do we need this?

anthrotype commented 3 years ago

When adding arcs to a Path, skia internally stores them as conic curves. We need to get rid of conic curves when we convert a Path to one that we can consume with the FontTools Pen protocol.

We have a method on pathops.Path called convertConicsToQuads that takes a tolerance parameter (a float). Internally this calls SkPath::CovertConicToQuads method. The latter in turns takes a pow2 parameter which defines the maximum number of quads to use to approximate the conic (count is 2 to the pow2):

https://github.com/google/skia/blob/52a4379f03f7cd4e1c67eb69a756abc5838a658f/include/core/SkPath.h#L997-L1023

In private SkGeometry module, SkConic struct as a convenient computeQuadPOW2 method that given a float tolerance and the conic points and weight, return the most optimal pow2 value that approximate the conic within that tolerance.

That's what this method is doing. I can't simply include the header because it's private, so I rewrote it in cython. Does that clarify the intent?

behdad commented 3 years ago

Ah, arcs. Got it. We don't really need that but I see the library provides it. Thanks.

behdad commented 3 years ago

Anyway. I wasn't asking why you need it. Was suggesting to add a line to the code about what this function does, because the name wasn't clear. I didn't know what the POW there refers to. Had to read the original code to understand.

anthrotype commented 3 years ago

definitely thanks, I'll add a comment 👍