[camerax] Re-land SurfaceProducer migration with fix for camera preview rotation
Re-lands https://github.com/flutter/packages/pull/6462 with the following changes to buildPreview:
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.
Fixes https://github.com/flutter/flutter/issues/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 https://github.com/flutter/flutter/issues/149177.
Pre-launch Checklist
- [x] I read the Contributor Guide and followed the process outlined there for submitting PRs.
- [x] I read the Tree Hygiene page, which explains my responsibilities.
- [x] I read and followed the relevant style guides and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/packages repo does use
dart format.) - [x] I signed the CLA.
- [x] The title of the PR starts with the name of the package surrounded by square brackets, e.g.
[shared_preferences] - [x] I linked to at least one issue that this PR fixes in the description above.
- [x] I updated
pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes. - [x] I updated
CHANGELOG.mdto add a description of the change, following repository CHANGELOG style, or this PR is exempt from CHANGELOG changes. - [x] I updated/added relevant documentation (doc comments with
///). - [x] I added new tests to check the change I am making, or this PR is test-exempt.
- [x] All existing and new tests are passing.
@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 :)
From triage: @chinmaygarde Waiting on you to bring the tablet for testing the fix.
Oh, awesome. I can test it today and also bring the tablet down to SVL next week.
Just an update: This works on all the devices I have access to. Including the the landscape natural ones. But we are going to gather in SVL for some more manual testing. That didn't happen last week as I was OOO sick.
As discussed offline with @matanlurey, going to land the fixes to the camera preview rotation without Impeller support in https://github.com/flutter/packages/pull/7044 first and land this (which will essentially only include re-lading Impeller support) later on.
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?)
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!
Despite the names those are indeed different things
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.
To double check that this method is called if a surface gets destroyed, though, I can follow up internally :)
I see, perhaps you're right - if it works it works!
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 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.