Skip to content

[camera_android_camerax] Remove logic used to previously correct preview rotation #8256

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

Merged
merged 12 commits into from
Jan 9, 2025

Conversation

camsim99
Copy link
Contributor

@camsim99 camsim99 commented Dec 9, 2024

Removes correction logic used to the correct the rotation of the camera preview, since it appears* to not be needed anymore.

Partially addresses flutter/flutter#154241.

*This is not true for emulators, but if we are ok landing this issue for real devices, I'll file an issue for emulators and follow up there. jonahwilliams@ mentioned there is known different behavior in emulators with Impeller, so this fact is not totally a surprise.

My basis for landing this is:

  1. I tested the following devices that use both Impeller backends and this PR works:
  • Realme X2 Pro, Android 11 (API 30) that uses Vulkan
  • Pixel 3A, Android 12 (Android 31/32) that falls back to OpenGLES
  • Samsung Galaxy (Android 8.1.0) that uses OpenGLES
  1. If this misses an edge case (like a class of device), this PR will make it easier to debug what is going on, since the camera preview will now only introduce a rotation if the device itself is rotated (as per the logic in
    Widget _wrapInRotatedBox({required Widget child}) {
    if (kIsWeb || defaultTargetPlatform != TargetPlatform.android) {
    return child;
    }
    return RotatedBox(
    quarterTurns: _getQuarterTurns(),
    child: child,
    );
    }
    bool _isLandscape() {
    return <DeviceOrientation>[
    DeviceOrientation.landscapeLeft,
    DeviceOrientation.landscapeRight
    ].contains(_getApplicableOrientation());
    }
    int _getQuarterTurns() {
    final Map<DeviceOrientation, int> turns = <DeviceOrientation, int>{
    DeviceOrientation.portraitUp: 0,
    DeviceOrientation.landscapeRight: 1,
    DeviceOrientation.portraitDown: 2,
    DeviceOrientation.landscapeLeft: 3,
    };
    return turns[_getApplicableOrientation()]!;
    }
    ) with no additional logic concerning the sensor orientation and UI orientation as it does now.

Pre-launch Checklist

@camsim99 camsim99 changed the title [camera_android_camerax] Remove correction for preview rotation [camera_android_camerax] Correct preview rotation Dec 26, 2024
@camsim99
Copy link
Contributor Author

camsim99 commented Jan 7, 2025

Works for me as of today on the following devices I tested (and their Impeller logs):

  • Pixel 3A, Android 12 (Android 31/32)
    I/flutter (13294): [INFO:flutter/shell/platform/android/android_context_vk_impeller.cc(64)] Known bad Vulkan driver encountered, falling back to OpenGLES.
    I/flutter (13294): [IMPORTANT:flutter/shell/platform/android/android_context_gl_impeller.cc(94)] Using the Impeller rendering backend (OpenGLES).
  • Realme X2 Pro, Android 11 (API 30)
    I/flutter (10164): [IMPORTANT:flutter/shell/platform/android/android_context_vk_impeller.cc(60)] Using the Impeller rendering backend (Vulkan).
    Vulkan
  • Samsung Galaxy (Android 8.1.0)
    I/flutter (11694): [IMPORTANT:flutter/shell/platform/android/android_context_gl_impeller.cc(94)] Using the Impeller rendering backend (OpenGLES).

@camsim99 camsim99 marked this pull request as ready for review January 8, 2025 17:17
@camsim99 camsim99 requested review from matanlurey and a team January 8, 2025 17:17
@camsim99 camsim99 changed the title [camera_android_camerax] Correct preview rotation [camera_android_camerax] Remove logic used to previously correct preview rotation Jan 9, 2025
@camsim99 camsim99 added the autosubmit Merge PR when tree becomes green via auto submit App label Jan 9, 2025
@auto-submit auto-submit bot merged commit aad08ec into flutter:main Jan 9, 2025
77 checks passed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 10, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 10, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 13, 2025
github-merge-queue bot pushed a commit to flutter/flutter that referenced this pull request Jan 13, 2025
flutter/packages@6554751...3c3bc68

