[vector_graphics_compiler] Fix tspan transform applied twice by mackings · Pull Request #13129 · flutter/packages

mackings · 2026-10-05T11:27:33Z

A tspan without its own x/y inside a translated g was drawn with the translation applied twice.

The parent text has x/y, so the compiler applies the transform to its position. The tspan has no position, so it kept the transform as well. The renderer then applied that transform on top of a position that already had it.

Now a tspan without its own position follows its parent: if the parent already applied the transform, the tspan does not apply it again. The bug is in vector_graphics_compiler, which flutter_svg uses to parse SVGs.

Fixes flutter/flutter#189925

Pre-Review Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the gemini-code-assist bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.

Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. ↩ ↩²

A tspan without its own x/y inside a translated group kept the group transform, even though its parent text had already applied it to its position. The transform is no longer applied again in that case. Fixes flutter/flutter#189925

gemini-code-assist

Code Review

This pull request resolves an issue where a tspan without its own position is drawn with its parent group transform applied twice. It updates ResolvingVisitor to track the active text position and passes it as a parent parameter to computeTextPosition to determine if the transform was already applied. Feedback on the changes highlights a potential bug where a tspan's local transform could be discarded if it has no position, and suggests ensuring continuesTransformedParent is only true when attributes.transform is null.

gemini-code-assist · 2026-10-05T11:32:13Z

+    final bool continuesTransformedParent =
+        parent != null &&
+        parent.transform == null &&
+        x == null &&
+        y == null &&
+        dx == null &&
+        dy == null;


If the tspan has its own local transform (i.e., attributes.transform != null) but no position of its own, continuesTransformedParent will evaluate to true. This causes consumeTransform to be true (if the computed transform is encodable in a rect), which sets the TextPosition's transform to null, completely discarding the tspan's local transform. To prevent discarding the local transform of the tspan, we should ensure continuesTransformedParent is only true when the tspan does not have its own local transform.

Suggested change

final bool continuesTransformedParent =

parent != null &&

parent.transform == null &&

x == null &&

y == null &&

dx == null &&

dy == null;

final bool continuesTransformedParent =

parent != null &&

parent.transform == null &&

attributes.transform == null &&

x == null &&

y == null &&

dx == null &&

dy == null;

Thanks for the suggestion! I tried it, but attributes.transform is never null (it defaults to identity), so this check would always be false and the fix would stop working. A transform on a tspan isn't valid SVG anyway, so I'll keep it as is.

b-luk · 2026-10-05T23:22:31Z

+    // A tspan without its own position continues from the parent's position.
+    // If the parent already applied the transform to its position, the
+    // transform must not be applied again.
+    final bool continuesTransformedParent =


nit: I think the name is a bit confusing, because TransformedParent reads as "a parent that has a transform", but the condition is parent.transform == null. It makes sense once you realize that parent.transform == null because the parent consumed its transform, but it's not intuitive from the naming. What do you think about naming it something like inheritsConsumedTransform? And maybe updating the comment to explain that the part about "If the parent already applied the transform to its position" means "parent.transform == null".

Good point, thanks! Renamed it to inheritsConsumedTransform and updated the comment to say the parent's transform is null in that case.

flutter-dashboard · 2026-10-13T17:11:20Z

This pull request is not mergeable in its current state, likely because of a merge conflict. Pre-submit CI jobs were not triggered. Pushing a new commit to this branch that resolves the issue will result in pre-submit jobs being scheduled.

b-luk

Please merge from upstream to resolve the CHANGELOG conflict.

b-luk · 2026-10-13T18:02:01Z

    final bool consumeTransform =
        computedTransform == AffineMatrix.identity ||
-        (computedTransform.encodableInRect && (hasXY || hasDxDy));
+        (computedTransform.encodableInRect && (hasXY || hasDxDy || inheritsConsumedTransform));


Maybe this is me being pedantic, but I don't think inheritsConsumedTransform should be a condition that makes consumeTransform true. If we're inheriting from the parent, we don't want to consume the current transform (i.e. we don't want to apply our current transform to our x/y/dx/dy.) It just so happens that if we're inheriting from the parent, consuming the current transform is a no-op because we have no x/y/dx/dy, so this doesn't do anything incorrect. But this is not immediately clear from the code, so I think it makes the code harder to follow and it makes the naming misleading.

If the parent already consumed the transform, it just means we don't want to pass the transform to the returned TextPosition. So instead of making inheritsConsumedTransform a condition for consumeTransform, I think it's clearer if we just use it below as transform: (inheritsConsumedTransform || consumeTransform) ? null : computedTransform,. What do you think?

Makes sense, that's clearer. Changed it to transform: (inheritsConsumedTransform || consumeTransform) ? null : computedTransform and left consumeTransform as it was.

# Conflicts: # packages/vector_graphics_compiler/CHANGELOG.md

mackings · 2026-10-15T04:59:32Z

Merged upstream. Since 1.3.1 is now the alpha mask change, I moved this entry to 1.3.2.

b-luk

LGTM. Thanks, @mackings

gaaclarke

lgtm

