Add Uno SVG control package, tests, sample, and docs#474
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cf2b9f5169
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| var normalizedPath = SvgSource.NormalizePath(path).ToString(); | ||
| var cacheKey = SvgCacheKey.Create(normalizedPath, parameters); |
There was a problem hiding this comment.
NormalizePath and LoadAsync are both invoked here without any baseUri, so Path="Assets/Icon.svg" is always treated as ms-appx:///Assets/Icon.svg instead of being resolved relative to the XAML file that declared the control. That means any Svg used from a page/resource dictionary in a subfolder cannot load page-relative assets even though SvgSource.NormalizePath(..., baseUri) explicitly supports that scenario.
Useful? React with 👍 / 👎.
| var candidateScaleX = hasWidth ? availableSize.Width / sourceSize.Width : 1.0; | ||
| var candidateScaleY = hasHeight ? availableSize.Height / sourceSize.Height : 1.0; | ||
|
|
||
| if (stretch == Stretch.Uniform) | ||
| { |
There was a problem hiding this comment.
When only one dimension is constrained and that constraint is larger than the SVG, the unconstrained axis is left at 1.0 and Stretch.Uniform ends up using Math.Min(..., 1.0). For example, a 100x50 SVG measured with availableSize=(∞, 200) returns 100x50 here instead of scaling to 400x200, so Svg controls with only Height or only Width set will fail to stretch up to the requested size.
Useful? React with 👍 / 👎.
| if (clone.HasPathSource) | ||
| { | ||
| await clone.ReLoadAsync(parameters, cancellationToken).ConfigureAwait(false); | ||
| } |
There was a problem hiding this comment.
This unconditionally reloads every path-backed SvgSource clone from disk/appx, even when no extra CSS/entities need to be applied. If a caller loads a SvgSource, edits source.Svg.Model, calls RebuildFromModel(), and then assigns that object to Svg.SvgSource, those in-memory changes are discarded and the control renders the original file contents instead of the edited model.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ebcc161acb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| result = await LoadExternalSourceAsync(externalSource, cancellationToken).ConfigureAwait(false); | ||
| } | ||
| else if (!string.IsNullOrWhiteSpace(Path)) | ||
| { | ||
| result = await LoadPathAsync(Path!, cancellationToken).ConfigureAwait(false); |
There was a problem hiding this comment.
ReloadSourceAsync awaits the path/external-source loaders with ConfigureAwait(false) and then immediately calls SetCurrentSource, ClearSource, and the invalidate methods on the control. For any non-inline load (Path or a path-backed SvgSource), that continuation can resume on a worker thread after file/ms-appx/http I/O, but SKCanvasElement/DependencyObject state in Uno is dispatcher-affine. In practice this makes async image loads prone to cross-thread exceptions or redraw races as soon as the control loads from anything other than the inline Source code path.
Useful? React with 👍 / 👎.
| { | ||
| Debug.WriteLine("Failed to load Uno svg control source."); | ||
| Debug.WriteLine(e); | ||
| DisposeResultIfOwned(result); | ||
| ClearSource(); |
There was a problem hiding this comment.
This catch always calls ClearSource() even if the failing task belongs to an older loadVersion. If Path/SvgSource changes quickly and the earlier request throws after a newer one has started, the stale task will cancel _pendingLoadCts and clear _svg, wiping out the newer request. That race is easy to hit with local files because SvgSource.LoadImplAsync only checks cancellation before parsing the file, so a malformed old SVG can still blank a later valid one.
Useful? React with 👍 / 👎.
| previous.Dispose(); | ||
| } | ||
|
|
||
| DisposeCache(); |
There was a problem hiding this comment.
ClearSource() always disposes _cache, but _cache is only recreated when EnableCache changes from false to true. After any clear path (for example setting Path/Source to null or hitting a load error), subsequent LoadPathAsync calls run with EnableCache == true and _cache == null, so nothing is cached until the property is toggled off and back on. That silently disables the control's cache mode for the remainder of its lifetime.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 69ef521929
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (path.StartsWith("/Assets/", StringComparison.OrdinalIgnoreCase) | ||
| || string.Equals(path, "/Assets", StringComparison.OrdinalIgnoreCase)) | ||
| { | ||
| return new Uri($"ms-appx://{path}", UriKind.Absolute); |
There was a problem hiding this comment.
Because only /Assets is special-cased here, a packaged SVG referenced as Path="/Images/logo.svg" falls through to the absolute-file handling and resolves to the local filesystem instead of ms-appx:///Images/logo.svg. That breaks any app asset stored outside the Assets folder for both Svg.Path and SvgSource.LoadAsync, even though those files are valid package resources.
Useful? React with 👍 / 👎.
| private void OnUnloaded(object sender, RoutedEventArgs e) | ||
| { | ||
| CancelPendingLoad(); |
There was a problem hiding this comment.
If a Path or SvgSource load is still in flight when the same Svg instance is temporarily removed from the visual tree (for example via ItemsRepeater virtualization or tab/page reuse), OnUnloaded cancels the request and there is no matching Loaded handler to queue it again. Since the source properties are unchanged, the control stays blank after it is reattached until some other property change forces QueueSourceReload().
Useful? React with 👍 / 👎.
| source._skSvg = skSvg; | ||
| source._picture = picture; |
There was a problem hiding this comment.
Replacing _skSvg and _picture here (and in LoadFromCachedStream a few lines below) leaks the previous native objects on every in-place reload. Calls such as SvgSource.ReLoadAsync, PrepareWorkingSource on an already-loaded source, or path-backed SvgSource clones in LoadExternalSourceAsync will accumulate abandoned SKSvg/SKPicture instances when CSS or themes are changed repeatedly.
Useful? React with 👍 / 👎.
1 participant
PR Summary: Add
Svg.Controls.Skia.UnoBranch
feature/svg-controls-skia-unoCommit breakdown
3f265718Add Svg.Controls.Skia.Uno package7764851aAdd Uno control testsf03cc9a5Add Uno desktop and mobile samplecf2b9f51Document Uno SVG packageOverview
This change adds a new Uno Platform control package,
Svg.Controls.Skia.Uno, modeled afterSvg.Controls.Skia.Avaloniawhere the API maps cleanly. The Uno implementation renders directly onUno.WinUI.Graphics2DSK.SKCanvasElementand exposes the same core control-facing concepts:Path,Source, reusableSvgSource, stretch behavior, cache control, wireframe rendering, filter disabling, zoom/pan, hit testing, and access to the underlyingSKSvg.The package is intentionally scoped to the Uno Skia renderer path in v1. Avalonia-only concepts such as
SvgImage,SvgResource, and markup extensions were not ported; Uno usage is centered onSvgplus reusableSvgSourceresources.Main implementation changes
New package:
Svg.Controls.Skia.UnoAdded a new packable Uno library project under
src/Svg.Controls.Skia.Uno/with:Uno.Svg.Skia.SvgUno.Svg.Skia.SvgSourceUno.Svg.Skia.StretchDirectionKey behavior:
SKCanvasElementRenderOverride(SKCanvas canvas, Size area)SvgSource > Path > SourceprecedenceEnableCache,Wireframe,DisableFilters,Zoom,PanX,PanYZoomToPoint(...),TryGetPicturePoint(...), andHitTestElements(...)SvgSourceinstances before applying per-control CSS or render optionspath + css + entities, not path aloneUno-safe
SvgSourceAdded async loading APIs:
LoadAsync(...)ReLoadAsync(...)Path handling includes:
/Assets/...mapped toms-appx:///Assets/...ms-appx,file, andhttp(s)URIsbaseUriThe implementation keeps sync loaders for:
StreamSvgDocumentSDK and dependency alignment
Updated:
global.jsonto pinUno.Sdkviamsbuild-sdksbuild/SkiaSharp.v3.propsbuild/SkiaSharp.Native.v3.propsThe SkiaSharp v3 overrides were aligned to
3.119.1because current Uno Skia packages restore against that version.Tests and solution integration
Added
tests/Svg.Controls.Skia.Uno.UnitTests/and wired it intoSvg.Skia.slnx.Coverage added for:
SvgSource.LoadAsync(...)SvgSource.ReLoadAsync(...)SvgSource.Clone()SvgSource.RebuildFromModel()SvgSourceusageSvgSourceWireframeandDisableFiltersThe standalone Uno sample was intentionally not added to the root solution so the default repo build/test path remains workload-free.
Sample app
Added
samples/UnoSvgSkiaSample/as a standalone Uno single-project sample.The sample demonstrates:
PathSourceSvgSourceresourcesCurrentCssDocumentation updates
Added new docs pages:
site/articles/packages/svg-controls-skia-uno.mdsite/articles/getting-started/quickstart-uno.mdsite/articles/xaml/uno-svg-control.mdsite/articles/advanced/uno-sample-publishing.mdUpdated:
site/config.scribanto includeSvg.Controls.Skia.Unoin generated API docs with a per-projectTargetFramework=net10.0overrideVerification performed
Executed successfully:
dotnet build src/Svg.Controls.Skia.Uno/Svg.Controls.Skia.Uno.csproj -c Releasedotnet test tests/Svg.Controls.Skia.Uno.UnitTests/Svg.Controls.Skia.Uno.UnitTests.csproj -c Releasedotnet build samples/UnoSvgSkiaSample/UnoSvgSkiaSample.csproj -c Release -f net10.0-desktopdotnet build Svg.Skia.slnx -c Releasedotnet test Svg.Skia.slnx -c Releasedotnet format --no-restoreScope and current limits
SvgImageSuggested PR description
Summary
Add a new Uno Platform SVG control package,
Svg.Controls.Skia.Uno, backed bySvg.SkiaandSKCanvasElement.What changed
Svg.Controls.Skia.UnowithSvgandSvgSourceValidation
dotnet build Svg.Skia.slnx -c Releasedotnet test Svg.Skia.slnx -c Releasedotnet build samples/UnoSvgSkiaSample/UnoSvgSkiaSample.csproj -c Release -f net10.0-desktop