Skip to content

Commit

Permalink
iOS fix scroll issues (#776)
Browse files Browse the repository at this point in the history
## PR scope changed
The velocity calculation formula is a separate issue now, it will take
all iOS platform specific datapoints into account, so no input data
correction is needed.
Duplicating data points will be fixed in
#752.
 
## Proposed Changes
1. Refactor touch sending on iOS, throw SkikoPointerEvent out as
redundant
2. Send coalesced touches(higher than display refresh rate touches) as
historical data (don't send them for synthetics). Users can retrieve it
now and use in case they need accurate touches data.
3. ~Adjust velocity calculator to filter duplicated data points in a
single timestamp (because synthetic touches), leading to inadequate
velocity (unexpected scrolls to top)~
4. ~Override position reported in the `ended` phase with last known
position of the touch. Solves two problems~:
4.1. ~Current velocity calculator plays bad with that value, because
it's slightly exaggerated to help UIKit with proper fling calculation
(opaque implementation detail) and since iOS can report two events in
the same frame (`moved` and `ended` with exaggerated position with small
timestamp delta, it causes awkward unexpected motions in the end of
scroll which was not supposed to lead to a fling)~
4.2. ~Delta in `ended` phase is not taken into consideration when
scrolling UIScrollView (proved by reverse engineering). Passing that
metadata in call tree of Draggable implementation will require internal
common API changes.~

## Testing

Test: N/A

## Issues Fixed

Fixes: [inadequate fling velocity during "quick-drag-and-stop" touch
events
sequence](JetBrains/compose-multiplatform#3335).
~Awkward motion caused by `ended` delta in the end of the drag without a
fling~.

## API Change

`ComposeScene.Pointer` now has a new constructor argument `val
historical: List<HistoricalChange> = emptyList()`

## Video

~https://github.com/JetBrains/compose-multiplatform-core/assets/4167681/bd5f530f-d26e-4a65-ab54-8085b22b5e1d~

---------

Co-authored-by: dima.avdeev <dima.avdeev@jetbrains.com>
  • Loading branch information
elijah-semyonov and dima-avdeev-jb committed Sep 27, 2023
1 parent 95de7ad commit 3a95834
Show file tree
Hide file tree
Showing 6 changed files with 82 additions and 58 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ internal data class PointerInputEventData(
val pressure: Float,
val type: PointerType,
val issuesEnterExit: Boolean = false,
val historical: List<HistoricalChange> = mutableListOf(),
val historical: List<HistoricalChange>,
val scrollDelta: Offset = Offset.Zero
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -888,6 +888,8 @@ class ComposeScene internal constructor(
* Pressure of the pointer. 0.0 - no pressure, 1.0 - average pressure
*/
val pressure: Float = 1.0f,

val historical: List<HistoricalChange> = emptyList()
) {
override fun equals(other: Any?): Boolean {
if (this === other) return true
Expand Down Expand Up @@ -961,6 +963,7 @@ private fun pointerInputEvent(
it.pressed,
it.pressure,
it.type,
historical = it.historical,
scrollDelta = scrollDelta
)
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -214,5 +214,6 @@ internal class SyntheticEventSender(
type,
issuesEnterExit,
scrollDelta = Offset(0f, 0f),
historical = emptyList() // we don't copy historical for synthetic
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ class TestPointerInputEventData(
position,
down,
pressure = 1.0f,
PointerType.Mouse
PointerType.Mouse,
historical = listOf()
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,10 @@ import androidx.compose.ui.geometry.Offset
import androidx.compose.ui.geometry.Rect
import androidx.compose.ui.input.InputMode
import androidx.compose.ui.input.key.KeyEvent
import androidx.compose.ui.input.pointer.HistoricalChange
import androidx.compose.ui.input.pointer.PointerEventType
import androidx.compose.ui.input.pointer.PointerId
import androidx.compose.ui.input.pointer.PointerType
import androidx.compose.ui.input.pointer.toCompose
import androidx.compose.ui.interop.LocalLayerContainer
import androidx.compose.ui.interop.LocalUIKitInteropContext
Expand All @@ -40,12 +43,15 @@ import androidx.compose.ui.platform.*
import androidx.compose.ui.text.input.PlatformTextInputService
import androidx.compose.ui.uikit.*
import androidx.compose.ui.unit.*
import androidx.compose.ui.util.fastMap
import kotlin.math.floor
import kotlin.math.roundToInt
import kotlin.math.roundToLong
import kotlin.math.roundToLong
import kotlinx.cinterop.CValue
import kotlinx.cinterop.ExportObjCClass
import kotlinx.cinterop.ObjCAction
import kotlinx.cinterop.objcPtr
import kotlinx.cinterop.readValue
import kotlinx.cinterop.useContents
import kotlinx.coroutines.Dispatchers
Expand All @@ -54,10 +60,11 @@ import org.jetbrains.skiko.OS
import org.jetbrains.skiko.OSVersion
import org.jetbrains.skiko.SkikoKeyboardEvent
import org.jetbrains.skiko.SkikoPointerEvent
import org.jetbrains.skiko.currentNanoTime
import platform.CoreGraphics.CGPoint
import org.jetbrains.skiko.available
import platform.CoreGraphics.CGAffineTransformIdentity
import platform.CoreGraphics.CGAffineTransformInvert
import platform.CoreGraphics.CGPoint
import platform.CoreGraphics.CGPointMake
import platform.CoreGraphics.CGRectMake
import platform.CoreGraphics.CGSize
Expand Down Expand Up @@ -615,24 +622,27 @@ internal actual class ComposeWindow : UIViewController {
!scene.hitTestInteropView(position)
}

override fun onPointerEvent(event: SkikoPointerEvent) {
val scale = density.density
override fun onTouchesEvent(view: UIView, event: UIEvent, phase: UITouchesEventPhase) {
val density = density.density

scene.sendPointerEvent(
eventType = event.kind.toCompose(),
pointers = event.pointers.map {
eventType = phase.toPointerEventType(),
pointers = event.touchesForView(view)?.map {
val touch = it as UITouch
val id = touch.hashCode().toLong()

val position = touch.offsetInView(view, density)

ComposeScene.Pointer(
id = PointerId(it.id),
position = Offset(
x = it.x.toFloat() * scale,
y = it.y.toFloat() * scale
),
pressed = it.pressed,
type = it.device.toCompose(),
pressure = it.pressure.toFloat(),
id = PointerId(id),
position = position,
pressed = touch.isPressed,
type = PointerType.Touch,
pressure = touch.force.toFloat(),
historical = event.historicalChangesForTouch(touch, view, density)
)
},
timeMillis = event.timestamp,
} ?: emptyList(),
timeMillis = (event.timestamp * 1e3).toLong(),
nativeEvent = event
)
}
Expand Down Expand Up @@ -682,6 +692,43 @@ internal actual class ComposeWindow : UIViewController {
}
}

private fun UITouch.offsetInView(view: UIView, density: Float): Offset =
locationInView(view).useContents {
Offset(x.toFloat() * density, y.toFloat() * density)
}

private fun UIEvent.historicalChangesForTouch(touch: UITouch, view: UIView, density: Float): List<HistoricalChange> {
val touches = coalescedTouchesForTouch(touch) ?: return emptyList()

return if (touches.size > 1) {
// subList last index is exclusive, so the last touch in the list is not included
// because it's the actual touch for which coalesced touches were requested
touches.subList(0, touches.size - 1).map {
val historicalTouch = it as UITouch
HistoricalChange(
uptimeMillis = (historicalTouch.timestamp * 1e3).toLong(),
position = historicalTouch.offsetInView(view, density)
)
}
} else {
emptyList()
}
}

private val UITouch.isPressed
get() = when (phase) {
UITouchPhase.UITouchPhaseEnded, UITouchPhase.UITouchPhaseCancelled -> false
else -> true
}

private fun UITouchesEventPhase.toPointerEventType(): PointerEventType =
when (this) {
UITouchesEventPhase.BEGAN -> PointerEventType.Press
UITouchesEventPhase.MOVED -> PointerEventType.Move
UITouchesEventPhase.ENDED -> PointerEventType.Release
UITouchesEventPhase.CANCELLED -> PointerEventType.Release
}

private fun UIViewController.checkIfInsideSwiftUI(): Boolean {
var parent = parentViewController

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,17 @@ internal interface SkikoUIViewDelegate {

fun pointInside(point: CValue<CGPoint>, event: UIEvent?): Boolean

fun onPointerEvent(event: SkikoPointerEvent)
fun onTouchesEvent(view: UIView, event: UIEvent, phase: UITouchesEventPhase)

fun retrieveInteropTransaction(): UIKitInteropTransaction

fun render(canvas: Canvas, targetTimestamp: NSTimeInterval)
}

internal enum class UITouchesEventPhase {
BEGAN, MOVED, ENDED, CANCELLED
}

@Suppress("CONFLICTING_OVERLOADS")
@ExportObjCClass
internal class SkikoUIView : UIView, UIKeyInputProtocol, UITextInputProtocol {
Expand Down Expand Up @@ -289,8 +293,8 @@ internal class SkikoUIView : UIView, UIKeyInputProtocol, UITextInputProtocol {

_touchesCount += touches.size

withEvent?.let {
delegate?.onPointerEvent(it.toSkikoPointerEvent(SkikoPointerEventKind.DOWN))
withEvent?.let { event ->
delegate?.onTouchesEvent(this, event, UITouchesEventPhase.BEGAN)
}
}

Expand All @@ -299,16 +303,16 @@ internal class SkikoUIView : UIView, UIKeyInputProtocol, UITextInputProtocol {

_touchesCount -= touches.size

withEvent?.let {
delegate?.onPointerEvent(it.toSkikoPointerEvent(SkikoPointerEventKind.UP))
withEvent?.let { event ->
delegate?.onTouchesEvent(this, event, UITouchesEventPhase.ENDED)
}
}

override fun touchesMoved(touches: Set<*>, withEvent: UIEvent?) {
super.touchesMoved(touches, withEvent)

withEvent?.let {
delegate?.onPointerEvent(it.toSkikoPointerEvent(SkikoPointerEventKind.MOVE))
withEvent?.let { event ->
delegate?.onTouchesEvent(this, event, UITouchesEventPhase.MOVED)
}
}

Expand All @@ -317,35 +321,11 @@ internal class SkikoUIView : UIView, UIKeyInputProtocol, UITextInputProtocol {

_touchesCount -= touches.size

withEvent?.let {
delegate?.onPointerEvent(it.toSkikoPointerEvent(SkikoPointerEventKind.UP))
withEvent?.let { event ->
delegate?.onTouchesEvent(this, event, UITouchesEventPhase.CANCELLED)
}
}

private fun UIEvent.toSkikoPointerEvent(kind: SkikoPointerEventKind): SkikoPointerEvent {
val pointers = touchesForView(this@SkikoUIView).orEmpty().map {
val touch = it as UITouch
val (x, y) = touch.locationInView(this@SkikoUIView).useContents { x to y }
SkikoPointer(
x = x,
y = y,
pressed = touch.isPressed,
device = SkikoPointerDevice.TOUCH,
id = touch.hashCode().toLong(),
pressure = touch.force
)
}

return SkikoPointerEvent(
x = pointers.centroidX,
y = pointers.centroidY,
kind = kind,
timestamp = (timestamp * 1_000).toLong(),
pointers = pointers,
platform = this
)
}

override fun inputDelegate(): UITextInputDelegateProtocol? {
return _inputDelegate
}
Expand Down Expand Up @@ -755,14 +735,6 @@ private fun NSWritingDirection.directionToStr() =
else -> "unknown direction"
}

private val UITouch.isPressed
get() =
phase != UITouchPhase.UITouchPhaseEnded &&
phase != UITouchPhase.UITouchPhaseCancelled

private val Iterable<SkikoPointer>.centroidX get() = asSequence().map { it.x }.average()
private val Iterable<SkikoPointer>.centroidY get() = asSequence().map { it.y }.average()

private fun toSkikoKeyboardEvent(
event: UIPress,
kind: SkikoKeyboardEventKind
Expand Down

0 comments on commit 3a95834

Please sign in to comment.