…r#194564) flutter/packages@da40eaf...93eede7 2026-10-16 41930132+hellohuanlin@users.noreply.github.com [camera]fix a crash when switching camera while recording due to disconnected capturePhotoOputput (flutter/packages#13248) 2026-10-16 okorohelijah@google.com [various] Update iOS example apps to the 15.0 deployment target (flutter/packages#13223) 2026-10-16 36861262+QuncCccccc@users.noreply.github.com [material_ui] Add M3E `IconButton` example (flutter/packages#13085) 2026-10-15 magder@google.com [ci] Increase deprecation check minimum to iOS 16 (flutter/packages#13211) 2026-10-15 victor.orozco@cloudsufi.com [google_sign_in] Remove the public iOS handleOpen helper (flutter/packages#13273) 2026-10-15 okorohelijah@google.com [google_fonts] Make the pendingFonts test deterministic (flutter/packages#13262) 2026-10-15 48776784+mackings@users.noreply.github.com [vector_graphics_compiler] Fix tspan transform applied twice (flutter/packages#13129) 2026-10-15 engine-flutter-autoroll@skia.org Roll Flutter from 426be96 to 5c7e45a (21 revisions) (flutter/packages#13270) 2026-10-15 37883572+edwin-hollen@users.noreply.github.com [flutter_svg] Add golden widget tests for SVG alpha masks (flutter/packages#13245) If this roll has caused a breakage, revert this CL and set the roller to dry run mode using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC flutter-ecosystem@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

github-actions Bot added p: vector_graphics triage-engine Should be looked at in engine triage labels Oct 5, 2026

gemini-code-assist Bot reviewed Oct 5, 2026

View reviewed changes

gaaclarke requested a review from b-luk October 5, 2026 20:53

b-luk added the CICD Run CI/CD label Oct 5, 2026

b-luk requested changes Oct 6, 2026

View reviewed changes

Rename continuesTransformedParent to inheritsConsumedTransform

dc0d27d

flutter-dashboard Bot removed the CICD Run CI/CD label Oct 9, 2026

mackings requested a review from b-luk October 11, 2026 08:57

b-luk added the CICD Run CI/CD label Oct 13, 2026

b-luk requested changes Oct 13, 2026

View reviewed changes

mackings added 2 commits October 15, 2026 05:57

Merge remote-tracking branch 'upstream/main' into vgc-tspan-transform

441827f

# Conflicts: # packages/vector_graphics_compiler/CHANGELOG.md

Drop the transform for inherited positions instead of consuming it

383a8f4

flutter-dashboard Bot removed the CICD Run CI/CD label Oct 15, 2026

mackings requested a review from b-luk October 15, 2026 05:01

b-luk added the CICD Run CI/CD label Oct 15, 2026

b-luk approved these changes Oct 15, 2026

View reviewed changes

b-luk requested a review from gaaclarke October 15, 2026 19:58

gaaclarke approved these changes Oct 15, 2026

View reviewed changes

gaaclarke added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 15, 2026

auto-submit Bot merged commit 293b18c into flutter:main Oct 15, 2026
14 checks passed

engine-flutter-autoroll mentioned this pull request Oct 16, 2026

Roll Packages from da40eaf4548a to 93eede7768d6 (9 revisions) flutter/flutter#194564

Merged

Navigation Menu

Repository navigation

[vector_graphics_compiler] Fix tspan transform applied twice - #13129

[vector_graphics_compiler] Fix tspan transform applied twice#13129
auto-submit[bot] merged 4 commits into
flutter:mainfrom
mackings:vgc-tspan-transform

mackings commented Oct 5, 2026

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

gemini-code-assist Bot Oct 5, 2026

Uh oh!

mackings Oct 5, 2026

Uh oh!

b-luk Oct 5, 2026

Uh oh!

mackings Oct 9, 2026

Uh oh!

flutter-dashboard Bot commented Oct 13, 2026

Uh oh!

b-luk left a comment

Uh oh!

b-luk Oct 13, 2026

Uh oh!

mackings Oct 15, 2026

Uh oh!

mackings commented Oct 15, 2026

Uh oh!

b-luk left a comment

Uh oh!

gaaclarke left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

mackings commented Oct 5, 2026

Pre-Review Checklist

Footnotes

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Oct 5, 2026

Choose a reason for hiding this comment

Uh oh!

mackings Oct 5, 2026

Choose a reason for hiding this comment

Uh oh!

b-luk Oct 5, 2026

Choose a reason for hiding this comment

Uh oh!

mackings Oct 9, 2026

Choose a reason for hiding this comment

Uh oh!

flutter-dashboard Bot commented Oct 13, 2026

Uh oh!

b-luk left a comment

Choose a reason for hiding this comment

Uh oh!

b-luk Oct 13, 2026

Choose a reason for hiding this comment

Uh oh!

mackings Oct 15, 2026

Choose a reason for hiding this comment

Uh oh!

mackings commented Oct 15, 2026

Uh oh!

b-luk left a comment

Choose a reason for hiding this comment

Uh oh!

gaaclarke left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants