Aspect ratio of saved image doesn't match setTargetResolution when locking display orientation and setting target rotation with OrientationEventListener [148763432]

Fixed

Bug

Status Update

No update yet.

Description

ch...@google.com

created issue #1

Feb 3, 2020 08:46AM

Please note: This component is for the CameraX API used in Jetpack. Please DO NOT file Pixel Camera issues here.

Please describe your issue and include details such as the version of CameraX you are using and any relevant logs related to your issue.

# If at all possible, capture an Android logcat (

https://developer.android.com/studio/command-line/logcat) when you're experiencing the issue, preferably while
# the camera is still active.

CAMERAX VERSION (ex - 1.0.0-alpha07)
1.0.0-alpha09

CAMERA APPLICATION NAME AND VERSION: (Settings > Apps > (app name) > version)
N/A

ANDROID OS BUILD NUMBER: (Settings > About > Build number)
N/A

DEVICE NAME: (Nexus 5X, Samsung S6, etc)
N/A

DESCRIPTION:
Aspect ratio of saved image doesn't match setTargetResolution when locking display orientation and setting target rotation with OrientationEventListener. What is the correct way to set target resolution when display orientation is locked?

STEPS TO REPRODUCE:
1. Lock application in portrait mode.
2. Create ImageCapture use case with setTargetResolution(Size(1080, 1920)).
3. Use OrientationEventListener to set the target rotation.
4. Take a picture in landscape orientation.

OBSERVED RESULTS:
The saved image is 608x1080.

EXPECTED RESULTS:
The saved image should be 1920x1080.

REPRODUCIBILITY: (5 of out 5, 1 out of 100, etc)
5 of out 5

ADDITIONAL INFORMATION:
N/A

CODE FRAGMENTS (this will help us troubleshoot your issues):
<activity
android:name=".camera.camerax.CameraActivity"
android:configChanges="keyboardHidden|orientation|screenSize"
android:screenOrientation="portrait" />

private fun createCaptureUseCase(): ImageCapture {
return ImageCapture.Builder()
.setTargetName("Capture")
.setTargetResolution(Size(1080, 1920))
.setCaptureMode(ImageCapture.CAPTURE_MODE_MINIMIZE_LATENCY)
.build()
}

Comments

ch...@google.com <ch...@google.com> #2Feb 6, 2020 03:04AM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit c60f33e229e31ab328ef6b59dab63b264954831c
Author: Alexandre Elias <aelias@google.com>
Date: Fri Jul 10 16:23:09 2020

Semantics no-op cleanups

Partly in response to lmr's broad code review, I did a pass of
superficial API/implementation cleanup. The main changes are:

- I changed each Boolean SemanticsProperty where false is equivalent to
not being present to take "Unit" instead. This is conceptually
clearer: it avoids questions like "can I cancel out a semantics from a
merged child by setting it to false?" Because "property = Unit" looks
weird, I also changed the style of these to "property()".

- I moved the Semantics id generator closer to where it's used, in
SemanticsModifierCore. I made it internal and an AtomicInt.
(Note that integer ids are heavily used in the Android
AccessibilityNodeInfo APIs so I can't simply remove them entirely.)

- I deleted dead code. Some examples include SemanticsHintOverrides,
a public API not connected to anything, and SemanticsPropertyKey
merge() open method which is never called. (In both cases I have
a different plan in mind for accessibility.)

Fixes: 145951226
Fixes: 145955412
Test: existing tests
Relnote: "Single-value semantics properties now use a calling style.
For example, 'semantics { hidden = true }' is now written as:
'semantics { hidden() }'."

Change-Id: Ic1afd12ea22c926babc9662f1804d80b33aa0cfc

M ui/integration-tests/benchmark/src/androidTest/java/androidx/ui/benchmark/test/LayoutNodeModifierBenchmark.kt
M ui/ui-core/api/0.1.0-dev15.txt
M ui/ui-core/api/current.txt
M ui/ui-core/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-core/api/public_plus_experimental_current.txt
M ui/ui-core/api/restricted_0.1.0-dev15.txt
M ui/ui-core/api/restricted_current.txt
M ui/ui-core/src/androidAndroidTest/kotlin/androidx/ui/graphics/vector/VectorTest.kt
M ui/ui-core/src/androidAndroidTest/kotlin/androidx/ui/semantics/SemanticsTests.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidActuals.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidComposeView.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidComposeViewAccessibilityDelegateCompat.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidPopup.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/Expect.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsConfiguration.kt
D ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsHintOverrides.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsModifier.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsNode.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsOwner.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsWrapper.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/semantics/SemanticsProperties.kt
M ui/ui-foundation/api/0.1.0-dev15.txt
M ui/ui-foundation/api/current.txt
M ui/ui-foundation/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-foundation/api/public_plus_experimental_current.txt
M ui/ui-foundation/api/restricted_0.1.0-dev15.txt
M ui/ui-foundation/api/restricted_current.txt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Clickable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Dialog.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Scroller.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/selection/Selectable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/selection/Toggleable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/semantics/FoundationSemanticsProperties.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ButtonTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/CardTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/CheckboxScreenshotTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/RadioButtonScreenshotTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ScaffoldTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/SnackbarTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/SurfaceTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ripple/RippleIndicationTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/textfield/TextFieldScreenshotTest.kt
M ui/ui-material/src/main/java/androidx/ui/material/AppBar.kt
M ui/ui-material/src/main/java/androidx/ui/material/TextFieldImpl.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/AssertsTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/CallSemanticsActionTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/ErrorMessagesTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/FindersTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/PrintToStringTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/ScrollToTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/TextActionsTest.kt
M ui/ui-test/src/main/java/androidx/ui/test/Actions.kt
M ui/ui-test/src/main/java/androidx/ui/test/Filters.kt
M ui/ui-text/api/0.1.0-dev15.txt
M ui/ui-text/api/current.txt
M ui/ui-text/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-text/api/public_plus_experimental_current.txt
M ui/ui-text/api/restricted_0.1.0-dev15.txt
M ui/ui-text/api/restricted_current.txt
M ui/ui-text/src/commonMain/kotlin/androidx/ui/text/CoreTextField.kt
M ui/ui-text/src/commonMain/kotlin/androidx/ui/text/TextSemanticsProperties.kt

https://android-review.googlesource.com/1360099

ch...@google.com <ch...@google.com> Feb 12, 2020 08:08AM

Accepted by ch...@google.com.

ap...@google.com <ap...@google.com> #3Feb 19, 2020 05:17PM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit 1c0b007d341921698823749e290b49cff0f179d6
Author: charcoalchen <charcoalchen@google.com>
Date: Fri Feb 07 17:37:41 2020

Updates target Resolution/AspectRatioCustom to match target rotation

- Corresponding targetResolution and targetApectRatioCustom values will
be automatically updated when targetRotation is updated after the use
case is created.
- Add more detailed javadoc descriptions to setTargetResolution,
setTargetRotation and setTargetAspectRatioCustom to explain what will
be affected when setting these values.
- Remove mUseCaseConfigBuilder since keeping it there will cause unused
ErrorProne issue

Relnote: "Fix the issue that aspect ratio of saved image is incorrect
after changing target rotation value by ImageCapture#setTargetRotation."

Bug:

b/148763432

Test: manual test and ./gradlew bOS
Change-Id: I474ea093775c723930b94cdfbdf2d4f0d93b3612

M camera/camera-camera2/src/androidTest/java/androidx/camera/camera2/ImageAnalysisTest.java
M camera/camera-camera2/src/androidTest/java/androidx/camera/camera2/ImageCaptureTest.java
M camera/camera-camera2/src/test/java/androidx/camera/camera2/internal/SupportedSurfaceCombinationTest.java
M camera/camera-core/src/main/java/androidx/camera/core/ImageAnalysis.java
M camera/camera-core/src/main/java/androidx/camera/core/ImageCapture.java
M camera/camera-core/src/main/java/androidx/camera/core/UseCase.java
M camera/camera-core/src/main/java/androidx/camera/core/VideoCapture.java
A camera/camera-core/src/main/java/androidx/camera/core/internal/utils/UseCaseConfigUtil.java
A camera/camera-core/src/main/java/androidx/camera/core/internal/utils/package-info.java

https://android-review.googlesource.com/1229540

ap...@google.com <ap...@google.com> #4Feb 19, 2020 10:13PM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit 7713a197bd9f99c1120b340ff5b797e2a3fa0f2c
Author: charcoalchen <charcoalchen@google.com>
Date: Wed Feb 19 18:38:31 2020

Change setTargetAspectRatioCustom API name to setCropAspectRatio

ImageCapture#setTargetAspectRatioCustom is public API and used for
cropping output image. New API name makes it easier to understand for
the developers.

Bug:

b/148763432

Test: manual test and ./gradlew bOS
Change-Id: I15a606c6b4e5ebbb57d5aa67ac10617d4df3165a

M camera/camera-camera2/src/androidTest/java/androidx/camera/camera2/ImageCaptureTest.java
M camera/camera-core/api/1.0.0-beta01.txt
M camera/camera-core/api/current.txt
M camera/camera-core/api/public_plus_experimental_1.0.0-beta01.txt
M camera/camera-core/api/public_plus_experimental_current.txt
M camera/camera-core/api/restricted_1.0.0-beta01.txt
M camera/camera-core/api/restricted_current.txt
M camera/camera-core/src/main/java/androidx/camera/core/ImageCapture.java
M camera/camera-view/src/main/java/androidx/camera/view/CameraXModule.java

https://android-review.googlesource.com/1239310

ch...@google.com <ch...@google.com> #5Feb 20, 2020 02:53AM

Marked as fixed.

The issue will happen in the following three general use cases.

Case 1. Create ImageCapture with target resolution → update target rotation → get wrong output image
Case 2. Create ImageCapture without target resolution → update target rotation → get wrong output image
Case 3. Create ImageCapture with AspectRatio.RATIO_16_9 → update target rotation → get wrong 4:3 output image

Root cause for each case:
For 1, setTargetResolution will also call setTargetAspectRatioCustom but it isn’t updated to match the new target rotation.
For 2, there is a default 4:3 custom aspect ratio setting but it isn’t updated to match the new target rotation.
For 3, there is a default 4:3 custom aspect ratio setting to crop output image even developers have set target aspect ratio as 16:9.

Solution:
Automatically updating corresponding target resolution and aspect ratio custom values when target rotation value is updated. Don’t set the default custom aspect ratio setting when there is target aspect ratio set by the developers.

Issue 148763432