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

fix: correct android preview orientation #1329

Conversation

navaronbracke
Copy link
Collaborator

@navaronbracke navaronbracke commented Feb 14, 2025

This PR adjusts the preview orientation for Android.

Previously the formula was not entirely correct.

Fixes #1319

@navaronbracke
Copy link
Collaborator Author

cc @@camsim99 I applied your latest patch changes in here to test it, too.

This now works correctly for devices that have handlesCropAndRotation set to false (so roughly API >= 30).
However, the preview is incorrectly rotated for devices that have handlesCropAndRotation set to true.
I did not see any changes between Flutter 3.27 and 3.29 when testing this.

Since the code path for handlesCropAndRotation set to true is equal to the old implementation (pre SurfaceProducer), I think that that behavior is a separate bug? With handlesCropAndRotation: true I would have expected the preview to always be rotated correctly automatically, but apparently that isn't the case.

I'll also need to test this change on a device that is naturally landscape oriented, I have not verified it there yet.

@camsim99
Copy link

Thanks for testing! What device are you testing on for handlesCropAndRotation is true? And is it an emulator? Either way, I agree; that would be a separate bug that is not camera-specific.

I don't know if naturally landscape oriented devices will work yet but if it does, that'd be a happy surprise, so let me know.

@navaronbracke
Copy link
Collaborator Author

navaronbracke commented Feb 14, 2025

I was testing on these devices

Android 10: https://www.gsmarena.com/motorola_one_(p30_play)-9310.php
Android 13: https://www.gsmarena.com/samsung_galaxy_xcover6_pro-11600.php

I did not test emulators, since I know that those might not be working properly yet.
I'll try to see if I can find a tablet to test on as well.

@navaronbracke
Copy link
Collaborator Author

navaronbracke commented Feb 14, 2025

Also, the Motorola had its Vulkan support fixed (Adreno 5xx GPU) in Flutter 3.29, but I doubt that that is related, as it still reports handlesCropAndRotation: true, which indicates it is using the old API under the hood.

@@ -621,7 +638,7 @@ class MobileScanner(
}
val paint = Paint().apply { colorFilter = ColorMatrixColorFilter(colorMatrix) }

val invertedBitmap = Bitmap.createBitmap(bitmap.width, bitmap.height, bitmap.config)
val invertedBitmap = Bitmap.createBitmap(bitmap.width, bitmap.height, bitmap.config!!)
Copy link
Collaborator Author

@navaronbracke navaronbracke Feb 17, 2025

Choose a reason for hiding this comment

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

This is a change that wasn't surfaced when using Kotlin 1.7, hence the bump to Kotlin 1.8. For some reason the compiler now checked for null specifically for this case (and probably others but I didn't see any changes). The bitmap config is provided when creating the original bitmap

@@ -19,7 +19,7 @@ dependencies:
flutter:
sdk: flutter

image_picker: ^1.0.4
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Bumping image picker (and updating the pubspec.lock locally) fixed a build issue with the removal of the Android v1 embedding.

@@ -191,7 +191,7 @@ class MobileScannerHandler(
"textureId" to it.id,
"size" to mapOf("width" to it.width, "height" to it.height),
"naturalDeviceOrientation" to it.naturalDeviceOrientation,
"isPreviewPreTransformed" to it.isPreviewPreTransformed,
"handlesCropAndRotation" to it.handlesCropAndRotation,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Renamed to match the actual name from the SurfaceProducer API.

@@ -11,3 +11,5 @@ GeneratedPluginRegistrant.java
key.properties
**/*.keystore
**/*.jks

**/.cxx
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I had to add this line, since Flutter 3.29 would add a .cxx folder when compiling on Android. New flutter created templates did include a change like this, too.

@@ -356,7 +365,6 @@ class MethodChannelMobileScanner extends MobileScannerPlatform {

_textureId = null;
_pausing = false;
_surfaceProducerDelegate?.dispose();
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The disposal of the "delegate" is no longer needed, since the stream subscription is moved to a widget.

@@ -319,9 +331,6 @@ class MethodChannelMobileScanner extends MobileScannerPlatform {
startResult,
cameraDirection,
);
_surfaceProducerDelegate?.startListeningToDeviceOrientation(
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is now handled by a separate widget.


/// This widget represents a camera preview that rotates itself,
/// based on changes in the device orientation.
final class RotatedPreview extends StatefulWidget {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We don't use a StreamBuilder directly, due to some unknown issue with it.
The authors of the camera_android_camerax plugin had the same problem of the preview not updating correctly with the new rotation.

@codecov-commenter
Copy link

codecov-commenter commented Feb 18, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (7947e61) to head (8586736).

Additional details and impacted files
@@            Coverage Diff            @@
##           develop     #1329   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           12        12           
  Lines          105       105           
=========================================
  Hits           105       105           
Flag Coverage Δ
unittests 100.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@navaronbracke navaronbracke marked this pull request as ready for review February 18, 2025 15:17
@juliansteenbakker juliansteenbakker merged commit 7f47cb2 into juliansteenbakker:develop Feb 18, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7.0.0-beta.6: Incorrect camera preview orientation
4 participants