Status Update
Comments
ja...@gmail.com <ja...@gmail.com> #2
This is a particularly hard device to come by - do you happen to have access to the device? If so could you provide us with the output of: adb shell dumpsys media.camera > info.txt
Thanks!
ma...@google.com <ma...@google.com> #3
Stacktrace:
Caused by: java.lang.IllegalArgumentException: Can not get supported output size under supported maximum for the format: 34
at androidx.camera.camera2.internal.SupportedSurfaceCombination.getSupportedOutputSizes(SupportedSurfaceCombination.java:355)
at androidx.camera.camera2.internal.SupportedSurfaceCombination.getSuggestedResolutions(SupportedSurfaceCombination.java:197)
at androidx.camera.camera2.internal.Camera2DeviceSurfaceManager.getSuggestedResolutions(Camera2DeviceSurfaceManager.java:198)
at androidx.camera.core.CameraX.calculateSuggestedResolutions(CameraX.java:943)
at androidx.camera.core.CameraX.bindToLifecycle(CameraX.java:293)
at androidx.camera.lifecycle.ProcessCameraProvider.bindToLifecycle(ProcessCameraProvider.java:227)
Below are some findings based on our debugging
When Dex is connected
previewConfig.getMaxResolution() is returning "731x411" as maxSize.
Inside Preview.Builder.build() -> Default_MAX_resolution is set to "CameraX.getSurfaceManager().getPreviewSize()" which is 731x411
this is being picked as maxSize.
While rendering maxSize is 731x411 and minSize is 640x480 and below are available outputSizes
0 = {Size@11860} "4032x3024"
1 = {Size@11861} "3984x2988"
2 = {Size@11862} "4032x2268"
3 = {Size@11863} "3024x3024"
4 = {Size@11864} "2976x2976"
5 = {Size@11865} "3840x2160"
6 = {Size@11866} "3264x2448"
7 = {Size@11867} "4032x1960"
8 = {Size@11868} "2880x2160"
9 = {Size@11869} "3264x1836"
10 = {Size@11870} "2160x2160"
11 = {Size@11871} "2560x1440"
12 = {Size@11872} "2224x1080"
13 = {Size@11873} "2048x1152"
14 = {Size@11874} "1920x1080"
15 = {Size@11875} "1440x1080"
16 = {Size@11876} "1088x1088"
17 = {Size@11877} "1280x720"
18 = {Size@11878} "1024x768"
19 = {Size@11879} "1056x704"
20 = {Size@11880} "960x720"
21 = {Size@11881} "960x540"
22 = {Size@11882} "720x720"
23 = {Size@11883} "800x450"
24 = {Size@11884} "720x480"
25 = {Size@11885} "640x480"
26 = {Size@11886} "352x288"
27 = {Size@11887} "320x240"
28 = {Size@11888} "256x144"
29 = {Size@11889} "176x144"
and couldn't find any size in this range.
When Dex not connected
minsize = 640x480
maxsize = 1920x1080
0 = {Size@11836} "4032x3024"
1 = {Size@11837} "3984x2988"
2 = {Size@11838} "4032x2268"
3 = {Size@11839} "3024x3024"
4 = {Size@11840} "2976x2976"
5 = {Size@11841} "3840x2160"
6 = {Size@11842} "3264x2448"
7 = {Size@11843} "4032x1960"
8 = {Size@11844} "2880x2160"
9 = {Size@11845} "3264x1836"
10 = {Size@11846} "2160x2160"
11 = {Size@11847} "2560x1440"
12 = {Size@11848} "2224x1080"
13 = {Size@11849} "2048x1152"
14 = {Size@11850} "1920x1080"
15 = {Size@11851} "1440x1080"
16 = {Size@11852} "1088x1088"
17 = {Size@11853} "1280x720"
18 = {Size@11854} "1024x768"
19 = {Size@11855} "1056x704"
20 = {Size@11856} "960x720"
21 = {Size@11857} "960x540"
22 = {Size@11858} "720x720"
23 = {Size@11859} "800x450"
24 = {Size@11860} "720x480"
25 = {Size@11861} "640x480"
26 = {Size@11862} "352x288"
27 = {Size@11863} "320x240"
28 = {Size@11864} "256x144"
29 = {Size@11865} "176x144"
and we have 12 available sizes in this range
Camera2DeviceSurfaceManager.java:: getPreviewSize()
mCameraSupportedSurfaceCombinationMap.get(cameraId).getSurfaceDefinition().getPreviewSize() = "1920x1080"
cameraId=0
g0...@gmail.com <g0...@gmail.com> #4
The issue root cause is that CameraX will default filter out sizes smaller than 640x480. For Preview, the max size will be limited to under display size. I checked the HW spec info for the issue related devices. Display size of FUJITSU F-04J/F-05J is 360x640. That will result int that no size exists in the conditions that is larger or equal to 640x480 and smaller or equal to 360x640.
A temporary workaround for this situation is to use Preview.Builder#setTargetResolution() to set a size smaller than 640x480 to bypass the problem.
For device FUJITSU arrowsM04, I checked its HW spec info and its display size I found is 1280x720. It seems that the problem should not exist in the device.
Could you confirm that the problem exist on arrowsM04 device? What will be the returned value when using Display#getRealSize to obtain the display size?
lp...@google.com <lp...@google.com> #5
> A temporary workaround for this situation is to use Preview.Builder#setTargetResolution() to set a size smaller than 640x480 to bypass the problem.
OK. I will try it.
> Could you confirm that the problem exist on arrowsM04 device?
We receive the crash report (Crashlytics) that this crash has occurred on arrowsM04.
We don't have this device so we can't confirm that the problem really exist on arrowsM04.
> What will be the returned value when using Display#getRealSize to obtain the display size?
We can't investigate it for the same reason.
Thank you.
ap...@google.com <ap...@google.com> #6
This issue happened on devices that the display size is smaller than 640x480. In original auto-resolution mechanism, supported sizes smaller than 640x480 will be default filter out.
The auto-resolution mechanism encodes the guaranteed configurations tables in CameraDevice#createCaptureSession(SessionConfiguration). It defines that the PREVIEW size is the small one of the device display size and 1080p. The PREVIEW size will be the maximal size limitation for Preview use case. The reason it limits the size to display size and 1080p is the stream output in display size or 1080p has been able to provide good enough preview quality. Therefore, auto-resolution mechanism will limit the selected size to be smaller than the small one of the device display size and 1080p.
With above two conditions, in this issue, all sizes smaller than 640x480 have been filter out, therefore, there is no size smaller than the display size 320x240 can be selected to use. And cause the exception.
Solution:
When the display size is smaller than 640x480, auto-resolution mechanism won't filter out those small sizes smaller than 640x480. This makes those small size be left and can be selected for the Preview use case on small display devices.
The solution has been merged and will be included in next CameraX release.
ap...@google.com <ap...@google.com> #7
Hello.
This crash still occurs.
- CAMERAX VERSION: 1.0.0-beta4
- ANDROID OS BUILD NUMBER: Android 7.1.1
- DEVICE NAME: FUJITSU F-02H
We receive following crash report from FUJITSU F-02H. So far We have received this crash report only from F-02H.
java.lang.IllegalArgumentException
Can not get supported output size under supported maximum for the format: 34
androidx.camera.camera2.internal.SupportedSurfaceCombination.getSupportedOutputSizes (SupportedSurfaceCombination.java:349)
androidx.camera.camera2.internal.SupportedSurfaceCombination.getSuggestedResolutions (SupportedSurfaceCombination.java:197)
androidx.camera.camera2.internal.Camera2DeviceSurfaceManager.getSuggestedResolutions (Camera2DeviceSurfaceManager.java:198)
androidx.camera.core.CameraX.calculateSuggestedResolutions (CameraX.java:949)
androidx.camera.core.CameraX.bindToLifecycle (CameraX.java:351)
androidx.camera.lifecycle.ProcessCameraProvider.bindToLifecycle (ProcessCameraProvider.java:230)
(our application's package name).CameraFragment.bindCameraUseCases (CameraFragment.java:174)
ap...@google.com <ap...@google.com> #8
Could you help to provide the following information to clarify the issue?
1. Is the full name of the device Fujitsu Arrows NX F-02H that has a 1440x2560 display?
2. Please help to provide the supported output sizes of ImageFormat.PRIVATE that is obtained by StreamConfigurationMap#getOutputSizes(int).
ap...@google.com <ap...@google.com> #9
- Is the full name of the device Fujitsu Arrows NX F-02H that has a 1440x2560 display?
Yes
- Please help to provide the supported output sizes of ImageFormat.PRIVATE that is obtained by StreamConfigurationMap#getOutputSizes(int).
Since we don't have this device, we'll try to collect this information in the next version of our app. The next version will be released later this month.
ap...@google.com <ap...@google.com> #10
Hello.
- Please help to provide the supported output sizes of ImageFormat.PRIVATE that is obtained by StreamConfigurationMap#getOutputSizes(int).
We have collected the output of the device where the crash occurs.
Device1
- Model : arrows Be F-05J
- Android Version : 7.1.1
- Supported output sizes of ImageFormat.PRIVATE
CameraId 0: 480x480
CameraId 1: 2048x1536 ,1920x1080 ,1280x720 ,960x720 ,640x480 ,320x240 ,176x144
Device2
- Model : Fujitsu arrows M04
- Android Version : 7.1.1
- Supported output sizes of ImageFormat.PRIVATE
CameraId 0: 480x480
CameraId 1: 2048x1536 ,1920x1080 ,1280x720 ,960x720 ,640x480 ,320x240 ,176x144
Additional Information
CameraX version : 1.0.0-beta04
We collect the supported output sizes by following code.
val errorString = buildString {
append("The supported output sizes of ImageFormat.PRIVATE: ")
(requireContext().getSystemService(Context.CAMERA_SERVICE) as CameraManager).apply {
cameraIdList.forEachIndexed { index, cameraId ->
val msg = if (VERSION.SDK_INT >= VERSION_CODES.M) {
val configurationMap =
getCameraCharacteristics(cameraId).get(CameraCharacteristics.SCALER_STREAM_CONFIGURATION_MAP)
val sizes = configurationMap?.getOutputSizes(ImageFormat.PRIVATE)
"CameraId $index: ${sizes?.joinToString(" ,")}"
} else {
"CameraId $index: This device version is under M."
}
append(msg)
}
}
}
ap...@google.com <ap...@google.com> #11
ap...@google.com <ap...@google.com> #12
I tried to find the device specs and both 720x1280
size display. For the camera id 0 device, it is a different case that the display size is larger than 640x480
but the device only supports a 480x480
size. The case also caused the same IllegalArgumentException and was also fixed by 1.0.0-beta04
release. Before 480x480
size would be filtered out and then caused the IllegalArgumentException. After it was merged, the 640x480
size threshold was removed and then the 480x480
size would be kept and selected to use.
It looks like 1.0.0-beta04
release had been used to collect the supported sizes information. But the issue should have been fixed by 1.0.0-beta04
release. Did you only check the device model name to collect the supported sizes information or collect the information when the IllegalArgumentException issue happens again?
CameraX's 1.0.0-beta04
version. Maybe you can also consider to upgrade to the latest 1.0.0-rc01
version for your application. Thanks.
ap...@google.com <ap...@google.com> #13
Did you only check the device model name to collect the supported sizes information or collect the information when the IllegalArgumentException issue happens again?
We collect informations only from the device on which IllegalArgumentException happened.
Our latest app uses CameraX version 1.0.0-beta10
and this issue still occurres.
However we don't receive crash report from Fujitsu arrows Be F-05J
or Fujitsu arrows M04
so far. (This doesn't mean this issue is fixed on these devices because our app is heavily rely on camera so these device's user wouldn't use our app anymore.)
Instead, we receive crash report from
- Model : Fujitsu F-03K
- Android Version : 7.1.2
- Supported output sizes of ImageFormat.PRIVATE
CameraId 0 : 480x480
CameraId 1 : 2048x1536 ,1920x1080 ,1280x720 ,960x720 ,640x480 ,320x240 ,176x144
lp...@google.com <lp...@google.com> #14
I missed some settings when I simulated the issue by robolectric test so that I was not able to reproduce it. Now, I can reproduce the issue if the device only supports one 480x480 resolution. I'm working on the solution and target to make it included in next release.
ap...@google.com <ap...@google.com> #15
Branch: androidx-main
commit 69d15dff7bb857ee33a0f643ff42a0f8bc475ab2
Author: charcoalchen <charcoalchen@google.com>
Date: Fri Jan 08 18:30:03 2021
Fixed IllegalArgumentException issue happened when all preview supported sizes are smaller than 640x480 and display size is larger than 640x480.
Do not filter out sizes smaller than 640x480 when all preview supported sizes are smaller than 640x480 and display size is larger than 640x480.
Relnote:"Fixed IllegalArgumentException issue happened when all preview supported sizes are smaller than 640x480 and display size is larger than 640x480."
Bug: 150506192
Test: SupportedSurfaceCombinationTest
Change-Id: I2a63ce8e2ad42a9cc060c8635ac3603bf440b1ec
M camera/camera-camera2/src/main/java/androidx/camera/camera2/internal/SupportedSurfaceCombination.java
M camera/camera-camera2/src/test/java/androidx/camera/camera2/internal/SupportedSurfaceCombinationTest.java
ap...@google.com <ap...@google.com> #16
ap...@google.com <ap...@google.com> #17
Branch: androidx-main
commit 3f8a328a3d4a7b420d9696126e893b106a4b9cd6
Author: Louis Pullen-Freilich <lpf@google.com>
Date: Wed Dec 27 20:43:56 2023
Updates tv-material components to expose a nullable MutableInteractionSource
Components that previously exposed an API such as:
interactionSource: MutableInteractionSource = remember { MutableInteractionSource() }
Have been updated to instead expose a nullable MutableInteractionSource that defaults to null. This allows for some components to never create and remember a MutableInteractionSource, and allows for some other components to lazily create one only when needed. Some components always need a MutableInteractionSource currently, so if null they will just create a new one internally - so for those cases there are no changes, but it allows for future optimizations.
Test: updateApi, existing tests
Bug:
Relnote: "tv-material components exposing a MutableInteractionSource in their API have been updated to now expose a nullable MutableInteractionSource that defaults to null. There are no semantic changes here: passing null means that you do not wish to hoist the MutableInteractionSource, and it will be created inside the component if needed. Changing to null allows for some components to never allocate a MutableInteractionSource, and allows for other components to only lazily create an instance when they need to, which improves performance across these components. If you are not using the MutableInteractionSource you pass to these components, it is recommended that you pass null instead. It is also recommended that you make similar changes in your own components."
Change-Id: I309b436d212ef53897979df40da9b1768377893f
M tv/tv-material/api/current.txt
M tv/tv-material/api/restricted_current.txt
M tv/tv-material/src/main/java/androidx/tv/material3/Button.kt
M tv/tv-material/src/main/java/androidx/tv/material3/Card.kt
M tv/tv-material/src/main/java/androidx/tv/material3/CardLayout.kt
M tv/tv-material/src/main/java/androidx/tv/material3/Checkbox.kt
M tv/tv-material/src/main/java/androidx/tv/material3/Chip.kt
M tv/tv-material/src/main/java/androidx/tv/material3/IconButton.kt
M tv/tv-material/src/main/java/androidx/tv/material3/ListItem.kt
M tv/tv-material/src/main/java/androidx/tv/material3/NavigationDrawerItem.kt
M tv/tv-material/src/main/java/androidx/tv/material3/RadioButton.kt
M tv/tv-material/src/main/java/androidx/tv/material3/Surface.kt
M tv/tv-material/src/main/java/androidx/tv/material3/Switch.kt
M tv/tv-material/src/main/java/androidx/tv/material3/Tab.kt
M tv/tv-material/src/main/java/androidx/tv/material3/WideButton.kt
ap...@google.com <ap...@google.com> #18
Branch: androidx-main
commit f8fa920a5a088277028bfa4c186aedb235d253c0
Author: Louis Pullen-Freilich <lpf@google.com>
Date: Wed Dec 27 20:42:07 2023
Updates wear material and wear material3 components to expose a nullable MutableInteractionSource
Components that previously exposed an API such as:
interactionSource: MutableInteractionSource = remember { MutableInteractionSource() }
Have been updated to instead expose a nullable MutableInteractionSource that defaults to null. This allows for some components to never create and remember a MutableInteractionSource, and allows for some other components to lazily create one only when needed. Some components always need a MutableInteractionSource currently, so if null they will just create a new one internally - so for those cases there are no changes, but it allows for future optimizations.
Test: updateApi, existing tests
Bug:
Relnote: "Wear material and wear material3 components exposing a MutableInteractionSource in their API have been updated to now expose a nullable MutableInteractionSource that defaults to null. There are no semantic changes here: passing null means that you do not wish to hoist the MutableInteractionSource, and it will be created inside the component if needed. Changing to null allows for some components to never allocate a MutableInteractionSource, and allows for other components to only lazily create an instance when they need to, which improves performance across these components. If you are not using the MutableInteractionSource you pass to these components, it is recommended that you pass null instead. It is also recommended that you make similar changes in your own components."
Change-Id: Ib90fc2736d2311e467d7c2a3fef4df757afaf525
M wear/compose/compose-material-core/src/androidTest/kotlin/androidx/wear/compose/materialcore/ButtonTest.kt
M wear/compose/compose-material-core/src/androidTest/kotlin/androidx/wear/compose/materialcore/SelectionControlsTest.kt
M wear/compose/compose-material-core/src/androidTest/kotlin/androidx/wear/compose/materialcore/ToggleButtonTest.kt
M wear/compose/compose-material-core/src/main/java/androidx/wear/compose/materialcore/Button.kt
M wear/compose/compose-material-core/src/main/java/androidx/wear/compose/materialcore/Card.kt
M wear/compose/compose-material-core/src/main/java/androidx/wear/compose/materialcore/RepeatableClickable.kt
M wear/compose/compose-material-core/src/main/java/androidx/wear/compose/materialcore/SelectionControls.kt
M wear/compose/compose-material-core/src/main/java/androidx/wear/compose/materialcore/Slider.kt
M wear/compose/compose-material-core/src/main/java/androidx/wear/compose/materialcore/ToggleButton.kt
M wear/compose/compose-material/api/current.txt
M wear/compose/compose-material/api/restricted_current.txt
M wear/compose/compose-material/src/main/java/androidx/wear/compose/material/Button.kt
M wear/compose/compose-material/src/main/java/androidx/wear/compose/material/Card.kt
M wear/compose/compose-material/src/main/java/androidx/wear/compose/material/Chip.kt
M wear/compose/compose-material/src/main/java/androidx/wear/compose/material/SwipeToReveal.kt
M wear/compose/compose-material/src/main/java/androidx/wear/compose/material/ToggleButton.kt
M wear/compose/compose-material/src/main/java/androidx/wear/compose/material/ToggleChip.kt
M wear/compose/compose-material/src/main/java/androidx/wear/compose/material/ToggleControl.kt
M wear/compose/compose-material3/api/current.txt
M wear/compose/compose-material3/api/restricted_current.txt
M wear/compose/compose-material3/src/main/java/androidx/wear/compose/material3/Button.kt
M wear/compose/compose-material3/src/main/java/androidx/wear/compose/material3/Card.kt
M wear/compose/compose-material3/src/main/java/androidx/wear/compose/material3/IconButton.kt
M wear/compose/compose-material3/src/main/java/androidx/wear/compose/material3/RadioButton.kt
M wear/compose/compose-material3/src/main/java/androidx/wear/compose/material3/TextButton.kt
M wear/compose/compose-material3/src/main/java/androidx/wear/compose/material3/ToggleButton.kt
lp...@google.com <lp...@google.com> #19
ap...@google.com <ap...@google.com> #20
Branch: androidx-main
commit 2e1799e019e275415a95804a8beb5a5a0bf3f86f
Author: Louis Pullen-Freilich <lpf@google.com>
Date: Wed Jan 10 02:53:19 2024
Removes AbstractClickablePointerInputNode, flattens pointer input logic
Pointer input handling is now handled inline: click and hover handling as well as their corresponding interactions are all handled in one place, removing the need for a separate pointer input node for clicks, and removing the HoverableNode delegation.
Bug:
Test: existing tests
Change-Id: I755c468d84f538aef3354a385c81e8c38fa69738
M compose/foundation/foundation/src/androidInstrumentedTest/kotlin/androidx/compose/foundation/selection/SelectableTest.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/Clickable.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/Hoverable.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/selection/Selectable.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/selection/Toggleable.kt
ap...@google.com <ap...@google.com> #21
Branch: androidx-main
commit b657f2f2082e2a1d17c2c259792ac9f3a57d2e0a
Author: Louis Pullen-Freilich <lpf@google.com>
Date: Fri Nov 24 13:28:02 2023
Adds ripple API to material3 library
The new ripple API replaces the deprecated rememberRipple API - it creates a ripple instance that will use the default theme values. ripple does not need to be remembered, similar to modifiers - if the parameters compare equally, then the same node can be reused internally. Also adds RippleDefaults that contain the default values used by material3.
This CL also migrates all material3 components to use the new ripple. There is a temporary CompositionLocal, LocalUseFallbackRippleImplementation, which can be set to true to fallback to using the old RippleTheme / rememberRipple implementation, but this is strongly discouraged as it is much less performant than the new implementation. This fallback will be removed in the next stable release, and exists here to aid migration.
Bug:
Bug:
Test: RippleTest
Relnote: "Adds new ripple API in material3 which replaces the deprecated rememberRipple. Also adds a temporary CompositionLocal, LocalUseFallbackRippleImplementation, to revert material3 components to using the deprecated rememberRipple / RippleTheme APIs. This will be removed in the next stable release, and is only intended to be a temporary migration aid for cases where you are providing a custom RippleTheme. See
Change-Id: I34cbc2834133de4f3e8dde389ed4dab8c54b0c95
M compose/material3/material3/api/current.txt
M compose/material3/material3/api/restricted_current.txt
M compose/material3/material3/src/androidInstrumentedTest/kotlin/androidx/compose/material3/RippleTest.kt
M compose/material3/material3/src/commonMain/kotlin/androidx/compose/material3/Checkbox.kt
M compose/material3/material3/src/commonMain/kotlin/androidx/compose/material3/IconButton.kt
M compose/material3/material3/src/commonMain/kotlin/androidx/compose/material3/MaterialTheme.kt
M compose/material3/material3/src/commonMain/kotlin/androidx/compose/material3/Menu.kt
M compose/material3/material3/src/commonMain/kotlin/androidx/compose/material3/NavigationBar.kt
M compose/material3/material3/src/commonMain/kotlin/androidx/compose/material3/NavigationRail.kt
M compose/material3/material3/src/commonMain/kotlin/androidx/compose/material3/RadioButton.kt
A compose/material3/material3/src/commonMain/kotlin/androidx/compose/material3/Ripple.kt
M compose/material3/material3/src/commonMain/kotlin/androidx/compose/material3/Slider.kt
M compose/material3/material3/src/commonMain/kotlin/androidx/compose/material3/Surface.kt
M compose/material3/material3/src/commonMain/kotlin/androidx/compose/material3/Switch.kt
M compose/material3/material3/src/commonMain/kotlin/androidx/compose/material3/Tab.kt
ap...@google.com <ap...@google.com> #22
Branch: androidx-main
commit c1dfd3a4e8af6f3b9182211f39883962ee81385b
Author: Louis Pullen-Freilich <lpf@google.com>
Date: Fri Nov 24 13:30:18 2023
Adds RippleConfiguration APIs to material3
RippleConfiguration and LocalRippleConfiguration allow for per-component / sub-tree customization of ripples using fixed values. For example, to change the color of a component you don't control, or to disable a ripple for a component. In most cases the default values should be used: these APIs are an escape hatch for customization of individual components / limited sub-trees. For wider changes and custom design systems, you should instead build your own ripple using createRippleModifierNode.
Bug:
Test: RippleTest
Relnote: "Adds RippleConfiguration and LocalRippleConfiguration to allow for per-component / sub-tree customization of ripples using fixed values. For example, to change the color of a component you don't control, or to disable a ripple for a component. In most cases the default values should be used: these APIs are an escape hatch for customization of individual components / limited sub-trees. For wider changes and custom design systems, you should instead build your own ripple using createRippleModifierNode. For more information, see
Change-Id: I7b5d62fd50ee78bb3559f83886aa1e7d9f964fb1
M compose/material3/material3/api/current.txt
M compose/material3/material3/api/restricted_current.txt
M compose/material3/material3/src/androidInstrumentedTest/kotlin/androidx/compose/material3/RippleTest.kt
M compose/material3/material3/src/commonMain/kotlin/androidx/compose/material3/Ripple.kt
ap...@google.com <ap...@google.com> #23
Branch: androidx-main
commit 6b42889368193eccb3bb2b1af1850f0a2a76feca
Author: Louis Pullen-Freilich <lpf@google.com>
Date: Thu Dec 28 04:08:41 2023
Updates material3 components to expose a nullable MutableInteractionSource
Components that previously exposed an API such as:
interactionSource: MutableInteractionSource = remember { MutableInteractionSource() }
Have been updated to instead expose a nullable MutableInteractionSource that defaults to null. This allows for some components to never create and remember a MutableInteractionSource, and allows for some other components to lazily create one only when needed. Some components always need a MutableInteractionSource currently, so if null they will just create a new one internally - so for those cases there are no changes, but it allows for future optimizations.
Test: updateApi, existing tests
Bug:
Relnote: "Material3 components exposing a MutableInteractionSource in their API have been updated to now expose a nullable MutableInteractionSource that defaults to null. There are no semantic changes here: passing null means that you do not wish to hoist the MutableInteractionSource, and it will be created inside the component if needed. Changing to null allows for some components to never allocate a MutableInteractionSource, and allows for other components to only lazily create an instance when they need to, which improves performance across these components. If you are not using the MutableInteractionSource you pass to these components, it is recommended that you pass null instead. It is also recommended that you make similar changes in your own components."
Change-Id: I41abb601499b4a735b6302b96cdc1f0d066dbbdc
M compose/material3/material3-adaptive-navigation-suite/api/current.txt
M compose/material3/material3-adaptive-navigation-suite/api/restricted_current.txt
M compose/material3/material3-adaptive-navigation-suite/src/commonMain/kotlin/androidx/compose/material3/adaptive/navigation-suite/NavigationSuiteScaffold.kt
M compose/material3/material3/api/current.txt
M compose/material3/material3/api/restricted_current.txt
M compose/material3/material3/src/androidMain/kotlin/androidx/compose/material3/AndroidMenu.android.kt
M compose/material3/material3/src/androidMain/kotlin/androidx/compose/material3/SearchBar.android.kt
M compose/material3/material3/src/commonMain/kotlin/androidx/compose/material3/Button.kt
M compose/material3/material3/src/commonMain/kotlin/androidx/compose/material3/Card.kt
M compose/material3/material3/src/commonMain/kotlin/androidx/compose/material3/Checkbox.kt
M compose/material3/material3/src/commonMain/kotlin/androidx/compose/material3/Chip.kt
M compose/material3/material3/src/commonMain/kotlin/androidx/compose/material3/FloatingActionButton.kt
M compose/material3/material3/src/commonMain/kotlin/androidx/compose/material3/IconButton.kt
M compose/material3/material3/src/commonMain/kotlin/androidx/compose/material3/Label.kt
M compose/material3/material3/src/commonMain/kotlin/androidx/compose/material3/Menu.kt
M compose/material3/material3/src/commonMain/kotlin/androidx/compose/material3/NavigationBar.kt
M compose/material3/material3/src/commonMain/kotlin/androidx/compose/material3/NavigationDrawer.kt
M compose/material3/material3/src/commonMain/kotlin/androidx/compose/material3/NavigationRail.kt
M compose/material3/material3/src/commonMain/kotlin/androidx/compose/material3/OutlinedTextField.kt
M compose/material3/material3/src/commonMain/kotlin/androidx/compose/material3/RadioButton.kt
M compose/material3/material3/src/commonMain/kotlin/androidx/compose/material3/SegmentedButton.kt
M compose/material3/material3/src/commonMain/kotlin/androidx/compose/material3/Surface.kt
M compose/material3/material3/src/commonMain/kotlin/androidx/compose/material3/Switch.kt
M compose/material3/material3/src/commonMain/kotlin/androidx/compose/material3/Tab.kt
M compose/material3/material3/src/commonMain/kotlin/androidx/compose/material3/TextField.kt
ap...@google.com <ap...@google.com> #24
Branch: androidx-main
commit 176632f8e826524658d782b03c9d27892d047819
Author: Louis Pullen-Freilich <lpf@google.com>
Date: Wed Jan 31 15:59:06 2024
Turns off auto invalidation in clickable / focusable
This causes a lot of extra work when input parameters are changed, such as invalidating focus nodes - even though most of these parameters do not affect the state of the node tree.
Bug:
Fixes:
Test: ClickableTest
Test: TabRowBenchmark
Change-Id: I2fcc8732784d27fde072ac739f917f6da5a432a6
M compose/foundation/foundation/src/androidInstrumentedTest/kotlin/androidx/compose/foundation/ClickableTest.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/Clickable.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/Focusable.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/FocusedBounds.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/relocation/BringIntoViewRequester.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/relocation/BringIntoViewResponder.kt
lp...@google.com <lp...@google.com> #25
Summarizing a discussion we had today: One of the long term ideas / goals we had for clickable was to be able to fully remove focus* logic, when in touch mode. There are some problems with this approach:
a) When we are in touch mode, and we swap to non-touch mode with a key press, this will also request focus. This means that in order for us to handle this properly, we need to have a stable focus tree by this point in time: if we just observeReads and delegate() a focus node, this will happen after we apply changes, so the nodes won't be attached when we try and focus. Instead we would probably need to advance the snapshot, to force synchronous creation of the focus node tree, but focus invalidation currently happens after we apply changes, so if we immediately add the nodes the focus tree will be in a broken state, so this would require some unknown amount of rewriting to make sure we can initialize the tree inline, safely, to make this use case work.
b) behavior gets a bit dangerous / undefined if we do this, consider the following:
Button(Modifier.focusProperties { ... }...)
Depending on touch mode, this focus properties modifier would either apply to the focus target inside button, or the next focus target within - this is very misleading and can lead to subtle behavioral problems. It's also hard to define this 'correctly', if you only want to affect the button in non-touch mode: for the reasons listed in a) it would be difficult to synchronize the creation of this focus properties modifier, with the time we create the focus target, to make sure that everything is in sync at the same time.
We might want to revisit this in the future, but given the constraints / unknown work here, and the performance investigations we did, it looks like we can still get close to this performance baseline with some other optimizations, so focusing on making sure that when we aren't focusable, as little work is done as possible inside focus.
ap...@google.com <ap...@google.com> #26
Branch: androidx-main
commit a3b31c9837990f5d556aeda56c056d464848af3a
Author: Louis Pullen-Freilich <lpf@google.com>
Date: Tue Apr 09 01:59:53 2024
Adds APIs to FocusTargetModifierNode for observing focus changes, configuring focusability, and requesting focus
This allows delegating nodes to just delegate to one FocusTargetModifierNode, instead of needing to add separate requester / event / properties nodes to modify the delegated and owned target node.
Also migrates focusable / clickable to use this, which reduces the cost of focus invalidation as we can remove all the other node types.
FocusProperties APIs for configurating focus requesters will be added in the future to FocusTargetModifierNode.
Bug:
Test: FocusableTest
Test: ClickableTest
Test: FocusTargetModifierNodeTest
Relnote: "Added FocusTargetModifierNode#requestFocus(), FocusTargetModifierNode#focusability, and a new FocusTargetModifierNode() overload that exposes FocusTargetMode and a callback for when focus state changes. This makes it simpler to implement delegated modifiers that should be focusable, as you do not need to implement requester / event / properties nodes as well, in order to modify the target node."
Change-Id: I27f84d34307dc78bbd14ecda328ea9cdc5b63b8b
M compose/foundation/foundation/src/androidInstrumentedTest/kotlin/androidx/compose/foundation/FocusableTest.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/Clickable.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/Focusable.kt
M compose/ui/ui/api/current.ignore
M compose/ui/ui/api/current.txt
M compose/ui/ui/api/restricted_current.ignore
M compose/ui/ui/api/restricted_current.txt
A compose/ui/ui/src/androidInstrumentedTest/kotlin/androidx/compose/ui/focus/FocusTargetModifierNodeTest.kt
M compose/ui/ui/src/androidInstrumentedTest/kotlin/androidx/compose/ui/node/NodeUtils.kt
M compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/focus/FocusEventModifierNode.kt
M compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/focus/FocusInvalidationManager.kt
M compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/focus/FocusOwnerImpl.kt
M compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/focus/FocusTargetModifierNode.kt
M compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/focus/FocusTargetNode.kt
M compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/focus/FocusTransactions.kt
A compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/focus/Focusability.kt
lp...@google.com <lp...@google.com> #27
After the last CL, there are no more high priority areas of work related to / inside clickable itself that we are tracking for performance - clickable is roughly 4x in perf microbenchmarks since 1.5*. There is ongoing work into optimizing parts of the focus system, but that's at a lower level than clickable and won't affect clickable's implementation. Closing this bug accordingly
Description
multiple state objects, multiple GlobalPositionAwareNodes which invalidate layout every time this updates, and many allocations and initialization work.
We can expect there to be dozens of clickables on a relatively average screen, and much more in some situations, so it being cheap is incredibly important. Moreover, most of them will never get interacted with, and we ought to pay as little for those as possible during initialization.
In the CL linked below, I have laid out my ideas for how I think we should refactor Clickable. Broadly this amounts to:
1. refactor indication to not need composition
2. make clickable correspond to a single element / node, even including delegated nodes, until there is an interaction that warrants an "inflation" of the node to include the more expensive things like indication etc.
More context on this issue can be found in aosp/2729638