Velocity Tracker has Issue [223440806]

Fixed

Bug

Status Update

No update yet.

Description

al...@gmail.com

created issue #1

Mar 9, 2022 07:50AM

Velocity Tracker in compose has an issue.
When I scroll on a composable and concurrently change offset of that composable by scroll delta ,every point that add to Velocity Tracker approximately has same offset , so velocity calculate incorrect.
The issue can be solved if drag amount be used instead of offset of pointer.
When layout is fixed it work well but when layout move by changing offset or translation this issue will be happen.

Comments

so...@google.com <so...@google.com> Mar 10, 2022 05:53PM

Assigned to je...@google.com.

ni...@google.com <ni...@google.com> Mar 16, 2022 03:26PM

Reassigned to co...@google.com.

le...@google.com <le...@google.com> Apr 11, 2022 11:15AM

Reassigned to le...@google.com.

le...@google.com <le...@google.com> #2Apr 12, 2022 02:06PM

Hey thank you for filing this issue. Could you provide a sample code that we can use to verify the described behavior?

Thank you!

al...@gmail.com <al...@gmail.com> #3Apr 13, 2022 12:01PM

in this code i use NestedScrollConnection on LazyColumn and when offset change the given velocity in onPreFling method is incorrect.
for example i move my finger up on screen and you can see in following logcat velocity in y axis is positive

TAG: velocity:(0.0, 388.90985) px/sec offset:780.03516
TAG: velocity:(0.0, 743.0752) px/sec offset:667.8938
TAG: velocity:(0.0, 292.08224) px/sec offset:583.4952
TAG: velocity:(0.0, 993.74994) px/sec offset:471.7256
TAG: velocity:(0.0, 638.26825) px/sec offset:404.34738
TAG: velocity:(0.0, 482.3507) px/sec offset:329.2629
TAG: velocity:(0.0, 233.61378) px/sec offset:247.59741
TAG: velocity:(0.0, 758.8499) px/sec offset:158.94847
TAG: velocity:(0.0, 679.20667) px/sec offset:87.78984
TAG: velocity:(0.0, 725.84314) px/sec offset:2.3688889

and when offset reach to zero ,
i don't decrease offset. so
layout is fixed,but now when i move my finger up the velocity in y axis is negative

TAG: velocity:(0.0, -929.37604) px/sec offset:0.0
TAG: velocity:(0.0, -2.2106879) px/sec offset:0.0
TAG: velocity:(0.0, -472.8846) px/sec offset:0.0
TAG: velocity:(0.0, -1433.1675) px/sec offset:0.0
TAG: velocity:(0.0, -1240.49) px/sec offset:0.0

const val max = 900f
@Composable
fun NestedScrollTest() {
var offset by remember {
mutableStateOf(max)
}
fun scrollBy(ds: Float): Float {
if (offset == 0f && ds < 0f) return 0.0f
if (offset == max && ds > 0f) return 0.0f
val pv = offset
offset = (ds + offset).coerceIn(0f, max)
return offset - pv
}
val nestedScrollConnection = remember {
object : NestedScrollConnection {
override fun onPreScroll(
available: Offset,
source: NestedScrollSource
): Offset {
return if (available.y < 0) {
val consumed = scrollBy(available.y)
Offset(x = 0f, y = consumed)
} else Offset.Zero
}
override fun onPostScroll(
consumed: Offset,
available: Offset,
source: NestedScrollSource
): Offset {
return if (abs(available.y) > 0f &&
available.y > 0f
) {
Offset(0f, scrollBy(available.y))
} else
super.onPostScroll(consumed, available, source)
}

override suspend fun onPreFling(available: Velocity): Velocity {
Log.i(TAG, "velocity:$available offset:$offset")
return super.onPreFling(available)
}

}
}

Column(modifier = Modifier.fillMaxSize()) {
Box(
modifier = Modifier
.fillMaxWidth()
.height(80.dp)
)
LazyColumn(
modifier = Modifier
.graphicsLayer {
translationY = offset
}
.nestedScroll(connection = nestedScrollConnection)
.fillMaxWidth()
.weight(1f)
) {
items(100) {
Box(
modifier = Modifier
.fillMaxWidth()
.height(60.dp)
.background(Color.Blue)
)
Spacer(modifier = Modifier.height(8.dp))
}
}
}
}

le...@google.com <le...@google.com> #4Apr 20, 2022 05:16PM

Hi, thank you for filing this and proving a sample.

We're updating the way velocity is calculated in the velocity tracker, but the way we add points will remain the same. I agree that the value presented by the onPreFling method seems strange, but you need to take into consideration that it hasn't been consumed by the children of the given NestedScrollConnection. Ideally, you might want to have a look at the onPostFling method where we report the velocity available after the consumption by the children (in you case the LazyColumn is automatically added as part of the nested scroll system, so it will consume part of the velocity). This is because we want to let the LazyColumn consume everything it can before we take the rest of the velocity and use it to change its offset. I took the liberty to update your sample so you can see how that would be achieved. Let me know if that's your intention. Let me know what you find.

    val flingSpec = rememberSplineBasedDecay<Float>()
    val nestedScrollConnection = remember {
        object : NestedScrollConnection {
           
            ...

            override suspend fun onPostFling(consumed: Velocity, available: Velocity): Velocity {
                val initialVelocity = available.y
                return if (abs(initialVelocity) > 1f) {
                    var velocityLeft = initialVelocity
                    var lastValue = 0f
                    AnimationState(
                        initialValue = 0f,
                        initialVelocity = initialVelocity,
                    ).animateDecay(flingSpec) {
                        val delta = value - lastValue
                        val consumed = scrollBy(delta)
                        lastValue = value
                        velocityLeft = this.velocity
                        // It might be necessary to cancel the animation if the fling direction change, this.cancelAnimation()
                    }
                    Velocity(available.x, velocityLeft)
                } else {
                    Velocity(available.x, initialVelocity)
                }

            }
        }
    }

ap...@google.com <ap...@google.com> #5May 19, 2022 03:44PM

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

commit 5b42a22514d0847dbce104a9f12216afa555b2bd
Author: Levi Albuquerque <levima@google.com>
Date: Fri May 13 13:06:45 2022

Improvements on how to calculate velocity based on pointer position

In this CL I updated the way we add data from an InputEventChange into
the velocity tracker. Now we add points deltas instead of event positions.
This will guarantee that points are consistent even if the target element
is moving, which was not considering when adding the input event position.

Test: Added tests to check the behaviour in scrolling.
Fixes: 216582726
Bug: 223440806
Bug: 227709803

Relnote: "When adding InputEventChange events to Velocity Tracker we will
consider now deltas instead of positions, this will guarantee the velocity
is correctly calculated for all cases even if the target element moves"

Change-Id: I51ec384a0424829b680b4666c7d3ce49227f45de

M compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/input/pointer/util/VelocityTracker.kt
M compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/input/nestedscroll/NestedScrollModifierTest.kt

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

le...@google.com <le...@google.com> May 19, 2022 03:47PM

Marked as fixed.

le...@google.com <le...@google.com> May 29, 2022 10:42AM

Status: Assigned (reopened)

ap...@google.com <ap...@google.com> #6Jun 8, 2022 09:03AM

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

commit a66e75c2cb06d1d754055093e47cbdb6162d7aeb
Author: Levi Albuquerque <levima@google.com>
Date: Fri May 27 10:59:21 2022

Improvements on how to calculate velocity based on pointer position

In this CL I updated the way we add data from an InputEventChange into
the velocity tracker. Now we add points deltas instead of event positions.
This will guarantee that points are consistent even if the target element
is moving, which was not considering when adding the input event position.

Test: Added tests to check the behaviour in scrolling.
Fixes: 216582726
Bug: 223440806
Bug: 227709803

Relnote: "When adding InputEventChange events to Velocity Tracker we will
consider now deltas instead of positions, this will guarantee the velocity
is correctly calculated for all cases even if the target element moves"

Change-Id: If9ef3b9069e8f15b9b049bc3cd8b08bfd91edd49

M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/ScrollableTest.kt
M compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/input/pointer/util/VelocityTracker.kt
M compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/input/nestedscroll/NestedScrollModifierTest.kt

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

ap...@google.com <ap...@google.com> #7Jun 28, 2022 07:29PM

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

commit 4e1e7aa76fe04d05362fa5cc319bd490401dfa61
Author: Levi Albuquerque <levima@google.com>
Date: Thu Jun 09 13:47:56 2022

Improvements on how to calculate velocity based on pointer position

In this CL I updated the way we add data from an InputEventChange into
the velocity tracker. Now we add points deltas instead of event positions.
This will guarantee that points are consistent even if the target element
is moving, which was not considering when adding the input event position.

Test: Added tests to check the behaviour in scrolling.
Fixes: 216582726
Bug: 223440806
Bug: 227709803

Relnote: "When adding InputEventChange events to Velocity Tracker we will
consider now deltas instead of positions, this will guarantee the velocity
is correctly calculated for all cases even if the target element moves"

Change-Id: Icea9d76a43643a6b17da11f3c539d27cb8fa6f6e

M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/list/LazyListTest.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/ScrollableTest.kt
M compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/input/pointer/util/VelocityTracker.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/list/BaseLazyListTestWithOrientation.kt
M compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/input/nestedscroll/NestedScrollModifierTest.kt

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

le...@google.com <le...@google.com> Jun 28, 2022 07:30PM