Skip to content

Conversation

@Brooooooklyn
Copy link
Owner

@Brooooooklyn Brooooooklyn commented Dec 21, 2025

Problem Summary

The skiac_path_transform_self function in skia-c/skia_c.cpp drops trailing moveTo commands when transforming a path. This causes bugs like the closePath-arc test failure where
moveTo calls between arcs are lost, resulting in lines being drawn instead of separate subpaths.

Root Cause

Skia intentionally trims trailing moveTo verbs in SkPathData::MakeNoCheck() via trim_trailing_move() because trailing moveTo creates "empty contours" which Skia considers invalid
for immutable paths.

The problematic data flow:

SkPathBuilder (has trailing moveTo)
    ↓ builder.snapshot()
SkPath (still has trailing moveTo)
    ↓ makeTransform(matrix)
SkPathData::MakeTransform() → MakeNoCheck() → trim_trailing_move()
    ↓
SkPath (trailing moveTo REMOVED)
    ↓ replace_from_path()
SkPathBuilder(SkPath)
    ↓
Builder now permanently lost the trailing moveTo

Solution

Fix skiac_path_transform_self to preserve trailing moveTo by:

  1. Detecting if the builder has a trailing moveTo before transform
  2. Saving the point if present
  3. Performing the transform (which loses trailing moveTo)
  4. Restoring the trailing moveTo (with transformed coordinates)

Note

Upgrades Skia to Chrome m144, fixes Path2D transform to retain trailing moveTo, adjusts ARMv7 build flags, updates CI to build armv7, and adds funding metadata to npm packages.

  • C++ bindings (skia-c/skia_c.cpp):
    • Preserve trailing moveTo in skiac_path_transform and skiac_path_transform_self by detecting/restoring the final move after SkPath::makeTransform.
  • Build script (scripts/build-skia.js):
    • For armv7-unknown-linux-gnueabihf, disable SkPathData backend (-DSK_DISABLE_PATHDATA) and set target_cpu="armv7a" to avoid zlib CRC32 issue.
  • CI (.github/workflows/CI.yaml):
    • Replace ARMv7 test job with a dedicated build-armv7-linux-gnueabihf job and make publish depend on it.
  • Docs:
    • Update Skia badge to chrome/m144 in README.md and README-zh.md.
  • Packages (npm/*/package.json):
    • Add funding field to platform-specific packages.

Written by Cursor Bugbot for commit 850f8ae. This will update automatically on new commits. Configure here.

Copilot AI review requested due to automatic review settings December 21, 2025 10:15
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the Skia dependency to Chrome M144 and implements logic to preserve trailing moveTo commands during path transformations, which were previously lost during Skia's path creation process. Additionally, funding information has been added to all platform-specific package.json files.

  • Updated Skia submodule from M143 to M144
  • Fixed path transformation functions to preserve trailing moveTo operations
  • Added GitHub Sponsors funding metadata across all platform packages

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
skia Updated submodule commit to M144 version
skia-c/skia_c.cpp Enhanced skiac_path_transform and skiac_path_transform_self to detect and restore trailing moveTo commands after transformation
README.md Updated Skia version badge from M143 to M144
README-zh.md Updated Skia version badge from M143 to M144
npm/win32-x64-msvc/package.json Added GitHub Sponsors funding information
npm/win32-arm64-msvc/package.json Added GitHub Sponsors funding information
npm/linux-x64-musl/package.json Added GitHub Sponsors funding information
npm/linux-x64-gnu/package.json Added GitHub Sponsors funding information
npm/linux-riscv64-gnu/package.json Added GitHub Sponsors funding information
npm/linux-arm64-musl/package.json Added GitHub Sponsors funding information
npm/linux-arm64-gnu/package.json Added GitHub Sponsors funding information
npm/linux-arm-gnueabihf/package.json Added GitHub Sponsors funding information
npm/darwin-x64/package.json Added GitHub Sponsors funding information
npm/darwin-arm64/package.json Added GitHub Sponsors funding information
npm/android-arm64/package.json Added GitHub Sponsors funding information

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@Brooooooklyn Brooooooklyn merged commit dfb65a3 into main Dec 21, 2025
49 checks passed
@Brooooooklyn Brooooooklyn deleted the release branch December 21, 2025 14:17
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