Status Update
Comments
st...@google.com <st...@google.com>
ak...@google.com <ak...@google.com> #2
Hi, I've had a go at repro'ing this, see code below. I'm not quite able to see the same ugly jump seen in the video. Is the issue that the fling is stopped prematurely then you have to fling again? Can you try and repro with the code below?
@Composable
fun DrawCircleWithScreenRadius(text: String = "", color: Color = Color.Blue) {
val context = LocalContext.current
val screenWidth = with(LocalDensity.current) {
context.resources.displayMetrics.widthPixels.toDp()
}
Box(modifier = Modifier.size(screenWidth)) {
Canvas(modifier = Modifier.fillMaxSize()) {
drawCircle(
color = color,
radius = screenWidth.value,
center = center
)
}
Text(text = text, color = Color.Black, modifier = Modifier.align(Alignment.Center))
}
}
@Composable
@Preview
fun HorizontalPagerBugRepro(
modifier: Modifier = Modifier,
) {
Box(
modifier
.fillMaxSize()
.background(color = Color.Black)
) {
val pagerItems: List<@Composable () -> Unit> = listOf(
{ DrawCircleWithScreenRadius("Page 0") },
{
Box(modifier = Modifier
.fillMaxSize()
.clip(CircleShape)) {
val sclItems = (1..20).map { "Button $it" }
ScalingLazyColumn() {
items(sclItems) { item ->
Chip(onClick = {}, colors = ChipDefaults.chipColors(), border = ChipDefaults.chipBorder(), content = {Text(item)})
}
}
}
},
{ DrawCircleWithScreenRadius("Page 2") },
{ DrawCircleWithScreenRadius("Page 3") },
{ DrawCircleWithScreenRadius("Page 4") })
val pagerState = rememberPagerState(
initialPage = 0,
pageCount = { 5 })
class HorizontalPageIndicatorState(pagerState: PagerState) : PageIndicatorState {
override val pageOffset: Float = pagerState.currentPageOffsetFraction
override val selectedPage: Int = pagerState.currentPage
override val pageCount: Int = pagerState.pageCount
}
HorizontalPager(state = pagerState) { page ->
val item = pagerItems[page]
item()
}
HorizontalPageIndicator(HorizontalPageIndicatorState(pagerState))
}
}
```
to...@gmail.com <to...@gmail.com> #3
The issue is that the fling goes to top then does this. Go to the bottom of the page then fling it will scroll to top then jump.
I've provided a full repro in OP just clone the branch and run the project as indicated with all the repro steps.
It's all based on Google Horologist code, just added more content to be sure the top and bottom items are no more composed then recomposed during the fling.
The global layout includes appscaffold and screenscaffold and other things. I mentioned the pager / SLC because when using SLC in a subscreen with the same appscaffold and screenscaffold without the pager the issue is not present.
Using
Box(modifier = Modifier
.fillMaxSize()
.clip(CircleShape)) {
val sclItems = (1..20).map { "Button $it" }
ScalingLazyColumn() {
items(sclItems) { item ->
Chip(onClick = {}, colors = ChipDefaults.chipColors(), border = ChipDefaults.chipBorder(), content = {Text(item)})
}
}
}
As the content of the pager screen in the repro code I gave in OP does produce the same issue.
ak...@google.com <ak...@google.com> #4
Thanks, I want to repro this in a clean Compose / no Horologist setting to 1) rule out any potential Horologist issue causing this 2) help pinpoint the issue so I can fix the underlying bug. If you have any tips to tweak the code I have to repro that would be helpful, otherwise I will try repro'ing in Horologist and slowly strip away bits to get a clean repro.
to...@gmail.com <to...@gmail.com> #5
I unfortunately don't. As indie dev I'm not payed by the hour, so it's hard to produce more than a fully working repro that already takes time. Specially when reporting dozens of issues.
ak...@google.com <ak...@google.com> #6
No worries, I'll try the Horologist code you linked when I get a chance.
gr...@google.com <gr...@google.com> #7
Can these be duped, @akhosa@google.com
to...@gmail.com <to...@gmail.com> #8
For the record passing to the underlying androidx.wear.compose.foundation.lazy.ScalingLazyColumn(
flingBehavior = rememberSnapFlingBehavior(object: SnapLayoutInfoProvider {
override fun calculateSnapOffset(velocity: Float): Float {
return 0f
}
}),
Does fix this.
And this is still happening with snapshot 12206807 (Compose 1.8 and Wear 1.5)
ys...@google.com <ys...@google.com> #9
Comment from tolriq in slack
removing that spec does fix the issue. I do not have time to investigate more if the spec is just wrong to use here, or there's a bug in compose default flingBehavior, but since the root is in Horologist I'm pinging you as you probably know why it's used like that
@Composable
@OptIn(ExperimentalFoundationApi::class)
public fun flingParams(
pagerState: PagerState,
): TargetedFlingBehavior {
return PagerDefaults.flingBehavior(
state = pagerState,
pagerSnapDistance = PagerSnapDistance.atMost(0),
snapAnimationSpec = tween(150, 0),
)
}
st...@google.com <st...@google.com> #10
Aman - please verify this workaround is not needed with your prototype Pager component - thanks.
ak...@google.com <ak...@google.com> #11
I have had some time to look into this issue. I was able to checkout the repo in bug description and repro the issue. Checking out latest version of Horologist I confirmed it still exists. Then I was able to produced a simplified repro in AndroidX, see code snippet below:
@Composable
fun SimpleHorizontalPagerSample() {
// Creates a horizontal pager with 10 elements
val state = rememberPagerState { 10 }
HorizontalPager(
state = state,
) { page ->
Box(modifier = Modifier.fillMaxSize(), contentAlignment = Alignment.Center) {
ScalingLazyColumn() {
items(20) {
BasicText(text = "Page $page", style = TextStyle(color = Color.White))
}
}
}
}
}
With this snippet there is a slight jump at the end of a fast fling to the top or bottom of the SLC. This is still reproducible on the new Wear Pager component. As #9 mentions, this seems to be caused by setting flingBehaviour:
@Composable
@OptIn(ExperimentalFoundationApi::class)
public fun flingParams(
pagerState: PagerState,
): TargetedFlingBehavior {
return PagerDefaults.flingBehavior(
state = pagerState,
pagerSnapDistance = PagerSnapDistance.atMost(0),
snapAnimationSpec = tween(150, 0),
)
}
In particular the troublesome line is the snapAnimationSpec, removing this prevents the jump at the end of the fling:
snapAnimationSpec = tween(150, 0)
ak...@google.com <ak...@google.com> #12
Using a snapAnimationSpec of spring(Spring.DampingRatioNoBouncy, 2000f)
avoids the double jump but still ensures the page contents settles quickly (within 150ms). See attached slowmo videos of before and and after functionality of going from tween to spring animation, as you can see the double jump does not happen in the PagerSpring video.
ap...@google.com <ap...@google.com> #13
Project: platform/frameworks/support
Branch: androidx-main
Author: Aman Khosa <
Link:
Change Wear Compose foundation pager snapAnimationSpec
Expand for full commit details
Change Wear Compose foundation pager snapAnimationSpec
Currently the snapAnimationSpec is set to a tween with a duration
of 150ms. This was set to ensure the Pager settles quickly so that
contents are focused and clickable, see b/345191109.
However a tween animation causes an ugly double jump when flinging
fast to the top or bottom of a SLC inside of a VerticalPager, see
b/349781047.
Changing to a Spring animation with no bounciness and high
stiffness achieves the same behaviour of settling quickly, and
avoids the double jump. Added a test to ensure this is the case.
Bug: 303807950
Bug: 345191109
Bug: 350466286
Bug: 349781047
Test: Added test case to PagerTest.kt
Change-Id: I9bfefca9998e856279d63d2595f3bed0d67639bb
Files:
- M
wear/compose/compose-foundation/src/androidTest/kotlin/androidx/wear/compose/foundation/PagerTest.kt
- M
wear/compose/compose-foundation/src/main/java/androidx/wear/compose/foundation/pager/Pager.kt
Hash: 335faecba455a29d7a21c5e369b7c95ee4a5c1ce
Date: Thu Oct 24 12:42:02 2024
ak...@google.com <ak...@google.com> #14
This issue is now fixed in the Wear HorizontalPager, the double jump is no longer visible with the new snapAnimationSpec. This fix has also been implemented in Horologist here:
gr...@google.com <gr...@google.com> #15
ap...@google.com <ap...@google.com> #16
Project: platform/frameworks/support
Branch: androidx-main
Author: Aman Khosa <
Link:
Change Wear foundation pager snapAnimationSpec
Expand for full commit details
Change Wear foundation pager snapAnimationSpec
Currently the snapAnimationSpec is set to a tween with a duration
of 150ms. This was set to ensure the Pager settles quickly so that
contents are focused and clickable, see b/345191109.
However a tween animation causes an ugly double jump when flinging
fast to the top or bottom of a SLC inside of a VerticalPager, see
b/349781047.
Changing to a Spring animation with no bounciness and high
stiffness achieves the same behaviour of settling quickly, and
avoids the double jump. Added a test to ensure this is the case.
Test to ensure the Pager settles quickly can be ran locally.
Bug: 303807950
Bug: 345191109
Bug: 350466286
Bug: 349781047
Test: Added test case to PagerTest.kt
Relnote: "Change pager snapAnimationSpec from Tween to Spring"
Change-Id: I10d0275b7f3b957af279893d2ed52d63e42b8115
Files:
- M
wear/compose/compose-foundation/api/current.txt
- M
wear/compose/compose-foundation/api/restricted_current.txt
- M
wear/compose/compose-foundation/src/androidTest/kotlin/androidx/wear/compose/foundation/PagerTest.kt
- M
wear/compose/compose-foundation/src/main/java/androidx/wear/compose/foundation/pager/Pager.kt
Hash: 2850a0327fe61851a8f53f70add7b5a0e3b11228
Date: Thu Oct 24 12:42:02 2024
gr...@google.com <gr...@google.com> #17
na...@google.com <na...@google.com> #18
The following release(s) address this bug.It is possible this bug has only been partially addressed:
androidx.wear.compose:compose-foundation:1.5.0-alpha06
Description
Component used: Horologist master Version used: Master from 06/27 Devices/Android versions reproduced on: Any
When a ScalingLazyColumn is in a pager, fling creates a very ugly jump when settling at top or bottom.
Repro:https://github.com/Tolriq/horologist/tree/repro_jump run the media.sample swipe to 2nd screen and swipe up / down to fling to top or bottom to see the jump.
See attached video.