Skip to content

Commit

Permalink
Fixed unexpected fling animation over scrolling content (#1039)
Browse files Browse the repository at this point in the history
  • Loading branch information
ASalavei committed Feb 5, 2024
1 parent 217cc36 commit be1d75a
Show file tree
Hide file tree
Showing 10 changed files with 202 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import androidx.compose.ui.platform.AccessibilitySyncOptions
import androidx.compose.ui.window.ComposeUIViewController
import bugs.IosBugs
import bugs.ProperContainmentDisposal
import bugs.ComposeAndNativeScroll
import bugs.StartRecompositionCheck
import platform.UIKit.UIViewController

Expand Down Expand Up @@ -42,6 +43,7 @@ fun IosDemo(arg: String, makeHostingController: ((Int) -> UIViewController)? = n
NativeModalWithNaviationExample,
UIKitViewOrder,
ProperContainmentDisposal,
ComposeAndNativeScroll
) + listOf(makeHostingController).mapNotNull {
it?.let {
SwiftUIInteropExample(it)
Expand Down
121 changes: 121 additions & 0 deletions compose/mpp/demo/src/uikitMain/kotlin/bugs/ComposeAndNativeScroll.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
/*
* Copyright 2024 The Android Open Source Project
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package bugs

import androidx.compose.foundation.background
import androidx.compose.foundation.horizontalScroll
import androidx.compose.foundation.layout.Box
import androidx.compose.foundation.layout.Column
import androidx.compose.foundation.layout.Row
import androidx.compose.foundation.layout.Spacer
import androidx.compose.foundation.layout.fillMaxSize
import androidx.compose.foundation.layout.fillMaxWidth
import androidx.compose.foundation.layout.height
import androidx.compose.foundation.layout.size
import androidx.compose.foundation.rememberScrollState
import androidx.compose.material.Text
import androidx.compose.mpp.demo.Screen
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.graphics.Color
import androidx.compose.ui.interop.UIKitView
import androidx.compose.ui.unit.dp
import androidx.compose.ui.util.fastForEachIndexed
import platform.CoreGraphics.CGRectMake
import platform.CoreGraphics.CGSizeMake
import platform.UIKit.UIColor
import platform.UIKit.UIScrollView
import platform.UIKit.UIView

private val ItemsHeight: Double = 200.0
private val ItemsWidth: Double = 100.0
private val Colors = listOf(
Color.Blue,
Color.Red,
Color.Green,
Color.Black,
Color.Cyan,
Color.Gray,
Color.Red,
Color.Yellow,
Color.LightGray,
Color.Magenta
)

val ComposeAndNativeScroll = Screen.Example("ScrollDraggingTest") {
Column(
modifier = Modifier.fillMaxSize(),
horizontalAlignment = Alignment.CenterHorizontally
) {
Text("Compose:")
Box(
modifier = Modifier
.fillMaxWidth()
.height(ItemsHeight.dp)
.horizontalScroll(rememberScrollState())
) {
Row(modifier = Modifier.height(ItemsHeight.dp)) {
(Colors + Colors).forEach { item ->
Box(
Modifier
.size(width = ItemsWidth.dp, height = ItemsHeight.dp)
.background(item)
)
}
}
}
Spacer(Modifier.height(10.dp))
Text("UIKit:")
Box(modifier = Modifier.height(ItemsHeight.dp)) {
UIKitView(::makeScrollView, Modifier.height(ItemsHeight.dp))
}
}
}

private fun makeScrollView(): UIScrollView {
val scroll = UIScrollView()
val uiColors = listOf(
UIColor.blueColor,
UIColor.redColor,
UIColor.greenColor,
UIColor.blackColor,
UIColor.cyanColor,
UIColor.grayColor,
UIColor.redColor,
UIColor.yellowColor,
UIColor.brownColor,
UIColor.magentaColor
)
scroll.setContentSize(
CGSizeMake(width = uiColors.count() * 2 * ItemsWidth, height = ItemsHeight)
)

(uiColors + uiColors).fastForEachIndexed { i, uiColor ->
scroll.addSubview(
UIView(
frame = CGRectMake(
i.toDouble() * ItemsWidth,
0.0,
ItemsWidth,
ItemsHeight
)
).also {
it.setBackgroundColor(uiColor)
})
}
return scroll
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,14 @@

package androidx.compose.ui.input.pointer.util

internal actual const val AssumePointerMoveStoppedMilliseconds: Int = 40
internal actual const val HistorySize: Int = 20

/**
* Some platforms (e.g. iOS) ignore certain events during velocity calculation.
* Some platforms (e.g. iOS) filter certain gestures during velocity calculation.
*/
internal actual fun VelocityTracker.shouldUse(event: PointerInputChange): Boolean = true
internal actual fun VelocityTracker1D.shouldUseDataPoints(
points: FloatArray,
times: FloatArray,
count: Int
): Boolean = true
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ import kotlin.math.abs
import kotlin.math.sign
import kotlin.math.sqrt

private const val AssumePointerMoveStoppedMilliseconds: Int = 40
private const val HistorySize: Int = 20
internal expect val AssumePointerMoveStoppedMilliseconds: Int
internal expect val HistorySize: Int

// TODO(b/204895043): Keep value in sync with VelocityPathFinder.HorizonMilliSeconds
private const val HorizonMilliseconds: Int = 100
Expand Down Expand Up @@ -139,21 +139,21 @@ class VelocityTracker1D internal constructor(

/**
* Constructor to create a new velocity tracker. It allows to specify whether or not the tracker
* should consider the data ponits provided via [addDataPoint] as differential or
* should consider the data points provided via [addDataPoint] as differential or
* non-differential.
*
* Differential data ponits represent change in displacement. For instance, differential data
* Differential data points represent change in displacement. For instance, differential data
* points of [2, -1, 5] represent: the object moved by "2" units, then by "-1" units, then by
* "5" units. An example use case for differential data points is when tracking velocity for an
* object whose displacements (or change in positions) over time are known.
*
* Non-differential data ponits represent position of the object whose velocity is tracked. For
* Non-differential data points represent position of the object whose velocity is tracked. For
* instance, non-differential data points of [2, -1, 5] represent: the object was at position
* "2", then at position "-1", then at position "5". An example use case for non-differential
* data points is when tracking velocity for an object whose positions on a geometrical axis
* over different instances of time are known.
*
* @param isDataDifferential [true] if the data ponits provided to the constructed tracker
* @param isDataDifferential [true] if the data points provided to the constructed tracker
* are differential. [false] otherwise.
*/
constructor(isDataDifferential: Boolean) : this(isDataDifferential, Strategy.Impulse)
Expand Down Expand Up @@ -228,6 +228,7 @@ class VelocityTracker1D internal constructor(
val newestSample: DataPointAtTime = samples[index] ?: return 0f

var previousSample: DataPointAtTime = newestSample
var previousDirection: Boolean? = null

// Starting with the most recent PointAtTime sample, iterate backwards while
// the samples represent continuous motion.
Expand All @@ -249,7 +250,7 @@ class VelocityTracker1D internal constructor(
sampleCount += 1
} while (sampleCount < HistorySize)

if (sampleCount >= minSampleSize) {
if (sampleCount >= minSampleSize && shouldUseDataPoints(dataPoints, time, sampleCount)) {
// Choose computation logic based on strategy.
return when (strategy) {
Strategy.Impulse -> {
Expand Down Expand Up @@ -348,7 +349,12 @@ private fun Array<DataPointAtTime?>.set(index: Int, time: Long, dataPoint: Float
/**
* Some platforms (e.g. iOS) ignore certain events during velocity calculation.
*/
internal expect fun VelocityTracker.shouldUse(event: PointerInputChange): Boolean
internal expect fun VelocityTracker1D.shouldUseDataPoints(
points: FloatArray,
times: FloatArray,
count: Int
): Boolean


/**
* Track the positions and timestamps inside this event change.
Expand Down Expand Up @@ -383,10 +389,6 @@ private fun VelocityTracker.addPointerInputChangeLegacy(event: PointerInputChang
resetTracking()
}

if (!shouldUse(event)) {
return
}

// To calculate delta, for each step we want to do currentPosition - previousPosition.
// Initially the previous position is the previous position of the current event
var previousPointerPosition = event.previousPosition
Expand Down Expand Up @@ -576,7 +578,7 @@ internal fun polyFitLeastSquares(
* should be provided in reverse chronological order. The returned velocity is in "units/ms",
* where "units" is unit of the [dataPoints].
*
* Calculates the resulting velocity based on the total immpulse provided by the data ponits.
* Calculates the resulting velocity based on the total impulse provided by the data points.
*
* The moving object in these calculations is the touchscreen (if we are calculating touch
* velocity), or any input device from which the data points are generated. We refer to this
Expand Down Expand Up @@ -609,7 +611,7 @@ internal fun polyFitLeastSquares(
* The final formula is:
* vfinal = sqrt(2) * sqrt(sum((v[i]-v[i-1])*|v[i]|)) for all i
* The absolute value is needed to properly account for the sign. If the velocity over a
* particular segment descreases, then this indicates braking, which means that negative
* particular segment decreases, then this indicates braking, which means that negative
* work was done. So for two positive, but decreasing, velocities, this contribution would be
* negative and will cause a smaller final velocity.
*
Expand Down Expand Up @@ -661,7 +663,7 @@ private fun calculateImpulseVelocity(
return 0f
}
val dataPointsDelta =
// For differential data ponits, each measurement reflects the amount of change in the
// For differential data points, each measurement reflects the amount of change in the
// subject's position. However, the first sample is discarded in computation because we
// don't know the time duration over which this change has occurred.
if (isDataDifferential) dataPoints[0]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,14 @@

package androidx.compose.ui.input.pointer.util

import androidx.compose.ui.input.pointer.PointerInputChange
internal actual const val AssumePointerMoveStoppedMilliseconds: Int = 40
internal actual const val HistorySize: Int = 20

/**
* Some platforms (e.g. iOS) ignore certain events during velocity calculation.
* Some platforms (e.g. iOS) filter certain gestures during velocity calculation.
*/
internal actual fun VelocityTracker.shouldUse(event: PointerInputChange): Boolean = true
internal actual fun VelocityTracker1D.shouldUseDataPoints(
points: FloatArray,
times: FloatArray,
count: Int
): Boolean = true
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,7 @@ internal class SyntheticEventSender(
type,
issuesEnterExit,
scrollDelta = Offset(0f, 0f),
historical = emptyList() // we don't copy historical for synthetic
historical = emptyList(), // we don't copy historical for synthetic
originalEventPosition = position
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ class TestPointerInputEventData(
down,
pressure = 1.0f,
PointerType.Mouse,
historical = listOf()
historical = listOf(),
originalEventPosition = position
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -118,16 +118,17 @@ internal fun PointerInputEvent(
timeMillis,
pointers.map {
PointerInputEventData(
it.id,
timeMillis,
it.position,
it.position,
it.pressed,
it.pressure,
it.type,
id = it.id,
uptime = timeMillis,
positionOnScreen = it.position,
position = it.position,
down = it.pressed,
pressure = it.pressure,
type = it.type,
issuesEnterExit = it.type == PointerType.Mouse,
historical = it.historical,
scrollDelta = scrollDelta
scrollDelta = scrollDelta,
originalEventPosition = it.position
)
},
buttons,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,35 @@

package androidx.compose.ui.input.pointer.util

import androidx.compose.ui.input.pointer.PointerInputChange
import kotlin.math.abs

internal actual const val AssumePointerMoveStoppedMilliseconds: Int = 100
internal actual const val HistorySize: Int = 40 // Increased to store history on 120 Hz devices

private const val MinimumGestureDurationMilliseconds: Int = 50
private const val MinimumGestureSpeed: Float = 1.0f // Minimum tracking speed, dp/ms

/**
* Some platforms (e.g. iOS) ignore certain events during velocity calculation.
* Some platforms (e.g. iOS) filter certain gestures during velocity calculation.
*/
internal actual fun VelocityTracker.shouldUse(event: PointerInputChange): Boolean = event.pressed
internal actual fun VelocityTracker1D.shouldUseDataPoints(
points: FloatArray,
times: FloatArray,
count: Int
): Boolean {
if (count == 0) {
return false
}

val timeDelta = abs(times[0] - times[count - 1])
if (timeDelta < MinimumGestureDurationMilliseconds) {
return false
}

val distance = abs(points[0] - points[count - 1])
if (distance / timeDelta < MinimumGestureSpeed) {
return false
}

return true
}
Original file line number Diff line number Diff line change
Expand Up @@ -627,9 +627,11 @@ private fun UIEvent.historicalChangesForTouch(
// because it's the actual touch for which coalesced touches were requested
touches.dropLast(1).map {
val historicalTouch = it as UITouch
val position = historicalTouch.offsetInView(view, density)
HistoricalChange(
uptimeMillis = (historicalTouch.timestamp * 1e3).toLong(),
position = historicalTouch.offsetInView(view, density)
position = position,
originalEventPosition = position
)
}
} else {
Expand Down

0 comments on commit be1d75a

Please sign in to comment.