Skip to content

Migrate Skia to m143 and SkPathBuilder#81

Merged
anthrotype merged 13 commits intomainfrom
update-skia-m143
Nov 7, 2025
Merged

Migrate Skia to m143 and SkPathBuilder#81
anthrotype merged 13 commits intomainfrom
update-skia-m143

Conversation

@anthrotype
Copy link
Member

This PR updates skia-pathops to use Skia m143 (chrome/m143) and migrates from mutable SkPath to immutable SkPath with SkPathBuilder for path construction.

Skia is removing mutability from SkPath (tracked in issue 40040287) to make it immutable and thread-safe. The deprecated mutable path methods (moveTo, lineTo, etc.) are now hidden behind SK_HIDE_PATH_EDIT_METHODS and will be removed in a future Skia release.

The core changes by @bungeman in 386fea4 update the Cython bindings to use the new skia API. The rest is me updating the skia-builder to m143 and ensuring the existing skia-pathops Python API remains unchanged.

There are still three test failures seemingly due to Skia m143 behavioral changes which I still need to investigate.

anthrotype and others added 8 commits November 6, 2025 21:42
Upgrade from chrome/m113 to chrome/m143-no-deps@149648a.

This new skia-builder version includes:
- SK_HIDE_PATH_EDIT_METHODS commented out to enable SkPath mutation methods
- :pathops added to main skia component dependencies
Skia m143 uses std::optional::value() which requires macOS 10.13+.
Using 11.0 to match Skia's build configuration.
As tracked in issue 40040287, Skia is removing mutability from SkPath
(this change currently guarded behind `SK_HIDE_PATH_EDIT_METHODS`) and
adding SkPathBuilder for path creation. This will allow SkPath to be
immutable and thread safe, which will bring both correctness and
performance benefits.

While testing to see what needs to change in google3 to allow SkPath to
become immutable with cl/825665428 it was discovered that
`google3/third_party/py/pathops/_skia/core.pxd` is using and exposing
mutating SkPath methods. This change updates `core.pxd` to remove the
mutable SkPath methods, adds SkPathBuilder, and changes Path to wrap a
SkPathBuilder.
Required to use the 3-argument version of SkDashPathEffect::Make()
with pointer/count instead of SkSpan.
Cython 3.2.0 has a regression where walrus operators with C++
optional<T> types generate invalid C++ code. Expand to explicit
while loops with .has_value() checks.
The SkPath::isConvex() method still exists in Skia m143, so restore
the property. Adapted to call .snapshot() since self.path is now
SkPathBuilder.
Update to skia-builder commit 501d7a3 which enables SK_HIDE_PATH_EDIT_METHODS
now that the SkPathBuilder migration is complete.
The arcTo() method and ArcSize enum were unnecessarily removed in commit
386fea4. It's still present in SkPathBuilder as an SVG-style arc method,
taking rx, ry, xAxisRotate, largeArc (ArcSize), sweep (Direction) and
endpoint x, y.
@anthrotype
Copy link
Member Author

anthrotype commented Nov 7, 2025

Regarding the 3 remaining test failures, I can confirm they are the result of intentional behavior changes in latest Skia (I don't exactly know from which version on, but it's evident in m143).

  1. OpBuilderTest::test_resolve: the test contains a pathological input (a path with an extreme spike at hundreds of millions of units), the new skia simplifies away this degenerate artifact, so I think we should just update the test.

  2. PathTest::test_allow_open_contour: this used to confirm that two consecutive moveTo() calls produced two single-point contours. In Skia m143 the builder now overwrites any preceding standalone kMove (SkPathBuilder::moveTo replaces the last point instead of appending), so by the time the second moveTo runs the first move point is already gone. See https://skia-review.googlesource.com/c/skia/+/1033357

  3. test_reverse_path[operations11-expected11]: this failure is somehow related to the previous one, but distinct; the test feeds a path with only a single moveTo through Path.reverse() and expects that it survives the reversal unchanged. Even if SkPathBuilder keeps the verb (when not overwritten by another consecutive moveTo, as per above), the new SkPathIter explicitly trims any trailing kMove verbs at the end of a path (see include/core/SkPathIter.h), so when we iterate over the contours we see nothing and the reversed path is empty. See https://skia-review.googlesource.com/c/skia/+/1041696

So basically we can no longer store and get back contours that consists of nothing but a single moveTo(x, y) as they won't survive once they touch SkPathBuilder or SkPathIter.

It's true that single-point contours can be found sometimes in font land e.g. used as points to anchor components or marks (font source formats nowadays store things like that as distinct "anchors" rather than as part of the contour outlines).

But even when we were using the old skia m113 with mutable SkPath, as soon as we passed those single-point paths through PathOps (Path.simplify, Path.op, OpBuilder, etc.) they were already doomed because a lone moveTo never emits an edge so the result path would come back without it. The only reason those two tests ever saw them was they hadn’t been fed into PathOps yet; they were really validating storage details that the latest Skia no longer preserves.
And people call into skia-pathops specifically to run the boolean ops, not merely to round-trip glyph anchors. Therefore I'm going to adjust the expectations to match m143 and document the behavioral change: single-point open contours are now dropped during path construction/iteration, and any PathOps call will discard them too.

- Bump python_requires to >=3.10 in setup.py
- Add Python 3.14 wheels for all platforms, but skip free-threaded for now until we actually test that they do work
- Enable PyPy 3.11 wheels on all platforms (Linux, macOS, Windows) and remove obsolete PyPy setuptools pin from pyproject.toml
- Remove redundant macOS x86_64-only builds (universal2 covers both
architectures)
@bungeman
Copy link
Contributor

bungeman commented Nov 7, 2025

The SkArcSize not being SkPathBuilder::ArcSize makes me a little uneasy, but it's fine. LGTM (though I don't see any buttons to push as I don't think I have any real permissions here).

@anthrotype
Copy link
Member Author

anthrotype commented Nov 7, 2025

Yeah, that was to mirror what we do with other similarly nested enums. I think as a follow up we could expose the new SkPathBuilder API to the Python level (a PathBuilder companion to Path) to mimic more closely the actual Skia interface, then I can have the ArcSize enum nested in there among other things. But not hugely urgent in my opinion.

@anthrotype anthrotype merged commit 076057a into main Nov 7, 2025
12 checks passed
@anthrotype anthrotype deleted the update-skia-m143 branch November 7, 2025 18:48
anthrotype added a commit to googlefonts/picosvg that referenced this pull request Nov 11, 2025
After updating skia-pathops from m113 to m143 in fonttools/skia-pathops#81, 5 tests were failing on macOS due to floating-point precision differences in stroke expansion.
These differences arise from cross-platform floating-point variations (macOS ARM vs Linux x86_64) and minor algorithmic improvements in skia-pathops m143.
Rather than regenerating all expected files, we round both actual and
expected output to the minimum precision needed for each test.
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

Successfully merging this pull request may close these issues.

2 participants