Repository navigation
Add SVG marker, filter, and font-weight/style matching support - #23
Merged
Merged
Conversation
added 3 commits
September 26, 2026 18:03
Adds support for rendering <marker> elements referenced via marker-start, marker-mid, and marker-end presentation attributes on line, polyline, polygon, and path elements. - Parse marker-start/marker-mid/marker-end as url(#id) references, added to RenderState/ApplyPresentationAttributes following the existing presentation-attribute cascade pattern. - Parse <marker> element attributes: markerWidth/markerHeight, refX/refY, markerUnits (strokeWidth default, userSpaceOnUse), orient (auto, auto-start-reverse, fixed angle), and optional viewBox with meet-fit scaling. - Compute ordered vertex lists (position + tangent + start/mid/end classification) from the already-built local-space Path data for line/polyline/polygon/path; rect/circle/ellipse never receive markers. - Compose the per-vertex marker transform (ref offset, content/units scale, orientation, vertex position) and render marker content via the existing recursive RenderElement with a fresh cascaded RenderState that does not inherit the referencing shape's fill or stroke. - Reuse existing elementDepth/totalElements/workBudget bounds for marker content, and add a dedicated markerDepth cycle guard mirroring <use>'s MaxUseDepth mechanism so marker-to-marker reference cycles are bounded rather than causing unbounded recursion. Dangling marker references tolerantly render nothing. - Update SvgCodec class-level XML doc remarks to move marker from the ignored-construct list into the supported subset, and document the out-of-scope marker sub-features (markerContentUnits, content overflow/clipping). Adds 16 new tests covering basic marker-end rendering, combined marker-start/mid/end placement, orient=auto tangent correctness for horizontal/vertical/diagonal lines, auto-start-reverse, markerUnits scaling in both modes, viewBox-based content fitting, dangling reference tolerance, marker reference cycle detection, multi-subpath path start/end placement, shape exclusions, and marker content non-inheritance of the referencing shape's presentation state, plus a fixture-based arrowhead rendering test. Updates the SVG codec design and verification documentation and adds new reqstream requirements (with system/subsystem decomposition) for marker rendering, orientation/units handling, and reference-cycle safety. Updates the README SVG feature bullet and user guide.
Implement filter="url(#id)" rendering for the common solid-color halo/ background use case, replacing the prior silent skip of <filter> elements. - Parse <filter> defs via the existing BuildIdIndex mechanism, with objectBoundingBox filter regions (default -10% -10% 120% 120%); userSpaceOnUse tolerantly falls back to the same default. - Evaluate filter primitive chains in document order: feFlood, feGaussianBlur (clamped stdDeviation), feOffset, feComposite (over/in/out/atop/xor), and feMerge, against SourceGraphic rendered into an offscreen temporary Surface sized to the filter region. Unsupported primitive types are tolerant no-op passthroughs. - Composite the filtered result back onto the main surface using the existing Surface.CompositeOver/CompositeOverSpan APIs. - Dangling filter references render the element normally, matching the codec's established dangling-reference tolerance convention. - Add resource-safety bounds: temporary surface dimensions are clamped via Surface.MaxDimension, blur stdDeviation is clamped, and a new primitive-count/region-area work budget (MaxFilterPrimitivesPerFilter / MaxFilterPrimitiveWorkUnits) rejects pathological filter chains with a tolerant fallback instead of an unbounded hang. - Update SvgCodec's class-level XML doc remarks to move filter from the out-of-scope list into the supported subset. Tests: add unit and fixture tests covering flood-only halos, flood+blur+composite/merge halos, feComposite in/atop/xor operators, dangling filter references, unsupported-primitive passthrough, pathological filter-region/blur/primitive-count bounding, and default vs. explicit filter regions. Update the pre-existing InkscapeFilters fixture test now that its filter chain is genuinely evaluated. Update design doc, verification doc, reqstream requirements (system/ subsystem/unit), README, and user guide to document the new filter support, mirroring the conventions established by the marker support added in a16d185.
Add SvgFontFace/SvgFontStyle types and additive Load overloads accepting IReadOnlyDictionary<string, IReadOnlyList<SvgFontFace>> so callers can register multiple weight/style variants per font family. Cascade font-weight/font-style presentation attributes through RenderState the same way font-family/font-size already do, and select the closest matching face (exact style match, then closest numeric weight, then boldness-side tiebreak) in MatchFont, falling back gracefully when no close variant is registered. Existing single-font-per-family overloads remain unchanged, wrapping each font as one normal-weight/normal-style face internally. Adds unit tests proving bold/italic face selection via distinguishable synthetic glyph outlines, closest-match fallback, cascading inheritance, and legacy single-font-per-family regression behavior. Updates design, verification, reqstream (unit/subsystem/system), README, and user guide documentation to match.
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Unresolved source-compatibility, marker, filter, font-matching, and test reliability issues remain.
Review effort: Lite
Findings: 1
Open (6)
New overload makes existing null calls ambiguous · New Filter budget ignores feMerge node processing cost · New Filter bounds reject valid stroked lines · New Closed paths omit the closing tangent for marker orientation · New Stroke width is scaled twice in marker construction · New Integer weight distance calculation can overflow · New
What changed in this PR
Adds SVG marker and filter rendering, plus font weight/style-aware matching in SvgCodec, with tests and documentation updates.
Changes:
- Adds marker geometry, orientation, scaling, and cycle handling.
- Adds bounded filter evaluation and compositing.
- Adds multi-face font matching APIs.
- Expands fixtures, tests, documentation, and requirements.
| File | Description |
|---|---|
test/DemaConsulting.CanvasNet.Tests/SvgFixtures/README.md |
Documents SVG fixtures. |
test/DemaConsulting.CanvasNet.Tests/SvgFixtures/label-halo.svg |
Adds filter fixture coverage. |
test/DemaConsulting.CanvasNet.Tests/SvgFixtures/arrow-markers.svg |
Adds marker fixture coverage. |
test/DemaConsulting.CanvasNet.Tests/Codecs/SvgFixtureTests.cs |
Adds fixture integration tests. |
test/DemaConsulting.CanvasNet.Tests/Codecs/SvgCodecTests.cs |
Adds feature and regression tests. |
src/DemaConsulting.CanvasNet/Codecs/SvgFontStyle.cs |
Adds SVG font-style metadata. |
src/DemaConsulting.CanvasNet/Codecs/SvgFontFace.cs |
Adds registered font-face metadata. |
README.md |
Updates SVG feature documentation. |
docs/verification/canvas-net/codecs/svg-codec.md |
Adds verification coverage. |
docs/user_guide/introduction.md |
Documents public SVG APIs. |
docs/reqstream/canvas-net/codecs/svg-codec.yaml |
Adds detailed codec requirements. |
docs/reqstream/canvas-net/codecs.yaml |
Adds codec requirements. |
docs/reqstream/canvas-net.yaml |
Adds system requirements. |
docs/design/canvas-net/codecs/svg-codec.md |
Documents design and limitations. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Addresses 6 issues (1 high, 5 medium) found by Copilot code review on PR #23: 1. (HIGH) Ambiguous overload: renamed the richer Load(Stream/string, int, int, IReadOnlyDictionary<string, IReadOnlyList<SvgFontFace>>?) overloads to LoadWithFontFaces, eliminating overload-resolution ambiguity with an explicit `null` literal against the legacy single-font Load overload. Updated all call sites, XML doc <see cref> references, and docs. 2. (MEDIUM) Filter work-budget bypass: feMerge's upfront primitive-count charge now includes each feMergeNode child's own cost (instead of a flat 1), since ApplyFeMerge performs one full-surface composite per node. 3. (MEDIUM) Filter bounds reject stroked lines: the filter-region degeneracy guard and region computation now use stroke-expanded bounds (via a new ExpandBoundsForStroke helper) instead of bare centerline/fill bounds, so a horizontal/vertical stroked <line> filter is no longer misdetected as degenerate. 4. (MEDIUM) Missing closing tangent: BuildMarkerVertices now folds a closed subpath's implicit closing-segment tangent into both the last vertex's outgoing tangent and the first vertex's incoming tangent, so a closed polygon/path's marker-end orientation reflects the closing edge too. 5. (MEDIUM) Double stroke-width scaling: RenderMarkers now passes the raw, unscaled local stroke-width into RenderOneMarker (instead of an already-pixel-scaled effective width), so the shapeTransform composition is the only place the root/group pixel scale is applied. 6. (MEDIUM) Integer overflow in weight distance: SelectClosestFace/ IsBetterFace now compute weight distance using long arithmetic, avoiding OverflowException/silent-wrap for extreme caller-supplied font-weight values. Added 6 new regression tests (one per issue) plus updated design/ verification/reqstream/user-guide docs for the LoadWithFontFaces rename and the new test coverage. Test count: 4260 -> 4278 (6 new tests x 3 target frameworks). build.ps1 and lint.ps1 both pass.
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Unresolved critical filter resource-safety and moderate marker, filter, and font-selection issues remain.
Review effort: Lite
Findings: 1
Open (4)
Resolved since last review (6)
New overload makes existing null calls ambiguous Integer weight distance calculation can overflow Stroke width is scaled twice in marker construction Closed paths omit the closing tangent for marker orientation Filter bounds reject valid stroked lines Filter budget ignores feMerge node processing cost
…ion, marker orient default, stale doc - HIGH: RenderFilteredShape now skips filtered rendering (falls back to unfiltered) for a <filter> with zero primitive children, before any temporary SourceGraphic surface is allocated - previously a zero work-unit charge trivially passed the primitive-work budget check even for a huge filter region, allocating a large unused surface. - MEDIUM: feOffset's dx/dy-to-pixel conversion and feComposite's per-channel output rounding now use MathF.Round(value, MidpointRounding.AwayFromZero), matching Surface.cs's established compositing convention, instead of MathF.Round's default banker's (round-to-even) rounding. - MEDIUM: ParseMarkerOrient now treats an absent/blank orient attribute as the SVG spec's fixed 0-degree default, distinct from an explicit orient="auto"; only the explicit auto/auto-start-reverse keywords opt into tangent-following behavior. - LOW: Updated the stale "ambiguous null" doc text in docs/user_guide/introduction.md now that the richer overload was renamed to LoadWithFontFaces - Load(..., null) and LoadWithFontFaces(..., null) are both unambiguous. Added/updated regression tests for all four fixes and updated the design/verification/reqstream companion docs accordingly.
… marker content - RenderFilteredShape now renders SourceGraphic with Opacity forced to 1.0, then applies the element's own cascaded opacity exactly once as a coverage multiplier when compositing the filtered result onto the canvas, instead of baking opacity into the pre-filter source paint (per SVG opacity-applies-to-filtered-result semantics). - RenderElement now threads a suppressFilter flag (markerDepth > 0) down to RenderShapeWithFilter/RenderText so a filter attribute on marker content is never resolved/evaluated, matching this codec's documented 'filters on marker content have no effect' scope decision. - Added regression tests SvgCodec_Load_OpacityWithFeFloodFilter_AppliesOpacityToFilteredResultNotSource and SvgCodec_Load_MarkerContentWithFilterAttribute_FilterHasNoEffect. - Updated design/verification/reqstream docs for svg-codec accordingly.
Fixes a high-severity code-review finding: the existing per-filter work-budget check (IsFilterPrimitiveWorkWithinBudget / MaxFilterPrimitiveWorkUnits = 5,000,000) was enforced independently for each individual filtered element. A document defining one filter and referencing it from many shapes could therefore be charged up to 5,000,000 work units once per reference with no bound on the total number of references, letting total filter-evaluation CPU/memory grow arbitrarily with shape count. Adds a new FilterWorkBudget class, mirroring GeometryWorkBudget's existing 'mutable object threaded through the render walk' pattern, constructed once per Load call and threaded through RenderElement/RenderUse/RenderMarkers/RenderOneMarker/RenderText/ RenderShapeWithFilter down to RenderFilteredShape. Every filter application that gets past the existing per-filter checks now charges its region-weighted work unit into this cumulative running total; once a new MaxCumulativeFilterWorkUnits ceiling (50,000,000 - ten times the per-filter ceiling) would be exceeded, the filter application tolerantly falls back to unfiltered rendering for the remainder of that Load call, matching every other filter resource-bound's existing tolerant-fallback convention rather than throwing. Adds a regression test (13 shapes referencing one filter, each individually charging exactly the per-filter ceiling) proving the first 10 references are genuinely filtered while the 11th-13th fall back to unfiltered rendering once the cumulative ceiling is exceeded, without throwing and while completing in well under a second. Updates the design doc, verification doc, and reqstream requirement for CanvasNet-Codecs-SvgCodec-FilterResourceSafety to document the new cumulative budget, its chosen ceiling, and rationale.
- ParseFontStyle now splits font-style values on all XML/CSS whitespace (space, tab, newline, CR) instead of only literal space, so values like 'oblique<TAB>10deg' correctly resolve to Italic instead of silently falling back to the inherited style. - ParseFirstNumberToken now splits filter-primitive numeric attributes (e.g. stdDeviation) on all XML/CSS whitespace plus comma, instead of only literal space plus comma, so values like '2<TAB>3' correctly parse the first token as 2 instead of silently falling back to 0 (disabling blur). - Added regression tests covering tab-separated font-style and stdDeviation values for both fixes.
This was referenced Oct 5, 2026
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.



Summary
Adds three previously-missing SVG rendering features to
SvgCodec, filed as backlog items from a user report:<marker>support —marker-start/marker-mid/marker-endare now rendered (open/filled arrowheads, diamonds, circles, bars, etc.), includingorient(auto/auto-start-reverse/angle),markerUnits,viewBoxfitting, and cycle detection.<filter>support —feFlood,feGaussianBlur,feOffset,feComposite, andfeMergeare now rendered againstSourceGraphic/SourceAlpha, via an offscreen-surface compositing path with resource-safety bounds (primitive count, region area, and blur-radius ceilings) to prevent pathological documents from causing excessive CPU/memory use.SvgFontFace/SvgFontStyletypes andLoadoverloads let a caller register multiple font faces (e.g. regular/bold/italic) per family;SvgCodecnow selects the closest-matching face for eachtextelement's ownfont-weight/font-style. Existing single-font-per-familyLoadoverloads are unchanged (thin wrappers), so this is fully backward-compatible.Testing
4260/4260tests passing (build.ps1), up from4137before this branch.lint.ps1clean (dotnet format, markdown, YAML, spelling, ReqStream/ReviewMark/SysML2 compliance).Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com