Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[camera_avfoundation] 🐛 Fix inverted orientation strings #5261

Merged
merged 4 commits into from
Oct 31, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/camera/camera_avfoundation/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 0.9.13+7

* Fixes inverted orientation strings.

## 0.9.13+6

* Fixes incorrect use of `NSError` that could cause crashes on launch.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,11 @@ - (void)testOrientationNotifications {
expectedChannelOrientation:@"portraitUp"
cameraPlugin:cameraPlugin
messenger:mockMessenger];
[self rotate:UIDeviceOrientationLandscapeRight
[self rotate:UIDeviceOrientationLandscapeLeft
expectedChannelOrientation:@"landscapeLeft"
cameraPlugin:cameraPlugin
messenger:mockMessenger];
[self rotate:UIDeviceOrientationLandscapeLeft
[self rotate:UIDeviceOrientationLandscapeRight
expectedChannelOrientation:@"landscapeRight"
cameraPlugin:cameraPlugin
messenger:mockMessenger];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,9 @@ - (void)testFLTGetVideoFormatFromString {
- (void)testFLTGetUIDeviceOrientationForString {
XCTAssertEqual(UIDeviceOrientationPortraitUpsideDown,
FLTGetUIDeviceOrientationForString(@"portraitDown"));
XCTAssertEqual(UIDeviceOrientationLandscapeRight,
FLTGetUIDeviceOrientationForString(@"landscapeLeft"));
XCTAssertEqual(UIDeviceOrientationLandscapeLeft,
FLTGetUIDeviceOrientationForString(@"landscapeLeft"));
XCTAssertEqual(UIDeviceOrientationLandscapeRight,
FLTGetUIDeviceOrientationForString(@"landscapeRight"));
XCTAssertEqual(UIDeviceOrientationPortrait, FLTGetUIDeviceOrientationForString(@"portraitUp"));
XCTAssertEqual(UIDeviceOrientationUnknown, FLTGetUIDeviceOrientationForString(@"unknown"));
Expand All @@ -96,9 +96,9 @@ - (void)testFLTGetStringForUIDeviceOrientation {
XCTAssertEqualObjects(@"portraitDown",
FLTGetStringForUIDeviceOrientation(UIDeviceOrientationPortraitUpsideDown));
XCTAssertEqualObjects(@"landscapeLeft",
FLTGetStringForUIDeviceOrientation(UIDeviceOrientationLandscapeRight));
XCTAssertEqualObjects(@"landscapeRight",
FLTGetStringForUIDeviceOrientation(UIDeviceOrientationLandscapeLeft));
XCTAssertEqualObjects(@"landscapeRight",
FLTGetStringForUIDeviceOrientation(UIDeviceOrientationLandscapeRight));
XCTAssertEqualObjects(@"portraitUp",
FLTGetStringForUIDeviceOrientation(UIDeviceOrientationPortrait));
XCTAssertEqualObjects(@"portraitUp", FLTGetStringForUIDeviceOrientation(-1));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,9 @@ UIDeviceOrientation FLTGetUIDeviceOrientationForString(NSString *orientation) {
if ([orientation isEqualToString:@"portraitDown"]) {
return UIDeviceOrientationPortraitUpsideDown;
} else if ([orientation isEqualToString:@"landscapeLeft"]) {
return UIDeviceOrientationLandscapeRight;
} else if ([orientation isEqualToString:@"landscapeRight"]) {
return UIDeviceOrientationLandscapeLeft;
} else if ([orientation isEqualToString:@"landscapeRight"]) {
return UIDeviceOrientationLandscapeRight;
} else if ([orientation isEqualToString:@"portraitUp"]) {
return UIDeviceOrientationPortrait;
} else {
Expand All @@ -104,9 +104,9 @@ UIDeviceOrientation FLTGetUIDeviceOrientationForString(NSString *orientation) {
switch (orientation) {
case UIDeviceOrientationPortraitUpsideDown:
return @"portraitDown";
case UIDeviceOrientationLandscapeRight:
return @"landscapeLeft";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you dig into the history and see why we had this flipped before? was it just a mistake?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. This should be a mistake when learning the relation between AVCaptureVideoOrientation, UIInterfaceOrientation, and UIDeviceOrientation.

AVCaptureVideoOrientation should be flipped with the corresponding UIDeviceOrientation by applying the affine transform, which typically means:

  • AVCaptureVideoOrientationLandscapeLeft <=> UIDeviceOrientationLandscapeRight
  • AVCaptureVideoOrientationLandscapeRight <=> UIDeviceOrientationLandscapeLeft

But the previous PR over-handled the implementation by replacing those string results too, which makes it a mistake. The value corresponding to orientation strings should always be as-is.

See also: https://stackoverflow.com/questions/7845520/why-does-avcapturevideoorientation-landscape-modes-result-in-upside-down-still-i

case UIDeviceOrientationLandscapeLeft:
return @"landscapeLeft";
case UIDeviceOrientationLandscapeRight:
return @"landscapeRight";
case UIDeviceOrientationPortrait:
default:
Expand Down
2 changes: 1 addition & 1 deletion packages/camera/camera_avfoundation/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ name: camera_avfoundation
description: iOS implementation of the camera plugin.
repository: https://github.com/flutter/packages/tree/main/packages/camera/camera_avfoundation
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+camera%22
version: 0.9.13+6
version: 0.9.13+7

environment:
sdk: ">=2.19.0 <4.0.0"
Expand Down