[video_player] Fix layout issue caused by `Transform.rotate` not affecting space calculation. #8685

fa0311 · 2025-02-24T15:57:39Z

This pull request resolves #159724.
The issue occurs because Transform.rotate is used when rotation is not 0. Since Transform does not affect space calculations, this leads to the issues.

This is related to #7846.

Pre-launch Checklist

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

Signed-off-by: ふぁ <[email protected]>

flutter-dashboard · 2025-02-24T15:57:46Z

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group.

stuartmorgan-g · 2025-02-25T20:47:27Z

Thanks for the contribution! You’ve checked boxes in the PR checklist above that are not reflected in this PR, so I’m assuming this is a work in progress and am marking it as a Draft. Please review the checklist, updating the PR as appropriate, and when the checklist reflects the state of the PR as posted please feel free to mark it as ready for review.

Signed-off-by: ふぁ <[email protected]>

fa0311 · 2025-02-26T13:04:03Z

@stuartmorgan

I have added tests to verify the fix:

Updated existing tests to check for RotatedBox instead of Transform.rotate.
Added a new test to ensure RotatedBox is not used when rotation is not a multiple of 90.

Signed-off-by: ふぁ <[email protected]>

stuartmorgan-g · 2025-03-07T21:20:32Z

packages/video_player/video_player/lib/video_player.dart

