[camerax] Re-land SurfaceProducer migration with fix for camera preview rotation #6856

camsim99 · 2024-06-03T17:29:58Z

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

Switches on 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. This logic was added in a previous PR (#7044); this simply enables it for devices where Surfaces are not backed by a SurfaceTexture (see how this is determined).

Fixes flutter/flutter#149294.

Pre-launch Checklist

camsim99 · 2024-06-06T20:17:31Z

packages/camera/camera_android_camerax/lib/src/android_camera_camerax.dart

@@ -10,7 +10,7 @@ import 'package:camera_platform_interface/camera_platform_interface.dart';
 import 'package:flutter/services.dart'
    show DeviceOrientation, PlatformException;
 import 'package:flutter/widgets.dart'
-    show Size, Texture, Widget, visibleForTesting;
+    show RotatedBox, Size, Texture, Widget, visibleForTesting;


I used RotatedBox to rotate the preview mostly because that is what is used in the plugin code, but if there is a better way to rotate it, let me know.

Looks reasonable to me. @jonahwilliams?

packages/camera/camera_android_camerax/test/android_camera_camerax_test.dart

camsim99 · 2024-06-06T21:12:17Z

@chinmaygarde if you get the chance to test this out and/or bring the landscape device to SVL next week I would love to see how it behaves with this change!

Other than that fix (I left TODOs in the code), this PR can start going through code review :)

camsim99 · 2024-07-02T19:17:58Z

As discussed offline with @matanlurey, going to land the fixes to the camera preview rotation without Impeller support in #7044 first and land this (which will essentially only include re-lading Impeller support) later on.

