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

[camerax] Add fix for camera preview rotation on landscape-oriented devices and set up fix for Impeller support #7044

Merged
merged 25 commits into from
Jul 5, 2024

Conversation

camsim99
Copy link
Contributor

@camsim99 camsim99 commented Jul 2, 2024

Partially lands #6856. This PR specifically includes:

1: A fix for correctly rotating the camera preview

This fix is required for Surfaces not backed by a SurfaceTexture because they don't contain the transformation information needed to correctly rotate the camera preview. In that case, we use the logic described in https://developer.android.com/media/camera/camera2/camera-preview#orientation_calculation.

The fix is not currently used (the logic is not reachable) as Impeller support has not been added back to the plugin, but has been tested in #6856 and will be turned on when flutter/flutter#149294 is fixed.

Part of flutter/flutter#149294.

2: A fix for correctly rotating the camera preview on naturally landscape-oriented devices

I believe this issue was caused because we assume that the natural orientation of the device is portrait up. We fix this here by adding an extra rotation for the camera preview based on the natural orientation of the device.

Fixes flutter/flutter#149177.

Pre-launch Checklist

@camsim99 camsim99 marked this pull request as ready for review July 2, 2024 19:16
@camsim99 camsim99 requested a review from matanlurey as a code owner July 2, 2024 19:16
_subscriptionForDeviceOrientationChanges = onDeviceOrientationChanged()
.listen((DeviceOrientationChangedEvent event) {
currentDeviceOrientation = event.orientation;
});
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@matanlurey if you have a suggestion as to how to make these run in parallel, do let me know!

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah I didn't notice you already had Future.wait, honestly this is fine. LGTM.

_subscriptionForDeviceOrientationChanges = onDeviceOrientationChanged()
.listen((DeviceOrientationChangedEvent event) {
currentDeviceOrientation = event.orientation;
});
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah I didn't notice you already had Future.wait, honestly this is fine. LGTM.

@camsim99 camsim99 added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 5, 2024
@auto-submit auto-submit bot merged commit 97bad7e into flutter:main Jul 5, 2024
74 checks passed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 5, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 5, 2024
flutter/packages@754de19...97bad7e

2024-07-05 [email protected] [camerax] Add fix for camera preview rotation on landscape-oriented devices and set up fix for Impeller support (flutter/packages#7044)

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],[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
TahaTesser pushed a commit to TahaTesser/flutter that referenced this pull request Jul 8, 2024
flutter/packages@754de19...97bad7e

2024-07-05 [email protected] [camerax] Add fix for camera preview rotation on landscape-oriented devices and set up fix for Impeller support (flutter/packages#7044)

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],[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
victorsanni pushed a commit to victorsanni/flutter that referenced this pull request Jul 8, 2024
flutter/packages@754de19...97bad7e

2024-07-05 [email protected] [camerax] Add fix for camera preview rotation on landscape-oriented devices and set up fix for Impeller support (flutter/packages#7044)

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],[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
auto-submit bot pushed a commit that referenced this pull request Aug 19, 2024
…ew rotation (#6856)

Re-lands #6462 with the following change to `buildPreview`:

#### Switches on a fix for correctly rotating the camera preview
This fix is required for `Surface`s not backed by a `SurfaceTexture` because they don't contain the transformation information needed to correctly rotate the camera preview. In that case, we use the logic described in https://developer.android.com/media/camera/camera2/camera-preview#orientation_calculation. This logic was added in a previous PR (#7044); this simply enables it for devices where `Surface`s are not backed by a `SurfaceTexture` (see [how this is determined](https://github.com/flutter/packages/blob/f118119cf830fe369b34a04ed63c9ed66c647985/packages/camera/camera_android_camerax/android/src/main/java/io/flutter/plugins/camerax/SystemServicesHostApiImpl.java#L107-L120)).

Fixes flutter/flutter#149294.
@AlexV525
Copy link
Member

AlexV525 commented Sep 5, 2024

@camsim99 Could we have a similar fix with the video_player_android?

@camsim99
Copy link
Contributor Author

camsim99 commented Sep 5, 2024

@camsim99 Could we have a similar fix with the video_player_android?

Can you file an issue for this? I'm assuming you're suggesting a fix for landscape-oriented devices because video_player_android received Impeller support in #6989.

@AlexV525
Copy link
Member

AlexV525 commented Sep 6, 2024

Can you file an issue for this? I'm assuming you're suggesting a fix for landscape-oriented devices because video_player_android received Impeller support in #6989.

flutter/flutter#154696

sfshaza2 pushed a commit to flutter/website that referenced this pull request Oct 11, 2024
The current URL `https://github.com/flutter/flutter/packages/pull/7044`
does not exist, it should be
`https://github.com/flutter/packages/pull/7044` instead

_Description of what this PR is changing or adding, and why:_

In page about the [new surface
provider](https://docs.flutter.dev/release/breaking-changes/android-surface-plugins):
The URL of `this camera_android_camerax PR`
https://github.com/flutter/flutter/packages/pull/7044 does not exist, it
should be flutter/packages#7044 instead.

_Issues fixed by this PR (if any):_

None

_PRs or commits this PR depends on (if any):_

None

## Presubmit checklist

- [x] This PR is marked as draft with an explanation if not meant to
land until a future stable release.
- [x] This PR doesn’t contain automatically generated corrections
(Grammarly or similar).
- [x] This PR follows the [Google Developer Documentation Style
Guidelines](https://developers.google.com/style) — for example, it
doesn’t use _i.e._ or _e.g._, and it avoids _I_ and _we_ (first person).
- [x] This PR uses [semantic line
breaks](https://github.com/dart-lang/site-shared/blob/main/doc/writing-for-dart-and-flutter-websites.md#semantic-line-breaks)
of 80 characters or fewer.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autosubmit Merge PR when tree becomes green via auto submit App p: camera platform-android
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CameraPreview not working correctly on Android devices with natural landscape orientation
3 participants