-          child: child,
-        );
+  Widget build(BuildContext context) {
+    if (rotation % 90 != 0 || rotation == 0) {


Why is silently ignoring any rotation value that's not a multiple of 90 correct? If there are actually cases where this happens, wouldn't silently ignoring them be even more broken than the current behavior?

@stuartmorgan
The rotation value in this context can only be 0, 90, 180, or 270 degrees, as most video formats (such as MP4 and MOV) store rotation metadata in 90-degree increments within the tkhd (Track Header) box.

For example, in the current Flutter ExoPlayer backend, any value outside this range is ignored:

packages/packages/video_player/video_player_android/android/src/main/java/io/flutter/plugins/videoplayer/texture/TextureExoPlayerEventListener.java

Lines 67 to 74 in 2d3b24e

try {

reportedRotationCorrection = RotationDegrees.fromDegrees(rotationCorrection);

} catch (IllegalArgumentException e) {

// Rotation correction other than 0, 90, 180, 270 reported by Format. Because this is

// unexpected we apply no rotation correction.

reportedRotationCorrection = RotationDegrees.ROTATE_0;

rotationCorrection = 0;

}

Since RotatedBox only supports quarter-turn rotations, this shouldn't be an issue. However, if an unexpected rotation value appears, should we fall back to Transform.rotate instead of ignoring it?

If the expectation is that it's impossible for this Dart code to receive values that aren't multiples of 90 degrees, that should be expressed via an assert, not a runtime conditional.

I've added an assert to ensure that only multiples of 90 degrees are allowed.

The conditional statement should be updated to reflect that change as well.

I've updated the conditional statement to reflect the assert change

Signed-off-by: ふぁ <[email protected]>

stuartmorgan-g · 2025-03-20T13:51:09Z

packages/video_player/video_player/CHANGELOG.md

@@ -1,6 +1,7 @@
 ## 2.9.4

 * Reduces the position update interval from 500ms to 100ms.
+* Fix layout issue caused by Transform.rotate not affecting space calculation. 


Fixes

See the CHANGELOG style guide linked from the PR checklist.

stuartmorgan-g · 2025-03-20T13:51:47Z

packages/video_player/video_player/CHANGELOG.md

@@ -1,6 +1,7 @@
 ## 2.9.4


This needs a new version bump; the previous one was lost in a merge.

The issue has been fixed. A new version bump has been added.

The package's actual version also needs to be bumped.

https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8719859349019297185/+/u/Run_package_tests/CHANGELOG_and_version_validation/stdout

stuartmorgan-g · 2025-03-20T13:52:53Z

packages/video_player/video_player/lib/video_player.dart

+      return child;
+    }
+    if (rotation % 90 != 0) {
+      throw ArgumentError('Rotation must be a multiple of 90');


An argument error should be thrown at the point where it is passed as an argument, which is the constructor on line 884.

The issue has been fixed.

Signed-off-by: ふぁ <[email protected]>

stuartmorgan-g

LGTM once the version is fixed.

stuartmorgan-g · 2025-03-25T19:29:02Z

packages/video_player/video_player/CHANGELOG.md

@@ -1,6 +1,7 @@
 ## 2.9.4


The package's actual version also needs to be bumped.

https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8719859349019297185/+/u/Run_package_tests/CHANGELOG_and_version_validation/stdout

tarrinneal

lgtm after pubspec version bump.

…` not affecting space calculation. (flutter/packages#8685)

flutter/packages@125c117...07496eb 2025-04-02 [email protected] [multicast_dns] MDnsClient::listen supports onError callback (flutter/packages#8888) 2025-04-02 [email protected] Upgrade tests to use Xcode 16 and iOS 18 (flutter/packages#8968) 2025-04-01 [email protected] Manual roll Flutter from 1d954f4 to 05b5e79 (225 revisions) (flutter/packages#8960) 2025-04-01 [email protected] Drop deprecated HTML head meta tags (flutter/packages#8970) 2025-04-01 [email protected] Adjust PR checklist formatter discussion (flutter/packages#8924) 2025-04-01 [email protected] Update CODEOWNERS username (flutter/packages#8933) 2025-04-01 [email protected] [flutter_markdown] Added sizedImageBuilder to Markdown widget (flutter/packages#6739) 2025-04-01 [email protected] [google_maps_flutter] Skip impl copy of iOS tests (flutter/packages#8975) 2025-04-01 [email protected] [google_maps_flutter] Skip more hanging iOS tests (flutter/packages#8967) 2025-04-01 [email protected] [url_launcher] When not fully loaded, clicking close causes the callback to not be triggered correctly. (flutter/packages#8582) 2025-04-01 [email protected] [camera_avfoundation] Test utils and mocks swift migration - part 2 (flutter/packages#8892) 2025-03-31 [email protected] [webview_flutter] Skip flaky legacy tests on iOS (flutter/packages#8911) 2025-03-31 [email protected] [camera_android] Don't override default fps range when not recording (flutter/packages#8891) 2025-03-31 [email protected] [google_maps_flutter] Fix typo and remove duplicitous CHANGELOG entry (flutter/packages#8754) 2025-03-31 [email protected] [extension_gsi] Support the latest version of googleapis_auth (flutter/packages#8931) 2025-03-31 49699333+dependabot[bot]@users.noreply.github.com [dependabot]: Bump com.android.tools.build:gradle from 8.0.0 to 8.9.1 in /packages/pigeon/platform_tests/test_plugin/android (flutter/packages#8955) 2025-03-31 [email protected] [webview_flutter_wkwebview] Fixes crash when sending undefined message via javascript channel (flutter/packages#8776) 2025-03-31 49699333+dependabot[bot]@users.noreply.github.com [dependabot]: Bump com.android.tools.build:gradle from 8.0.0 to 8.9.1 in /packages/pigeon/platform_tests/alternate_language_test_plugin/android (flutter/packages#8946) 2025-03-31 [email protected] [interactive_media_ads] Fixes `AdEventType`s not triggering on iOS in release mode (flutter/packages#8918) 2025-03-31 [email protected] [google_maps_flutter] Skip test that hangs iOS CI (flutter/packages#8958) 2025-03-28 [email protected] Manual roll Flutter from b16430b to 1d954f4 (114 revisions) (flutter/packages#8922) 2025-03-28 [email protected] [camera_avfoundation] Test utils and mocks swift migration - part 3 (flutter/packages#8912) 2025-03-27 [email protected] Use a more deterministic way of waiting for ad widgets to be ready. (flutter/packages#8920) 2025-03-27 [email protected] [camera_avfoundation] Test utils and mocks swift migration - part 1 (flutter/packages#8890) 2025-03-27 [email protected] [pigeon] correct usage of extended generics in generator methods (flutter/packages#8910) 2025-03-27 [email protected] [video_player] Fix layout issue caused by `Transform.rotate` not affecting space calculation. (flutter/packages#8685) 2025-03-27 [email protected] [shared_preferences] Fix a late initialized error with the example app (flutter/packages#8540) 2025-03-26 [email protected] [various] Disable sandbox in Chrome dart tests (flutter/packages#8909) 2025-03-25 [email protected] [tool] Move changed file detection to base command class (flutter/packages#8730) 2025-03-25 [email protected] [Camera] Add lens type information (iOS) (flutter/packages#8723) 2025-03-25 [email protected] [pigeon] kotlin equality methods (flutter/packages#8887) 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 [email protected] 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 --------- Co-authored-by: Maurice Parrish <[email protected]>

…` not affecting space calculation. (flutter/packages#8685)

)

[video_player] using RotatedBox

00cdd54

Signed-off-by: ふぁ <[email protected]>

fa0311 requested a review from tarrinneal as a code owner February 24, 2025 15:57

github-actions bot added the p: video_player label Feb 24, 2025

stuartmorgan-g marked this pull request as draft February 25, 2025 20:47

stuartmorgan-g removed the request for review from tarrinneal February 25, 2025 20:47

fa0311 and others added 2 commits February 26, 2025 21:47

[video_player] update test

96b33d9

Signed-off-by: ふぁ <[email protected]>

Merge branch 'main' into use-rotated-box

9de259b

fa0311 marked this pull request as ready for review February 26, 2025 13:03

[video_player] update version

960f01c

Signed-off-by: ふぁ <[email protected]>

tarrinneal requested review from stuartmorgan-g and tarrinneal March 3, 2025 18:10

fa0311 mentioned this pull request Mar 6, 2025

[video_player_android] Flutter player widget does not scale correctly if video uses rotation correction flutter/flutter#159724

Closed

stuartmorgan-g reviewed Mar 7, 2025

View reviewed changes

fa0311 and others added 3 commits March 8, 2025 16:43

Merge branch 'main' into use-rotated-box

5d9b87d

[video_player] add assertion for rotation to be a multiple of 90

6be9577

Signed-off-by: ふぁ <[email protected]>

Merge branch 'main' into use-rotated-box

5a4ec4f

fa0311 requested a review from stuartmorgan-g March 13, 2025 17:15

fa0311 added 2 commits March 16, 2025 02:52

[video_player] fix test

4bea0c6

Signed-off-by: ふぁ <[email protected]>

[video_player] add assertion for rotation to be a multiple of 90

b11717b

Signed-off-by: ふぁ <[email protected]>

fa0311 force-pushed the use-rotated-box branch from d93c843 to b11717b Compare March 20, 2025 05:20

Merge branch 'main' into use-rotated-box

c5131a3

Signed-off-by: ふぁ <[email protected]>

stuartmorgan-g requested changes Mar 20, 2025

View reviewed changes

fa0311 added 2 commits March 21, 2025 02:08

[video_player] add assertion for rotation to be a multiple of 90

5ac9565

Signed-off-by: ふぁ <[email protected]>

[video_player] update CHANGLOG

74062cd

Signed-off-by: ふぁ <[email protected]>

fa0311 requested a review from stuartmorgan-g March 20, 2025 17:19

stuartmorgan-g approved these changes Mar 25, 2025

View reviewed changes

tarrinneal approved these changes Mar 25, 2025

View reviewed changes

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

e3d259d2a [video_player] Fix layout issue caused by `Transform.rotate…

b3d96b3

…` not affecting space calculation. (flutter/packages#8685)

engine-flutter-autoroll mentioned this pull request Mar 31, 2025

Roll Packages from 125c117a432f to c97691715ffb (19 revisions) flutter/flutter#166266

Closed

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

e3d259d2a [video_player] Fix layout issue caused by `Transform.rotate…

54edbae

…` not affecting space calculation. (flutter/packages#8685)

engine-flutter-autoroll mentioned this pull request Mar 31, 2025

Roll Packages from 125c117a432f to c97691715ffb (19 revisions) flutter/flutter#166280

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 1, 2025

e3d259d2a [video_player] Fix layout issue caused by `Transform.rotate…

750bcb5

…` not affecting space calculation. (flutter/packages#8685)

engine-flutter-autoroll mentioned this pull request Apr 1, 2025

Roll Packages from 125c117a432f to 0ef3d446fd44 (23 revisions) flutter/flutter#166341

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 1, 2025

e3d259d2a [video_player] Fix layout issue caused by `Transform.rotate…

eb9ce97

…` not affecting space calculation. (flutter/packages#8685)

engine-flutter-autoroll mentioned this pull request Apr 1, 2025

Roll Packages from 125c117a432f to 799a5908904c (24 revisions) flutter/flutter#166346

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 1, 2025

e3d259d2a [video_player] Fix layout issue caused by `Transform.rotate…

0178f20

…` not affecting space calculation. (flutter/packages#8685)

engine-flutter-autoroll mentioned this pull request Apr 1, 2025

Roll Packages from 125c117a432f to bba066f89dd4 (25 revisions) flutter/flutter#166349

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 1, 2025

e3d259d2a [video_player] Fix layout issue caused by `Transform.rotate…

3525f15

…` not affecting space calculation. (flutter/packages#8685)

engine-flutter-autoroll mentioned this pull request Apr 1, 2025

Roll Packages from 125c117a432f to 28676de193f2 (26 revisions) flutter/flutter#166352

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 1, 2025

e3d259d2a [video_player] Fix layout issue caused by `Transform.rotate…

5077fd8

…` not affecting space calculation. (flutter/packages#8685)

engine-flutter-autoroll mentioned this pull request Apr 1, 2025

Roll Packages from 125c117a432f to eb8cb324582e (27 revisions) flutter/flutter#166356

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 2, 2025

e3d259d2a [video_player] Fix layout issue caused by `Transform.rotate…

fd6767e

…` not affecting space calculation. (flutter/packages#8685)

engine-flutter-autoroll mentioned this pull request Apr 2, 2025

Roll Packages from 125c117a432f to 84ef201205ea (29 revisions) flutter/flutter#166435

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 2, 2025

e3d259d2a [video_player] Fix layout issue caused by `Transform.rotate…

ccb57f3

…` not affecting space calculation. (flutter/packages#8685)

engine-flutter-autoroll mentioned this pull request Apr 2, 2025

Roll Packages from 125c117a432f to 5cee1525909d (30 revisions) flutter/flutter#166456

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 2, 2025

e3d259d2a [video_player] Fix layout issue caused by `Transform.rotate…

9f05636

…` not affecting space calculation. (flutter/packages#8685)

engine-flutter-autoroll mentioned this pull request Apr 2, 2025

Roll Packages from 125c117a432f to 07496eb03035 (31 revisions) flutter/flutter#166457

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 2, 2025

e3d259d2a [video_player] Fix layout issue caused by `Transform.rotate…

892fd74

…` not affecting space calculation. (flutter/packages#8685)

engine-flutter-autoroll mentioned this pull request Apr 2, 2025

Roll Packages from 125c117a432f to 07496eb03035 (31 revisions) flutter/flutter#166469

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 2, 2025

e3d259d2a [video_player] Fix layout issue caused by `Transform.rotate…

b324acb

…` not affecting space calculation. (flutter/packages#8685)

engine-flutter-autoroll mentioned this pull request Apr 2, 2025

Roll Packages from 125c117a432f to 8f5844af8135 (32 revisions) flutter/flutter#166472

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 3, 2025

e3d259d2a [video_player] Fix layout issue caused by `Transform.rotate…

1bd7c79

…` not affecting space calculation. (flutter/packages#8685)

engine-flutter-autoroll mentioned this pull request Apr 3, 2025

Roll Packages from 125c117a432f to 8f5844af8135 (32 revisions) flutter/flutter#166534

Closed

marvin-kolja added a commit to marvin-kolja/packages that referenced this pull request May 4, 2025

fix(video_player_android): rotation layout issue (following flutter#8685

f579d5e

)

marvin-kolja mentioned this pull request May 5, 2025

[video_player_android] Fix incorrect dimensions swap #9199

Open

11 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[video_player] Fix layout issue caused by `Transform.rotate` not affecting space calculation. #8685

[video_player] Fix layout issue caused by `Transform.rotate` not affecting space calculation. #8685

fa0311 commented Feb 24, 2025 •

edited

Loading

flutter-dashboard bot commented Feb 24, 2025

stuartmorgan-g commented Feb 25, 2025

fa0311 commented Feb 26, 2025

stuartmorgan-g Mar 7, 2025

fa0311 Mar 8, 2025

stuartmorgan-g Mar 10, 2025

fa0311 Mar 13, 2025

tarrinneal Mar 17, 2025

fa0311 Mar 20, 2025

stuartmorgan-g Mar 20, 2025

stuartmorgan-g Mar 20, 2025

fa0311 Mar 20, 2025

stuartmorgan-g Mar 25, 2025

stuartmorgan-g Mar 20, 2025

fa0311 Mar 20, 2025

stuartmorgan-g left a comment

stuartmorgan-g Mar 25, 2025

tarrinneal left a comment

	try {
	reportedRotationCorrection = RotationDegrees.fromDegrees(rotationCorrection);
	} catch (IllegalArgumentException e) {
	// Rotation correction other than 0, 90, 180, 270 reported by Format. Because this is
	// unexpected we apply no rotation correction.
	reportedRotationCorrection = RotationDegrees.ROTATE_0;
	rotationCorrection = 0;
	}

[video_player] Fix layout issue caused by Transform.rotate not affecting space calculation. #8685

[video_player] Fix layout issue caused by Transform.rotate not affecting space calculation. #8685

Conversation

fa0311 commented Feb 24, 2025 • edited Loading

Pre-launch Checklist

flutter-dashboard bot commented Feb 24, 2025

stuartmorgan-g commented Feb 25, 2025

fa0311 commented Feb 26, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stuartmorgan-g left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tarrinneal left a comment

Choose a reason for hiding this comment

[video_player] Fix layout issue caused by `Transform.rotate` not affecting space calculation. #8685

[video_player] Fix layout issue caused by `Transform.rotate` not affecting space calculation. #8685

fa0311 commented Feb 24, 2025 •

edited

Loading