Skip to content

Commit 48852c8

Browse files
ASalaveiASalavei
authored
Fix crash in CupertinoOverscrollEffect (#2456)
Fix imbalanced touches calculation to prevent assertion. Add multitouch scroll tests. Fixes https://youtrack.jetbrains.com/issue/CMP-9028/iOS-Multiple-scrollable-components-with-CupertinoOverscrollEffectFactory-crash-when-scrolling-and-releasing-fingers-at-the-same ## Release Notes ### Fixes - iOS - Fix crash when dragging two Scrollable components with two fingers
1 parent e689fde commit 48852c8

File tree

7 files changed

+197
-17
lines changed

7 files changed

+197
-17
lines changed

‎compose/foundation/foundation/src/uikitMain/kotlin/androidx/compose/foundation/cupertino/CupertinoOverscrollEffect.kt‎

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@ import androidx.compose.ui.input.nestedscroll.NestedScrollSource
3636
import androidx.compose.ui.input.pointer.PointerEvent
3737
import androidx.compose.ui.input.pointer.PointerEventPass
3838
import androidx.compose.ui.input.pointer.PointerEventType
39+
import androidx.compose.ui.input.pointer.changedToDown
40+
import androidx.compose.ui.input.pointer.changedToUp
3941
import androidx.compose.ui.layout.Measurable
4042
import androidx.compose.ui.layout.MeasureResult
4143
import androidx.compose.ui.layout.MeasureScope
@@ -57,6 +59,7 @@ import kotlin.math.abs
5759
import kotlin.math.sign
5860
import kotlinx.coroutines.CoroutineScope
5961
import kotlinx.coroutines.cancel
62+
import kotlinx.coroutines.currentCoroutineContext
6063
import kotlinx.coroutines.isActive
6164

6265
private enum class CupertinoScrollSource {
@@ -389,7 +392,7 @@ internal class CupertinoOverscrollEffect(
389392
}
390393

391394
springAnimationScope?.cancel()
392-
springAnimationScope = CoroutineScope(coroutineContext)
395+
springAnimationScope = CoroutineScope(currentCoroutineContext())
393396
springAnimationScope?.run {
394397
AnimationState(
395398
Float.VectorConverter,
@@ -413,7 +416,7 @@ internal class CupertinoOverscrollEffect(
413416
springAnimationScope = null
414417
}
415418

416-
if (coroutineContext.isActive) {
419+
if (currentCoroutineContext().isActive) {
417420
// The spring is critically damped, so in case spring-fling-spring sequence is slightly
418421
// offset and velocity is of the opposite sign, it will end up with no animation
419422
overscrollOffset = Offset.Zero
@@ -471,13 +474,15 @@ private class CupertinoOverscrollNode(
471474
pass: PointerEventPass,
472475
bounds: IntSize
473476
) {
474-
if (pass == PointerEventPass.Final) {
475-
if (pointerEvent.type == PointerEventType.Press) {
476-
pointersDown++
477-
} else if (pointerEvent.type == PointerEventType.Release) {
478-
pointersDown--
479-
assert(pointersDown >= 0) { "pointersDown cannot be negative" }
477+
if (pass == PointerEventPass.Initial) {
478+
pointerEvent.changes.forEach { change ->
479+
if (change.changedToDown()) {
480+
pointersDown++
481+
} else if (change.changedToUp()) {
482+
pointersDown--
483+
}
480484
}
485+
assert(pointersDown >= 0) { "pointersDown cannot be negative" }
481486
}
482487
}
483488

‎compose/ui/ui/src/uikitInstrumentedTest/kotlin/androidx/compose/ui/interaction/BasicInteractionTest.kt‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,23 +9,35 @@ import androidx.compose.foundation.ComposeFoundationFlags
99
import androidx.compose.foundation.ExperimentalFoundationApi
1010
import androidx.compose.foundation.ScrollState
1111
import androidx.compose.foundation.background
12+
import androidx.compose.foundation.gestures.awaitEachGesture
13+
import androidx.compose.foundation.horizontalScroll
1214
import androidx.compose.foundation.layout.Box
1315
import androidx.compose.foundation.layout.Column
16+
import androidx.compose.foundation.layout.Row
1417
import androidx.compose.foundation.layout.fillMaxSize
1518
import androidx.compose.foundation.layout.fillMaxWidth
1619
import androidx.compose.foundation.layout.height
1720
import androidx.compose.foundation.layout.safeDrawingPadding
21+
import androidx.compose.foundation.layout.size
1822
import androidx.compose.foundation.text.BasicTextField
1923
import androidx.compose.foundation.text.input.TextFieldState
2024
import androidx.compose.foundation.verticalScroll
2125
import androidx.compose.material.Button
2226
import androidx.compose.material.Text
2327
import androidx.compose.material.TextField
2428
import androidx.compose.runtime.MutableState
29+
import androidx.compose.runtime.getValue
2530
import androidx.compose.runtime.mutableStateOf
31+
import androidx.compose.runtime.remember
32+
import androidx.compose.runtime.setValue
2633
import androidx.compose.ui.Alignment
2734
import androidx.compose.ui.Modifier
2835
import androidx.compose.ui.graphics.Color
36+
import androidx.compose.ui.input.pointer.PointerEventPass
37+
import androidx.compose.ui.input.pointer.PointerEventType
38+
import androidx.compose.ui.input.pointer.changedToDown
39+
import androidx.compose.ui.input.pointer.changedToUp
40+
import androidx.compose.ui.input.pointer.pointerInput
2941
import androidx.compose.ui.layout.boundsInWindow
3042
import androidx.compose.ui.layout.onGloballyPositioned
3143
import androidx.compose.ui.platform.testTag
@@ -35,6 +47,7 @@ import androidx.compose.ui.test.findNodeWithLabel
3547
import androidx.compose.ui.test.findNodeWithTag
3648
import androidx.compose.ui.test.firstNodeOrNull
3749
import androidx.compose.ui.test.runUIKitInstrumentedTest
50+
import androidx.compose.ui.test.utils.up
3851
import androidx.compose.ui.text.input.TextFieldValue
3952
import androidx.compose.ui.unit.DpOffset
4053
import androidx.compose.ui.unit.DpRect
@@ -46,6 +59,7 @@ import kotlin.test.Test
4659
import kotlin.test.assertEquals
4760
import kotlin.test.assertFalse
4861
import kotlin.test.assertTrue
62+
import kotlin.time.Duration.Companion.seconds
4963
import kotlinx.cinterop.ExperimentalForeignApi
5064
import platform.UIKit.UIPasteboard
5165

@@ -230,6 +244,64 @@ class BasicInteractionTest {
230244
assertTrue(textFieldState.isFullySelected())
231245
}
232246

247+
@Test
248+
fun testTapsCountingWithMultiTouch() = runUIKitInstrumentedTest {
249+
var touchesDown = 0
250+
var touchesUp = 0
251+
252+
setContent {
253+
Column {
254+
Box(
255+
modifier = Modifier
256+
.size(250.dp)
257+
.testTag("Box 1")
258+
.pointerInput(Unit) {
259+
awaitEachGesture {
260+
while (true) {
261+
val event = awaitPointerEvent(pass = PointerEventPass.Initial)
262+
event.changes.forEach { change ->
263+
if (change.changedToDown()) {
264+
touchesDown++
265+
} else if (change.changedToUp()) {
266+
touchesUp++
267+
}
268+
}
269+
}
270+
}
271+
}
272+
)
273+
Box(
274+
modifier = Modifier
275+
.size(250.dp)
276+
.testTag("Box 2")
277+
.pointerInput(Unit) {
278+
awaitEachGesture {
279+
while (true) {
280+
awaitPointerEvent(pass = PointerEventPass.Initial)
281+
}
282+
}
283+
}
284+
)
285+
}
286+
}
287+
288+
val tap1 = findNodeWithTag("Box 1").touchDown()
289+
val tap2 = findNodeWithTag("Box 2").touchDown()
290+
291+
assertEquals(1, touchesDown)
292+
assertEquals(0, touchesUp)
293+
294+
tap1.dragBy(dx = 20.dp, duration = 0.1.seconds)
295+
tap2.dragBy(dx = 20.dp, duration = 0.1.seconds)
296+
297+
tap1.up()
298+
tap2.up()
299+
waitForIdle()
300+
301+
assertEquals(1, touchesDown)
302+
assertEquals(1, touchesUp)
303+
}
304+
233305
@OptIn(ExperimentalFoundationApi::class)
234306
private fun runContextMenuTest(
235307
newContextMenuEnabled: Boolean,

‎compose/ui/ui/src/uikitInstrumentedTest/kotlin/androidx/compose/ui/scroll/ScrollTest.kt‎

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,10 @@
1717
package androidx.compose.ui.scroll
1818

1919
import androidx.compose.foundation.ScrollState
20-
import androidx.compose.foundation.background
20+
import androidx.compose.foundation.horizontalScroll
2121
import androidx.compose.foundation.layout.Box
2222
import androidx.compose.foundation.layout.Column
23+
import androidx.compose.foundation.layout.Row
2324
import androidx.compose.foundation.layout.fillMaxSize
2425
import androidx.compose.foundation.layout.fillMaxWidth
2526
import androidx.compose.foundation.layout.height
@@ -36,6 +37,7 @@ import androidx.compose.runtime.remember
3637
import androidx.compose.runtime.setValue
3738
import androidx.compose.ui.Alignment
3839
import androidx.compose.ui.Modifier
40+
import androidx.compose.ui.background
3941
import androidx.compose.ui.graphics.Color
4042
import androidx.compose.ui.layout.boundsInWindow
4143
import androidx.compose.ui.layout.onGloballyPositioned
@@ -848,6 +850,45 @@ internal class ScrollTest {
848850
assertEquals(DpRect(DpOffset(x = 0.dp, y = 100.dp), DpSize(screenSize.width, 200.dp)), uiKitViewRect())
849851
assertEquals(DpOffset.Zero, contentOffset())
850852
}
853+
854+
@Test
855+
fun testMultiTouchScroll() = runUIKitInstrumentedTest {
856+
val state1 = ScrollState(0)
857+
val state2 = ScrollState(0)
858+
setContent {
859+
Column(modifier = Modifier.fillMaxSize()) {
860+
Row(modifier = Modifier.testTag("Row 1").horizontalScroll(state1)) {
861+
repeat(20) {
862+
Box(modifier = Modifier.size(200.dp))
863+
}
864+
}
865+
Row(modifier = Modifier.testTag("Row 2").horizontalScroll(state2)) {
866+
repeat(20) {
867+
Box(modifier = Modifier.size(200.dp))
868+
}
869+
}
870+
}
871+
}
872+
873+
val tap1 = findNodeWithTag("Row 1").touchDown()
874+
val tap2 = findNodeWithTag("Row 2").touchDown()
875+
876+
waitForIdle()
877+
878+
// Simulate simultaneous drag of two fingers
879+
repeat(2) {
880+
tap1.dragBy(dx = (-25).dp, duration = (0.5).seconds)
881+
tap2.dragBy(dx = (-50).dp, duration = (0.5).seconds)
882+
}
883+
884+
tap1.up()
885+
tap2.up()
886+
887+
waitForIdle()
888+
889+
assertEquals((50 - CUPERTINO_TOUCH_SLOP) * density.density, state1.value.toFloat())
890+
assertEquals((100 - CUPERTINO_TOUCH_SLOP) * density.density, state2.value.toFloat())
891+
}
851892
}
852893

853894
@Composable

‎compose/ui/ui/src/uikitInstrumentedTest/kotlin/androidx/compose/ui/test/UIKitInstrumentedTest.kt‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import androidx.compose.ui.scene.ComposeHostingViewController
2323
import androidx.compose.ui.test.utils.center
2424
import androidx.compose.ui.test.utils.getTouchesEvent
2525
import androidx.compose.ui.test.utils.moveToLocationOnWindow
26+
import androidx.compose.ui.test.utils.resetTouches
2627
import androidx.compose.ui.test.utils.toCGPoint
2728
import androidx.compose.ui.test.utils.touchDown
2829
import androidx.compose.ui.test.utils.up
@@ -227,19 +228,19 @@ internal class UIKitInstrumentedTest {
227228
*
228229
* @param position The position on the root hosting controller.
229230
*/
230-
fun tap(position: DpOffset): UITouch {
231+
fun tap(position: DpOffset) {
231232
return touchDown(position).up()
232233
}
233234

234235
/**
235236
* Simulates a tap gesture for a given AccessibilityTestNode.
236237
*/
237-
fun AccessibilityTestNode.tap(): UITouch {
238+
fun AccessibilityTestNode.tap() {
238239
val frame = frame ?: error("Internal error. Frame is missing.")
239240
return tap(frame.center())
240241
}
241242

242-
fun AccessibilityTestNode.doubleTap(): UITouch {
243+
fun AccessibilityTestNode.doubleTap() {
243244
val frame = frame ?: error("Internal error. Frame is missing.")
244245
tap(frame.center())
245246
delay(50)
@@ -343,6 +344,7 @@ internal class MockAppDelegate: NSObject(), UIApplicationDelegateProtocol {
343344
window.resignKeyWindow()
344345
}
345346

347+
_window?.resetTouches()
346348
_window?.resignKeyWindow()
347349
_window?.windowScene = null
348350
_window?.rootViewController = UIViewController()
@@ -395,8 +397,8 @@ internal fun UIKitInstrumentedTest.findFocusedUITextInput(): UITextInputProtocol
395397
if (view.isFirstResponder) {
396398
return view
397399
}
398-
view.subviews.forEach {
399-
findFirstResponder(it as UIView)?.let { return it }
400+
view.subviews.forEach { view ->
401+
findFirstResponder(view as UIView)?.let { return it }
400402
}
401403
return null
402404
}

‎compose/ui/ui/src/uikitInstrumentedTest/kotlin/androidx/compose/ui/test/utils/UITouch+Utils.kt‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616

1717
package androidx.compose.ui.test.utils
1818

19+
import androidx.compose.test.utils.endAllTouches
20+
import androidx.compose.test.utils.endTouch
1921
import androidx.compose.test.utils.getTouchesEvent
2022
import androidx.compose.test.utils.send
2123
import androidx.compose.test.utils.setLocationInWindow
@@ -40,10 +42,16 @@ internal fun UIWindow.touchDown(location: DpOffset): UITouch {
4042
}
4143
}
4244

45+
@OptIn(ExperimentalForeignApi::class)
4346
internal fun UIWindow.getTouchesEvent(): UIEvent {
4447
return UITouch.getTouchesEvent()
4548
}
4649

50+
@OptIn(ExperimentalForeignApi::class)
51+
internal fun UIWindow.resetTouches() {
52+
UITouch.endAllTouches()
53+
}
54+
4755
@OptIn(ExperimentalForeignApi::class)
4856
internal fun UITouch.moveToLocationOnWindow(location: DpOffset) {
4957
setLocationInWindow(location.toCGPoint())
@@ -59,8 +67,8 @@ internal fun UITouch.hold(): UITouch {
5967
}
6068

6169
@OptIn(ExperimentalForeignApi::class)
62-
internal fun UITouch.up(): UITouch {
70+
internal fun UITouch.up() {
6371
setPhase(UITouchPhase.UITouchPhaseEnded)
6472
send()
65-
return this
73+
endTouch()
6674
}

‎testutils/testutils-xctest/src/uikitMain/objc/CMPTestUtils/UITouch+Test.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,14 @@ NS_ASSUME_NONNULL_BEGIN
2626
fromEdge:(BOOL)fromEdge;
2727

2828
+ (UIEvent *)getTouchesEvent;
29+
+ (void)endAllTouches;
2930

3031
@property (assign) UITouchPhase phase;
3132
@property (assign) CGPoint locationInWindow;
3233

3334
- (void)send;
3435
- (void)updateTimestamp;
36+
- (void)endTouch;
3537

3638
@end
3739

0 commit comments

Comments
 (0)