Open
Conversation
….0-457-ga46d5732d9)
….40.0-787-g0f94c19331)
… freetype compatibility." This reverts commit bfccc58.
….40.0-795-gcbc694239b)
….40.0-1240-gb57e04d1c7)
…, and removed.
It was deprecated in m139, and gone in m140.
Milestone 139
-------------
* `SkFontMgr_New_FontConfig` with 1 parameter has been deprecated and will be removed in a future
release. Clients will need to call the other version providing an SkFontScanner (e.g.
`SkFontScanner_Make_FreeType()`)
Conflicts:
src/skia/Font.cpp
…favor of the `SkRecorder*` version.
Milestone 140
-------------
* `SkImage::isValid(GrRecordingContext*)` has been deprecated in favor of the `SkRecorder*` version.
To migrate do something like `image->isValid(ctx->asRecorder())`.
`SkImage::makeSubset(GrDirectContext*, ...)` has been deprecated in favor of the `SkRecorder*`
version. To migrate, do something like `image->makeSubset(ctx->asRecorder, ..., {})`
`SkImage::makeColorSpace(GrDirectContext*, ...)` has been deprecated in favor of the `SkRecorder*`
version. To migrate, do something like `image->makeColorSpace(ctx->asRecorder, ..., {})`
`SkImage::makeColorTypeAndColorSpace(GrDirectContext*, ...)` has been deprecated in favor of the
`SkRecorder*` version. To migrate, do something like
`image->makeColorTypeAndColorSpace(ctx->asRecorder, ..., {})`
In the case you are working with CPU-backed images, `skcpu::Recorder::TODO()` should work until
a `skcpu::Context` and `skcpu::Recorder` can be used properly.
…atrix.inverse()
m139:
bool setPolyToPoly(const SkPoint src[], const SkPoint dst[], int count);
m140:
bool setPolyToPoly(SkSpan<const SkPoint> src, SkSpan<const SkPoint> dst);
Before m140, only (deprecated):
[[nodiscard]] bool invert(SkMatrix* inverse) const;
Added in m140:
std::optional<SkMatrix> invert() const;
Old:
bool getSegment(SkScalar startD, SkScalar stopD, SkPath* dst, bool startWithMoveTo);
Newly added:
bool getSegment(SkScalar startD, SkScalar stopD, SkPathBuilder* dst, bool startWithMoveTo);
Old:
bool transform(const SkMatrix& matrix, SkRRect* dst) const;
Newly added:
std::optional<SkRRect> transform(const SkMatrix& matrix) const;
Old:
bool getBoundaryPath(SkPath* path) const;
Newly added:
SkPath getBoundaryPath() const;
was:
bool applyToPath(SkPath* dst, const SkPath& src) const;
m140:
bool applyToPath(SkPathBuilder* dst, const SkPath& src) const;
This reverts commit cd8af0c. Should not be needed in m140.
m140-public includes everything in m139-public. Merge remote-tracking branch 'origin/m139-public' into m140-public
Merge remote-tracking branch 'origin/m140-public' into m141-public
Merge remote-tracking branch 'origin/m141-public' into m142-public
Revert "Make artefacts available even when pytest fails." This reverts commit a057e3c.
Merge remote-tracking branch 'origin/m142-public' into m143-public
Merge remote-tracking branch 'origin/m143-public' into m144-public
…everse-apply to m144 cleanly.
Reversing "defines.remove('SK_DISABLE_LEGACY_NONCONST_SERIAL_PROCS')" also is likely strictly-speaking
incorrect: what we want is really removing the 'defines.remove("SK_HIDE_PATH_EDIT_METHODS")' line
while keeping the rest. It likely does not matter.
Forgotten to adjust the line-count when modifying the patch
Fixes kyamagu#350 See kyamagu#350 Segfault on pytest exit in CI against pybind11 3.0.1 This is a workaround. See upstream issues: pybind/pybind11#5976 [BUG]: Crash due to invalid free during cleanup, if any enum is registerd, due to attempting to free a string literal pybind/pybind11#5991 [BUG]: segfault at exit since upgrading to 3.0.x, probably due to "Make wrapped C++ functions pickleable" (pybind/pybind11#5580) Signed-off-by: Hin-Tak Leung <htl10@users.sourceforge.net>
…Format and VkImageLayout Signed-off-by: Hin-Tak Leung <htl10@users.sourceforge.net>
Signed-off-by: Hin-Tak Leung <htl10@users.sourceforge.net>
src/skia/Matrix.cpp:1562:13: error: return type 'Tuple<int, int>' must match previous return type
'Tuple<float &, float &>' when lambda expression has unspecified explicit return type
1562 | return py::make_tuple(-1, -1);
| ^
1 error generated.
Signed-off-by: Hin-Tak Leung <htl10@users.sourceforge.net>
…askit/0.40.0-2648-g2708a1b154)
The example code in the Canvas Creation section of the tutorial requires the `sys` module to run correctly, but the `import sys` statement was missing. This change adds the import sys to resolve the error.
Revert "Make artefacts available even when pytest fails." This reverts commit a057e3c.
m144-public-cleaned is identical to m144-public at this point, just with a cleaner (recent) history. Some of the recent and reverted changes on m144-public are problematic. Merge remote-tracking branch 'origin/m144-public' into m144-public-cleaned
There was a problem hiding this comment.
Pull request overview
Updates the project to Skia milestone m144, aligning Python bindings/tests/CI with upstream API changes (notably SkSpan-based signatures, Vulkan enum usage, and removal of ApplyPerspectiveClip).
Changes:
- Updated C++ pybind11 bindings to match Skia m144 API signatures (SkSpan conversions, overload disambiguation, Vulkan enums).
- Updated tests and docs to reflect API changes (e.g., Path.transform, PathBuilder.incReserve, Vulkan parameter types).
- Refreshed build/CI configuration and patches (new Skia submodule rev, Python 3.14 wheels, macOS runner updates, download-minimization patches).
Reviewed changes
Copilot reviewed 39 out of 39 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_path.py | Adjusts tests for removed ApplyPerspectiveClip arg and updated incReserve signature. |
| tests/test_grcontext.py | Switches Vulkan-related tests from integer constants to VkFormat/VkImageLayout enums. |
| src/skia/TextBlob.cpp | Updates binding calls to pass SkSpan-like arguments for positions/xforms/glyph buffers. |
| src/skia/SkTextOnPath.cpp | Adapts font glyph APIs to SkSpan-based Skia signatures. |
| src/skia/Region.cpp | Disambiguates overloaded getBoundaryPath binding using overload_cast. |
| src/skia/Rect.cpp | Updates point-array APIs to pass span-like arguments; disambiguates RRect::transform overload. |
| src/skia/PathMeasure.cpp | Disambiguates getSegment overload to match updated Skia signature(s). |
| src/skia/PathEffect.cpp | Updates SkStrokeRec::applyToPath and dash interval passing for SkSpan-based APIs. |
| src/skia/Path.cpp | Adds kDefault PathDirection, updates signatures for Make/Polygon/transform/incReserve, and overload disambiguation. |
| src/skia/Matrix.cpp | Removes ApplyPerspectiveClip binding, updates mapping APIs to span/signature changes. |
| src/skia/Image.cpp | Updates image color/validity/subset APIs for recorder/options-based Skia changes. |
| src/skia/GrContext_vk.cpp | Expands and formalizes Vulkan enums for VkFormat/VkImageLayout in bindings. |
| src/skia/GrContext.cpp | Disambiguates MakeGL overload; aligns MakeVk overload typing. |
| src/skia/Font.cpp | Updates Linux fontmgr factory and multiple span-based font/typeface APIs. |
| src/skia/Canvas.cpp | Updates drawPoints/drawAtlas to span-style arguments; disambiguates SVGCanvas::Make. |
| skia | Updates Skia submodule pointer to m144 revision. |
| setup.py | Bumps package version to 144.0. |
| scripts/build_macOS.sh | Updates applied patch set for m144 and reverses a SkPath immutability patch. |
| scripts/build_Windows.sh | Updates applied patch set for m144; swaps Windows GN patching strategy. |
| scripts/build_Linux.sh | Updates applied patch set for m144 and reverses a SkPath immutability patch. |
| relnotes/README.m144.md | Adds milestone release notes for m144 changes. |
| relnotes/README.m143.md | Adds milestone release notes for m143 changes. |
| relnotes/README.m142.md | Adds milestone release notes for m142 changes. |
| relnotes/README.m141.md | Adds milestone release notes for m141 changes. |
| relnotes/README.m140.md | Adds milestone release notes for m140 changes. |
| relnotes/README.m139.md | Adds milestone release notes for m139 changes and rationale for Vulkan enum change. |
| patch/skia-m144-minimize-download.patch | Minimizes Skia DEPS downloads for m144 and disables emsdk activation. |
| patch/skia-m143-minimize-download.patch | Minimizes Skia DEPS downloads for m143 and disables emsdk activation. |
| patch/skia-m142-minimize-download.patch | Minimizes Skia DEPS downloads for m142 and disables emsdk activation. |
| patch/skia-m141-minimize-download.patch | Minimizes Skia DEPS downloads for m141 and disables emsdk activation. |
| patch/skia-m140-minimize-download.patch | Minimizes Skia DEPS downloads for m140 and disables emsdk activation. |
| patch/skia-m139-minimize-download.patch | Minimizes Skia DEPS downloads for m139 and disables emsdk activation. |
| patch/fetch-gn-windows-arm64.diff | Adjusts GN fetching behavior for Windows ARM64 environments. |
| patch/0001-gn-Remove-msvc-env-setting.patch | Removes the previous GN/MSVC environment patch (file deleted). |
| patch/0001-Reland-Make-SkPath-immutable-on-GN-build.patch | Adds Skia GN patch (then reversed in build scripts). |
| patch/0001-Disable-OpenGL-for-Windows-on-ARM64.patch | Updates Skia patches for Ganesh GL paths and ARM64 OpenGL disabling. |
| docs/tutorial/canvas.rst | Updates tutorial imports to include sys in examples. |
| README.md | Updates supported Python versions/platform notes and links new milestone READMEs. |
| .github/workflows/ci.yml | Updates build matrix (macOS runners, Python 3.14), docs install step, and adds Fedora wheel artifact upload. |
Comments suppressed due to low confidence (5)
src/skia/PathEffect.cpp:1
- This wrapper both dereferences
dstunconditionally (soNonefrom Python would segfault) and never writes the computed result back into*dst(the output stays indst2). Suggestion: either (a) makedstnon-nullable in the binding (e.g., takeSkPath&), or (b) explicitly handledst == nullptrby throwing a Python exception, and on success assign the resulting path back to*dst(e.g., viadetach()/snapshot()on the builder) before returning.
src/skia/Path.cpp:1 SkPath::moveTois typically declared asmoveTo(const SkPoint&)(const reference). Usingpy::overload_cast<const SkPoint>is unlikely to match any overload and will fail to compile (or bind the wrong overload if one exists). Usepy::overload_cast<const SkPoint&>(&SkPath::moveTo)to match the actual signature.
src/skia/Rect.cpp:1- These bindings still use
&points[0], which is undefined behavior for an emptypointsvector (Python can pass[]). If empty input is invalid, raise a Python exception before calling into Skia; if it is valid, usepoints.data()(or{nullptr, 0}) and ensure Skia receives a zero-length span without dereferencing.
src/skia/Rect.cpp:1 - These bindings still use
&points[0], which is undefined behavior for an emptypointsvector (Python can pass[]). If empty input is invalid, raise a Python exception before calling into Skia; if it is valid, usepoints.data()(or{nullptr, 0}) and ensure Skia receives a zero-length span without dereferencing.
src/skia/Rect.cpp:1 - These bindings still use
&points[0], which is undefined behavior for an emptypointsvector (Python can pass[]). If empty input is invalid, raise a Python exception before calling into Skia; if it is valid, usepoints.data()(or{nullptr, 0}) and ensure Skia receives a zero-length span without dereferencing.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Owner
|
@HinTak Thanks! I've checked the changes, and they look good. Copilot left a few comments, where I believe the empty vector address cases are valid. Can you apply the suggestions in the branch? |
…inimize-download.patch
The name of the patch was auto-generated from its content (and the change has indeed been applied once, reverted, and relanded upstream) from `git format-patch -1 ...`. IMHO it should be kept as is, and the content should not be converted to forward-apply. I'll just remove the "-Reland" part to make copilot happier.
Most of them done programmatically with something like:
perl -pi -e 's/\&pts\[0\]/pts.data()/g' skia/*
Copilot points out that the latter is safer. Many(all?) of them were
touched by the recent SkSpan changes, plus a few more.
Collaborator
Author
|
@kyamagu I have done all the recently touched &x[0], plus a few more. I have a look at the remaining &×[0] (those not part of recent SkSpan change) and not too sure about changing working code, so left most of them alone. Maybe for a later time. Also removed a lot of stale patches. Don't agree with converting a revert to foreward apply, but dropping "-Reland"part is better. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
@kyamagu ready to go. Merging this will close most of the others. Sorry the history has gotten a bit muddled - I feel like recording some of the m139-143 being buildable is good in case somebody need to go back. Anyway, top of README.m144 has a small summary about change since m138, while the other new README's have more details.
I have not looked at m146 yet; m145 besides requiring large reverts, also seem to have problems on mac os recreating deleted files (I suspect /dev/null on github CI on mac os might not be available), so I would just leave that alone tor a while.