From 0716e5c057d8c15989aa600aa5fc47b043e7471e Mon Sep 17 00:00:00 2001 From: "Andrei.Salavei" Date: Thu, 14 Nov 2024 15:58:57 +0100 Subject: [PATCH 1/4] Batch frequent marked text edit commands --- .../ui/platform/IOSSkikoInput.uikit.kt | 30 ++++++------------ .../platform/UIKitTextInputService.uikit.kt | 31 +++++++++++++++++-- .../IntermediateTextInputUIView.uikit.kt | 21 +++++++++++-- 3 files changed, 57 insertions(+), 25 deletions(-) diff --git a/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/platform/IOSSkikoInput.uikit.kt b/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/platform/IOSSkikoInput.uikit.kt index 9850b1cd13cdc..2f1b03bb82391 100644 --- a/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/platform/IOSSkikoInput.uikit.kt +++ b/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/platform/IOSSkikoInput.uikit.kt @@ -16,7 +16,6 @@ package androidx.compose.ui.platform -import androidx.compose.ui.geometry.Offset import androidx.compose.ui.unit.DpOffset internal interface IOSSkikoInput { @@ -27,6 +26,16 @@ internal interface IOSSkikoInput { fun endFloatingCursor() + /** + * Delays all edit commands until [endEditBatch] is being called. + */ + fun beginEditBatch() + + /** + * Performs all editing commands, starting from the [beginEditBatch] call. + */ + fun endEditBatch() + /** * A Boolean value that indicates whether the text-entry object has any text. * https://developer.apple.com/documentation/uikit/uikeyinput/1614457-hastext @@ -116,23 +125,4 @@ internal interface IOSSkikoInput { * Returned value must be in range between 0 and length of text (inclusive). */ fun positionFromPosition(position: Long, offset: Long): Long - - object Empty : IOSSkikoInput { - override fun beginFloatingCursor(offset: DpOffset) = Unit - override fun updateFloatingCursor(offset: DpOffset) = Unit - override fun endFloatingCursor() = Unit - override fun hasText(): Boolean = false - override fun insertText(text: String) = Unit - override fun deleteBackward() = Unit - override fun endOfDocument(): Long = 0L - override fun getSelectedTextRange(): IntRange? = null - override fun setSelectedTextRange(range: IntRange?) = Unit - override fun selectAll() = Unit - override fun textInRange(range: IntRange): String = "" - override fun replaceRange(range: IntRange, text: String) = Unit - override fun setMarkedText(markedText: String?, selectedRange: IntRange) = Unit - override fun markedTextRange(): IntRange? = null - override fun unmarkText() = Unit - override fun positionFromPosition(position: Long, offset: Long): Long = 0 - } } \ No newline at end of file diff --git a/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/platform/UIKitTextInputService.uikit.kt b/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/platform/UIKitTextInputService.uikit.kt index 6d4aa26910aeb..74f98f3bae694 100644 --- a/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/platform/UIKitTextInputService.uikit.kt +++ b/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/platform/UIKitTextInputService.uikit.kt @@ -263,10 +263,27 @@ internal class UIKitTextInputService( return event.type == KeyEventType.KeyDown } + private val editCommandsBatch = mutableListOf() + private var editBatchDepth: Int = 0 + set(value) { + field = value + flushEditCommandsIfNeeded() + } + private fun sendEditCommand(vararg commands: EditCommand) { - val commandList = commands.toList() - _tempCurrentInputSession?.apply(commandList) - currentInput?.onEditCommand?.invoke(commandList) + _tempCurrentInputSession?.apply(commands.toList()) + + editCommandsBatch.addAll(commands) + flushEditCommandsIfNeeded() + } + + private fun flushEditCommandsIfNeeded() { + if (editBatchDepth == 0 && editCommandsBatch.isNotEmpty()) { + val commandList = editCommandsBatch.toList() + editCommandsBatch.clear() + + currentInput?.onEditCommand?.invoke(commandList) + } } private fun getCursorPos(): Int? { @@ -395,6 +412,14 @@ internal class UIKitTextInputService( floatingCursorTranslation = null } + override fun beginEditBatch() { + editBatchDepth++ + } + + override fun endEditBatch() { + editBatchDepth-- + } + /** * A Boolean value that indicates whether the text-entry object has any text. * https://developer.apple.com/documentation/uikit/uikeyinput/1614457-hastext diff --git a/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/window/IntermediateTextInputUIView.uikit.kt b/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/window/IntermediateTextInputUIView.uikit.kt index 694ada5602e0c..b9d9aa172897c 100644 --- a/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/window/IntermediateTextInputUIView.uikit.kt +++ b/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/window/IntermediateTextInputUIView.uikit.kt @@ -29,6 +29,8 @@ import kotlin.time.DurationUnit import kotlinx.cinterop.CValue import kotlinx.cinterop.readValue import kotlinx.cinterop.useContents +import kotlinx.coroutines.MainScope +import kotlinx.coroutines.launch import platform.CoreGraphics.CGPoint import platform.CoreGraphics.CGRect import platform.CoreGraphics.CGRectMake @@ -47,6 +49,7 @@ import platform.UIKit.UIEvent import platform.UIKit.UIKeyInputProtocol import platform.UIKit.UIKeyboardAppearance import platform.UIKit.UIKeyboardType +import platform.UIKit.UIPress import platform.UIKit.UIPressesEvent import platform.UIKit.UIReturnKeyType import platform.UIKit.UITextAutocapitalizationType @@ -66,7 +69,6 @@ import platform.UIKit.UITextRange import platform.UIKit.UITextSelectionRect import platform.UIKit.UITextStorageDirection import platform.UIKit.UIView -import platform.UIKit.UIPress import platform.darwin.NSInteger private val NoOpOnKeyboardPresses: (Set<*>) -> Unit = {} @@ -88,6 +90,8 @@ internal class IntermediateTextInputUIView( } } + private val mainScope = MainScope() + /** * Callback to handle keyboard presses. The parameter is a [Set] of [UIPress] objects. * Erasure happens due to K/N not supporting Obj-C lightweight generics. @@ -233,7 +237,12 @@ internal class IntermediateTextInputUIView( location.toInt() to length.toInt() } val relativeTextRange = locationRelative until locationRelative + lengthRelative - input?.setMarkedText(markedText, relativeTextRange) + + // Due to iOS specifics, [setMarkedText] can be called several times in a row. Batching + // helps to avoid text input problems caused by too frequent dispaltching of input commands. + input?.withBatch { + input?.setMarkedText(markedText, relativeTextRange) + } } /** @@ -496,6 +505,14 @@ internal class IntermediateTextInputUIView( fun resetOnKeyboardPressesCallback() { onKeyboardPresses = NoOpOnKeyboardPresses } + + private fun IOSSkikoInput.withBatch(update: () -> Unit) { + beginEditBatch() + update() + mainScope.launch { + endEditBatch() + } + } } private class IntermediateTextPosition(val position: Long = 0) : UITextPosition() From 7cdde06d84eb32f07bc7a833f18eb0d76402693c Mon Sep 17 00:00:00 2001 From: "Andrei.Salavei" Date: Fri, 29 Nov 2024 13:31:37 +0100 Subject: [PATCH 2/4] Add force flush before render and on input end --- .../compose/ui/platform/UIKitTextInputService.uikit.kt | 5 +++-- .../androidx/compose/ui/scene/ComposeSceneMediator.uikit.kt | 1 + .../compose/ui/window/IntermediateTextInputUIView.uikit.kt | 3 ++- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/platform/UIKitTextInputService.uikit.kt b/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/platform/UIKitTextInputService.uikit.kt index 74f98f3bae694..b3d431558e0cd 100644 --- a/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/platform/UIKitTextInputService.uikit.kt +++ b/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/platform/UIKitTextInputService.uikit.kt @@ -162,6 +162,7 @@ internal class UIKitTextInputService( } override fun stopInput() { + flushEditCommandsIfNeeded() currentInput = null _tempCurrentInputSession = null currentImeOptions = null @@ -277,8 +278,8 @@ internal class UIKitTextInputService( flushEditCommandsIfNeeded() } - private fun flushEditCommandsIfNeeded() { - if (editBatchDepth == 0 && editCommandsBatch.isNotEmpty()) { + fun flushEditCommandsIfNeeded(force: Boolean = false) { + if ((force || editBatchDepth == 0) && editCommandsBatch.isNotEmpty()) { val commandList = editCommandsBatch.toList() editCommandsBatch.clear() diff --git a/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/scene/ComposeSceneMediator.uikit.kt b/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/scene/ComposeSceneMediator.uikit.kt index 19d6b1834bc58..867d9f089c63c 100644 --- a/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/scene/ComposeSceneMediator.uikit.kt +++ b/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/scene/ComposeSceneMediator.uikit.kt @@ -466,6 +466,7 @@ internal class ComposeSceneMediator( } fun render(canvas: Canvas, nanoTime: Long) { + textInputService.flushEditCommandsIfNeeded(force = true) scene.render(canvas, nanoTime) } diff --git a/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/window/IntermediateTextInputUIView.uikit.kt b/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/window/IntermediateTextInputUIView.uikit.kt index b9d9aa172897c..1e4a249826ba2 100644 --- a/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/window/IntermediateTextInputUIView.uikit.kt +++ b/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/window/IntermediateTextInputUIView.uikit.kt @@ -239,7 +239,8 @@ internal class IntermediateTextInputUIView( val relativeTextRange = locationRelative until locationRelative + lengthRelative // Due to iOS specifics, [setMarkedText] can be called several times in a row. Batching - // helps to avoid text input problems caused by too frequent dispaltching of input commands. + // helps to avoid text input problems, when Composables use parameters set during + // recomposition instead of the current ones. input?.withBatch { input?.setMarkedText(markedText, relativeTextRange) } From 430cd6276eb6c778b165ce71d474300dbd5142c5 Mon Sep 17 00:00:00 2001 From: "Andrei.Salavei" Date: Fri, 29 Nov 2024 14:11:27 +0100 Subject: [PATCH 3/4] Add force flush --- .../androidx/compose/ui/platform/UIKitTextInputService.uikit.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/platform/UIKitTextInputService.uikit.kt b/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/platform/UIKitTextInputService.uikit.kt index b3d431558e0cd..87fa885ad75a1 100644 --- a/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/platform/UIKitTextInputService.uikit.kt +++ b/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/platform/UIKitTextInputService.uikit.kt @@ -162,7 +162,7 @@ internal class UIKitTextInputService( } override fun stopInput() { - flushEditCommandsIfNeeded() + flushEditCommandsIfNeeded(force = true) currentInput = null _tempCurrentInputSession = null currentImeOptions = null From 4b5b13e8487e694ef4adfa76fb24e4ee2849b1dc Mon Sep 17 00:00:00 2001 From: "Andrei.Salavei" Date: Fri, 29 Nov 2024 16:00:38 +0100 Subject: [PATCH 4/4] Elaborate with detailed comments --- .../compose/ui/window/IntermediateTextInputUIView.uikit.kt | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/window/IntermediateTextInputUIView.uikit.kt b/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/window/IntermediateTextInputUIView.uikit.kt index 1e4a249826ba2..817e8ffee60f5 100644 --- a/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/window/IntermediateTextInputUIView.uikit.kt +++ b/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/window/IntermediateTextInputUIView.uikit.kt @@ -240,7 +240,12 @@ internal class IntermediateTextInputUIView( // Due to iOS specifics, [setMarkedText] can be called several times in a row. Batching // helps to avoid text input problems, when Composables use parameters set during - // recomposition instead of the current ones. + // recomposition instead of the current ones. Example: + // 1. State "1" -> TextField(text = "1") + // 2. setMarkedText "12" -> Not equal to TextField(text = "1") -> State "12" + // 3. setMarkedText "1" -> Equal to TextField(text = "1") -> State remains "12" + // scene.render() - Recomposes TextField + // 4. State "12" -> TextField(text = "12") - Invalid state. Should be TextField(text = "1") input?.withBatch { input?.setMarkedText(markedText, relativeTextRange) }