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 b90079595f33f58fece04026a97faa0d243acdb1
Author: Yuichi Araki <yaraki@google.com>
Date: Wed Sep 18 16:55:49 2019

Change the way to detect mismatch between POJO and query

This fixes cursor mismatch warnings with expandProjection.

Bug: 140759491
Test: QueryMethodProcessorTest
Change-Id: I7659002e5e0d1ef60fc1af2a625c0c36da0664d8

M room/compiler/src/main/kotlin/androidx/room/processor/QueryMethodProcessor.kt
M room/compiler/src/main/kotlin/androidx/room/solver/TypeAdapterStore.kt
M room/compiler/src/main/kotlin/androidx/room/solver/query/result/PojoRowAdapter.kt
M room/compiler/src/test/kotlin/androidx/room/processor/QueryMethodProcessorTest.kt
M room/compiler/src/test/kotlin/androidx/room/testing/TestProcessor.kt

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

https://goto.google.com/android-sha1/b90079595f33f58fece04026a97faa0d243acdb1

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

The following changes were cherrypicked through Coastguard (

https://android-build.googleplex.com/coastguard/)
Release Track: (

https://android-build.googleplex.com/coastguard/dashboard/6093116828680192/#/cherrypickHistory/all)
Created: 2019-10-14 17:23:42.610596+00:00
Changes: aosp/1123258

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

Verified by kl...@google.com.

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

commit bdde5a1a970ddc9007b28de4aa29d60ffa588f08
Author: Yigit Boyar <yboyar@google.com>
Date: Thu Apr 16 16:47:05 2020

Re-factor how errors are dismissed when query is re-written

This CL changes how we handle errors/warnings if query is
re-written.
There was a bug in expandProjection where we would report warnings
for things that Room already fixes automatically (

b/140759491

).
The solution to that problem (I7659002e5e0d1ef60fc1af2a625c0c36da0664d8)
solved it by deferring validating of columns until after re-write
decision is made. Unfortunately, this required changing PojoRowAdapter
to have a dummy mapping until it is validating, make it hard to use
as it does have a non-null mapping which is not useful.

This CL partially reverts that change and instead rely on the log
deferring logic we have in Context. This way, we don't need to break
the stability of PojoRowAdapter while still having the ability to
drop warnings that room fixes. This will also play nicer when we
have different query re-writing options that can use more information
about the query results.

Bug: 153387066
Bug: 140759491
Test: existing tests pass
Change-Id: I2ec967c763d33d7a3ff02c1a13c6953b460d1e5f

M room/compiler/src/main/kotlin/androidx/room/log/RLog.kt
M room/compiler/src/main/kotlin/androidx/room/processor/QueryMethodProcessor.kt
M room/compiler/src/main/kotlin/androidx/room/solver/TypeAdapterStore.kt
M room/compiler/src/main/kotlin/androidx/room/solver/query/result/PojoRowAdapter.kt

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

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