Optimize SKColor hex parsing and add ReadOnlySpan<char> overloads#4345
Merged
Conversation
Rewrite SKColor.TryParse to parse hex color strings ("#RGB", "#ARGB",
"#RRGGBB", "#AARRGGBB") with a manual, allocation-free hex parser instead
of byte.TryParse/uint.TryParse + NumberStyles.HexNumber. This removes the
per-channel parse overhead and the #if split between the span path and the
allocating net4x/netstandard2.0 path (which built a `new string(char, 2)`
per channel), unifying behavior across all target frameworks.
The core now runs on ReadOnlySpan<char> (Trim().TrimStart('#') + a length
switch), and new public overloads are exposed:
SKColor.Parse(ReadOnlySpan<char>)
SKColor.TryParse(ReadOnlySpan<char>, out SKColor)
The string overloads delegate to the span ones via AsSpan(), matching the
pattern already used elsewhere in the binding (SKFont, SKTextBlob, ...).
These are pure additions, so the change is ABI-safe. Re-basing to a span
also lets the JIT drop bounds checks in the fixed-length cases.
Behavior is preserved for every input the old code accepted, with one
intentional tightening: whitespace directly after '#' in the 6/8-digit
forms (e.g. "# 12345") was previously accepted as a side effect of
NumberStyles.HexNumber's AllowLeadingWhite and is now rejected, matching
the stricter behavior the 3/4-digit path already had. Parsing remains
case-insensitive.
Measured on net10.0 (Apple M3 Pro): ~1.4-2.3x faster across all shapes
with zero allocations (e.g. "#FFF" 8.4ns -> 3.8ns, "#FFFFFFFF" ~9ns ->
6ns, invalid fast-reject 3.7ns -> 1.7ns).
Adds SKColorTest coverage (whitespace, case-insensitivity, null/empty,
named-color rejection, span overloads, slice parsing) and benchmark
projects documenting the strategy shootout, the span-vs-string
comparison, and the public New-vs-Old numbers.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
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 -- 4345PowerShell / Windows: iex "& { $(irm https://raw.githubusercontent.com/mono/SkiaSharp/main/scripts/get-skiasharp-pr.ps1) } 4345"Step 2 — Add the local NuGet source dotnet nuget add source ~/.skiasharp/hives/pr-4345/packages --name skiasharp-pr-4345More options
Or download manually from Azure Pipelines — look for the Remove the source when you're done: dotnet nuget remove source skiasharp-pr-4345 |
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. |
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Rewrites
SKColor.TryParseto parse hex color strings with a manual, allocation-free hex parser instead ofbyte.TryParse/uint.TryParse+NumberStyles.HexNumber, and adds publicReadOnlySpan<char>overloads forParse/TryParse.Why
SKColor.Parse/TryParsesit on hot paths for anyone hydrating colors from XAML/CSS/JSON/theme files. The old implementation:byte.TryParseper channel anduint.TryParsefor the 6/8-digit forms (culture +NumberStylesoverhead), and#ifsplit where the net4x/netstandard2.0 path allocated anew string(char, 2)per channel.Changes
ReadOnlySpan<char>(Trim().TrimStart('#')+ a length switch, with small case-insensitive nibble helpers). Removes the#ifsplit — one implementation for all TFMs.stringoverloads delegate via.AsSpan(), matching the existing pattern inSKFont/SKTextBlob/etc.):ff/FF/fF), null/empty/#, named-color rejection, span/string agreement, slice parsing, invalid/empty span rejection. 35SKColorTestcases pass, 0 failed.benchmarks/) documenting the work: a strategy shootout (if-chain vs lookup table vs branchless vs switch), the span-vs-string comparison, and the public New-vs-Old numbers.Interop / native surface
None. This is a pure-managed change with no P/Invoke involved anywhere:
SKColoris areadonly structwrapping a singleuint— it has no native handle and noSKObject/IntPtrbacking.Parse/TryParseand the privateTryParseNibble/TryParseBytehelpers are 100% managed C#: noSkiaApi.*/sk_*calls, noDllImport/LibraryImport, noMarshal, nostackalloc/fixed, no native memory.uint.externals/skia/**) or generated bindings (*.generated.cs) are touched, soexternals-downloadis sufficient for local builds and there is no native rebuild requirement.Behavior
Preserved for every input the old code accepted, with one intentional tightening: whitespace immediately after
#in the 6/8-digit forms (e.g."# 12345") was previously accepted as a side effect ofNumberStyles.HexNumber'sAllowLeadingWhite, and is now rejected — matching the stricter behavior the 3/4-digit path already had. Parsing stays case-insensitive.Performance
net10.0, Apple M3 Pro, BenchmarkDotNet — zero allocations, ~1.4–2.3× faster across all shapes:
#FFF#FFFF#FFFFFF#FFFFFFFF#A7A8A9A0(varying)Red(invalid)Notes
#ifguard existed only because that path used thebyte/uint.TryParsespan overloads (netstandard2.1+); this trimming-only span code compiles everywhere.