Support non-scaling SVG strokes#514
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: 55ce64b41c
ℹ️ 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".
| skCanvas.Save(); | ||
| skCanvas.ResetMatrix(); | ||
| skCanvas.DrawPath(transformedPath, paint); |
There was a problem hiding this comment.
This path now draws IsStrokeNonScaling strokes in device space by transforming the geometry and resetting the canvas matrix, so the rendered stroke width no longer follows the node transform. Hit testing still goes through SvgSceneHitTestService/GeometryHitTestService.ContainsStroke(..., node.StrokeWidth), which assumes stroke width is local-space and therefore transform-scaled. That mismatch causes interactive hits to be too narrow when the content is downscaled and too wide when upscaled whenever vector-effect="non-scaling-stroke" is used.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 2fdf1a513 (Fix non-scaling stroke hit testing). SvgSceneHitTestService now passes node.IsStrokeNonScaling into a new GeometryHitTestService.ContainsStroke overload. For non-scaling strokes, the hit tester flattens the path, maps the flattened points through node.TotalTransform, and compares the original scene-space point against that transformed geometry with the authored stroke width as the tolerance, so the hit region matches the device-space draw path. I also changed structural traversal bounds to use renderable paint bounds so ancestor groups are not pruned before a non-scaling stroke child is checked. Covered by HitTest_Point_NonScalingStroke_UsesDeviceSpaceStrokeWidth; focused tests, dotnet build Svg.Skia.slnx -c Release --no-restore, and dotnet test Svg.Skia.slnx -c Release --no-restore pass.
| @@ -710,14 +779,30 @@ public void Draw(SkiaSharp.SKCanvas canvas) | |||
| } | |||
| else if (!TryDrawAnimationLayers(canvas)) | |||
There was a problem hiding this comment.
The non-scaling fallback is only entered when TryDrawAnimationLayers(canvas) returns false, so any document using animation-layer caching renders via cached SKPicture replay and skips ContainsNonScalingStroke/SkiaModel.Draw. In that mode, vector-effect="non-scaling-stroke" strokes still scale with the caller’s canvas transform (for example after canvas.Scale(...)), which defeats the feature for animated content.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 180a5c191 (Render non-scaling animation layers live). TryDrawAnimationLayers now captures both retained layer models and native pictures. Each layer checks ContainsNonScalingStroke(model); layers with non-scaling strokes are drawn via SkiaModel.Draw(model, canvas) so the existing path-transform/reset-matrix behavior runs under the caller canvas transform, while layers without this marker still replay cached native pictures. Covered by Draw_AnimationLayerCachingWithNonScalingStroke_PreservesStrokeWidth, which verifies active animation layer caching under canvas.Scale(0.5f, 0.5f); focused tests, dotnet build Svg.Skia.slnx -c Release --no-restore, and dotnet test Svg.Skia.slnx -c Release --no-restore pass.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 180a5c191d
ℹ️ 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".
| skCanvas.ResetMatrix(); | ||
| skCanvas.DrawPath(transformedPath, paint); |
There was a problem hiding this comment.
This branch resets the canvas matrix before drawing non-scaling strokes, which keeps stroke width constant but also drops the active transform that Skia uses to map shader coordinates. As a result, transformed strokes that use gradient/pattern shaders render with incorrect color mapping (e.g., gradients no longer follow the transformed geometry) whenever vector-effect="non-scaling-stroke" is set under a non-identity canvas transform.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f8ba315a95
ℹ️ 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".
| canvas.Scale(scaleX, scaleY); | ||
| SkiaModel.Draw(model, canvas); |
There was a problem hiding this comment.
When saving at scaleX/scaleY below 1, any document that contains a non-scaling stroke now takes this live-model path and draws directly at the target scale, bypassing SKPictureExtensions.ToImage's oversampled render/downsample path. That means adding one vector-effect="non-scaling-stroke" element to an SVG that also embeds raster images reintroduces the downscaled-raster aliasing that the normal Picture.ToImage export path avoids; the live path needs equivalent raster downsampling while keeping the non-scaling strokes at target-device width.
Useful? React with 👍 / 👎.
1 participant
PR Summary: Support
vector-effect: non-scaling-strokeOverview
This change adds focused support for SVG
vector-effect: non-scaling-strokeso strokes keep their authored width when SVG content is rendered under down-scaling or other canvas transforms. The implementation is intentionally narrow: it supports thenoneandnon-scaling-strokevalues needed for issue #275 without attempting broad SVG 2 vector-effect coverage.Motivation
Issue #275 reports that down-scaled SVG content with
vector-effect="non-scaling-stroke"renders strokes too thin because stroke width is scaled together with geometry. Browsers preserve the stroke width for this value by applying the transform to the path geometry while keeping stroke painting in device-space stroke units. Svg.Skia previously had no representation for this attribute, so the renderer could not distinguish regular strokes from non-scaling strokes.Changes
SVG Model And Parsing
SvgVectorEffectwith converter support for kebab-case SVG values.VectorEffectproperty onSvgVisualElement.vector-effectin the compatibility style attribute set so inline CSS such asstyle="vector-effect: non-scaling-stroke"is recognized.Scene Graph And Paint Metadata
IsStrokeNonScalingto the shimSKPaintrepresentation and preserved it through clone/versioning behavior.VectorEffectsetting from SVG visual elements into scene graph nodes and stroke paints.IsStrokeNonScalingtoSvgSceneNodeso later rendering stages can make transform-aware decisions.Skia Rendering And Export
SkiaModel.DrawPathso stroke paths marked as non-scaling are transformed into current canvas space, then drawn with the canvas matrix reset. This preserves path placement while keeping stroke width unscaled.SKSvg.Drawto bypass cached native picture replay when the model contains non-scaling stroke paths, because a cachedSKPicturecannot adapt stroke width to the caller's current canvas transform.SKSvg.Saveto render the live model for non-scaling stroke content instead of converting from a cached native picture image.OnDrawbehavior in the live-modelSKSvg.Drawpath.Tests
SKPaint.IsStrokeNonScalingproperty.SKSvg.Drawpreserves stroke width under an external canvas scale and still raisesOnDraw.vector-effect="non-scaling-stroke"style="vector-effect: non-scaling-stroke"Commit Breakdown
07bc73384 Add vector-effect scene metadata693598dbd Render non-scaling strokes under transforms55ce64b41 Add non-scaling stroke regression testsValidation
dotnet format Svg.Skia.slnx --no-restore --include ...git diff --checkdotnet build Svg.Skia.slnx -c Release --no-restoredotnet test Svg.Skia.slnx -c Release --no-restoreThe full test suite passed. The build still reports existing warnings unrelated to this change, including known NuGet vulnerability warnings in sample projects and existing nullable/obsolete warnings.
Notes And Limitations
SKSvg.Picturedo not get dynamic non-scaling-stroke behavior, because a staticSKPicturecannot reinterpret stroke widths relative to a later caller transform. The publicSKSvg.DrawandSKSvg.Savepaths now detect this case and render through the live model instead.