2025-01-11 49699333+dependabot[bot]@users.noreply.github.com
[dependabot]: Bump the test-dependencies group across 2 directories with
1 update (flutter/packages#8412)
2025-01-11 49699333+dependabot[bot]@users.noreply.github.com
[dependabot]: Bump org.json:json from 20241224 to 20250107 in
/packages/in_app_purchase/in_app_purchase_android/example/android/app
(flutter/packages#8411)
2025-01-11 [email protected] Roll Flutter from
4b23b81 to 864d4f5 (50 revisions) (flutter/packages#8408)
2025-01-10 49699333+dependabot[bot]@users.noreply.github.com
[dependabot]: Bump org.json:json from 20240303 to 20250107 in
/packages/in_app_purchase/in_app_purchase_android/android
(flutter/packages#8413)
2025-01-10 49699333+dependabot[bot]@users.noreply.github.com
[dependabot]: Bump org.json:json from 20241224 to 20250107 in
/packages/in_app_purchase/in_app_purchase/example/android/app
(flutter/packages#8410)
2025-01-10 49699333+dependabot[bot]@users.noreply.github.com
[dependabot]: Bump io.mockk:mockk from 1.13.13 to 1.13.14 in
/packages/pigeon/platform_tests/test_plugin/android
(flutter/packages#8357)
2025-01-10 [email protected] Fix dependabot test-dependencies group
io.mockk regex (flutter/packages#8406)
2025-01-10 49699333+dependabot[bot]@users.noreply.github.com
[shared_pref]: Bump androidx.datastore:datastore from 1.0.0 to 1.1.1 in
/packages/shared_preferences/shared_preferences_android/android
(flutter/packages#7306)
2025-01-10 [email protected] [url_launcher_windows] Correct logging url
(flutter/packages#8107)
2025-01-10 49699333+dependabot[bot]@users.noreply.github.com
[dependabot]: Bump io.mockk:mockk from 1.13.13 to 1.13.14 in
/packages/shared_preferences/shared_preferences_android/android
(flutter/packages#8358)
2025-01-10 49699333+dependabot[bot]@users.noreply.github.com
[dependabot]: Bump the androidx group across 3 directories with 1 update
(flutter/packages#8329)
2025-01-10 49699333+dependabot[bot]@users.noreply.github.com
[dependabot]: Bump org.json:json from 20240303 to 20241224 in
/packages/in_app_purchase/in_app_purchase_android/example/android/app
(flutter/packages#8372)
2025-01-10 [email protected] [url_launcher][web] Better support for
semantics in the Link widget (flutter/packages#6711)
2025-01-10 [email protected] [camera]:
Activate leak testing for sub packages (flutter/packages#8353)
2025-01-09 [email protected]
[camera_android_camerax] Remove logic used to previously correct preview
rotation (flutter/packages#8256)
2025-01-09 [email protected] [go_router]
Rephrases readme to better describe the current status
(flutter/packages#8403)

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
maheshj01 pushed a commit to maheshj01/flutter that referenced this pull request Jan 15, 2025
…er#161515)

flutter/packages@6554751...3c3bc68

2025-01-11 49699333+dependabot[bot]@users.noreply.github.com
[dependabot]: Bump the test-dependencies group across 2 directories with
1 update (flutter/packages#8412)
2025-01-11 49699333+dependabot[bot]@users.noreply.github.com
[dependabot]: Bump org.json:json from 20241224 to 20250107 in
/packages/in_app_purchase/in_app_purchase_android/example/android/app
(flutter/packages#8411)
2025-01-11 [email protected] Roll Flutter from
4b23b81 to 864d4f5 (50 revisions) (flutter/packages#8408)
2025-01-10 49699333+dependabot[bot]@users.noreply.github.com
[dependabot]: Bump org.json:json from 20240303 to 20250107 in
/packages/in_app_purchase/in_app_purchase_android/android
(flutter/packages#8413)
2025-01-10 49699333+dependabot[bot]@users.noreply.github.com
[dependabot]: Bump org.json:json from 20241224 to 20250107 in
/packages/in_app_purchase/in_app_purchase/example/android/app
(flutter/packages#8410)
2025-01-10 49699333+dependabot[bot]@users.noreply.github.com
[dependabot]: Bump io.mockk:mockk from 1.13.13 to 1.13.14 in
/packages/pigeon/platform_tests/test_plugin/android
(flutter/packages#8357)
2025-01-10 [email protected] Fix dependabot test-dependencies group
io.mockk regex (flutter/packages#8406)
2025-01-10 49699333+dependabot[bot]@users.noreply.github.com
[shared_pref]: Bump androidx.datastore:datastore from 1.0.0 to 1.1.1 in
/packages/shared_preferences/shared_preferences_android/android
(flutter/packages#7306)
2025-01-10 [email protected] [url_launcher_windows] Correct logging url
(flutter/packages#8107)
2025-01-10 49699333+dependabot[bot]@users.noreply.github.com
[dependabot]: Bump io.mockk:mockk from 1.13.13 to 1.13.14 in
/packages/shared_preferences/shared_preferences_android/android
(flutter/packages#8358)
2025-01-10 49699333+dependabot[bot]@users.noreply.github.com
[dependabot]: Bump the androidx group across 3 directories with 1 update
(flutter/packages#8329)
2025-01-10 49699333+dependabot[bot]@users.noreply.github.com
[dependabot]: Bump org.json:json from 20240303 to 20241224 in
/packages/in_app_purchase/in_app_purchase_android/example/android/app
(flutter/packages#8372)
2025-01-10 [email protected] [url_launcher][web] Better support for
semantics in the Link widget (flutter/packages#6711)
2025-01-10 [email protected] [camera]:
Activate leak testing for sub packages (flutter/packages#8353)
2025-01-09 [email protected]
[camera_android_camerax] Remove logic used to previously correct preview
rotation (flutter/packages#8256)
2025-01-09 [email protected] [go_router]
Rephrases readme to better describe the current status
(flutter/packages#8403)

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
@FrankenApps
Copy link

I fear this broke the live camera preview on some Android devices (where the preview is now distorted and rotated by 90°). For example see: khoren93/flutter_zxing#169

@camsim99
Copy link
Contributor Author

I fear this broke the live camera preview on some Android devices (where the preview is now distorted and rotated by 90°). For example see: khoren93/flutter_zxing#169

Preview rotation correction logic was added back in camera_android_camerax: 0.6.14; please try it out!

@FrankenApps
Copy link

Works like a charm. Thanks

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants