Fixed
Status Update
Comments
ma...@google.com <ma...@google.com> #2
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
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
le...@google.com <le...@google.com>
ap...@google.com <ap...@google.com> #3
Project: platform/frameworks/support
Branch: androidx-master-dev
commit 76ad175d0c681ddc6a1b86ff35b180f97049da0d
Author: leo huang <leohuang@google.com>
Date: Sat Jul 11 11:15:41 2020
Fix CameraControl.enableTorch is not working
The issue is caused by a race condition. Camera2CameraControl#setActive(true) is called on the main thread, but #setActive(false) is called on the camera thread. It will cause the control to be in a wrong state and not work properly.
The solution is to post all operations to camera thread to ensure the API calls are executed in the correct order.
But it must update the "LiveData" in advance on the main thread to maintain an existing behavior, i.e. user can immediately obtain the new LiveData value through continuous API calls on the main thread. However, trying to maintain this behavior will conflict with the above solution. There are 2 issues that need to be corrected to ensure the final LiveData value is correct.
(1) LiveData that was updated earlier on the main thread may reset by "setActive(false)" executed on the camera thread. This issue can be resolved by updating LiveData again in the posted camera operation since the tasks on the camera thread always have correct order.
(2) Before updating LiveData on the main thread, it cannot directly check the "active state" of CameraControl because the "active state" is only correct on the camera thread. The solution is to create a synchronized "use count" that can be queried on all threads. In this way, "use count" represents whether CameraControl accepts new requests, "active state" represents whether the requests are sent to Camera.
"Relnote: Fix the CameraControl unable to work by a race condition"
Bug: 152333890
Bug: 160928870
Bug: 160714166
Test: ./gradlew camera:camera-camera2:connectedAndroidTest; ./gradlew camera:camera-camera2:test
Change-Id: I2279f7bda1a9edf90af0c46b2db749d59821e0cc
M camera/camera-camera2/src/androidTest/java/androidx/camera/camera2/CameraControlDeviceTest.java
M camera/camera-camera2/src/androidTest/java/androidx/camera/camera2/internal/Camera2CameraControlTest.java
M camera/camera-camera2/src/androidTest/java/androidx/camera/camera2/internal/ZoomControlDeviceTest.java
M camera/camera-camera2/src/main/java/androidx/camera/camera2/internal/Camera2CameraControl.java
M camera/camera-camera2/src/main/java/androidx/camera/camera2/internal/Camera2CameraImpl.java
M camera/camera-camera2/src/main/java/androidx/camera/camera2/internal/FocusMeteringControl.java
M camera/camera-camera2/src/main/java/androidx/camera/camera2/internal/TorchControl.java
M camera/camera-camera2/src/main/java/androidx/camera/camera2/internal/ZoomControl.java
M camera/camera-camera2/src/test/java/androidx/camera/camera2/internal/TorchControlTest.java
M camera/camera-camera2/src/test/java/androidx/camera/camera2/internal/ZoomControlTest.java
https://android-review.googlesource.com/1345356
Branch: androidx-master-dev
commit 76ad175d0c681ddc6a1b86ff35b180f97049da0d
Author: leo huang <leohuang@google.com>
Date: Sat Jul 11 11:15:41 2020
Fix CameraControl.enableTorch is not working
The issue is caused by a race condition. Camera2CameraControl#setActive(true) is called on the main thread, but #setActive(false) is called on the camera thread. It will cause the control to be in a wrong state and not work properly.
The solution is to post all operations to camera thread to ensure the API calls are executed in the correct order.
But it must update the "LiveData" in advance on the main thread to maintain an existing behavior, i.e. user can immediately obtain the new LiveData value through continuous API calls on the main thread. However, trying to maintain this behavior will conflict with the above solution. There are 2 issues that need to be corrected to ensure the final LiveData value is correct.
(1) LiveData that was updated earlier on the main thread may reset by "setActive(false)" executed on the camera thread. This issue can be resolved by updating LiveData again in the posted camera operation since the tasks on the camera thread always have correct order.
(2) Before updating LiveData on the main thread, it cannot directly check the "active state" of CameraControl because the "active state" is only correct on the camera thread. The solution is to create a synchronized "use count" that can be queried on all threads. In this way, "use count" represents whether CameraControl accepts new requests, "active state" represents whether the requests are sent to Camera.
"Relnote: Fix the CameraControl unable to work by a race condition"
Bug: 152333890
Bug: 160928870
Bug: 160714166
Test: ./gradlew camera:camera-camera2:connectedAndroidTest; ./gradlew camera:camera-camera2:test
Change-Id: I2279f7bda1a9edf90af0c46b2db749d59821e0cc
M camera/camera-camera2/src/androidTest/java/androidx/camera/camera2/CameraControlDeviceTest.java
M camera/camera-camera2/src/androidTest/java/androidx/camera/camera2/internal/Camera2CameraControlTest.java
M camera/camera-camera2/src/androidTest/java/androidx/camera/camera2/internal/ZoomControlDeviceTest.java
M camera/camera-camera2/src/main/java/androidx/camera/camera2/internal/Camera2CameraControl.java
M camera/camera-camera2/src/main/java/androidx/camera/camera2/internal/Camera2CameraImpl.java
M camera/camera-camera2/src/main/java/androidx/camera/camera2/internal/FocusMeteringControl.java
M camera/camera-camera2/src/main/java/androidx/camera/camera2/internal/TorchControl.java
M camera/camera-camera2/src/main/java/androidx/camera/camera2/internal/ZoomControl.java
M camera/camera-camera2/src/test/java/androidx/camera/camera2/internal/TorchControlTest.java
M camera/camera-camera2/src/test/java/androidx/camera/camera2/internal/ZoomControlTest.java
Description
CAMERAX VERSION: 1.0.0-beta06 CameraView Version: 1.0.0-alpha13
ANDROID OS BUILD NUMBER: QQ3A.200605.001
DEVICE NAME: Pixel 4
DESCRIPTION:
mCameraControlInternal.setActive(true);
is called immediately upon receiving a request to attach use cases.On the other hand
mCameraControlInternal.setActive(false);
is called only once all use cases have been detached.This causes a race condition if the
CameraRepository
is set from Inactive to Active very quickly. The result of this race condition is that controls on the camera are disabled, but the camera use cases are successfully attached (ie. zoom functionality does not work).STEPS TO REPRODUCE:
I am able to reproduce this by using a custom lifecycle observer that registers the
ON_STOP
lifecycle event when media is captured and the preview is shown. Then I register theON_START
event when the media is dismissed.This is so that we do not continue to run the camera while the view finder is covered by our media preview.
OBSERVED RESULTS:
If a user captures media then dismisses it quickly enough, the race condition observed above can occur. This results in a state where the camera is functioning properly but controls are disabled.
REPRODUCIBILITY: 5 of 5
I have attached a screen shot where you can see that
tryDetachUseCases (773)
is called afterattachUseCases (648)
. The line numbers are slightly different from the Android source I link above but this is the effect I am describing,Camera2CameraControl.setActive(false)
is called afterCamera2CameraControl.setActive(true)
incorrectly.