Skip to content

Compose SVG glyph transforms in document order - #571

Open
jafin wants to merge 1 commit into
SixLabors:mainfrom
jafin:fix/svg-transform-order
Open

jafin wants to merge 1 commit into
SixLabors:mainfrom
jafin:fix/svg-transform-order

Conversation

@jafin

@jafin jafin commented Oct 17, 2026

Copy link
Copy Markdown
Contributor

Prerequisites

  • I have written a descriptive pull-request title
  • I have verified that there are no overlapping pull-requests open
  • I have verified that I am following matches the existing coding patterns and practice as demonstrated in the repository. These follow strict Stylecop rules 👮.
  • I have provided test coverage for my change (where applicable)

Description

Fixes #570.

Matrix3x2 transforms row vectors, so in a * b the transform a applies first. SvgGlyphSource composed SVG transforms the other way round. This PR fixes the order in three places, and the rotation centre:

  • Transform lists: ParseTransform now prepends each entry (m = t * m), so the list applies right to left as SVG requires.
  • Nested elements: Walk now computes nodeTransform * parentLocalTransform.
  • <use>: now computes translate(x, y) * nodeTransform * parentLocalTransform. The x/y offset and the element's own transform were already in the right order relative to each other; only the parent transform was misplaced.
  • rotate(angle cx cy): now rotates about (cx, cy), using Matrix3x2.CreateRotation(radians, centerPoint).

gradientTransform goes through the same ParseTransform, so a gradient with more than one transform in its list is fixed as well.

Tests

SvgTransformTests renders a triangle under each transform and compares its device-space points with an untransformed triangle at hand-calculated coordinates. Five of the six cases fail on main. The sixth checks the x/y-before-transform order inside <use>, which was already correct.

SvgTestFonts is a small shared helper that adds an SVG table to SixLaborsSampleAB.ttf, so tests can write SVG glyph documents inline. #569 has a private copy of the same helper. Whichever of the two merges second can switch to the shared one.

The full suite passes on net10.0 in Release, apart from HarfBuzzCorpusTests (2 tests) and TextShaperTests.ZawgyiScriptOverrideAppliesThroughTextOptionsAndTextRun, which fail the same way on a clean main in my environment.

I also enabled SUPPORTS_DRAWING locally to compare the Issues_462 emoji snapshots. Their differences from the reference images are identical with and without this change, so it doesn't change the Noto Color Emoji SVG output. (12 of those 16 snapshots differ from the references on main on my machine anyway, the COLRv1 ones included.)

Noticed but not changed

Two more transform issues in the same file, left out to keep this PR focused:

  • A transform on the root <svg> element is passed to Walk as the starting transform and is also stored in PaintedCanvasMetadata.RootTransform, which PaintedGlyphMetrics.ComputeSourceToUpem applies again. When the glyph is the root element itself, Walk parses the attribute a third time.
  • BuildLinearGradient and BuildRadialGradient multiply gradientTransform together along an href chain. In SVG, a referencing gradient inherits the attribute only when it doesn't set its own.

Matrix3x2 transforms row vectors, so the left operand of a product applies
first. SvgGlyphSource composed transforms the other way round:

- A transform list such as "translate(100 0) scale(2)" was applied left to
  right; SVG applies it right to left.
- A child's transform was applied after its parent's instead of before.
- A <use> element's parent transform was applied before its own transform
  and x/y offset instead of last.

Also honour the centre point of rotate(angle cx cy), which was ignored.
@codecov

codecov Bot commented Oct 17, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 66.66667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 80%. Comparing base (4eb468d) to head (fe6688a).

Files with missing lines Patch % Lines
...xLabors.Fonts/Tables/General/Svg/SvgGlyphSource.cs 66% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@          Coverage Diff          @@
##            main    #571   +/-   ##
=====================================
  Coverage     80%     80%           
=====================================
  Files        408     408           
  Lines      35802   35802           
  Branches    5542    5543    +1     
=====================================
+ Hits       28885   28914   +29     
+ Misses      5488    5448   -40     
- Partials    1429    1440   +11     
Flag Coverage Δ
unittests 80% <66%> (+<1%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SVG glyph transforms are composed in reverse order, and rotate(a cx cy) ignores its centre

1 participant