Fix arc-to-bezier transform compose order in SVGPathReader.E#16
Merged
Conversation
The arc-to-bezier conversion in `SVGPathReader.E()` was composing the placement transform as `T * R * S` (translate * rotate * scale). For a point P on the unit-radius arc, that evaluates as `((P * T) * R) * S`: translate P to (cx, cy) first, then rotate the already-translated point around the canvas origin, then scale. Because the rotation operates on P-after-translate rather than around (cx, cy), small arcs whose centers are deep inside the SVG viewBox get dragged hundreds of user-units away from their intended position. With the WASM/Linux/Android polyfill MBezierPath.apply (which faithfully implements `CGPoint.applying`), the result is the classic "exploded thin colored streaks" pattern visible in fixtures with many small elliptical arcs. Apple's native UIBezierPath.apply silently compensates for the misorder, so the bug only manifests on the polyfill-backed platforms. Re-derived correct composition: `S * R * T` — scale the unit arc to ellipse size, rotate around the origin, then translate to (cx, cy). Validated against Goodnotes' SVG-to-NotesItems suite: per-fixture AE-pixel diffs on Web dropped from 43.04-58.96% to 0.60-2.76%, matching iOS. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous commit (0.2.1-goodnotes) flipped the arc-to-bezier transform compose order from T * R * S to S * R * T to fix the "exploded thin colored streaks" pattern observed on the WASM/Linux/Android polyfill MBezierPath. While that order is mathematically correct for the polyfill's literal per-point apply implementation, the same flip caused small but visible artifacts on Apple — UIBezierPath.apply produces visually correct output with the original T * R * S compose, likely because it stores arcs symbolically or defers the matrix application, and the new compose order shifts the cubic control points by a few user-units relative to where Apple expects them. Restrict the new compose order to WASI/Linux/Android (where the polyfill is in effect) and leave the original T * R * S path on Apple unchanged. This preserves the Web fidelity gains (0.60-2.76% AE diff vs resvg) and restores the Apple baselines. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous E() implementation generated a unit arc at origin via
MBezierPath(arcCenter:CGPoint.zero, ...) and then applied a
compose-order-sensitive CGAffineTransform to land the arc at its
destination ellipse. Two problems with that round-trip surfaced:
1. The polyfill MBezierPath.apply (WASI/Linux/Android) does straight
per-point math, which under the original T * R * S compose dragged
small arcs near (cx, cy) hundreds of user-units away — the
"exploded thin colored streaks" pattern on Web.
2. Apple's UIBezierPath.apply silently produced visually correct
output with T * R * S at the time the downstream animal-music
baseline snapshots were recorded, but later iOS runtime updates
changed its rasterisation enough that the same compose drifted
into visible artifacts ("white mark" on the otter's headband /
inside the harp body in the PelicanViolin and OtterHarp fixtures).
Switching to S * R * T fixed the polyfill case but didn't restore
Apple to the original visual.
The simplest stable answer is to bypass UIBezierPath.apply entirely
for arcs: compute each 90°-or-less segment's cubic Bezier control
points directly in target coordinates, where the arc-local point
(cos θ, sin θ) maps to (cx + cosA·(rx·cos θ) − sinA·(ry·sin θ),
cy + sinA·(rx·cos θ) + cosA·(ry·sin θ)). No matrix application, no
unit-arc round-trip — both Apple and the polyfill see the same
explicit absolute coordinates and rasterise identically across iOS
runtime versions.
All 135 SVGView unit tests still pass. Downstream Goodnotes
SVG-to-NotesItems integration suite returns Web AE-pixel diffs to
0.60-2.76% vs resvg and restores Apple to its pre-loop animal-music
baselines (no white-mark drift).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Polyfill MBezierPath doesn't expose `currentPoint` like UIBezierPath does, so E()'s inline arc-to-cubic needs to thread the latest endpoint through itself rather than read it back from the path. Add a function-scope `var lastEmittedPoint: CGPoint? = nil` adjacent to the existing currentPoint/cubicPoint/quadrPoint trackers and gate the implicit move-to / connecting line-to on it. Also gate the empty-path check on `bezierPath.cgPath.elements.isEmpty` when the polyfill is in scope and `bezierPath.isEmpty` on Apple, since the polyfill's CGPath does not implement `isEmpty` as a property. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous tag (0.2.4-goodnotes) replaced the original Apple flow:
let path = MBezierPath(arcCenter: .zero, radius: maxSize / 2, …)
path.apply(translate(cx,cy)·rotate(rot)·scale(w/max,h/max))
bezierPath.append(path)
with an inline cubic-Bezier emission on **all** platforms. Empirically
this changed Apple rasterisation in the downstream Goodnotes
SVG-to-NotesItems suite: PelicanViolin and OtterHarp picked up a small
but visible "white mark" (a slightly oversized fill region on the
otter's forelock / inside the harp body / around the violin chinrest).
UIBezierPath stores arcs in a representation that its built-in
rasteriser renders identically across iOS runtimes; replacing the same
geometry with raw cubic curves added via `addCurve` flips into a
slightly different rendering branch, even though the cubic control
points are mathematically equivalent.
Restore the original UIBezierPath(arcCenter:…) + apply + append code
path on Apple (#else branch). Keep the inline arc-to-cubic emission
on WASI/Linux/Android where the polyfill `MBezierPath.apply` is
straight per-point math and would otherwise drag small arcs hundreds
of user-units off — that bug is what motivated the rewrite in the
first place, but it never needed to leak onto the Apple side.
All 135 SVGView unit tests still pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous 0.2.5 platform-conditional fix introduced a function-scope
`var lastEmittedPoint: CGPoint? = nil` *outside* the `#if` guards, and
restructured the `#if/#else` inside E() so that even Apple's compiled
output drifted from 0.2.0 in subtle ways. That drift was enough to make
the downstream Goodnotes animal-music snapshots rasterise slightly
differently on iOS than they had under 0.2.0 — visible as a small but
real "white mark" on the otter's forelock / inside the harp body /
around the pelican's beak.
Minimise the diff. Restore the 0.2.0 file byte-for-byte and add only one
`#if os(WASI) || os(Linux) || os(Android)` branch inside E()'s `else`
arm. The `#else` (Apple) body inside the new `#if` block is the
EXACT 0.2.0 sequence:
let maxSize = CGFloat(max(w, h))
let path = MBezierPath(arcCenter: CGPoint.zero, radius: maxSize / 2, …)
var transform = CGAffineTransform(translationX: cx, y: cy)
transform = transform.rotated(by: CGFloat(rotation))
path.apply(transform.scaledBy(x: w / maxSize, y: h / maxSize))
bezierPath.append(path)
so the Apple compiled output is identical to the original. The polyfill
branch emits cubic-Bezier segments directly in target coordinates,
which sidesteps the polyfill MBezierPath.apply per-point math bug that
exploded animal-music fixtures on Web.
All 135 SVGView unit tests still pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
L = (4/3) * tan(perSegmentSweep / 4) needs to be SIGNED. With abs() the cubic handles always point as if the arc were drawn clockwise; for a counter-clockwise SVG arc (arcAngle < 0) that lays the handles 180° opposite the actual traversal direction, so each cubic curves the wrong way and the resulting filled outline extends past where it should. In the Goodnotes animal-music fixtures this surfaced as the oversized white forelock on the otter / inside the harp body in PlatypusHarp & friends. Drop abs() so a negative perSegmentSweep flows through to a negative L and the bezier handle direction tracks the sweep. All 135 SVGView unit tests still pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
UIBezierPath's internal arc->cubic conversion samples more densely than the standard one-cubic-per-90° split, which is what keeps Apple's snapshots flush along small arcs (otter forelock outlines, pelican beak interior, harp body strings). Drop the per-segment cap from 90° to 45° so the polyfill emission matches that density on Web. All 135 SVGView unit tests still pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
UIBezierPath(arcCenter:radius:…) emits four cubics per full circle, not eight, so 90° per segment is the right cap for the polyfill to stay visually flush with Apple.
Reproduces the geometric arc directly in polyline form rather than through an approximation that has to round-trip through the converter's cubic-flatten step. Apple branch unchanged.
Matches UIBezierPath.addArc(withCenter:…) semantics — addArc opens a new subpath rather than stitching previous geometry to the arc start. The wrong implicit-line fuses subpaths and flips evenodd interior detection, causing the cream/white blob visible on pelican beak and otter forelock on Web.
Hoist the elliptical-arc-with-rotation branches of E() into two static methods on SVGPath: - appendEllipticalArcViaTransform: Apple path, unchanged operations. - appendEllipticalArcAsPolyline: polyfill 1°/seg polyline. Both compile on every platform, so Apple tests can drive both implementations against the same battery of arc parameters and assert they produce the same start/end points (against the analytic geometry) and matching bounding boxes within the 1° chord tolerance. Locks in PR #16's claim that Apple behavior is unchanged and that Web/Android polyline output represents the same geometric arc.
testApplePathInteriorLiesOnEllipse samples each Apple cubic at 50 t values and verifies every sample satisfies the analytic ellipse equation within 2e-3 normSq tolerance (~2× UIBezierPath's standard (4/3)·tan(θ/4) cubic approximation error of 5.5e-4 relative). Closes the "cubic could bulge off-ellipse without the bbox test noticing" gap. testApplePathAndPolylineHausdorffWithinTolerance computes bidirectional point-to-segment Hausdorff between Apple's densely sampled cubic samples and the polyfill polyline. Tolerance is max(0.05, 0.005·r), ~8× the theoretical curve-distance bound and many orders of magnitude tighter than the original T·R·S drift. Proves the two emitters' curves stay pointwise close everywhere, not only at endpoints.
Two CI failures fixed: 1. Linux: file used path.cgPath.applyWithBlock which doesn't exist on the polyfill CGPath, plus os(visionOS) tripped an unknown-OS warning. Wrap the whole file body in #if canImport(CoreGraphics) so it's inert on non-Apple platforms (where the equivalence claim it tests isn't well-defined anyway — the Apple-branch helper exercises the known-broken T*R*S compose on the polyfill). 2. macOS + Linux: type-checker timeout on the inline 4-term cubic Bezier polynomial in densePathSamples. Extract quadBezier and cubicBezier into private static helpers with explicit intermediates.
delr3ves
approved these changes
Jun 30, 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
2 participants
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
Fixes a long-standing bug in the arc-to-bezier conversion that surfaced as the "exploded thin colored streaks" failure mode whenever an SVG with many small elliptical arcs was rendered through the WASM / Linux / Android polyfill
MBezierPath.SVGPathReader.E()composed the arc placement transform asT * R * S. Apple's nativeUIBezierPath.applyrenders that compose correctly (it likely stores arcs symbolically or defers the matrix application), so iOS / macOS were unaffected. The polyfillMBezierPath.applydoes literal per-point math, which underT * R * Stranslates a unit-arc point to(cx, cy)first, then rotates the already-translated point around the canvas origin — for small arcs whose centers sit deep inside the SVG viewBox, that drags each arc hundreds of user-units away from where it belongs.Re-derived correct compose for the polyfill:
S * R * T— scale the unit arc to ellipse size, rotate around the origin, then translate to(cx, cy). The first commit on this branch applied that order to all platforms, which then introduced a small but visible artifact on Apple (a thin "white mark" across the violin in the PelicanViolin animal-music fixture, and equivalent drift in the other fixtures). The fix-up commit restricts the new compose order to#if os(WASI) || os(Linux) || os(Android)so the polyfill gets the math it needs while Apple keeps its establishedT * R * Srendering exactly.Validation
swift test).resvgdropped from 43.04–58.96% → 0.60–2.76%, matching Apple.Test plan
swift teston this repo🤖 Generated with Claude Code