Skip to content

Bump SkiaSharpGenerator to CppAst 0.21.4 and make it work on Linux#3714

Merged
mattleibow merged 4 commits into
mono:mainfrom
ramezgerges:dev/generator-cppast-bump
Apr 23, 2026
Merged

Bump SkiaSharpGenerator to CppAst 0.21.4 and make it work on Linux#3714
mattleibow merged 4 commits into
mono:mainfrom
ramezgerges:dev/generator-cppast-bump

Conversation

@ramezgerges

Copy link
Copy Markdown
Contributor

Description of Change

Bugs Fixed

  • Fixes #

API Changes

None.

Behavioral Changes

None.

Required skia PR

mono/skia#186

PR Checklist

  • Has tests (if omitted, state reason in description)
  • Rebased on top of main at time of PR
  • Merged related skia PRs
  • Changes adhere to coding standard
  • Updated documentation

ramezgerges and others added 3 commits April 22, 2026 01:33
The generator was pinned to CppAst 0.13.0 (from 2021), which transitively
required libclang 15 plus an libtinfo5 build with pre-libtinfo6 symbol
naming. That libtinfo5 was dropped from Ubuntu 24.04 onward, making the
generator unrunnable on modern Linux hosts without shipping a jammy-era
libtinfo.so.5 shim. The macOS-only include-path discovery also left Linux
contributors unable to parse the Skia headers at all.

Bump CppAst to 0.21.4 (pulls ClangSharp 18.1.0.4 / libclang 18.1.0). That
libclang is built with no ncurses dependency, so the libtinfo problem
goes away entirely.

Three small adjustments to keep the regenerated output byte-compatible
with what CppAst 0.13 produced:

 * BaseTool.GetCppType — strip `const` as suffix as well as prefix (newer
   CppAst emits `T const *` instead of `const T*`), and collapse the
   space before `*`/`&` that the new emitter inserts. Without this, the
   opaque-handle lookup in skiaTypes misses on every `const T*`
   parameter and every handle falls through to the raw-name fallback,
   producing thousands of lines of mangled signatures.

 * BaseTool.cs standard mappings — pin C `long` / `unsigned long` to
   Int32 / UInt32. CppAst 0.13 silently conflated `long` with `int`,
   which gave Int32 on both. CppAst 0.21 reports `long` faithfully,
   which would otherwise take the existing Int64 mapping and break the
   Windows ABI (C `long` is 32-bit under LLP64).

 * Generator.cs FormatCppSignature — normalize CppAst's ToString output
   in the `// extern …` / `// typedef …` / struct-field comments so they
   still read `const T*` with no space before the star. Purely cosmetic
   but saves ~500 lines of whitespace-only diff.

 * BaseTool.cs Linux branch — add a Linux arm of the include-path
   discovery that matches the macOS branch: invoke `clang -print-resource-dir`
   for the built-in headers and add /usr/include{,/x86_64-linux-gnu} for
   the system ones.

With these, regenerating all five `.generated.cs` files against the
current headers produces output that's byte-identical to what's checked
in, modulo two comment-header lines where CppAst 0.21 correctly reports
`long offset` for sk_stream_move where 0.13 mislabeled it `int offset`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… to int32_t

C `long` has a different size on Windows (LLP64, 32-bit) and Linux/macOS
(LP64, 64-bit), so there is no single managed type that maps correctly
on every platform. The previous commit kept `long` mapped to Int32 to
preserve the pre-CppAst-bump behaviour — but that same mapping was
silently misrepresenting the native ABI on Linux, where the .NET
marshaller zero-extended the 32-bit value into a 64-bit register that
the native `long` parameter then read as a huge positive number.

Make the generator refuse C `long` outright. Any header using it now
fails generation with a message pointing at int32_t / int64_t. Applies
to `long`, `signed long`, `unsigned long`, and their `long int` aliases.
`long long` (always 64-bit) keeps its Int64 mapping; it's unambiguous.

To let the guard fire without breaking the existing headers, also
narrow the only two places Skia's C API still uses `long` — the
sk_stream_move function and the sk_managedstream_move_proc callback —
to int32_t. That's a skia-submodule bump. The bindings don't change
shape (they were already Int32), but the ABI is now exact rather than
platform-dependent.

On the managed side, the backward-compat `SKStream.Move(long)` overload
previously silently truncated values beyond int range. Mark it
[Obsolete] with a message pointing at `Move(int)`, and switch its
implementation to `checked((int)offset)` so an overflowing value throws
OverflowException instead of being quietly cropped.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions

github-actions Bot commented Apr 22, 2026

Copy link
Copy Markdown
Contributor

📦 Try the packages from this PR

Warning

Do not run these scripts without first reviewing the code in this PR.

Step 1 — Download the packages

bash / macOS / Linux:

curl -fsSL https://raw.githubusercontent.com/mono/SkiaSharp/main/scripts/get-skiasharp-pr.sh | bash -s -- 3714

PowerShell / Windows:

iex "& { $(irm https://raw.githubusercontent.com/mono/SkiaSharp/main/scripts/get-skiasharp-pr.ps1) } 3714"

Step 2 — Add the local NuGet source

dotnet nuget add source ~/.skiasharp/hives/pr-3714/packages --name skiasharp-pr-3714
More options
Option Description
--successful-only / -SuccessfulOnly Only use successful builds
--force / -Force Overwrite previously downloaded packages
--list / -List List available artifacts without downloading
--build-id ID / -BuildId ID Download from a specific build

Or download manually from Azure Pipelines — look for the nuget artifact on the build for this PR.

Remove the source when you're done:

dotnet nuget remove source skiasharp-pr-3714

@mattleibow

Copy link
Copy Markdown
Contributor

/azp run

@azure-pipelines

Copy link
Copy Markdown
No pipelines are associated with this pull request.

mattleibow added a commit to mono/skia that referenced this pull request Apr 23, 2026
Use int32_t instead of long for stream move offset (#186)

Context: mono/SkiaSharp#3714

C `long` is 32-bit on Windows (LLP64) but 64-bit on Linux/macOS (LP64),
so `sk_stream_move` and `sk_managedstream_move_proc` had a silently
platform-dependent parameter size. The SkiaSharp bindings mapped this to
Int32, which was correct on Windows but wrong on LP64 targets — the .NET
marshaller zero-extends the 32-bit value into a 64-bit register, so
negative offsets like -1 arrive as 0x00000000FFFFFFFF on the native side.

Narrow the C-API surface to `int32_t` so the ABI is unambiguously 32-bit
on every platform. `SkManagedStream::move(long)` still matches upstream
SkStream's signature internally; a range check against INT32_MIN /
INT32_MAX now returns false on overflow rather than silently truncating
before calling through to the managed MoveProc.

Stream offsets are now explicitly capped at ±2 GB on all platforms.
Windows behaviour is unchanged; Linux gains correct handling of negative
offsets at the cost of the theoretical >2 GB range that nobody could
reliably use through the managed bindings anyway.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Point at the merge commit (bc1f05a1) that includes the int32_t stream
move fix, so the submodule tracks the skiasharp branch tip.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@mattleibow
mattleibow merged commit edc7bff into mono:main Apr 23, 2026
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants