From ee14b9387825422a27495fc7334f151e68a559d3 Mon Sep 17 00:00:00 2001 From: Alexander Maryanovsky Date: Wed, 24 Jan 2024 02:32:10 +0200 Subject: [PATCH] Fix SkiaParagraph.getLineForVerticalPosition (#1012) --- compose/material/material/build.gradle | 5 ++ .../androidx/compose/material/TextLayout.kt | 69 +++++++++++++++++++ .../compose/ui/text/SkiaParagraph.skiko.kt | 33 ++++++--- 3 files changed, 97 insertions(+), 10 deletions(-) create mode 100644 compose/material/material/src/commonTest/kotlin/androidx/compose/material/TextLayout.kt diff --git a/compose/material/material/build.gradle b/compose/material/material/build.gradle index 67e2b1a581bf8..3a8f4c429b995 100644 --- a/compose/material/material/build.gradle +++ b/compose/material/material/build.gradle @@ -167,6 +167,11 @@ if(AndroidXComposePlugin.isMultiplatformEnabled(project)) { implementation(libs.testUiautomator) } + commonTest.dependencies { + implementation(kotlin("test")) + implementation(project(":compose:ui:ui-test")) + } + desktopTest.dependencies { implementation(project(":compose:ui:ui-test-junit4")) implementation(libs.truth) diff --git a/compose/material/material/src/commonTest/kotlin/androidx/compose/material/TextLayout.kt b/compose/material/material/src/commonTest/kotlin/androidx/compose/material/TextLayout.kt new file mode 100644 index 0000000000000..292a231f106a2 --- /dev/null +++ b/compose/material/material/src/commonTest/kotlin/androidx/compose/material/TextLayout.kt @@ -0,0 +1,69 @@ +/* + * 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 androidx.compose.material + +import androidx.compose.foundation.layout.fillMaxSize +import androidx.compose.foundation.text.BasicTextField +import androidx.compose.runtime.remember +import androidx.compose.ui.Modifier +import androidx.compose.ui.test.ExperimentalTestApi +import androidx.compose.ui.test.runComposeUiTest +import androidx.compose.ui.text.TextLayoutResult +import kotlin.test.Test +import kotlin.test.assertEquals + +/** + * Tests text layout in text fields. + */ +@OptIn(ExperimentalTestApi::class) +class TextLayout { + @Test + fun getLineForOffsetReturnsLastLineWhenOffsetIsBeyondLast() = runComposeUiTest { + lateinit var textLayoutResult: TextLayoutResult + + setContent { + val text = remember { "X\n".repeat(100) } + + BasicTextField( + value = text, + onValueChange = {}, + onTextLayout = { textLayoutResult = it }, + modifier = Modifier.fillMaxSize() + ) + } + + assertEquals(100, textLayoutResult.getLineForOffset(Int.MAX_VALUE)) + } + + @Test + fun getLineForVerticalPositionReturnsLastLineWhenPositionIsBeyondLastLine() = runComposeUiTest { + lateinit var textLayoutResult: TextLayoutResult + + setContent { + val text = remember { "Compose rules!\n".repeat(100) } + + BasicTextField( + value = text, + onValueChange = {}, + onTextLayout = { textLayoutResult = it }, + modifier = Modifier.fillMaxSize() + ) + } + + assertEquals(100, textLayoutResult.getLineForVerticalPosition(Float.MAX_VALUE)) + } +} \ No newline at end of file diff --git a/compose/ui/ui-text/src/skikoMain/kotlin/androidx/compose/ui/text/SkiaParagraph.skiko.kt b/compose/ui/ui-text/src/skikoMain/kotlin/androidx/compose/ui/text/SkiaParagraph.skiko.kt index c7bb7f15ea59b..efa2bf3e17f1d 100644 --- a/compose/ui/ui-text/src/skikoMain/kotlin/androidx/compose/ui/text/SkiaParagraph.skiko.kt +++ b/compose/ui/ui-text/src/skikoMain/kotlin/androidx/compose/ui/text/SkiaParagraph.skiko.kt @@ -187,17 +187,10 @@ internal class SkiaParagraph( private fun lineMetricsForOffset(offset: Int): LineMetrics? { checkOffsetIsValid(offset) - val metrics = lineMetrics - if (metrics.isEmpty()) { - return null - } - val index = metrics.asList().binarySearch { - if (offset >= it.endIncludingNewline) -1 else 1 + return lineMetrics.binarySearchFirstMatchingOrLast { + offset < it.endIncludingNewline } - - // The search will always return a negative value because the comparison never returns 0 - return metrics[(-index - 1).coerceAtMost(metrics.lastIndex)] } override fun getLineHeight(lineIndex: Int) = @@ -239,7 +232,7 @@ internal class SkiaParagraph( } private fun getLineMetricsForVerticalPosition(vertical: Float): LineMetrics? { - return lineMetrics.firstOrNull { vertical < it.baseline + it.descent } + return lineMetrics.binarySearchFirstMatchingOrLast { vertical < it.baseline + it.descent } } override fun getHorizontalPosition(offset: Int, usePrimaryDirection: Boolean): Float { @@ -613,3 +606,23 @@ private fun LineMetrics.copy( ) private fun IRange.toTextRange() = TextRange(start, end) + +/** + * Returns the first item satisfying [predicate], or the last item in the array if none satisfy it. + * + * Returns `null` if the array is empty. + */ +private inline fun Array.binarySearchFirstMatchingOrLast( + crossinline predicate: (T) -> Boolean): T? +{ + if (this.isEmpty()) { + return null + } + + val index = this.asList().binarySearch { + if (predicate(it)) 1 else -1 + } + + // The search will always return a negative value because the comparison never returns 0 + return this[(-index - 1).coerceAtMost(this.lastIndex)] +}