[skia-sync] Merge upstream chrome/m151 bug fixes - #4642
Closed
mattleibow wants to merge 41 commits into
Closed
Conversation
mattleibow
added
partner/agentic-workflows
mattleibow
added
partner/agentic-workflows
Contributor
📦 Try the packages from this PRWarning 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 -- 4642PowerShell / Windows: iex "& { $(irm https://raw.githubusercontent.com/mono/SkiaSharp/main/scripts/get-skiasharp-pr.ps1) } 4642"Step 2 — Add the local NuGet source dotnet nuget add source ~/.skiasharp/hives/pr-4642/packages --name skiasharp-pr-4642More options
Or download manually from Azure Pipelines — look for the Remove the source when you're done: dotnet nuget remove source skiasharp-pr-4642 |
Contributor
|
📖 Documentation Preview The documentation for this PR has been deployed and is available at: 🔗 View Staging Site This preview will be updated automatically when you push new commits to this PR. This comment is automatically updated by the documentation staging workflow. |
Contributor
📊 SkiaSharp benchmarks — PR #4642⭐ this PR (full source build) vs 🌙 nightly · Linux · Windows · macOS
Highlights⏱️ Time — 🔴 68 slower · 🟢 8 faster
Full per-OS benchmark deltasLinux⏱️ Time (vs 🌙 nightly
Windows⏱️ Time (vs 🌙 nightly
macOS⏱️ Time (vs 🌙 nightly
|
Contributor
📦 Artifact size reportPackages from this PR (build Total Packages
+37 package(s) unchanged (< 50.0 KB). Per-file changes
|
| File | Size |
|---|---|
runtimes/win-x86/native/libHarfBuzzSharp.pdb |
21.9 MB → 22.2 MB (🔴 +272.0 KB) |
runtimes/win-arm64/native/libHarfBuzzSharp.pdb |
22.3 MB → 22.5 MB (🔴 +192.0 KB) |
SkiaSharp.NativeAssets.WinUI
| File | Size |
|---|---|
runtimes/win-arm64/native/libGLESv2.pdb |
64.5 MB → 64.4 MB (🟢 −80.0 KB) |
runtimes/win-x64/native/libGLESv2.pdb |
66.6 MB → 66.6 MB (🟢 −64.0 KB) |
runtimes/win-x86/native/libGLESv2.pdb |
67.2 MB → 67.2 MB (🟢 −32.0 KB) |
Informational only — this never blocks the PR. Native binaries are labelled by os/arch.
) Add a PR-time artifact-size check to the size-tracking workflow (#4505) The benchmark workflow already runs on PRs, but artifact sizes were only tracked nightly, so there was no PR-time signal when a change balloons a package or a shipped native binary. This adds that guard without rebuilding anything in Actions and without touching the Azure DevOps pipeline. Rather than repacking in CI, it reacts to the AzDO job that already builds the NuGets. The work is folded into the single "Track - Artifact Sizes" workflow via a shared `resolve` job that picks the mode (mirroring the benchmark tracker's resolve -> ... -> report shape): - nightly (schedule / manual dispatch): the existing measure + persist + dashboard flow, unchanged. - pr (the AzDO "Package NuGets" check run completing, or a manual dispatch with a build id): pull the exact build id out of the check-run event, download that build's `nuget` artifact (public, anonymous, dnceng-public/public), measure every .nupkg and every file inside, diff against the latest nightly baseline on the aw-data branch, and post/update a size-diff PR comment. Informational only: the pr mode never fails a check or blocks the PR, and never writes to aw-data. Notable design points: - `resolve` verifies check-run builds are real PRs (refs/pull/<n>/merge) before proceeding, so pushes to main/branches don't trigger a pointless ~1 GB download. - The pr job carries its own `pull-requests: write`; nightly stays read-only. - Package measurement (measure_nupkg) and render helpers are shared across modes; the comment sorts by |delta|, flags growth over 500 KB / 2% with a warning, applies a 50 KB noise floor, and filters OPC-metadata churn. - persist-aw-data.yml is unaffected: the workflow name is unchanged, and pr-mode runs upload no `agent` artifact and are gated out by its head_branch == main check. Files: - .github/workflows/track-artifact-sizes.yml -- merged workflow (nightly + pr). - scripts/infra/perf/sizes/measure_pr.py -- download + measure a build's `nuget` artifact; resolves the PR number from the build source branch (fork-safe). - scripts/infra/perf/sizes/render_pr_md.py -- per-package + per-file size diff vs the nightly baseline, with native-binary labels. Validation: the resolve -> download -> measure -> diff -> comment chain was exercised end-to-end via workflow_dispatch against real build 1517880 (~1 GB `nuget` artifact, 40 packages, 21 native), and the `resolve` mode selection was unit-tested across schedule / dispatch / matching + non-matching check_run / fork-without-PR-hint / non-PR branch build payloads, including live AzDO source-branch verification. The `check_run` trigger only becomes active once this file is on the default branch, so that event delivery is exercised organically after merge (until then it can be driven manually with a build id). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9864b16f-0950-498e-8a61-12dceeb87107
…terization) (#4507) The Blazor WASM integration tests could only run against net10.0, and would fail against a .NET 11 preview because the pinned Microsoft.Playwright 1.49.0 bundles Chromium 131, which does not support the WebAssembly exception-handling (exnref) feature the .NET 11 WASM runtime requires. On net11 the Blazor app aborts during startup with: MONO_WASM: Assert failed: This browser/engine doesn't support WASM exception handling CompileError: WebAssembly.compileStreaming(): invalid value type 'exn', enable with --experimental-wasm-exnref so the app never renders and the test times out — not a SkiaSharp defect, purely a stale test browser. Fixes: - Bump Microsoft.Playwright 1.49.0 -> 1.55.0 (Chromium 140, which supports the required WASM exception handling). - Parameterize the generated temp projects' target framework and SDK pin via -p:BaseFramework / -p:SdkVersion / -p:SdkAllowPrerelease (default net10.0, so existing behavior is unchanged). This lets the harness smoke-test the packages on a newer band, e.g.: dotnet test -p:BaseFramework=net11.0 -p:SdkVersion=11.0.100-preview.6 -p:SdkAllowPrerelease=true Verified locally against SkiaSharp 4.151.0-rc.1.1 / HarfBuzzSharp 14.2.1-rc.1.1: BlazorTests pass on both net10.0 and net11.0 (SKCanvasView + SKGLView). Copilot-Session: 88526fb6-7129-4a2b-bbf6-47f32912d079 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Chrome Stable (and Extended stable) is m150 today, so the supported stable line is 4.150 only. The 4.148 entry was stale drift left over from a prior milestone. Removing it makes the release-notes support grouping match the live Chrome channels; the security-audit milestone-schedule drift-check now reports OK for both paths (stable=[4.150], preview=[4.151]). Copilot-Session: f3248de6-df37-46f5-8bb2-9778033f9450 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…4504) * Add compact PR benchmark comment to the benchmark tracking workflow On `pull_request` runs of Track - Benchmarks, post (and update) a single informational comment summarising this PR's performance vs the baseline. - New `scripts/infra/perf/benchmarks/render_pr_md.py` reads the same per-(OS, role) histories the `report` job already downloads, compares the source-built `pr` leg against the nightly baseline (falling back to released stables), and emits a COMPACT marker comment: a Highlights section with the biggest time and allocation movers, plus a collapsed <details> block of full per-OS deltas. Only benchmarks that move beyond the shared 5% noise threshold are surfaced. Time/byte formatters, trend dots and the history loader are imported from render_md.py and _common.py rather than duplicated. - Wire it into the `report` job with a PR-only render step and a github-script find-update-or-create step keyed on `<!-- skiasharp-pr-benchmarks -->`, using the same marker-comment pattern as pr-artifacts-comment.yml. Adds `pull-requests: write`; the post step is best-effort (continue-on-error) so it never blocks the PR and tolerates read-only fork tokens. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 807194d1-0dc9-4eae-8c10-c23549d0398f * Note that PR benchmark times are raw and noisy for microbenchmarks The ⭐ PR and baseline legs run on separate CI runners, so raw wall-clock time carries cross-runner variance and microbenchmarks can swing run-to-run. Add a caveat to the comment (and a code note) making clear the times are raw BenchmarkDotNet means, that small time deltas should be treated as noise, that allocations are the deterministic/reliable signal, and that the interactive perf-dashboard applies smoothing for the trend view. Raw numbers are kept as-is. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 807194d1-0dc9-4eae-8c10-c23549d0398f --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Latest: Merge pull request #182 from mono/automation/write-api-docs Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Use FreeType for Nano Server font support (#4280) Context: #1105 Skia change: mono/skia#272 (merged as 5952f46) Nano Server has no DirectWrite, so SkiaSharp's default font manager fell back to an empty manager that could neither enumerate system fonts nor create typefaces from caller-supplied data — text drawing silently did nothing on Nano Server even when the app loaded a font itself. This also blocked using a FreeType-backed build on Windows for the scenarios in #1105 (e.g. the PDF performance issue #3906 and font-render consistency between Linux and Windows). Thread a `useFreeType` switch through the Windows native build and enable it for the Nano Server variant, so the Nano `libSkiaSharp` is built with FreeType and the "custom empty" font manager. Combined with the merged Skia C-API change, typefaces loaded via `SKTypeface.FromStream/FromData/ FromFile` now render on Nano Server. Normal Windows builds are unaffected (FreeType off, DirectWrite retained). * native/windows/build.cake: add a `useFreeType` argument/`USE_FREETYPE` env switch and map it to GN args — `skia_use_freetype`, `skia_enable_fontmgr_custom_empty`, and `skia_enable_fontmgr_win` (disabled when FreeType is on). `skia_use_system_freetype2` is pinned to `false` alongside the other `skia_use_system_*` flags so the bundled FreeType is always used. * native/nanoserver/build.cake: pass `useFreeType=true` for the Nano Server build. * externals/skia: bump to 5952f46 (merged mono/skia#272), which adds the `SK_BUILD_FOR_NANOSERVER` font-manager branch (custom-empty via FreeType) and scopes the Windows FreeType include-config build workaround to the freetype-android define branch. * cgmanifest.json: update the Skia component commit hash to match. Verified on CI: the Native Windows "Nano Server x64" leg builds bundled FreeType, links libSkiaSharp with the FreeType-backed manager, and passes nano-api-scan (~46m compile, no issues). Known limitation: the custom-empty manager has no system fonts, so the default typeface still renders nothing on Nano Server — fonts must be loaded explicitly. Redirecting the default manager at a fonts directory (SkFontMgr_New_Custom_Directory) is a possible follow-up. Co-authored-by: Jeff Skaistis <jskaistis@easypost.com> Co-authored-by: Matthew Leibowitz <mattleibow@live.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Harden milestone audit and document WASM release testing (#4510) Three related release-tooling improvements, all surfaced while publishing SkiaSharp 4.151.0-rc.1. Milestone audit: only assign work to releases that actually shipped ------------------------------------------------------------------ scripts/infra/milestones/audit-milestones.ps1 assigns merged PRs (and the issues they close) to the milestone of the release they shipped in. Two gaps let it assign work to releases that never shipped: - An unshipped preview collected PRs it never released. 4.151.0-preview.3 had a branch and a milestone but was never tagged, yet the audit parked 21 PRs on it even though those commits actually shipped in rc.1. A range is now only assigned to a release that has a git tag; an untagged preview's commits roll forward to the next shipped release (preview.3 -> rc.1). - Commits on main past the last release branch were parked in a speculative "stable" milestone before stable was cut. They are now left alone until the next release is actually cut and tagged. The branch listing now logs the published tags and marks each branch shipped or unshipped, so the roll-forward is visible before anything is applied. Also fixes three latent bugs found while testing the audit: - $errors was incremented on a failed update but never initialized, so the summary printed a blank count instead of a number. - A single-branch match (e.g. -Version 4.150.1) collapsed $rawBranches to a scalar string, so $allBranches[0] returned the character 'o' from "origin/..." and produced a bogus "Milestone 'o' not found". Git results are now wrapped in @(). - The previous-release boundary used a descending string sort that ranked 4.148.0-rc.1 above the bare stable 4.148.0, leaking that version's rc->stable commits into the next preview.1. It now prefers the bare stable when versions tie. Verified by dry-run: 4.151.0 rolls preview.3's 21 PRs into rc.1 and skips the 5 trailing commits; 4.150.0 (fully shipped) is unchanged (0 to fix, 53 correct). Wire the audit into release-publish ----------------------------------- Added Step 8a to the release-publish skill: run the audit after tagging and before closing the milestone, so every publish keeps milestone assignments clean incrementally instead of drifting between releases. Document WASM release testing ----------------------------- Added a Test Properties table and a WASM (Blazor) workflow to the release-testing skill, styled after the existing Android emulator workflow. The table documents every -p: property the integration project accepts; the workflow shows how to run BlazorTests on a non-default .NET band by changing BaseFramework + SdkVersion, with the per-band wasm-tools and Playwright/Chromium exnref caveats. Uses netX.0 placeholders so it does not go stale. Co-authored-by: Matthew Leibowitz <mattleibow@live.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…port (#4519) Update DeviceRunners to 0.1.0-preview.12 for dotnet test --filter support (#4519) Changes: mattleibow/DeviceRunners@0.1.0-preview.11...0.1.0-preview.12 The WASM (SkiaSharp.Tests.Wasm) and device (SkiaSharp.Tests.Devices) test projects referenced DeviceRunners 0.1.0-preview.11, in which `dotnet test --filter ...` is silently ignored on WASM. `--filter` support for device and WASM runs only landed in preview.12 (mattleibow/DeviceRunners#143), so the version we shipped predated the feature entirely. The full filter chain is new in preview.12, which is why nothing we could wire locally would have made it work on preview.11: * Testing.Targets now forwards $(VSTestTestCaseFilter) (from `dotnet test --filter`) to the DeviceRunners CLI. * The CLI passes it on — WASM as a `?device-runners-filter=<expr>` query param, Maui/Windows as a `--device-runners-filter <expr>` arg. * The Blazor runner's AddCliConfiguration(url) reads that query param and the Maui runner's AddCliConfiguration() reads the arg; both call SetTestCaseFilter(...). preview.11's reader only understood `device-runners-autorun`. Bump all DeviceRunners packages in both projects preview.11 -> preview.12. No test code changes are required: Blazor's UseVisualTestRunner auto-wires AddCliConfiguration, and MauiProgram.cs already calls it — the bump alone enables the filter. preview.12 was verified to restore cleanly and stay API-compatible; a full end-to-end `--filter` run was not executed. Note: NuGet.config restricts restore to the dnceng dotnet-public mirror, into which preview.12 was only partially upstreamed. These IDs had to be mirrored at 0.1.0-preview.12 before CI restore would succeed: * DeviceRunners.VisualRunners.Blazor * DeviceRunners.VisualRunners.Xunit3 * DeviceRunners.UITesting.Xunit3 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
…ount (#4512) Build the internal name->Variable dictionary with an explicit capacity (names.Length) instead of the default (zero) capacity. The constructor already knows the exact final entry count up front, so the default-sized map needlessly rehashes several times while it is filled, allocating throwaway bucket/entry arrays on a per-uniform-set (potentially per-frame) path. Behaviour is identical: dictionary capacity is a pure sizing hint. Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…d path (#4498) Encode text directly into a byte[byteCount + nullBytes] buffer instead of concatenating a '\0' onto the source string before encoding. The trailing zero bytes match the encoding of a '\0' exactly (UTF-8=1, UTF-16=2, UTF-32=4 bytes), so output is byte-identical while removing one managed string allocation and copy per call on the native-marshalling hot path. Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…r failure path (#4455) * Fix CoTaskMem pixel-buffer leak when SKImage.Create raster fails SKImage.Create(SKImageInfo) allocates an unmanaged pixel buffer with Marshal.AllocCoTaskMem and relies on the native raster image's release proc to free it. When sk_image_new_raster fails (e.g. a valid-BytesSize info Skia rejects), no image is created, the release proc never runs, and the buffer leaks. Free it explicitly on the failure path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Skip pixel-buffer leak test on platforms without Process memory API The FailedRasterCreateDoesNotLeakPixelBuffer regression test measures the leak via System.Diagnostics.Process.PrivateMemorySize64. That API is not supported on WASM (no System.Diagnostics.Process at all), iOS, or Mac Catalyst, so the test threw PlatformNotSupportedException and failed the Tests WASM (Linux) and Tests iOS (macOS) legs (and Mac Catalyst). Guard the test with SkipOnPlatform(IsBrowser || IsIOS || IsMacCatalyst), matching the existing platform-skip pattern in this suite. The SKImage.Create fix itself is platform-agnostic and remains exercised by the desktop and Android test legs, where PrivateMemorySize64 is available. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4649389c-a7bd-443d-8c26-7c6b16cda819 * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Apply suggestions from code review Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Matthew Leibowitz <mattleibow@live.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Copilot-Session: 4649389c-a7bd-443d-8c26-7c6b16cda819
…erver (#4515) Add containerized test legs: Azure Linux, Alpine/musl, Nano Server (#4515) Run the SkiaSharp console test suite inside Docker containers as gating CI legs so the bindings are verified on runtime environments the existing matrix never covered: glibc Linux without a desktop, musl (Alpine), the same two with no font/dependency packages installed, and Windows Nano Server. These environments regularly surface real behavioral differences (font managers, allocator behavior, missing system libraries) that the desktop agents hide, and until now nothing exercised them. Five gating legs are added, each reusing the bootstrapper's existing `docker:` mechanism (compliant YAML tasks, not an ad-hoc script) rather than a new runner: * Linux (Azure Linux) — glibc, fontconfig + fonts * Linux (Azure Linux, No Dependencies) — glibc, nothing installed * Linux (Alpine) — musl, fontconfig + fonts * Linux (Alpine, No Dependencies) — musl, nothing installed * Windows (Nano Server) — the FreeType-backed nano build ~~ Native selection without rebuilding ~~ The test build selects a prebuilt native library by platform via a new `SkiaSharpNativePlatform` MSBuild property, copying from output/native/<platform>/<arch> instead of the OS-derived globs. This lets a Linux host test the Alpine, Nano Server, etc. libraries produced by the native pipeline without rebuilding them per leg. The arch condition uses $(Platform) when explicitly set (not AnyCPU) and otherwise the detected $(BuildArch), so AnyCPU test runs still resolve the right binary. ~~ Fontless and No-Dependency environments ~~ The No-Dependency images install nothing on top of the base .NET SDK image; that is the point of the leg — prove the native library loads and runs on a bare base (it does: libstdc++/libgcc/ICU already ship in the base images). Because those environments have no system font manager, font-dependent tests self-skip at runtime via a probe (SKFontManager.Default cannot match a character) instead of a hardcoded platform filter, so the same assembly does the right thing on every leg. The font-equipped images install fontconfig + DejaVu, and — since Azure Linux ships no emoji font in any of its repos — the Azure Linux image downloads Noto Emoji pinned by commit and verified by SHA-256 into a fonts directory so the Unicode/emoji tests have coverage. ~~ Windows Nano Server ~~ Nano Server uses the FreeType-backed native build (no system fonts, but it can load typefaces from streams/files). Getting it green required: * detecting Nano at runtime from the OS (registry InstallationType), not by probing a feature, so behavior is correct regardless of what is installed; * splitting the XPS document tests, which require the Windows XPS Object Model / DirectWrite that Nano lacks (SupportsXps => Windows && !Nano); * skipping sn.exe strong-name re-sign and mdoc.exe doc generation in the Nano container build, which are unavailable there; * a Nano-specific raster golden for the text render. ~~ Skip the heavy bitmap-threading stress variant on musl ~~ Adding the first musl legs surfaced a latent hang, not a new regression: SKBitmapThreadingTest.ImageScalingMultipleThreadsTest(100 threads, 1000 iterations) deliberately creates ~100K undisposed native allocations to stress GC-finalizer throughput (#3608). It already self-skips on x86, where "the GC can't keep up and Skia's native allocator fails"; on x64 musl the same pressure stalls the allocator/finalizer and the run hangs until hangdump kills the leg (exit 7). Reproduced deterministically by running the managed tests against the real CI x64 musl native library under emulation. The existing self-skip is extended to musl (glibc keeps up and still runs it); the lighter (10,10)/(10,100) variants keep running there, so the #3608 regression guard is retained. Adds IsGlibc/IsMusl test helpers (musl == Linux and not glibc, via the gnu_get_libc_version probe already used for fonts). ~~ Job display names ~~ Test-job names now lead with the platform, with brackets only for genuine variations: Windows (.NET Framework)/(.NET Core)/(Nano Server), macOS, Linux / Linux (Azure Linux[, No Dependencies]) / Linux (Alpine[, No Dependencies]), Android, iOS, Mac Catalyst, WASM / WASM (.NET Preview). Design and per-leg font strategy are documented in documentation/dev/containerized-testing.md. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
[build] Convert all .sln solutions to .slnx (#4528) Moves every solution in the repo off the legacy `.sln` format onto the modern XML-based `.slnx` format, matching the convention the Gallery samples already use (they were authored natively as `.slnx`). This keeps the solution files diff-friendly and consistent across the tree. Ran `dotnet sln <file> migrate` on all 30 `.sln` files and removed the originals. Migration preserved solution configurations, including the native C++ solutions' custom x64 / Win32 / ARM64 platforms. The 19 `.slnf` solution filters were kept rather than rewritten, and each one's `solution.path` was repointed from `X.sln` to `X.slnx`. This is the lowest-risk option and preserves the existing filter semantics exactly; the current SDK (10.0.301) loads a `.slnf` whose solution path is a `.slnx`, so no filtering behavior changes. Updated the 14 files that name a specific solution: the native `build.cake` scripts, `scripts/infra/caching/repo-deps.json`, both `.devcontainer` init scripts, `.vscode/settings.json`, the `documentation/dev` guides, and the affected agent skills. Left alone: `samples.cake` already discovers and rewrites `.sln`/`.slnf`/`.slnx` generically; `libs.cake`/`nuget.cake` reference the `.slnf` filters by their unchanged filenames; and the residual `.sln` mentions in `.gitattributes` (eol rule) and `.gitignore` (`*.sln.docstates`) are harmless. New `.slnx` files use LF via the existing `* text=auto` rule. Verified: all 34 `.slnx` parse via `dotnet sln list`, all 19 `.slnf` are valid JSON pointing to existing targets, and `dotnet build utils/Utils.slnx -t:Restore` succeeds. The two native C++ solutions build only on Windows CI (MSBuild 17.14+ supports `.slnx`), so they could not be built locally on macOS. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… + Linux (#4527) Add SkiaSharp.Vulkan.Silk.NET package and run Vulkan tests on Android + Linux (#4527) SkiaSharp's only managed Vulkan helper was SkiaSharp.Vulkan.SharpVk, pinned to SharpVk 0.4.2 (a 2018 release). SharpVk is effectively unmaintained and its native loader throws NotSupportedException on Android -- it only handles Windows and Linux -- so the Vulkan tests have always been desktop/Windows-only. With the Graphite GPU backend (#3968) bringing Vulkan to mobile, we need a managed Vulkan binding that actually works on Android. ~~ New package: SkiaSharp.Vulkan.Silk.NET ~~ Adds a convenience package (namespace SkiaSharp, netstandard2.0 -> net10.0) built on the actively maintained, cross-platform Silk.NET.Vulkan, mirroring the shape of the existing SharpVk package. GRSilkNetBackendContext maps Silk.NET's Instance/PhysicalDevice/Device/Queue (nint handles) plus a pinned PhysicalDeviceFeatures onto SkiaSharp's raw-IntPtr GRVkBackendContext and bridges the GetProcedureAddress delegate Skia uses to resolve Vulkan entry points. GRVkExtensionsSilkNetExtensions.Initialize(...) is a typed extension (plus an overload taking instance/device extension arrays). SkiaSharp.Vulkan.SharpVk is intentionally left untouched; nothing is removed. ~~ Core API: public GRVkExtensions constructor ~~ The parameterless GRVkExtensions() constructor is changed from private to public, mirroring native skgpu::VulkanExtensions (default-construct, then init) and SkiaSharp's own allocating SKPaint/SKPath constructors. This lets the Silk.NET helper expose a plain Initialize(...) extension instead of a bespoke Create(...) factory. GRVkExtensions.Create(...) is unchanged. This is the only change to a shipping package and is purely additive (a widened accessibility), so it is ABI/API-safe. ~~ Test restructuring: keep unsigned Vulkan deps off Apple + net48 ~~ The Vulkan tests bring Vulkan up through a headless Silk.NET context (SilkVkContext) that uses the OS-provided loader, so they run anywhere a Vulkan ICD is present. An earlier iteration folded the unsigned Silk.NET.Vulkan/SharpVk dependencies into the shared, strong-named SkiaSharp.Tests base library, which broke three CI legs: * iOS / Mac Catalyst -- the Apple AOT linker must resolve every native symbol at link time; there is no libvulkan on Apple, so the link failed with "clang++ ... Undefined symbols: _vkBindImageMemory" (and friends). * Windows .NET Framework (net48) -- the full framework enforces strong-name references at load time, so the strong-named host could not load the unsigned Silk.NET.Vulkan and threw FileLoadException "A strongly-named assembly is required" (0x80131044). .NET Core does not enforce this, which is why the net10 build was clean and the failure only surfaced at runtime on net48. To keep those dependencies out of the base library and out of every strong-named / Apple-linked assembly, the Vulkan tests now live in two dedicated, non-strong- named projects: * tests/SkiaSharp.Vulkan.Tests.Console -- a standalone Microsoft.Testing.Platform executable (net10.0 + net48, SignAssembly=false) that references the signed SkiaSharp.Tests.Console for shared infrastructure (unsigned -> signed is allowed) plus Silk.NET/SharpVk. This is the desktop runner (Linux/Lavapipe and Windows). * tests/SkiaSharp.Vulkan.Tests -- a library targeting only net10.0-android / -windows, referenced by the MAUI Devices host and registered in MauiProgram, so Vulkan never reaches the iOS/Mac Catalyst heads. Supporting changes so the separate assemblies integrate with the shared visual- regression harness: * CatalogReflection discovers renderers across the catalog, entry, and all loaded SkiaSharp.* assemblies, so the ganesh-vulkan renderer (in the separate Vulkan library) joins the shared VisualMatrixTests inside the single-process Devices host. This also preps the identical pattern for a future Direct3D test library: reference the library + register its assembly. * VisualMatrixTests matches only its own assembly's renderers (NamesIn(own assembly) instead of AllNames), so the base matrix and each satellite never double-run a cell when both are loaded in one process. * GPU rendering is serialized with an xUnit [Collection(DisableParallelization = true)] instead of the previous process-wide GpuRenderGate static lock. xUnit collections are scoped to a single assembly, so each assembly carries its own definition -- GpuRenderingCollection (base, GL/Metal) and VulkanGpuRenderingCollection (Vulkan). No cross-assembly lock is needed because the base and Vulkan tests never run concurrently: the Console hosts run them as separate processes, and DeviceRunners executes registered assemblies sequentially. * tests/Dockerfile.linux installs libvulkan1 + mesa-vulkan-drivers (Mesa Lavapipe software ICD) so the Vulkan GPU tests render headlessly in the container. ~~ Validation ~~ * Linux (repo test container + Mesa Lavapipe): Silk.NET Vulkan tests render and pass; the visual matrix is green against the seeded ganesh-vulkan.linux goldens. * Android (API 36 arm64 emulator, ranchu ICD): Vulkan comes up through libvulkan.so and all five ganesh-vulkan.android cells pass against seeded goldens -- the platform SharpVk could never reach -- with each renderer run exactly once (no double-run). * iOS / Mac Catalyst: the Devices heads build cleanly again; the earlier "Undefined symbols: _vk*" link failures are gone. * CI: green on the xamarin "SkiaSharp (Public)" pipeline; the previously-red iOS, Mac Catalyst, and Windows .NET Framework legs all report 0 failed tests. Windows Vulkan CI enablement (a software SwiftShader ICD + ganesh-vulkan.windows goldens) is delivered by the stacked follow-up #4531, which lands on top of this PR. Follow-up: the new public types and the now-public GRVkExtensions constructor still need generated mdoc XML via the normal docs flow. Co-authored-by: Matthew Leibowitz <mattleibow@live.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Modernize the pull request template
Replace the old plain-markdown PR template with a structured, graphics-library-
focused one and require its use when opening PRs.
Redesign informed by a survey of well-run graphics/game engines and .NET
interop libraries (Bevy, Godot, three.js, MonoGame, Avalonia, WPF/WinForms,
Flutter, ImageSharp, Silk.NET, mono/skia). Key changes:
- Real `##` headings instead of bold pseudo-headings, matching the modern
YAML issue forms.
- New "Areas Affected" checkboxes to route reviewers (managed vs native/C API
vs generated bindings vs native-dep/Skia update vs views vs tests).
- "API Changes" now uses fenced ```csharp blocks (Added / Obsoleted) so
multi-line signatures are readable; keeps the strict-ABI, additive-only rule.
- "Testing" upgraded from a lone checkbox to a narrative prompt covering the
CPU/GPU/platform matrix and golden-image updates.
- Collapsible before/after screenshots section for rendering changes.
- Native path reminders: companion mono/skia PR, submodule staging, and
`pwsh ./utils/generate.ps1` regeneration.
- Reference docs live in mono/SkiaSharp-API-docs, so the checklist asks
contributors to file a docs issue there rather than add XML comments here.
- Dropped stale/unverifiable items ("rebased on main", "adhere to coding
standard") that CI/merge-queue already handle.
AGENTS.md now instructs contributors to populate every section of the template
when opening a PR.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d1306b3c-b91d-4cf5-88f3-366c864bb414
* Use H3 headings in PR template
H2 renders heavy for an ~8-section form. Switch section headings to H3 (###),
matching the dotnet/maui sibling template while keeping real headings for
anchors/outline.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d1306b3c-b91d-4cf5-88f3-366c864bb414
* Group PR template into fewer headings with bold sub-sections
Collapse the eight H3 sections into five, nesting the context fields (Fixes,
Areas affected, Required skia PR) as bold sub-headings under Description — the
same pattern API changes already uses for Added/Obsoleted. Lighter visual
weight, same information.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d1306b3c-b91d-4cf5-88f3-366c864bb414
* Merge changes into one H2 section; use H2 for underlined section rules
Fold API and behavioral changes under a single "Changes" heading, leaving four
top-level sections (Description, Changes, Testing, Checklist). Promote those to
H2 so GitHub renders the underline rule between sections; bold sub-headings
carry the detail so the H2s read as dividers, not clutter.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d1306b3c-b91d-4cf5-88f3-366c864bb414
* Rename Fixes sub-heading to Related issues
Avoids the repetitive "Fixes" heading directly above "Fixes #".
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d1306b3c-b91d-4cf5-88f3-366c864bb414
* Make PR template lazy-friendly with copy-paste sub-templates
Default the body to a short, good-enough state: "Changes" defaults to "None."
and the before/after screenshot table is no longer always-on. The full formats
(Added/Obsoleted/Behavioral fences, the screenshot <details> block) now live as
copy-paste blocks inside each section's HTML comment — inert until an author
drops one in over the default. Optimizes for the common case (a short
description) while keeping rich structure one paste away.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d1306b3c-b91d-4cf5-88f3-366c864bb414
* Reorder Description sub-sections to issue, skia PR, areas
Put the companion "Required skia PR" right after the issue link, before the
"Areas affected" checkboxes.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d1306b3c-b91d-4cf5-88f3-366c864bb414
* Reframe Changes as public API + behavior, discourage file lists
Authors were dumping a file-by-file changelog into "Changes". Rescope the
section to just the public API surface (a single copy-paste code fence) and
observable behavior, and expand the comment to say explicitly that it is NOT a
file list — the diff already shows files and the narrative belongs in
Description. Merges the old Added/Obsoleted blocks into one Public API fence.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d1306b3c-b91d-4cf5-88f3-366c864bb414
---------
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d1306b3c-b91d-4cf5-88f3-366c864bb414
Keep the Skia gitlink and Component Governance registration aligned with mono/skia's skiasharp branch, including manifest-only drift repairs. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 47cd9a3e-b097-43f3-b41f-7188e91b388a
Latest: Merge pull request #293 from mono/mattleibow-patch-1 Commit: ea1f6a46c97545be81d028abe7f8f6e2f0b79854 Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
…#4531) Execute Vulkan visual tests on the Windows CI legs via a software ICD (#4531) Context: #4527 Follow-up to #4527, which added the Silk.NET Vulkan test path and enabled the ganesh-vulkan visual cells on Linux (Mesa lavapipe) and Android (emulator SwiftShader). Windows already compiled the tests and built libSkiaSharp with Vulkan, but the headless CI agent ships no Vulkan driver, so every ganesh-vulkan cell skipped — the backend was built and never exercised. Provision a software Vulkan ICD on the Windows test agent so those cells render and golden-compare instead of skipping, mirroring what the Linux leg does with lavapipe. A pwsh preBuildStep on both Windows test legs (.NET Core and .NET Framework) runs scripts/infra/native/windows/install-vulkan-icd.ps1, which downloads and SHA-256-verifies the Khronos loader (vulkan-1.dll) and Google SwiftShader (vk_swiftshader.dll + vk_swiftshader_icd.json) from pinned, prebuilt Silk.NET native packages, extracts them under externals/vulkan-icd, registers the ICD, and puts the loader on PATH. Register the ICD in HKLM\SOFTWARE\Khronos\Vulkan\Drivers rather than via VK_ICD_FILENAMES / VK_DRIVER_FILES: the agent runs elevated, and the loader ignores those environment variables under elevation, reading the driver list only from the registry. The script fails the leg on any download, checksum, or registry error instead of silently degrading back to a skip — provisioning can only add coverage, never hide a regression. Seed five ganesh-vulkan.windows goldens harvested from the agent's own SwiftShader renders; cross-implementation pixel differences mean Windows goldens must come from the Windows ICD, not from Linux/Android. A future driver-driven mismatch reseeds via scripts/infra/tests/extract-visual-goldens.py, the same flow as the other legs. CI-only change: no product, binding, or test-project code is touched (+88/-0), so apps running against real Vulkan drivers are unaffected. Verified on CI — the x64 Windows .NET Core and .NET Framework legs now run SkiaSharp.Vulkan.Tests.Console with 15 passed / 0 skipped, including all five VulkanVisualTests.RenderMatchesGolden ("ganesh-vulkan") cells, and golden extraction reports 0 mismatch / 0 unseeded. The 32-bit netfx (x86) slice still skips Vulkan (NotExecuted, never failed) because the provisioned SwiftShader ICD is 64-bit only. Linux and Android provisioning is unchanged. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Add a manual workflow input for the SkiaSharp target branch and carry it through checkout, concurrency, automation branch naming, and PR creation. Scheduled syncs continue to target main. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: dbf36b1f-f08e-411f-b059-62d111fe63dc
[skia-sync] Sync Skia with chrome/m151 bug fixes (#4538) Context: mono/skia#294 Changes: mono/skia@ea1f6a4...1af4051 Advance the Skia submodule to the merged same-milestone `chrome/m151` bug-fix sync. This brings in upstream commit `b4204f7ff9`, which prevents Ganesh readbacks from returning stale texture data when pending render tasks fail to flush. Update `cgmanifest.json` to record the new mono/skia and upstream commits. This is not a milestone or version bump. It includes no C API, binding, DEPS, or ABI changes. Co-authored-by: Matthew Leibowitz <mattleibow@live.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…4545) Add backend-neutral async RequestReadPixels to SKImage and SKSurface (#4545) Requires: mono/skia#295 Related: #3968 (Graphite backend) Docs: mono/SkiaSharp-API-docs#184 GPU-backed surfaces need a way to read pixels back to the CPU, and on some backends (notably Graphite) the asynchronous rescale-and-read is the *only* readback path — Skia gates synchronous GPU ReadPixels behind GPU_TEST_UTILS. The in-flight Graphite PR (#3968) had added this under Graphite-prefixed names, but the underlying C++ types (SkImage::AsyncReadResult, SkImage::RescaleGamma, SkImage::RescaleMode) are core and shared by the raster, Ganesh and Graphite backends — SkSurface merely aliases the SkImage ones. So this lands the feature generically on main; #3968 can then drop its SKGraphite* copies and consume it. Add RequestReadPixels overloads on SKImage and SKSurface that deliver the pixels to an Action<SKImageReadPixelsResult>. Raster reads invoke the callback synchronously; Ganesh defers until the work is submitted and drained via the new GRContext.CheckAsyncWorkCompletion(). The result is a non-owning, callback-scoped view (deliberately not an SKObject — the native pointer is non-ref-counted and reused); it is invalidated when the callback returns and throws ObjectDisposedException afterwards. Failure is signalled by a null result, and there is no state argument (capture it in the closure). * SKImageReadPixelsResult: PlaneCount, GetPlaneRowBytes and a raw zero-copy GetPlaneData(ReadOnlySpan<byte>), plus copy-out helpers that outlive the callback — CopyPlaneTo (tightly packed, bounded), ToArray, ToImage, ToBitmap. * SKImageRescaleGamma { Src, Linear } and SKImageRescaleMode { Nearest, Linear, RepeatedLinear, RepeatedCubic }. The convenience overload defaults to (Src, Nearest) — the least-transform choice, matching what Skia itself forces when no rescale is required (SurfaceContext normalises to kSrc/kNearest at 1:1). * Bumps externals/skia to the C API (mono/skia#295) and regenerates the P/Invoke bindings; the only hand-written generated piece is the fire-once DelegateProxies async-read proxy. Scope is the RGBA (count()==1) read; YUV420/YUVA420 are a clean additive follow-up, as the result type and accessors are already multi-plane. Tested by 18 SKAsyncReadPixelsTest cases: per-pixel pattern correctness read from an offset srcRect, a deterministic padding-strip unit test, destination-size and invalid-plane-index edge cases, use-after-callback throwing cleanly, and Ganesh deferred completion (asserting the false -> pump -> true transition where the backend supports it). Co-authored-by: Matthew Leibowitz <mattleibow@live.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Add Graphite GPU backend bindings for Vulkan, Metal, and Dawn (#3968) Context: mono/skia#236 Context: #4545 Tracking: #4555 Expose Skia's next-generation Graphite GPU backend through additive SkiaSharp APIs. The new surface follows Graphite's explicit record, snap, insert, and submit model while leaving the existing Ganesh APIs and behavior unchanged. Add managed wrappers for contexts, recorders, recordings, backend textures, texture information, and Vulkan, Metal, and Dawn backend contexts. Support wrapping caller-owned textures with release callbacks and provide SKGraphiteImageCache as the default image-provider strategy for uploading and reusing raster images. Enable Graphite in the native builds for supported desktop and mobile platforms. WebAssembly uses Dawn's emdawnwebgpu port so Graphite can render through browser WebGPU. Readback uses the existing backend-neutral asynchronous API because shipping Graphite surfaces do not support synchronous ReadPixels. Cover the new surface with API tests, backend-context and release callback tests, and a cross-backend visual regression matrix with platform-specific goldens. The iOS simulator's Graphite/Metal gradient limitation remains isolated to that environment and is tracked separately. Co-authored-by: Ramez Ragaa <ramezragaa@proton.me> Co-authored-by: Matthew Leibowitz <mattleibow@live.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…4550) On span-capable TFMs, write the native buffer straight to the target Stream as a ReadOnlySpan<byte> instead of copying it through a pooled managed buffer with Marshal.Copy. This removes one full memcpy per 80 KB chunk (and the intermediate buffer), making SaveTo ~2x faster with no allocation change. netstandard2.0 and .NET Framework keep the original buffered path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Matthew Leibowitz <mattleibow@live.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
[skia-sync] Update m151 with upstream stability fixes (#4568) Changes: mono/skia#301 Changes: google/skia@b4204f7...755b89e Advance the Skia submodule and Component Governance metadata to include four m151 font-rendering and Ganesh stability fixes. This is a same-milestone bug-fix sync, so ABI, assembly, soname and NuGet versions remain unchanged. Regeneration added only typedef aliases to the Resources, SceneGraph and Skottie bindings; no P/Invoke entries or public SkiaSharp APIs changed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
[Windows] Initialize the MSVC environment once per native build (#4570) Context: #4570 Skia change: mono/skia#306 Update the Skia submodule to remove per-edge vcvarsall invocation, then initialize the architecture-specific Visual C++ environment once before launching Ninja. Split GN generation from Ninja execution so Windows can capture the required MSVC and Windows SDK environment without changing other native build paths. Running vcvarsall for every compile, archive, and link edge added substantial process overhead. Clean hosted-agent builds for x86, x64, and ARM64 completed successfully with one initialization per job and reduced native build times by 54–57%. Co-authored-by: Matt Mitchell (.NET) <mmitche@microsoft.com> Co-authored-by: Matthew Leibowitz <mattleibow@live.com> Co-authored-by: Alexander Köplinger <alex.koeplinger@outlook.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
[CI] Move public builds to dnceng agents (#4565) Context: #4565 Move eligible Linux and managed Windows jobs from Microsoft-hosted agents to the dnceng NetCore-Public pool. Keep workloads requiring capabilities absent from the lean images on approved hosted agents: Android emulator tests need KVM, native Windows builds need the complete VS 2022 toolchain with Spectre libraries, and native macOS uses macOS 15. The hosted images previously supplied several undeclared prerequisites. Provision Python, Node 20, Android platform and build tools, and Chrome where required; pass AndroidSdkDirectory explicitly to MSBuild; and reuse existing Ninja and 7-Zip installations before installing fallbacks. Pin direct Chrome and 7-Zip downloads and verify their SHA-256 digests before installation. Also validate Chrome package identity, version, and architecture so compromised or unexpectedly changed artifacts fail before execution. Co-authored-by: Matthew Leibowitz <mattleibow@live.com> Co-authored-by: Matt Mitchell <mmitche@microsoft.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
[Windows] Stop pinning a Windows SDK version by default (#4588) Context: #4588 Context: 7c23c53 Follow-up: #4589 The benchmark-source (windows-latest, Windows, native/windows) leg of Track - Benchmarks died the instant it began the native build: Done. Made 114 targets from 63 files in 358ms Initializing the Visual C++ environment once for amd64. An error occurred when executing task 'libSkiaSharp'. Error: Process 'cmd.exe' failed with error: 1 RunNinjaWithVcVars invoked vcvarsall.bat with the default of the windowsSdkVersion argument, 10.0.22621.0, which matches the AzDO agents. The hosted runner is windows-2025-vs2026 and ships only Windows SDK 10.0.26100.0, and vcvarsall hard-fails on an SDK that is not installed: [ERROR:winsdk.bat] Windows SDK 10.0.22621.0 : 'C:\Program Files (x86)\Windows Kits\10\include\10.0.22621.0\um' not found or was incomplete Nothing needed that default. Before the MSVC environment was initialized explicitly there was no such argument at all: gn resolved win_sdk_version itself and the toolchain interpolated that resolved value into the vcvarsall command it generated, so the version was always one that existed on the machine and the two could never disagree. Hardcoding a constant only meant the build broke wherever that exact SDK was absent - this runner today, and equally a developer box carrying just a newer one. Default the argument to empty and omit it from both gn and vcvarsall when unset, so each detects an SDK that is present. Setting it still pins both together, so --windowsSdkVersion remains available to build against a specific SDK. * drop the `>nul` that hid the failure. It suppressed the vcvarsall banner, but `>nul` redirects only stdout and VsDevCmd writes its errors there too, so it discarded the diagnosis along with the banner - rerunning with `2>nul` still prints the [ERROR:winsdk.bat] lines, while `>nul` yields nothing but the exit code. The banner is five lines per architecture and worth keeping in the log. * this went unnoticed because benchmark-source runs on pull requests only and is skipped on main, so #4570 merged green without the path ever being exercised. * whether the vcvarsall call is needed at all is left open in #4589: every tool is invoked by absolute path and gn emits the SDK include and lib directories as explicit -imsvc and /LIBPATH: flags, so it may be redundant for our LLVM-based configuration. Co-authored-by: Matthew Leibowitz <mattleibow@live.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…ecar (#4603) Skia made Vulkan 1.1 the Ganesh minimum in m139 (0a3150b, "[vulkan] Require Vulkan 1.1 in Ganesh") and promoted the minimum-version check from a debug check to a fatal SK_ABORT in m140 (dfea59a). The consequence is severe and was undocumented on our side: code that previously got a graceful null back from GRContext.CreateVulkan on a Vulkan 1.0 instance or device -- and could fall back to raster -- now aborts the process. It is not an exception, so managed code can neither catch nor pre-empt it. Callers are exposed if the VkInstance was created with VkApplicationInfo .apiVersion below VK_API_VERSION_1_1 (including leaving it at 0, which the Vulkan spec treats as 1.0), or if GRVkBackendContext.MaxAPIVersion is left at its default 0. That default makes Skia fall back to vkEnumerateInstanceVersion -- the loader's version, not the version the instance was actually created with -- so it can over-declare and ask for entry points the instance cannot serve. The fix is both halves: declare at least 1.1 on the instance AND set MaxAPIVersion to match. 4.148.0 is the right page, not 4.151.0. Determined from the externals/skia SHA each release tag actually pins, and whether the two upstream commits are in its history: v3.119.4 7dbfc07d diverged from both (VulkanUtilsPriv.cpp absent) v4.147.0-preview.1.1 6f8139ad ahead of both (SK_ABORT present) v4.148.0 1a155bae ahead of both (SK_ABORT present) So the abort first ships in a 4.147.0 preview -- but 4.147.0 has only -preview.{1,2,3} tags and no v4.147.0, matching its release data (status: preview, superseded_by 4.148.0, "never released as stable"). v4.148.0 is the first stable tag, so it is where someone upgrading from 3.119.x meets this. It is also already the home for this class of change: the sidecar scopes itself to the v4 breaking changes from the m132/m147 upgrade that no signature diff can show, and already documents the GRVkBackendContextNative field removals. The new entry leads that Vulkan group and cross-references it -- that entry covers fields that disappeared (including fMinAPIVersion), this one covers the surviving fMaxAPIVersion whose default is now unsafe. Sidecar only, per spec section 3.7: this file is maintainer-owned and never machine-written, and editing it flips its sha256 in data.json on the next Prepare run, which drops the stale prose.json and re-polishes exactly this page. So the rendered 4.148.0.md, its prose.json and the data.json hash are deliberately left to CI rather than hand-written here. Related to #4600 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c512dee0-fb90-4ff0-9916-57567ecdca6a
[tests] Make GPU tests required; skip only when declared (#4575) Context: #4575 Context: documentation/dev/gpu-test-policy.md GPU tests brought a backend up inside a try/catch and called Assert.Skip on any exception. That made "this agent has no GPU" and "this binding is broken" indistinguishable, and reported both as green. The committed goldens recorded what it cost: ganesh-gl had a golden for macOS only, and graphite-dawn had none at all, so those backends had been skipping everywhere for as long as they had existed. Nobody could answer which backends actually ran. Invert the default. A backend is now required on every platform listed in GpuPolicy.RequiredOn, which mirrors the gn args in native/*/build.cake, and a skip has to be declared. Failing to bring a required backend up is a test failure, so RendererUnavailableException is deleted along with every catch that swallowed it. Skips come from exactly two places, kept separate because that distinction is what a reader needs when a test skips: * the compiled-in table is a statement about a platform — Metal is not required off Apple because we do not build it there * SKIASHARP_TEST_SKIP_GPU is a statement about one agent, declared per leg in scripts/azure-templates-stages-test.yml with its reason and tracking issue inline, so it can be removed when that agent gains the capability without touching the policy or the documentation Device and browser hosts never see the agent environment, so the same list reaches them through the SkiaSharpTestSkipGpu MSBuild property baked into runtimeconfig.json. An unrecognised backend id is an error, so a typo cannot quietly leave a backend required. ~~ Latent bugs the required tests surfaced ~~ Five real bugs, every one of them previously invisible as a skip: * DllImport used the unversioned libGL and libX11 SONAMEs, which are linker symlinks shipped only in -dev packages. Linux GL could never load, and every GL test skipped silently for as long as that binding existed. * GlxContext.Destroy never called XCloseDisplay — the binding was not even declared — leaking an X11 connection per GPU test. * SKBlenderTest asserted exact colour equality across two GPU shader paths. llvmpipe returns #ff5e0000 where the other path returns #ff5d0000. Raster stays exact; the GPU subclass allows a tolerance of one. * The Windows software Vulkan ICD was staged for win-x64 only, so x86 had no loader and 21 of 25 Vulkan tests failed. * Win32VkContext built a Vulkan 1.0 instance and set no MaxAPIVersion, so GRContext.CreateVulkan returned null and the test reported an ArgumentNullException three frames from the real problem. Skia has required 1.1 unconditionally since m139 [0] and acquires the core 1.1 device procs with no version gate; vkGetDeviceProcAddr gates those on the instance's apiVersion, so all nine came back null. It passed against CI's SwiftShader, which reports device 1.3.0, and failed on an AMD device reporting 1.4.329 — the result depended on the agent's driver. ~~ Limits that are declared, not fixed ~~ Fourteen opt-outs remain, each naming its cause. This is not a regression — every one was already skipping silently — but they are now enumerable, and * Metal is unusable on all three Apple hosts, for three unrelated reasons: virtualized Metal hangs the macOS host on shutdown (#4598), Mac Catalyst never sees the Mac2 GPU family (#4599), and the iOS simulator cannot compile Graphite's gradient pipeline (#4555) * Linux GL crashes the host once GLX loads, exiting 7 after 523, 312, 5068 and 0 tests with zero test failures and no TRX published. The XCloseDisplay fix above did not resolve it (#4590) * the headless browser the runner launches exposes no WebGPU adapter and the runner has no flag passthrough; reported upstream ~~ Structural fallout ~~ The .NET Framework leg is now two jobs. SwiftShader's LLVM JIT cannot resolve __chkstk in a 32-bit process, so software Vulkan crashes the host on x86 while x64 runs the same suite green. GpuPolicy models the OS but not the architecture, so separate jobs are what let x86 opt out while x64 keeps Vulkan required, and any future x64/x86 divergence becomes visible on its own leg. Mac Catalyst also stops being non-gating. It carried the repo's only continueOnError: true, added in #3247, which is the same failure mode this change exists to remove. ~~ Filed from the Vulkan investigation ~~ * #4600 — MaxAPIVersion's default of 0 makes Skia trust the loader's version rather than the instance's. Worse, since m140 [1] the minimum-version check is SK_ABORT rather than a graceful null, so a consumer who correctly declares Vulkan 1.0 now aborts the process instead of being able to fall back to raster. * #4601 — fDeviceLostProc and fDeviceLostContext are not exposed, so consumers get no notification on VK_ERROR_DEVICE_LOST. [0]: https://skia.googlesource.com/skia/+/0a3150b03b [1]: https://skia.googlesource.com/skia/+/dfea59a30f Co-authored-by: Matthew Leibowitz <mattleibow@live.com> Co-authored-by: Copilot <noreply@github.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
[bindings] Make generation deterministic across platforms (#4612) Context: #4612 SkiaSharpGenerator relied on filesystem enumeration and culture-sensitive string ordering, causing identical inputs to produce different function and type ordering across Windows, macOS, and Linux. Use normalized relative paths, ordinal comparisons, and canonical UTF-8/LF output. The resulting SkiaApi migration only reorders its existing 23,630 lines. Add CI that independently generates bindings on all three platforms, compares their outputs byte-for-byte, then verifies the canonical result matches the four committed Skia bindings. This detects cross-host nondeterminism, omitted regeneration, and manual generated-file edits. HarfBuzz freshness remains out of scope because its bindings are versioned separately. Co-authored-by: Matthew Leibowitz <mattleibow@live.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
[tests] Run ganesh-gl on the Windows and Linux CI legs (#4605) Context: #4605 Context: documentation/dev/gpu-test-policy.md Fixes: #4590 #4575 made a GPU backend required on every platform GpuPolicy.RequiredOn names and turned the silent skips into an enumerable list. ganesh-gl was the largest entry on it: required on Windows, macOS and Linux, but opted out of the two desktop legs that had no working GL stack. This closes those, so the Ganesh GL backend now runs on all three desktop platforms. This is not "GL was untested until now". GL was already covered in several places, and this fills in the ones that were not: * macOS already ran ganesh-gl through CGL. That is why ganesh-gl.macos was the only committed golden set — under the rules #4575 introduced, an unseeded backend fails, so those goldens exist because macOS was rendering. * Android, iOS and Mac Catalyst already exercise the views layer against real GL surfaces: MemoryLeakTests.SKGLViewHandlerDoesNotLeak runs and passes on all three device legs. * Windows had no OpenGL implementation at all, and Linux had GLX loading and then killing the host. Both are the low-level GRContext.CreateGl path the desktop console suite covers, and both are what this fixes. The five container legs and Nano Server keep their opt-out, and the comment now reads as a decision rather than a pending item: those images exist to prove SkiaSharp works with the bare minimum installed, so giving them an X server and Mesa would defeat their purpose. Two gaps this does not close, both pre-existing: SKGLView on WinUI has no device leg in this pipeline, and the Blazor SKGLView/WebGL test lives in SkiaSharp.Tests.Integration, which this pipeline does not run. ganesh-gl is not required on Browser at all; WASM's GPU backend is graphite-dawn, still opted out because the headless browser exposes no adapter. #4591 tracks the remainder. Every problem below was reproduced and measured before it was fixed: Mesa on Windows, and Ubuntu under WSL with Xvfb and GLX for Linux, which reproduced the CI crash faithfully enough to bisect it. ~~ Windows: no OpenGL at all ~~ The agents have no display driver carrying an OpenGL ICD, so opengl32.dll answers with the in-box generic implementation: OpenGL 1.1, no WGL_ARB_pixel_format, no WGL_ARB_pbuffer. WglContext cannot come up on that, and Ganesh needs 3.0+ regardless. scripts/infra/native/windows/install-mesa-gl.ps1 provisions Mesa the way Mesa documents for machines with no OpenGL driver [0]: libgallium_wgl.dll becomes System32\mesadrv.dll and SysWOW64\mesadrv.dll, registered under OpenGLDrivers\MSOGL. It follows install-vulkan-icd.ps1 — pinned version, SHA-256 pin, both bitnesses, fatal on any failure — and checks what provisioning is responsible for and no more: the driver is where opengl32 will look for it, its version resource matches the version just pinned rather than a leftover from an earlier run, and the registration reads back. Proving it renders belongs to the suite, and GRContextTest.CreateDefaultContextIsValid already does that on every leg where the policy requires ganesh-gl. ~~ Windows: SetPixelFormat and wglSetPixelFormat are not interchangeable ~~ With Mesa registered, provisioning still failed, and the diagnostics named it: mesadrv=C:\Windows\SYSTEM32\mesadrv.dll; pixelFormats=1332; chosenFormat=121 FAILED: wglCreateContext returned NULL (2000) Mesa is loaded and serving its own 1332 pixel formats, and 2000 is ERROR_INVALID_PIXEL_FORMAT. gdi32's ChoosePixelFormat reports Mesa's formats and SetPixelFormat returns true, but Mesa's per-HDC state is only created by its own wglSetPixelFormat, so wglCreateContext has nothing to look up. The two APIs turn out to be exactly inverted, consistently per driver: a hardware ICD is set up through GDI and refuses a context when the format was set the other way, and Mesa is the reverse. So the Wgl bootstrap tries GDI first — every real driver, and every developer running these tests locally — and falls back only when the context actually fails. A pixel format can be set once per DC and the class is CS_OWNDC, so the retry needs a fresh window; the first is destroyed before it is replaced. ~~ Linux: the crash was one test, not a race ~~ #4590 attributed the exit-7 host crash to nine test classes creating GLX contexts concurrently. That diagnosis was wrong. The crash reproduces with the suite serialized onto a single thread, and a trace shows every context created, used and destroyed on that one thread. The real cause is GRGlInterfaceTest.AssembleInterfaceIsValid, which resolved Skia's proc addresses with glXGetProcAddress alone. Under GLVND — the vendor-neutral libGL every current distro ships — that returns a dispatch stub for any name at all; glTotallyNotARealFunction comes back non-null. Skia was handed pointers that are not functions and walked off the end of the extension list in GrGLExtensions::init. It now resolves through dlsym on libGL.so.1, exactly as the macOS branch already did. The death point only looked random because the run order varied. ~~ Both: llvmpipe cannot compile one of Skia's shaders ~~ Mesa's LLVM shader JIT segfaults inside LLVMTypeOf, under GrGLOpsRenderPass::onDraw, building the fragment shader Skia generates for a runtime blender. Reproduced on two independent stacks — Mesa 25.2 with LLVM 20 on Linux, and 26.1 with LLVM 22 on Windows — so both legs pin softpipe, the non-JIT gallium rasterizer, which runs the identical tests green on both and caps at GL 3.3, well past the 3.0 Ganesh needs. Filed as #4604. Ruled out along the way, so nobody re-treads it: thread parallelism (--parallel none --max-threads 1 crashes identically), managed thread stack size at 2, 16 and 64 MB, LP_NUM_THREADS of 0 and 1, file-descriptor limits at 256, 1024 and 65536, and GL context churn — 400 create/use/destroy cycles are fine. ~~ Supporting changes ~~ * every class that creates a GPU context joins the existing GpuRenderingCollection, mirroring VulkanGpuRenderingCollection. This is what #4590 and #4591 asked for; it was not the crash fix, but concurrent GPU context use is a real hazard worth closing. Where only a nested class does GPU work (SKBlenderTest, SKRuntimeEffectTest) only that class joins, so its raster sibling stays parallel — roughly 300 of 6100 tests are serialized * GlxContext shares one X11 connection for the process instead of opening and closing one per test. That is the leak XCloseDisplay was added for in #4575, fixed at the source: one connection instead of 128 and counting, and no closing a display while a context minted from it is still current * XInitThreads runs before any other Xlib call * ganesh-gl.windows and ganesh-gl.linux goldens are harvested and committed, since a backend that renders for the first time is unseeded and fails until they exist ~~ Result ~~ * Windows (.NET Framework, x64) — 7415 tests, 0 failed, 23 skipped * Windows (.NET Framework, x86) — 7415 tests, 0 failed, 25 skipped * Windows (.NET Core) — 6117 tests, 0 failed, 31 skipped * Linux — 6117 tests, 0 failed, 34 skipped Linux previously exited 7 and published nothing at all. Measured locally, GL on versus off is 6079 against 5910 passing: 169 tests that had never run once. [0]: https://docs.mesa3d.org/drivers/llvmpipe.html#windows Co-authored-by: Matthew Leibowitz <mattleibow@live.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
[Issue Templates] Refresh SkiaSharp version dropdowns (#4622) Update `.github/ISSUE_TEMPLATE/bug-report.yml` with the current release metadata so new reports default to the latest stable version and users can accurately identify the last known good release. Keep pre-release, current, previous, deprecated, and obsolete labels aligned with the published release state. This is the generated output of `update-bug-template.py` from the automated issue-template refresh workflow. Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
…ixel source) (#4558) SKPixmap.ExtractSubset and WithColorType/WithColorSpace/WithAlphaType return a pixmap that points into the source pixmap's pixel memory (native does not copy), but did not root the pixel owner. Unlike PeekPixels (which sets pixelSource), these results kept no managed reference, so the pixel owner could be finalized while the returned pixmap was still in use -> use-after-free. Propagate the ultimate pixel owner (pixelSource ?? this) onto the returned pixmap, matching the existing PeekPixels rooting idiom. Managed C# only, no public signature change (ABI-safe). Adds red->green regression tests ExtractSubsetKeepsPixelSourceAlive and WithColorTypeKeepsPixelSourceAlive. Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ck (#4630) [performance] Cache HandleDictionary skip-registration reflection check (#4630) Context: #4629 HandleDictionary.GetInstance<T> and GetOrAddObject<T> run for every native-object wrap, but each call repeatedly used Type.IsAssignableFrom to determine whether the wrapper type skips global registration. That predicate is invariant for each closed wrapper type. Cache the result in a generic static holder so reflection runs once per T and the hot path reads a plain static boolean. This preserves behavior and does not change the public API or ABI. Add parity tests covering skip-registration and tracked wrapper types, plus an end-to-end deduplication check. Add a four-way benchmark comparing the existing cached-Type baseline, inline typeof, the PR cache, and inline typeof inside the generic cache. The generic cache variants are effectively equivalent; both avoid the repeated reflection cost without allocations. Co-authored-by: Matthew Leibowitz <mattleibow@live.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…t the SKRegion) (#4355) [memory-leak] Root SKRegion for iterator lifetimes (#4355) Context: #4354 SKRegion iterators hold native iterator state that references the source region's run data. Because SKRegion skips global handle registration, the region could be finalized while a SpanIterator was still alive, leaving the native iterator with a dangling pointer and causing use-after-free. Use SKObject.Referenced(this, region) for RectIterator, ClipIterator, and SpanIterator so each iterator keeps its source region alive for its entire lifetime. Add a regression test that forces collection and verifies the SpanIterator remains usable. This keeps the iterator family consistent, clears the keep-alive reference during disposal, and does not change the public API or ABI. Fixes #4354 Co-authored-by: Matthew Leibowitz <mattleibow@live.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Merges 2 upstream commits from google/skia chrome/m151: - [M151] Address incorrect handling of a map pointer in SkRP (86bb2f25f4) - Roll infra dep to 055b758759c89b0cfafc8264ae2100a9aa6582d3 (f7be6d7766) Same-milestone sync — only cgmanifest.json hash and submodule pointer updated. No C API, binding, or version changes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
mattleibow
force-pushed
the
skia-sync/m151
branch
from
August 4, 2026 16:16
903bbe9 to
a51e9cc
Compare
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
4 participants
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.
Automated upstream bug-fix sync for m151.
Companion skia PR: mono/skia#334
Summary
Same-milestone bug-fix sync (
m151 → m151). Pulls the two latest upstreamchrome/m151commits into mono/skia and updates the SkiaSharp submodule pointer. Not a version bump — milestone, soname, and NuGet versions are unchanged; onlycgmanifest.json'scommitHash/upstream_merge_commitmove forward.Paired PR: mono/skia
skia-sync/m151(see the sync summary in that PR).Breaking Change Analysis
None. The two upstream commits are:
86bb2f25f4[M151] Address incorrect handling of a map pointer in SkRPf7be6d7766Roll infra depNo public headers changed, no C API changes, no enum/struct/signature changes. Regeneration produced a zero-line diff to
SkiaApi.generated.cs.Version / Binding Updates
Handled by
.agents/skills/update-skia/scripts/update-versions.ps1 -Current 151 -Target 151 -UpstreamRef chrome/m151:cgmanifest.json:commitHashandupstream_merge_commitupdated to86bb2f25f46f4d620b4a26e738f59a9b6c22d2eb;chrome_milestoneunchanged (151).scripts/VERSIONS.txt: unchanged (same milestone → no version bump).scripts/azure-templates-variables.yml:SKIASHARP_VERSIONre-asserted at4.151.0(no change).externals/skia/include/c/sk_types.hSK_C_INCREMENT:0(unchanged).Verification greps skipped per script (same-milestone sync).
C# Wrapper Changes
None.
regenerate-bindings.ps1reported:No changes to bindings (C API signatures unchanged)No new functions foundbinding/HarfBuzzSharp/HarfBuzzApi.generated.csreverted per the standing policy (HarfBuzz updates are separate).Build & Test Results
dotnet cake --target=externals-linux --arch=x64→ ✅ (12m03s). Producedoutput/native/linux/x64/libSkiaSharp.so.151.0.0andlibHarfBuzzSharp.so.0.61421.0.dotnet build binding/SkiaSharp/SkiaSharp.csproj→ ✅ 0 errors, 4 warnings (unrelated NETSDK1202 workload-support notices).dotnet test tests/SkiaSharp.Tests.Console.slnx— seetest-output.txtartifact.SkiaSharp.Tests.dll (net10.0|x64): Failed: 169, Passed: 5922, Skipped: 33, Total: 6124SkiaSharp.Vulkan.Tests.dll (net10.0|x64): Failed: 19, Passed: 4, Skipped: 2, Total: 25Test failure analysis
Every observed failure has the same root cause:
These are GLX / Vulkan GPU tests that require a display / GPU stack, which is not provisioned in this headless workflow runner. The failures are environmental, not a regression from this sync — the two upstream commits are an internal SkSL fix and an infra dep roll, neither of which touches GPU init.
Per the AGENTS.md GPU policy this is technically a "no display" failure that should be treated as a real failure (not an inferred skip). Flagging for human attention below — this is a workflow-environment concern that pre-dates this sync and would reproduce on
mainunchanged.Items Needing Human Attention
GPU tests fail on this headless runner (169 + 19 failures, all
Failed to open X display). Not caused by this sync (upstream commits are SkSL-internal + infra). Options for the workflow owner:GlxContextcan initialize, orSKIASHARP_TEST_SKIP_GPU=1in the workflow env if this runner is intentionally CPU-only perGpuPolicy.Full log attached as
test-output.txtartifact.Paired mono/skia PR must merge first so the submodule pointer resolves on
origin/skiasharpbefore this PR merges (standard skia-sync sequence).Created by skia-upstream-sync.