[vector_graphics] Respect BoxFit parameter with viewbox by Gustl22 · Pull Request #11012 · flutter/packages

Gustl22 · 2026-02-12T22:17:59Z

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. ↩ ↩² ↩³

gemini-code-assist

Code Review

This pull request fixes an issue where BoxFit was not being respected for vector graphics with a viewBox. The change refactors the sizing logic in _VectorGraphicWidgetState to correctly use FittedBox. The PR also renames an internal variable for clarity and adds extensive new tests, including golden file tests, to verify the fix. The changes are logical and well-tested. I've included a few minor suggestions to align with Dart's style guide.

packages/vector_graphics/lib/src/vector_graphics.dart

Gustl22 · 2026-02-16T08:53:06Z

It seems, that the goldens fail, although they succeed locally on my Mac. As I am not sure, if we should keep them anyways, I wait until generating on Linux (as I need to set it up then).

Gustl22 · 2026-02-16T08:54:36Z

packages/vector_graphics/test/vector_graphics_test.dart

+    });
+  });
+
+  // TODO(gustl22): can be removed if redundant


Can remove goldens (if too costly for CI), but maybe is a good thing to have them in place (more intuitive to see, what's going on).

Gustl22 · 2026-03-04T17:14:29Z

@jtmcdole you are assigned as reviewer. Is it feasable to triage or delegate reviewing this PR? Thanks for your time.

jtmcdole · 2026-03-06T18:03:57Z

@jtmcdole you are assigned as reviewer. Is it feasable to triage or delegate reviewing this PR? Thanks for your time.

I must have been auto-assigend for owners reasons. When I see failing tests, I usually assume the PR is still in flux. Any chance you can address the failures first?

Gustl22 · 2026-03-06T18:17:45Z

@jtmcdole thank you for getting back. As I wrote here, it would be good to know, if goldens are actually desired in this case:
#11012 (comment)
I additionally wrote tests which work without goldens and they pass.
The goldens I wrote also pass locally on mac, but fail on CI probably due to different plaform rendering on Linux.

# Conflicts: # packages/vector_graphics/CHANGELOG.md

gaaclarke · 2026-03-09T22:09:27Z

If you are having a hard time creating a golden that passes on macos and linux, can't you just add 2 different goldens, one for each platform?

Gustl22 · 2026-03-10T07:17:08Z

@gaaclarke thank you for your proposal, I will probably remove my golden tests, as they still fail on CI even if rendered on Linux locally.
I would have liked the goldens as one can better see the transfomations. But they also don't exist for testing regular Boxfit images (maybe for that reason of conflicting rendering output).
I still test the boundaries already, though:
https://github.com/flutter/packages/pull/11012/changes#diff-6439a450e367819d9c9b7e7dd52cf4e8932ae767afd5850ef477b58d4bc65dd5R208

I would have liked to get a comment on what is your preferred testing strategy:

testing via goldens only
- more intuitive to see the outcome
- probably costly to check
- increases repository size
- problems on rendering on different platforms
test by checking transformed boundaries only
- fast to check
- precise values one can proof via calculation
- cannot see the effect at first glance (via an image)
keep both tests separately
keep both but combine them each in one test case for boundary & golden testing together

…-vector-boxfit

Gustl22 · 2026-03-10T10:18:24Z

I removed the redundant golden tests for now. If not sufficient, we can think about the approach how to add them. All tests pass now.

gaaclarke

Code looks good and I ran the it locally with the reproduction code to verify it does fix the problem. Thanks!

jtmcdole

LFTM

…utter/packages#11012)

flutter/packages@ee460d6...ecace66 2026-03-11 srawlins@google.com [pigeon] Produce a helpful error for bad method return type (flutter/packages#11204) 2026-03-11 srawlins@google.com [pigeon] Use hasLength and isEmpty in tests for better failure messages (flutter/packages#11205) 2026-03-11 dacoharkes@google.com [rfw] Opt out of icon tree shaking (flutter/packages#11216) 2026-03-11 srawlins@google.com [pigeon] Tidy imports and "ignore" comments (flutter/packages#11149) 2026-03-11 49699333+dependabot[bot]@users.noreply.github.com [dependabot]: Bump org.jetbrains.kotlin:kotlin-bom from 2.2.21 to 2.3.10 in /packages/pigeon/platform_tests/test_plugin/android (flutter/packages#10984) 2026-03-11 srawlins@google.com [pigeon] Improve casting and nullability-handling in generated code (flutter/packages#11163) 2026-03-10 engine-flutter-autoroll@skia.org Roll Flutter from 2ec61af to 195ae7b (36 revisions) (flutter/packages#11222) 2026-03-10 git@reb0.org [vector_graphics] Respect BoxFit parameter with viewbox (flutter/packages#11012) 2026-03-10 stuartmorgan@google.com Add AI contribution guidelines to PR checklist (flutter/packages#11195) 2026-03-10 zezohassam@gmail.com [video_player] Optimize caption retrieval with binary search in VideoPlayerController (flutter/packages#8347) If this roll has caused a breakage, revert this CL and stop the roller 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

[vector_graphics] Respect BoxFit parameter with viewbox (closes #158496)

3bf1aec

Gustl22 requested a review from jtmcdole as a code owner February 12, 2026 22:17

github-actions bot added p: vector_graphics triage-engine Should be looked at in engine triage labels Feb 12, 2026

gemini-code-assist bot reviewed Feb 12, 2026

View reviewed changes

remove this param

baa096a

Gustl22 force-pushed the 158496-vector-boxfit branch from 81bbbe6 to baa096a Compare February 12, 2026 22:23

Gustl22 commented Feb 16, 2026

View reviewed changes

Gustl22 added 4 commits March 6, 2026 22:59

Merge remote-tracking branch 'upstream/main' into 158496-vector-boxfit

2c7fc41

# Conflicts: # packages/vector_graphics/CHANGELOG.md

Render on linux to make CI happy

e380e76

Render on linux to make CI happy - Episode 2

bcbbacd

Render on linux to make CI happy - Episode 3

c1ad9d4

Gustl22 added 2 commits March 10, 2026 10:35

Merge remote-tracking branch 'refs/remotes/upstream/main' into 158496…

67a3235

…-vector-boxfit

refactor: remove redundant golden tests

3be9dc8

gaaclarke approved these changes Mar 10, 2026

View reviewed changes

Gustl22 changed the title ~~[vector_graphics] Respect BoxFit parameter with viewbox (closes #158496)~~ [vector_graphics] Respect BoxFit parameter with viewbox Mar 10, 2026

jtmcdole approved these changes Mar 10, 2026

View reviewed changes

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

auto-submit bot merged commit a643267 into flutter:main Mar 10, 2026
81 checks passed

Gustl22 deleted the 158496-vector-boxfit branch March 11, 2026 10:34

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 11, 2026

a643267e6 [vector_graphics] Respect BoxFit parameter with viewbox (fl…

3b6279b

…utter/packages#11012)

engine-flutter-autoroll mentioned this pull request Mar 11, 2026

Roll Packages from ee460d6a01fe to 1ea3725c4396 (9 revisions) flutter/flutter#183516

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 11, 2026

a643267e6 [vector_graphics] Respect BoxFit parameter with viewbox (fl…

ed40929

…utter/packages#11012)

engine-flutter-autoroll mentioned this pull request Mar 11, 2026

Roll Packages from ee460d6a01fe to ecace66e92c2 (10 revisions) flutter/flutter#183517

Merged

Gaurav-CareMonitor mentioned this pull request Mar 16, 2026

[vector_graphics] SVG size regression in version 1.1.20 - SVGs render too big flutter/flutter#183725

Open

Conversation

Gustl22 commented Feb 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-Review Checklist

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Gustl22 commented Feb 16, 2026

Uh oh!

Gustl22 Feb 16, 2026

Choose a reason for hiding this comment

Uh oh!

Gustl22 commented Mar 4, 2026

Uh oh!

jtmcdole commented Mar 6, 2026

Uh oh!

Gustl22 commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gaaclarke commented Mar 9, 2026

Uh oh!

Gustl22 commented Mar 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Gustl22 commented Mar 10, 2026

Uh oh!

gaaclarke left a comment

Choose a reason for hiding this comment

Uh oh!

jtmcdole left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Gustl22 commented Feb 12, 2026 •

edited

Loading

Gustl22 commented Mar 6, 2026 •

edited

Loading

Gustl22 commented Mar 10, 2026 •

edited

Loading