Skip to content

Port SKMatrix math to managed C# to remove native interop (#2779)#4241

Open
mattleibow wants to merge 10 commits into
mainfrom
mattleibow-dev-issue-2779-skmatrix-managed-math
Open

Port SKMatrix math to managed C# to remove native interop (#2779)#4241
mattleibow wants to merge 10 commits into
mainfrom
mattleibow-dev-issue-2779-skmatrix-managed-math

Conversation

@mattleibow

@mattleibow mattleibow commented Jun 25, 2026

Copy link
Copy Markdown
Collaborator

Fixes #2779.

Problem

SKMatrix is a pure managed struct, but every math operation (Invert, Concat, MapPoint, MapVector, MapRect, MapRadius, MapPoints/MapVectors) round-tripped through the Skia C API. Each call paid the full P/Invoke transition cost — pinning, marshalling and a managed→native→managed hop — even though the underlying math is a handful of floating-point operations. For hot paths (mapping points, inverting/concatenating transforms in a render loop) this interop dominates the cost.

What this does

Reimplements the math in managed C#, faithfully mirroring Skia's SkMatrix algorithms so results are bit-for-bit identical to the native path. The implementation follows the exact C++ code Skia runs through the C shim (AsMatrix/MakeAll → proc-table dispatch), matching float-vs-double usage, operation order, type-mask computation, the ±0/Inf/NaN edge behaviour, and the skvx min/max/shuffle semantics.

The batch MapPoints/MapVectors paths use System.Numerics.Vector4 (two points per vector lane, mirroring Skia's skvx::float4 procs Trans_pts/Scale_pts/Affine_vpts), with a hardware Vector128.Shuffle swizzle on modern runtimes (net8+) and a portable Vector4 fallback on older TFMs (netstandard2.0, net4x, net6). The scale/translate batch procs unroll to four points per iteration (two float4) exactly like Skia's Scale_pts/Trans_pts so the multiplies pipeline. Perspective mapping stays scalar (as it is in Skia), and perspective MapRect still delegates to native (Skia uses a non-trivial path-clipping routine there).

No public API changes — only method bodies changed, plus one new private static readonly bool gate. ABI is unchanged: every public signature is byte-identical to main (verified by diff); the only public-line edits are expression-body ↔ block-body reformatting.

Correctness

Bit-exactness is the hard requirement (SkiaSharp must not change rendering output). New SKMatrixManagedTests call the Skia C API directly (SkiaApi.sk_matrix_*) and assert bit-equality against the managed result across ~230 matrices (static + 200 seeded-random), including:

  • perspective, degenerate / non-invertible, collinear-row matrices
  • -0, Inf, NaN, MaxValue, Epsilon members
  • scale-by-infinity matrices that specifically exercise the sort_as_rect NaN semantics
  • odd/small batch counts {0,1,2,3,5,6,8,10,11,12} to hit the SIMD pair loop, the unrolled 4-point loop and the scalar tail (incl. counts that combine the single-pair lead-in with the unrolled loop)
  • in-place mapping (dst == src) for both points and vectors
  • overlapping shifted spans (dst one element ahead of src in a shared buffer) for vectors, which pins the reverse-iteration order of the perspective path
  • Inf/NaN points and rects

All 12 equivalence tests pass bit-exact, the 20 existing SKMatrixTests stay green, and 151 additional matrix-consuming tests (SKMatrix44, SKPath, SKCanvas, SKShader, SKRoundRect, SKRotationScaleMatrix) pass.

This was independently reviewed against the pinned Skia submodule sources (SkMatrix.cpp, SkRect.cpp, SkVx.h, SkFloatingPoint.h) and by two AI reviewers (GPT-5.5 and Opus 4.8). Two narrow native-parity gaps were found and fixed in this PR:

  1. MapVector affine operand order — native single mapVector routes through the SIMD Affine_vpts proc (y' = y*scaleY + x*skewY), not mapPointAffine (y' = x*skewY + y*scaleY). The managed vector path and the affine batch tail now use the Affine_vpts order. Bit-identical for finite inputs; this only affects NaN-payload propagation but makes the "bit-for-bit identical" claim hold for the vector path too.
  2. Perspective MapVectors iteration direction — native walks the perspective case back-to-front; the managed path now does the same, so overlapping shifted spans match native for all inputs (previously only the dst == src in-place case matched). Covered by a new test that is proven to fail with forward iteration.

Platform note: x86 .NET Framework (x87 FPU)

The managed math is bit-for-bit identical to native Skia on every runtime that rounds each floating-point operation to single precision: x64 .NET Framework, and all .NET Core / .NET 5+ on x64 / x86 / ARM64. Those use SSE2 / NEON scalar math, exactly like native Skia's compiled C++, and the equivalence tests assert strict bit-for-bit equality there.

The one exception is x86 .NET Framework, whose legacy JIT targets the x87 FPU. x87 evaluates float expressions with 80-bit intermediates and only rounds to float32 on store — behaviour explicitly permitted by ECMA-334 §8.3.7: "Floating-point operations may be performed with higher precision than the result type of the operation." In cancellation-prone paths (e.g. x*skewY + y*scaleY where the result nearly cancels) that widening amplifies sub-ULP product-rounding differences into thousands of ULP, and 0*∞-style inputs diverge categorically (native SSE yields NaN, x87's wider range yields a finite value). No numeric tolerance can bridge that without effectively disabling the test and shipping different rendering output on that one runtime.

So on x86 .NET Framework only, the diverging public methods (TryInvert/IsInvertible, Concat/PreConcat/PostConcat/Concat(ref), MapPoint, MapVector, MapPoints, MapVectors, MapRect, MapRadius) keep routing through the original native C API, so output stays byte-for-byte identical to previous releases. The routing is a single static readonly bool UseNativeMath (detected once via RuntimeInformation) — a predictable, always-false branch on every modern runtime, so the managed fast path and its benchmark wins are unaffected. The equivalence tests therefore assert strict bit-equality on all platforms.

Benchmarks

Before/after harness added (SKMatrixBenchmark, SKMatrixMapBatchBenchmark): the *_Native methods call the Skia C API directly (the old code path), the *_Managed methods use the new math. Apple M3 Pro, .NET 10, default job. All ops allocate zero in both before and after.

Scalar / single-value ops

Interop dominated the old cost, so removing it is a clear, reproducible win:

Operation Native Managed Speedup
Invert 24.1 ns 8.5 ns 2.8×
Concat 16.6 ns 10.3 ns 1.6×
MapPoint 7.4 ns ~0 ns¹ interop eliminated
MapPoint (perspective) 9.3 ns 0.3 ns 24×
MapVector 13.7 ns 1.7 ns
MapRadius 24.6 ns 8.8 ns 2.8×
MapRect (affine) 17.2 ns 9.4 ns 1.8×
MapRect (scale) 14.8 ns 5.1 ns 2.9×
MapRect (translate) 14.1 ns 5.2 ns 2.7×

¹ The scalar single-point ops are now cheap enough that the JIT folds the constant-input benchmark away; the point is that the P/Invoke transition is gone.

Batch ops vs item count

The array-taking ops are parameterised by count — 4 (low), 64 (medium), 1,024 (cache-resident) and 1,000,000 (very high) — to show how the managed SIMD path scales against Skia's native procs (affine → Affine_vpts, scale → Scale_pts, translate → Trans_pts). Ratio = managed / native (< 1.0 = managed faster):

Count MapPoints (affine) MapPoints (scale) MapPoints (translate) MapVectors (affine)
4 9.8→3.0 ns · 0.31× 10.2→3.7 ns · 0.36× 8.8→3.0 ns · 0.35× 10.6→3.9 ns · 0.36×
64 21.7→13.9 ns · 0.64× 17.8→14.0 ns · 0.78× 16.4→8.5 ns · 0.52× 21.4→14.3 ns · 0.67×
1,024 211→200 ns · 0.95× 180→141 ns · 0.68× 117→174 ns · 1.48ײ 215→210 ns · 0.98×
1,000,000 200→202 µs · 1.01× 163→163 µs · 1.00× 144→141 µs · 0.98× 204→207 µs · 1.01×

Takeaway: the managed advantage is largest at small counts and converges to parity as the array grows — exactly as expected, since the removed cost is a fixed per-call P/Invoke transition amortised over more elements. At 1,000,000 points the work is memory-bandwidth bound and the two paths are identical (0.98–1.01×).

² The 1,024-point arrays are 8 KB and stay L1-resident, so absolute times are tens-to-low-hundreds of ns and dominated by CPU frequency scaling on this laptop. The native baseline for the same op swings run-to-run (e.g. translate@1024 native measured 117 ns isolated vs 187 ns in the full sweep → managed ratio 1.48× vs 0.75× for identical code; scale@1024 swung the other way, 0.68× vs 1.46×). This size is a noisy transition point and is effectively on par either way.

Future improvements (out of scope here)

On every modern runtime the single remaining interop call in SKMatrix is perspective MapRect (SKMatrix.cs, the hasPerspective branch); everything else is fully managed. (On x86 .NET Framework, all of the ported math additionally routes back to native — see the platform note above.) Tracked follow-ups:

  • Perspective MapRect (the last interop) — tracked in [perf] Port perspective SKMatrix.MapRect to managed (remove last SKMatrix native interop) #4249. For a perspective matrix, mapping the four corners and taking their bounds is not correct: the transform can push corners onto or behind the w = 0 plane, and edges crossing that plane must be clipped. Skia builds a path from the rect and runs SkPathBuilder::transformSkPathPriv::PerspectiveClipSkHalfPlane + SkEdgeClipper::ClipPath (a full edge-clipping engine). A faithful port pulls in hundreds of lines of path-clipping infrastructure to remove one P/Invoke on the rare perspective path, so it stays native for now.
  • System.Numerics bridges (additive API) — tracked in Add System.Numerics bridges: SKMatrix↔Matrix3x2, SKColorF↔Vector4, and related #4250. SKPoint↔Vector2, SKPoint3↔Vector3 and SKMatrix44↔Matrix4x4 already exist; the useful gaps are SKMatrix↔Matrix3x2 (the 2×3 affine equivalent; explicit SKMatrix→Matrix3x2 drops perspective, implicit the other way) and SKColorF↔Vector4 (layout is already identical R,G,B,A). Purely additive, so ABI-safe, but expands public surface and needs API-review sign-off. (SKRect↔Vector4, Plane, Vector<T> were considered and recommended against.)
  • Wider SIMD for huge batches — the batch procs use Vector4 (two points per 128-bit lane). On x64 with AVX, Vector256 could pack four points per lane. Measured on ARM64/NEON it is a non-starter: Vector256 is emulated and ran 5.7–6.5× slower, while ILP-unrolling (4/8 points per iteration) gave no gain because large arrays are memory-bandwidth bound (the 1,000,000-point cases are already at parity above). Any benefit is x64-only, would need an Avx-gated second bit-exact path, and is unverified on the dev hardware — so it is deliberately not done.
  • Related upstream runtime issue for Vector4 swizzle codegen (affects the affine SwapXY): Add Matrix3x3 dotnet/runtime#16226.

Deliberate non-goal — FMA: the managed math intentionally does not use fused multiply-add (MathF.FusedMultiplyAdd / Vector* FMA). FMA rounds a*b + c once instead of twice, so it would diverge from Skia's separate multiply/add and break the bit-exactness guarantee (the equivalence tests confirm the native build does not contract these either). The scale/affine inner loops should not be "optimised" with FMA.

(Note: SKMatrix44 is already fully managed — backed by System.Numerics.Matrix4x4 with no interop — so it needs no similar work.)

Independent confirmation — permanent tracking suite (CI, Linux)

The regression-tracking benchmarks added for this port (MatrixMapPointsBenchmark
single-op methods + the new MatrixOpsBenchmark, in SkiaSharp.Benchmarks.Tracking)
now measure this change on CI hardware. Comparing this branch's source build (managed)
against the nightly package (native), averaged over repeated Linux runs:

Op (4096 individual calls) native → managed Δ
MapVector 180 → 28 µs −84%
MapPoint 76 → 16 µs −78%
MapRadius 229 → 79 µs −65%
MapRect 193 → 79 µs −59%
Concat (pre+post) 229 → 102 µs −56%
Invert 143 → 119 µs −16% (smaller op, noisier)
MapPoints (batch — one native call) 2.9 → 2.6 µs −9%

This independently reproduces the laptop micro-benchmarks above: removing the per-call
P/Invoke is a large win when it is paid per element (the single-op loops), and collapses
to parity for the batch overload that already made a single native call — the MapPoints
batch barely moves (−9%), exactly as the amortisation argument predicts.

Caveat: the tracking dashboard compares two separate CI runner VMs, so it carries a
~±25% run-to-run noise floor; these large effects clear it comfortably, but sub-25%
deltas on µs-scale benchmarks are within the noise and should be read from the
in-process A/B numbers above (that is why Invert, the smallest op, is the least stable).


Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com

@github-actions

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 -- 4241

PowerShell / Windows:

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

Step 2 — Add the local NuGet source

dotnet nuget add source ~/.skiasharp/hives/pr-4241/packages --name skiasharp-pr-4241
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-4241

@github-actions

Copy link
Copy Markdown
Contributor

📖 Documentation Preview

The documentation for this PR has been deployed and is available at:

🔗 View Staging Site
🔗 View Staging Docs
🔗 View Staging Gallery (Blazor)
🔗 View Staging Gallery (Uno Platform)
🔗 View Staging SkiaFiddle

This preview will be updated automatically when you push new commits to this PR.


This comment is automatically updated by the documentation staging workflow.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 ports SKMatrix math operations from native Skia C API calls to a managed C# implementation to eliminate P/Invoke overhead on hot paths, while preserving native behavior (including a runtime gate that keeps x86 .NET Framework on the native path to avoid x87 precision divergences). It also adds parity tests and benchmarks to validate/measure the change.

Changes:

  • Replaced native interop in SKMatrix operations (invert/concat/map/map-batch/map-radius/map-rect fast paths) with managed implementations intended to be bit-for-bit identical to Skia.
  • Added SKMatrixManagedTests that compare managed results against direct calls into SkiaApi.sk_matrix_* across a broad corpus, including aliasing/overlap cases.
  • Added BenchmarkDotNet benchmarks to compare old (direct native C API) vs new (managed) paths for scalar and batch operations.

Reviewed changes

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

File Description
tests/Tests/SkiaSharp/SKMatrixManagedTests.cs Adds native-vs-managed equivalence tests for SKMatrix math, including edge cases and aliasing/overlap scenarios.
binding/SkiaSharp/SKMatrix.cs Implements managed SKMatrix math + runtime UseNativeMath gate; replaces prior per-call native interop for most operations.
benchmarks/SkiaSharp.Benchmarks/Benchmarks/SKMatrixBenchmark.cs Adds before/after benchmarks comparing direct C API calls (“Native”) to the new managed path (“Managed”).

Comment on lines +15 to +21
private static bool NativeTryInvert (SKMatrix m, out SKMatrix inverse)
{
SKMatrix result;
var ok = SkiaApi.sk_matrix_try_invert (&m, &result);
inverse = result;
return ok;
}

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Stale — already addressed at the current PR head. NativeTryInvert now initializes the local before the call: var result = SKMatrix.Identity;, which is exactly the fix suggested here, so there is no uninitialized-stack read. (The managed TryInvert also explicitly sets inverse = Identity on the non-invertible path, matching the native shim.)

Comment on lines +108 to +110
private static bool BitEqual (float a, float b) =>
(float.IsNaN (a) && float.IsNaN (b)) ||
FloatBits (a) == FloatBits (b);

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

By design — not a defect. BitEqual enforces an exact bit match for every finite result and a NaN-for-NaN match for non-finite ones, deliberately ignoring NaN sign/payload. The native and managed paths legitimately reach NaN via different routes and can differ only in those payload/sign bits (e.g. MapRect of a rotation over an infinite rect: native 0x7FC00000 vs managed 0xFFC00000 — same quiet NaN, opposite sign). Skia never inspects NaN payloads, so comparing them would fail the suite on a provably insignificant difference while adding no real coverage. The bit-for-bit guarantee that matters — all finite outputs — is fully enforced.

Comment thread binding/SkiaSharp/SKMatrix.cs Outdated
Comment on lines +283 to +285
// Match the native shim: on failure the result is left as the
// default-constructed (identity) matrix.
inverse = Identity;

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Stale — the current comment is accurate and already says this. It reads: "…leaves its out matrix as the identity it was default-constructed with … (A default-constructed SKMatrix in C# is all-zero, so set it explicitly.)" It refers to the native C++ SkMatrix default-ctor (which is identity) and explicitly notes the C# default is all-zero, which is why the code assigns Identity. No misleading claim remains.

Comment on lines +876 to +885
if ((n & 1) != 0) {
*(Vector4*)d = *(Vector4*)s + trans;
s += 4; d += 4;
}
n >>= 1;
for (var i = 0; i < n; i++) {
*(Vector4*)(d + 0) = *(Vector4*)(s + 0) + trans;
*(Vector4*)(d + 4) = *(Vector4*)(s + 4) + trans;
s += 8; d += 8;
}

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Stale — resolved in commit 024893a ("Use unaligned Vector4 access in SKMatrix batch procs"). MapTranslateBatch now reads/writes via Unsafe.ReadUnaligned<Vector4> / Unsafe.WriteUnaligned; there is no raw *(Vector4*) dereference left, so the alignment/UB concern no longer applies.

Comment on lines +909 to +918
if ((n & 1) != 0) {
*(Vector4*)d = *(Vector4*)s * scale + trans;
s += 4; d += 4;
}
n >>= 1;
for (var i = 0; i < n; i++) {
*(Vector4*)(d + 0) = *(Vector4*)(s + 0) * scale + trans;
*(Vector4*)(d + 4) = *(Vector4*)(s + 4) * scale + trans;
s += 8; d += 8;
}

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Stale — same fix (commit 024893a). MapScaleBatch now uses Unsafe.ReadUnaligned<Vector4> / Unsafe.WriteUnaligned for all vector reads and writes; no aligned *(Vector4*) cast remains.

Comment on lines +935 to +939
for (var i = 0; i < pairs; i++) {
var o = i << 2;
var v = *(Vector4*)(s + o);
*(Vector4*)(d + o) = v * scale + SwapXY (v) * skew + trans;
}

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Stale — same fix (commit 024893a). MapAffineBatch now uses var v = Unsafe.ReadUnaligned<Vector4>(s + o); and Unsafe.WriteUnaligned(...); no aligned *(Vector4*) dereference remains.

mattleibow and others added 9 commits July 7, 2026 01:02
SKMatrix previously round-tripped through the Skia C API for every
math operation (Invert, Concat, MapPoint, MapVector, MapRect,
MapRadius, MapPoints/MapVectors). Each call paid the P/Invoke
transition cost even though the underlying math is trivial.

This reimplements the math in managed C#, faithfully mirroring Skia's
SkMatrix algorithms so the result is bit-for-bit identical to the
native path (verified against the Skia C API across ~230 matrices
including perspective, degenerate, -0, Inf and NaN inputs). The batch
MapPoints/MapVectors paths use System.Numerics.Vector4 (two points per
lane, matching skvx::float4) with a hardware Vector128.Shuffle swizzle
on modern runtimes and a portable fallback elsewhere.

No public API changes; only method bodies and private helpers change.

Benchmarks (Apple M3 Pro, .NET 10, native vs managed):

  Invert         25.6 ns -> 9.0 ns   (2.8x)
  MapVector      13.5 ns -> 1.8 ns   (7.5x)
  MapPoint        8.0 ns -> ~0 ns    (interop eliminated)
  MapPointPersp   9.5 ns -> 0.4 ns   (24x)
  MapRect        17.3 ns -> 9.3 ns   (1.86x)
  MapRadius      24.9 ns -> 8.8 ns   (2.8x)
  Concat         18.3 ns -> 11.6 ns  (1.6x)
  MapPoints 1024  269 ns -> 261 ns   (on par)

Adds SKMatrixManagedTests (native-vs-managed bit-exact equivalence,
incl. odd/small batch counts, in-place mapping and Inf/NaN edges) and
a before/after SKMatrixBenchmark.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adds the missing native-vs-managed benchmarks that the original PR lacked:
MapVectors batch (1024 and small), small-count MapPoints/MapVectors (where
the P/Invoke transition is not amortized), and MapRect for the scale and
translate fast paths. These confirm 2-2.8x wins on small/scalar workloads.

Benchmarking the new scale batch revealed the managed MapScaleBatch/
MapTranslateBatch processed only two points per iteration, which regressed
the 1024-point scale case. Reworked both to mirror Skia's Scale_pts/Trans_pts
exactly: a scalar lead-in for the odd point, one float4 pair, then a main loop
of two float4 (four points) per iteration so the multiplies pipeline. The
result stays bit-for-bit identical (verified by the equivalence tests over
varied counts) and the scale batch is now on par with / faster than native.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Splits the benchmark into a scalar class (single-value ops) and a
parameterised batch class so the array-taking operations (MapPoints
affine/scale/translate and MapVectors) are measured across a range of
item counts: 4 (low), 64 (medium), 1024 (cache-resident) and 1,000,000
(very high). This shows how the managed path scales with the number of
items versus Skia's native procs.

Result: the managed advantage is largest at small counts (1.3-3.2x, where
the fixed per-call P/Invoke transition is not amortised) and converges to
parity at very large counts (memory-bandwidth bound, ~1.0x). Cache-resident
sizes (~1024) are dominated by CPU frequency scaling and sit at parity
within run-to-run variance.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Address two faithfulness gaps found in review of the managed SKMatrix port:

- MapVector / affine batch tail now compute the y lane as y*scaleY + x*skewY,
  matching Skia's Affine_vpts proc (the path native mapVector routes through)
  rather than mapPointAffine. Bit-identical for finite inputs; only NaN-payload
  propagation differed, but this keeps the managed path bit-exact with native.
- Perspective MapVectors now iterates back-to-front to match SkMatrix::mapVectors,
  so overlapping dst/src spans (dst shifted ahead of src) produce the same result
  as the native path for all inputs, not just the in-place (dst==src) case.

Add an overlapping-span equivalence test (proven to fail with forward iteration)
and broaden batch counts to 6/10/11 so the single-pair lead-in and unrolled
4-point loop are exercised together.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Extend MapVectorsOverlappingSpansMatchNative to exercise both shift
directions (dst ahead of src, and dst behind src) across all test
matrices. The shifted-overlap topology is the only aliasing layout that
makes perspective iteration direction observable; disjoint and
same-offset in-place buffers are direction-invariant and cannot catch a
wrong loop order. Verified non-vacuous: reverting the perspective loop to
forward iteration fails this test.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Mirror the MapVectors overlap test for MapPoints. Every native mapPoints
proc iterates front-to-back (only mapVectors special-cases perspective
with a backward walk), so the managed batch path must stay forward for
all matrix types. Disjoint and same-offset in-place buffers are
direction-invariant and cannot observe this; the shifted-overlap layout
can. Guards against a future 'optimize to backward' or buffer-the-source
regression that would silently diverge from native on aliased spans.
Verified non-vacuous: snapshotting src before mapping fails this test.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add extreme-magnitude and determinant-threshold matrices to the shared
test set. Native SkMatrix computes some cofactor/concat cross-products in
float (scross, rowcol3) and others in double (dcross, muladdmul); the two
only diverge near float.MaxValue overflow or at the cubed nearly-zero
determinant threshold, which the random +/-100 matrices never reached.
These lock the per-path width contract for Determinant, Invert and Concat.
Verified non-vacuous: widening rowcol3 to double fails ConcatMatchesNative.

Also document why SortAsRect keeps its hand-rolled skvx min/max instead of
SKRect.Standardized or MathF.Min/Max: the skvx 'NaN keeps first operand'
semantics differ from both, the scale/translate map paths have no
finiteness probe so the difference is value-visible, and MathF is absent
on net462/net48/netstandard2.0.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The managed SKMatrix port is bit-for-bit identical to native Skia on every
runtime that rounds each float operation to single precision (x64 .NET
Framework and all .NET Core / .NET 5+ on x86, x64, ARM). It diverges only on
x86 .NET Framework, whose legacy JIT targets the x87 FPU and keeps 80-bit
extended-precision intermediates (permitted by ECMA-334 §8.3.7). In
cancellation-prone paths that widening amplifies sub-ULP product-rounding
differences into thousands of ULP, and 0*Inf style inputs differ
categorically (native SSE NaN vs x87 finite), so no numeric tolerance can
bridge it without changing rendering output.

To stay byte-for-byte identical there, gate the diverging public math
methods (TryInvert/IsInvertible, Concat/PreConcat/PostConcat/Concat(ref),
MapPoint, MapVector, MapPoints, MapVectors, MapRect, MapRadius) on a single
static readonly UseNativeMath flag that detects x86 .NET Framework and routes
through the original native C API, mirroring the pre-port implementation. The
flag is a predictable always-false branch on every modern runtime, so the
managed fast path (and its benchmark wins) is unaffected. No public API
changes.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Address review feedback on the managed SKMatrix port.

The SIMD batch procs (translate/scale/affine) read and write Vector4 via
`*(Vector4*)ptr`, which the CLI treats as a naturally-aligned ldobj/stobj.
SKPoint[] buffers are only 8-byte aligned, so this is technically undefined
behaviour and could fault on strict-alignment targets. RyuJIT happens to emit
unaligned moves on x64/ARM64 so it works today, but route the loads/stores
through Unsafe.ReadUnaligned/WriteUnaligned to be correct by construction. The
lowering is identical on x64/ARM64, so it is bit-exact and zero-cost (verified:
batch benchmark ratios unchanged within noise; all 12 equivalence + 20 existing
matrix tests still pass).

Also tighten two test/comment items flagged in review:

- NativeTryInvert left its out matrix uninitialised. The native shim always
  writes it (identity on the non-invertible path, since its C++ out matrix is
  default-constructed to identity), so this was benign, but initialise to
  Identity so the test never depends on that.
- Document that BitEqual deliberately treats any two NaNs as equal: native and
  managed legitimately reach NaN by different routes and disagree on the sign
  bit (e.g. MapRect of a rotation over an infinite rect: native 0x7FC00000 vs
  managed 0xFFC00000), which Skia never inspects. Reword the misleading
  "default-constructed (identity)" comment that read as C++ semantics in a C#
  file.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@mattleibow
mattleibow force-pushed the mattleibow-dev-issue-2779-skmatrix-managed-math branch from 8dc1b94 to 024893a Compare July 6, 2026 23:14
mattleibow added a commit that referenced this pull request Jul 8, 2026
…ready (#4361)

[skills] Add performance-fixer skill + make the benchmark harness AI-ready (#4361)

Context: dotnet/aspnetcore optimize-aspnetcore-performance (reference architecture)
Related: #4241 (SKMatrix native→managed, up to 24×), #4345 (SKColor alloc-free parse),
         #4247 (SKSurface.Canvas caching), #4362 (perf/* label taxonomy), #4330 (memory-leak-fixer)

SkiaSharp is a thin managed wrapper over native Skia, so its recurring, high-impact
performance family is the tax the C# layer adds between the caller and Skia — a P/Invoke
transition paid for math that is a few float ops, an allocation on a hot parse/convert
path, a native lookup redone on every getter, or per-element marshalling in a loop. We
have merged several such wins by hand (#4241, #4345, #4247); this adds an autonomous
scanner so more of them surface as code lands, without a human hunting for them.

Add a `performance-fixer` skill (sibling to `memory-leak-fixer`) plus a scheduled gh-aw
workflow that scans the managed binding for one hot-path opportunity, proves it, fixes it
(managed-only, ABI-safe), and files a linked issue + draft PR.

The defining discipline is two proofs, which a generic perf pass lacks: every fix must be
shown FASTER (a BenchmarkDotNet New-vs-Old measurement) AND behaviour-IDENTICAL (an
equivalence test — bit-exact vs SkiaApi.sk_* for numeric ports, across edge inputs, and
mutation-checked to prove it catches divergence). A faster answer that differs from Skia is
a rendering regression, not a win, so it is rejected. The skill also encodes the traps we
learned the hard way: the x86/x87 float-determinism native fallback (#4241), the ARM64
Vector256 regression, statistical benchmark rigor, and full behaviour parity including
exceptions, ownership/GC.KeepAlive lifetime, and rendered pixels.

~~ Reference structure ~~
References are split by area (mirroring the aspnetcore skill): references/hot-paths/*
(SkiaSharp code areas — geometry-math, color, handles-and-collections, text-and-fonts,
pixels-and-images) and references/bcl-patterns/* (the .NET techniques), plus
decision-framework.md (impact×complexity rubric + the two-proof gate), measuring.md,
signals.md (detection routing), and repo-helpers.md. A lean SKILL.md routes between them.

~~ Benchmark harness readiness ~~
So a scan never wastes time repairing infra: replace TheBenchmark.cs — a trap whose
[SimpleJob(Net48/Net70/Net80)] attributes silently skip off-Windows and whose empty bodies
taught none of the conventions — with a working TemplateBenchmark.cs (New-vs-Old +
[MemoryDiagnoser] + [Params] + return-sink that runs out of the box), and add
benchmarks/README.md documenting the add/run commands, --list flat / --job short for fast
iteration, the single-TFM rule, and the InternalsVisibleTo native-oracle pattern.

~~ Labels ~~
Findings are labelled tenet/performance plus one agent-chosen perf/* sub-type from the
shared taxonomy in .agents/skills/issue-triage/references/labels.md (#4362), matching the
memory-leak-fixer convention; the sub-type is decided by the win's dominant measured driver
(a removed P/Invoke → perf/interop, removed allocations → perf/allocations, ...).

~~ Validated end to end ~~
The workflow self-tests on any PR touching the skill via a forced dry-run. Those runs
correctly rejected a non-finding (SKFourByteTag's char[4] is already stack-allocated by
.NET 9/10 escape analysis) and found real, fully-proven wins (SKShaper glyph-array
extraction → perf/allocations; SKColor→SKColorF operator → perf/interop). A real dispatch
run created a correctly-labelled issue (#4369) and draft PR (#4370), confirming the
tenet/performance + perf/* labels land on both.

No binding/runtime code changes: a new skill, a new workflow, benchmark docs/template, and
two rows in AGENTS.md.

Co-authored-by: Matthew Leibowitz <mattleibow@live.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@mattleibow mattleibow added tenet/performance Performance related issues perf/interop P/Invoke marshalling / native interop overhead. Implies tenet/performance. labels Jul 8, 2026
mattleibow added a commit that referenced this pull request Jul 17, 2026
…) (#4480)

The tracking suite only measured SKMatrix.MapPoints (the batch overload), which
is the weakest indicator of the managed SKMatrix port in #4241: the batch does a
single native call for the whole array, so the per-call P/Invoke that #4241
removes is amortised to almost nothing. The single-op operations - where the
managed<->native transition is paid on every call - are where that win actually
shows, and they had no permanent coverage.

Split the SKMatrix surface into two tracked groups, mirroring how #4241 changes it:

  * MatrixMapPointsBenchmark (expanded, geometry mapping) - add MapPoint, MapRect,
    MapVector and MapRadius, each looped over the existing deterministic point set.
    The pre-existing MapPoints method and its Points params are left untouched (a
    shipped history key must never be renamed). All the new methods are
    allocation-free; MapPoints keeps its allocating batch behaviour as a signal.
  * MatrixOpsBenchmark (new, matrix algebra) - Concat (pre + post multiply) and
    Invert, the operations that produce another matrix rather than mapping geometry.

Local short-run against the 4.151 nightly shows the single-op methods are dominated
by interop overhead (MapPoint 24.6 us vs the MapPoints batch 1.88 us for the same
4096 points; MapRect 55 us, MapRadius 118 us, Concat 125 us, Invert 71 us) - all
zero-alloc - so #4241 should visibly drop their time lines while the batch barely
moves. The operands are affine (rotation/scale/translate), which is #4241's managed
fast path; perspective would fall back to native. Public API only, deterministic
(fixed seed), and verified to compile against both the 3.119.4 baseline and 4.151
nightly (no new #if needed).


Copilot-Session: ca69afac-998b-493d-a03c-d9b25dd3f15b

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Comment on lines +846 to +847
// process two points per iteration and handle a trailing odd point with
// the scalar formula (IEEE addition is commutative, so it is bit-identical).

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

The code is correct — this is at most a wording nit in the summary comment, not a behavioral issue. The scalar tails are bit-identical to native because they preserve native operand order, not because of commutativity: the affine tail computes (x*sx + y*kx) + tx / (y*sy + x*ky) + ty, matching Skia's Affine_vpts trailing-element lane (and the inline comment directly above that line already states this correctly). Fair point that "IEEE addition is commutative" is imprecise justification in the top-of-proc comment; it could read "the scalar tail preserves native operand order" instead. No code change needed.

mattleibow added a commit that referenced this pull request Jul 20, 2026
…ger math (#4385)

Port single-value SKPMColor premultiply/unpremultiply to managed integer math (#4385)

SKPMColor.PreMultiply(SKColor) and UnPreMultiply(SKPMColor) each round-tripped
a single colour through a native P/Invoke (sk_color_premultiply /
sk_color_unpremultiply) even though the underlying work is only a handful of
integer ops. Callers that convert one colour at a time — gradient ramps, palette
expansion, per-pixel software blends — hit these in tight loops, so the managed
to native transition cost dominated the actual conversion.

Reimplement both in managed C# using Skia's exact integer algorithms, so the
result is bit-exact with the native path for every input:

  * PreMultiply uses SkMulDiv255Round(c, a) = (p + (p >> 8)) >> 8, p = c*a + 128,
    per channel, with the opaque (a == 255) case passing through unchanged.
  * UnPreMultiply mirrors SkUnPreMultiply::gTable — a 256-entry
    round((255 << 24) / a) scale table built once at type init — then applies
    ApplyScale(scale, c) = (scale*c + (1 << 23)) >> 24, turning a per-call 32-bit
    divide into a table lookup.

The maths is integer-only, so it stays deterministic across all runtimes and
TFMs (no x87/float divergence, unlike float ports such as #4241) and matches the
native uint32 wrap even for out-of-gamut premultiplied colours where a channel
exceeds alpha. Only the method bodies change, so this is ABI-safe: the array
overloads stay native (already one P/Invoke per batch) and the cast operators are
untouched — both call these two methods, so they benefit automatically.

Correctness is proven, not assumed. SKPMColorEquivalenceTest exhaustively sweeps
alpha 0..255 x channel 0..255 for each R/G/B position against the native oracle
(sk_color_(un)premultiply) — a complete enumeration of every reachable input —
with distinct sentinels pinning the packing order, an opaque round-trip check,
and deliberately-wrong "teeth" guards proving the oracle comparison actually
detects divergence.

The Track - Benchmarks CI (ColorMathBenchmark) confirms the win with zero
allocations on every leg, managed vs the still-native nightly baseline on the
same runners:

  * Linux:   PreMultiply +32.9%, UnPreMultiply +31.6%
  * Windows: PreMultiply +24.1%, UnPreMultiply +20.9%
  * macOS:   PreMultiply +12.3%, UnPreMultiply  +2.4%

This also lays to rest the earlier "Linux regression" concern, which was
runner/baseline noise — Linux in fact benefits the most.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Matthew Leibowitz <mattleibow@live.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

perf/interop P/Invoke marshalling / native interop overhead. Implies tenet/performance. tenet/performance Performance related issues

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

[perf] Remove native interop in SKMatrix

2 participants