BasicMarquee sometimes does not work with frequent `velocity` updates [297974036]

Fixed

Bug

Status Update

No update yet.

Description

mi...@gmail.com

created issue #1

Aug 29, 2023 01:28PM

Jetpack Compose version: BOM 2023.08.00 (Compose 1.5.0, I suppose)

Jetpack Compose component(s) used:

androidx.compose.ui:ui

androidx.compose.material3:material3

Android Studio Build: #AI-223.8836.35.2231.10671973, built on August 17, 2023

Kotlin version: 223-1.9.0-release-358-AS8836.35.2231.10671973

Steps to Reproduce or Code Sample to Reproduce:

Add basicMarquee which starts immediately with custom velocity which changes depending from Slider component.
Launch app and just move thumb on Slider. Sometimes basicMarquee stop working. The content is displayed but not scrolling. If I repeat - sometimes everything returns to normal, sometimes not.
I assume that due to too frequent change of the velocity field basicMarquee sometimes does not work.

Full code, just in case:

Text(
	text = "Sample Text",
	modifier = Modifier
		.basicMarquee(
			iterations = Int.MAX_VALUE,
			animationMode = MarqueeAnimationMode.Immediately,
			delayMillis = 0,
			velocity = (2048 * sliderScrollSpeed).dp// sliderScrollSpeed = 0..1f
		),
)

Visual demonstration: https://i.imgur.com/zeaIKfN.mp4

GIF.gif

4.5 MB

View

Download

Comments

mi...@gmail.com <mi...@gmail.com> #2Aug 29, 2023 04:33PM

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

ha...@google.com <ha...@google.com> Aug 31, 2023 12:19PM

Assigned to kl...@google.com.

mi...@gmail.com <mi...@gmail.com> #3Sep 11, 2023 04:19PM

I discovered the exact conditions under which the bug is reproduced. For some reason, if recomposition happens while basicMarquee animation is active, the animation will stop (although the animation should start again from the beginning). And with the next recomposition, the animation will work again.

For example, if you change the number of basicMarquee iterations to 1, you can easily see that after basicMarquee animation ends, the next recomposition starts basicMarquee animation correctly. But if you recompose during basicMarquee animation, the animation will stop until the next recomposition.

Could you recommend some workaround to solve this problem? I'm new to Jetpack Compose and have no idea how to force stop internal animation of basicMarquee before recomposing. Or at least how to get the state of basicMarquee internal animation.

kl...@google.com <kl...@google.com> #4Sep 21, 2023 10:23PM

Verified by kl...@google.com.

I cannot reproduce this on the 1.6 alpha branch. I suspect it was fixed when migrated to the Modifier.Node API. If you can reproduce on the latest 1.6 alpha, please submit a reproducer project and I'll take another look.

mi...@gmail.com <mi...@gmail.com> #5Sep 22, 2023 12:30PM

The bug is still there. Reproduced on 1.6.0-alpha06. The bug is also reproduced on the emulator, which means it is not a device-specific bug. I have attached a project with a reproducible bug.

In the demo project I change the velocity of basicMarquee in an infinite loop. It is expected that after each velocity change the animation will start over again, but it "stuck" every second time (when the current scroll animation is active - this is a key part of reproducing the bug).

Minor clarification: source code for basicMarquee modifier is located at androidx.compose.foundation which according to my tests is part of androidx.compose.ui:ui-graphics. At least if I go to implementation in Android Studio it opens the source code of the file C:\Users\Mikhail\.gradle\caches\modules-2\files-2.1\androidx.compose.foundation\foundation-android\1.6.0-alpha06\fe06e22c416549d90173acb7828b3a13f781e8b2\foundation-android-1.6. 0-alpha06-sources.jar!\commonMain\androidx\compose\foundation\BasicMarquee.kt. The version in the path clearly indicates that I am testing the alpha branch. I'll clarify just in case.

basicMarqueeBug.zip

105 KB

Download

GIF.gif

150 KB

View

Download

kl...@google.com <kl...@google.com> #6Sep 25, 2023 05:49PM

Accepted by kl...@google.com.

I'm pretty sure this is a race condition introduced when I converted basicMarquee to a Modifier.Node.

Message last modified on Sep 25, 2023 05:58PM

ap...@google.com <ap...@google.com> #7Sep 26, 2023 12:24AM

Marked as fixed.

Project: platform/frameworks/support
Branch: androidx-main

commit cd77952e2a6847a3cea00125c0a7a8214418069c
Author: Zach Klippenstein <klippenstein@google.com>
Date: Mon Sep 25 11:34:38 2023

Fix basicMarquee not animating after velocity change.

Fixes:

b/297974036

Test: BasicMarqueeTest
Relnote: "Fixed `basicMarquee` not animating after velocity change."
Change-Id: Id2e77b47a3fd5cc046e59594cc1d5f01c99f34a0

M compose/foundation/foundation/src/androidInstrumentedTest/kotlin/androidx/compose/foundation/BasicMarqueeTest.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/BasicMarquee.kt

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

Issue 297974036

Description

Issue summary

Comments

mi...@gmail.com <mi...@gmail.com> #2Aug 29, 2023 04:33PM

ha...@google.com <ha...@google.com> Aug 31, 2023 12:19PM

mi...@gmail.com <mi...@gmail.com> #3Sep 11, 2023 04:19PM

kl...@google.com <kl...@google.com> #4Sep 21, 2023 10:23PM

mi...@gmail.com <mi...@gmail.com> #5Sep 22, 2023 12:30PM

kl...@google.com <kl...@google.com> #6Sep 25, 2023 05:49PM

ap...@google.com <ap...@google.com> #7Sep 26, 2023 12:24AM

Add comment

Issue metadata