…evices and set up fix for Impeller support (#7044) Partially lands #6856. This PR specifically includes: #### 1: 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. 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.

matanlurey · 2024-08-13T20:34:53Z

We need to add the lifecycle methods to correctly suspend/resume the camera surface:
https://docs.flutter.dev/release/breaking-changes/android-surface-plugins#migration-guide

(Unless I missed something and that's being handled elsewhere?)

camsim99 · 2024-08-14T17:52:31Z

We need to add the lifecycle methods to correctly suspend/resume the camera surface: https://docs.flutter.dev/release/breaking-changes/android-surface-plugins#migration-guide

(Unless I missed something and that's being handled elsewhere?)

I assume that the preview surface provider already handles that, but let me know if that doesn't cover everything!

matanlurey · 2024-08-15T18:25:01Z

Despite the names those are indeed different things

camsim99 · 2024-08-15T18:28:38Z

I just meant that the method will be called when a new surface is needed which causes CameraX to detach the surface from the camera and return a new surface (see our impl), effectively doing the same thing that doc describes, so I don't think there's anything else to do here.

camsim99 · 2024-08-15T18:32:20Z

To double check that this method is called if a surface gets destroyed, though, I can follow up internally :)

matanlurey · 2024-08-15T18:33:42Z

I see, perhaps you're right - if it works it works!

camsim99 · 2024-08-15T20:42:36Z

Followed up with the CameraX team and found out that we need to call SurfaceRequest.invalidate() (docs) in order for a new request for a surface to be called and thus, for the Preview.SurfaceProvider callback to handle creating a new surface.

This means we either have to save the requests we receive to invalidate it in the SurfaceProducer callback or we create a new SurfaceProvider instance every time a request for a surface comes in. I assume the latter is discouraged, can you confirm that @matanlurey ? edit: JK I think I found a middle ground :)

auto-submit · 2024-08-19T17:28:25Z

auto label is removed for flutter/packages/6856, due to - The status or check suite Mac_arm64 custom_package_tests master has failed. Please fix the issues identified (or deflake) before re-applying this label.

…ra preview rotation (flutter/packages#6856)

flutter/packages@7c1a05c...4d2d2e3 2024-08-20 [email protected] [webview] Add macOS support in implementation package (flutter/packages#6221) 2024-08-19 [email protected] Roll Flutter from a0c0453 to 6a28048 (22 revisions) (flutter/packages#7449) 2024-08-19 [email protected] [ci] Remove scorecard action (flutter/packages#7450) 2024-08-19 [email protected] [camerax] Re-land SurfaceProducer migration with fix for camera preview rotation (flutter/packages#6856) 2024-08-19 49699333+dependabot[bot]@users.noreply.github.com [camera]: Bump com.google.guava:guava from 33.2.1-android to 33.3.0-android in /packages/camera/camera_android_camerax/android (flutter/packages#7441) 2024-08-19 49699333+dependabot[bot]@users.noreply.github.com [sign_in]: Bump com.google.guava:guava from 33.2.1-android to 33.3.0-android in /packages/google_sign_in/google_sign_in_android/android (flutter/packages#7442) 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

@navaronbracke

_Description of what this PR is changing or adding, and why:_ Adds a note to the Impeller Android plugins migration to clarify that plugin authors that create a camera preview may additionally need to correct the rotation of the preview due to rotation information of produced `Surface`s being lost. _Issues fixed by this PR (if any):_ N/A _PRs or commits this PR depends on (if any):_ Should land after flutter/packages#6856 as this is mentioned as an example. cc @navaronbracke ## 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. --------- Co-authored-by: Parker Lougheed <[email protected]> Co-authored-by: Shams Zakhour (ignore Sfshaza) <[email protected]>

flutter/packages@7c1a05c...4d2d2e3 2024-08-20 [email protected] [webview] Add macOS support in implementation package (flutter/packages#6221) 2024-08-19 [email protected] Roll Flutter from a0c0453 to 6a28048 (22 revisions) (flutter/packages#7449) 2024-08-19 [email protected] [ci] Remove scorecard action (flutter/packages#7450) 2024-08-19 [email protected] [camerax] Re-land SurfaceProducer migration with fix for camera preview rotation (flutter/packages#6856) 2024-08-19 49699333+dependabot[bot]@users.noreply.github.com [camera]: Bump com.google.guava:guava from 33.2.1-android to 33.3.0-android in /packages/camera/camera_android_camerax/android (flutter/packages#7441) 2024-08-19 49699333+dependabot[bot]@users.noreply.github.com [sign_in]: Bump com.google.guava:guava from 33.2.1-android to 33.3.0-android in /packages/google_sign_in/google_sign_in_android/android (flutter/packages#7442) 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

Test 1 changes

200ac7f

github-actions bot added p: camera platform-android labels Jun 3, 2024

Check transformation info

8fe3230

stuartmorgan added the triage-android Should be looked at in Android triage label Jun 3, 2024

camsim99 added 3 commits June 3, 2024 15:51

Pixel fix

4fb6f74

change sensor orientation impl -- no change

b2fbc7d

Working state

754e0fd

camsim99 changed the title ~~[camerax] Fix camera preview to support Impeller~~ [camerax] Re-land SurfaceProducer migration with fixes for correcting camera preview Jun 4, 2024

camsim99 changed the title ~~[camerax] Re-land SurfaceProducer migration with fixes for correcting camera preview~~ [camerax] Re-land SurfaceProducer migration with fix for camera preview rotation Jun 4, 2024

camsim99 added 8 commits June 4, 2024 15:33

Corrections pt 1

cf7c4e6

Possible landscape tablet fix + working state for pixel, samsung

61b9631

Self-review, add tests, bump version

dfcfe32

Clean up plugin code

77ebbfa

Fix current tests

9e72c49

Add docs

ee6dbe9

Add most test coverage

2b1637e

Fix tests

0c8b230

camsim99 commented Jun 6, 2024

View reviewed changes

packages/camera/camera_android_camerax/test/android_camera_camerax_test.dart Outdated Show resolved Hide resolved

camsim99 added 4 commits June 6, 2024 13:38

Final review

c85942f

Add landscape fix guess

673344d

Update changelog with hopes and dreams

0de02e8

Merge remote-tracking branch 'upstream/main' into camx_preview_fix

989eea6

camsim99 marked this pull request as ready for review June 6, 2024 21:13

camsim99 requested review from chinmaygarde, matanlurey and jonahwilliams June 6, 2024 21:13

Rename var and change landscape guess

08fd90d

camsim99 marked this pull request as draft July 18, 2024 20:29

chinmaygarde mentioned this pull request Jul 30, 2024

[Impeller] AHB sampled textures use nearest sampling instead of linear. flutter/flutter#152579

Closed

camsim99 mentioned this pull request Aug 6, 2024

Add Impeller migration step for camera plugin authors flutter/website#11008

Merged

4 tasks

camsim99 added 2 commits August 13, 2024 11:28

Merge remote-tracking branch 'upstream/main' into camx_preview_fix

5e72187

Merge remote-tracking branch 'upstream/main' into camx_preview_fix

5c510a7

camsim99 requested a review from matanlurey August 13, 2024 18:44

camsim99 marked this pull request as ready for review August 13, 2024 18:44

Add surface producer callback

c91984e

matanlurey approved these changes Aug 16, 2024

View reviewed changes

version bump

a05fda8

camsim99 added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 19, 2024

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Aug 19, 2024

camsim99 added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 19, 2024

auto-submit bot merged commit 7740990 into flutter:main Aug 19, 2024
76 checks passed

engine-flutter-autoroll mentioned this pull request Aug 20, 2024

Roll Packages from 7c1a05c8b122 to 4d2d2e38a536 (6 revisions) flutter/flutter#153763

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 20, 2024

7740990 [camerax] Re-land SurfaceProducer migration with fix for came…

a7939e0

…ra preview rotation (flutter/packages#6856)

matanlurey mentioned this pull request Aug 23, 2024

Migrate rendering plugins to SurfaceProducer after stable release flutter/flutter#139702

Closed

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[camerax] Re-land SurfaceProducer migration with fix for camera preview rotation #6856

[camerax] Re-land SurfaceProducer migration with fix for camera preview rotation #6856

camsim99 commented Jun 3, 2024 •

edited

Loading

camsim99 Jun 6, 2024

matanlurey Jun 26, 2024

camsim99 commented Jun 6, 2024 •

edited

Loading

camsim99 commented Jul 2, 2024

matanlurey commented Aug 13, 2024

camsim99 commented Aug 14, 2024

matanlurey commented Aug 15, 2024

camsim99 commented Aug 15, 2024 •

edited

Loading

camsim99 commented Aug 15, 2024

matanlurey commented Aug 15, 2024

camsim99 commented Aug 15, 2024 •

edited

Loading

auto-submit bot commented Aug 19, 2024

[camerax] Re-land SurfaceProducer migration with fix for camera preview rotation #6856

[camerax] Re-land SurfaceProducer migration with fix for camera preview rotation #6856

Conversation

camsim99 commented Jun 3, 2024 • edited Loading

Switches on a fix for correctly rotating the camera preview

Pre-launch Checklist

camsim99 Jun 6, 2024

Choose a reason for hiding this comment

matanlurey Jun 26, 2024

Choose a reason for hiding this comment

camsim99 commented Jun 6, 2024 • edited Loading

camsim99 commented Jul 2, 2024

matanlurey commented Aug 13, 2024

camsim99 commented Aug 14, 2024

matanlurey commented Aug 15, 2024

camsim99 commented Aug 15, 2024 • edited Loading

camsim99 commented Aug 15, 2024

matanlurey commented Aug 15, 2024

camsim99 commented Aug 15, 2024 • edited Loading

auto-submit bot commented Aug 19, 2024

camsim99 commented Jun 3, 2024 •

edited

Loading

camsim99 commented Jun 6, 2024 •

edited

Loading

camsim99 commented Aug 15, 2024 •

edited

Loading

camsim99 commented Aug 15, 2024 •

edited

Loading