From b936815af62e12f3a461cb9282e389643195d125 Mon Sep 17 00:00:00 2001 From: Gibran Chevalley Date: Wed, 27 Nov 2024 15:34:51 +0100 Subject: [PATCH 1/3] refactor: Better write the getBoundingBoxesForRange algorithm --- .../ui/components/HighlightedText.kt | 39 +++++++++++-------- 1 file changed, 23 insertions(+), 16 deletions(-) diff --git a/app/src/main/java/com/infomaniak/swisstransfer/ui/components/HighlightedText.kt b/app/src/main/java/com/infomaniak/swisstransfer/ui/components/HighlightedText.kt index a1287b96b..8312d9737 100644 --- a/app/src/main/java/com/infomaniak/swisstransfer/ui/components/HighlightedText.kt +++ b/app/src/main/java/com/infomaniak/swisstransfer/ui/components/HighlightedText.kt @@ -165,35 +165,42 @@ private fun getRotatedRectanglePath(rect: Rect, angleDegrees: Double, @FloatRang } private fun TextLayoutResult.getBoundingBoxesForRange(start: Int, end: Int): List { + // No character case or start > end case + if (end - start <= 0) return emptyList() + + // Single character case + if (end - start == 1) return listOf(getBoundingBox(0)) + + var firstBoundingBoxRect: Rect? = null var previousRect: Rect? = null - var firstLineCharRect: Rect? = null val boundingBoxes = mutableListOf() + // More than one character case for (index in start..end) { - val rect = getBoundingBox(index) - val isLastRect = index == end + val currentRect = getBoundingBox(index) + val isLastCharacter = index == end - // Single char case - if (isLastRect && firstLineCharRect == null) { - firstLineCharRect = rect - previousRect = rect + // If we haven't seen the first character of the line, set it now + if (firstBoundingBoxRect == null) { + firstBoundingBoxRect = currentRect } - // `rect.right` is zero for the last space in each line - // Might be an issue: https://issuetracker.google.com/issues/197146630 - if (!isLastRect && rect.right == 0.0f) continue + // Check if we reached the end of the current bounding box (i.e. if we reached a new line or the last character in the range) + if (previousRect != null && (areOnDifferentLines(previousRect, currentRect) || isLastCharacter)) { + boundingBoxes.add(firstBoundingBoxRect.copy(right = currentRect.right)) - if (firstLineCharRect == null) { - firstLineCharRect = rect - } else if (previousRect != null && (previousRect.bottom != rect.bottom || isLastRect)) { - boundingBoxes.add(firstLineCharRect.copy(right = previousRect.right)) - firstLineCharRect = rect + // Start a new line (reset firstBoundingBoxRect to the current character's rect) + firstBoundingBoxRect = currentRect } - previousRect = rect + + previousRect = currentRect } + return boundingBoxes } +private fun areOnDifferentLines(previousRect: Rect, currentRect: Rect) = previousRect.bottom != currentRect.bottom + @PreviewLightAndDark @Preview(locale = "fr") @Preview(locale = "de") From 9db410477057b1daa79ac9bf487adbbc4c0ffd9e Mon Sep 17 00:00:00 2001 From: Gibran Chevalley Date: Wed, 27 Nov 2024 15:43:17 +0100 Subject: [PATCH 2/3] fix: Crash when the argument is the very end of the template The range passed as argument to getBoundingBoxesForRange should stop at the last index of the string to not crash with an out of bound exception. --- .../infomaniak/swisstransfer/ui/components/HighlightedText.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/java/com/infomaniak/swisstransfer/ui/components/HighlightedText.kt b/app/src/main/java/com/infomaniak/swisstransfer/ui/components/HighlightedText.kt index 8312d9737..c1fbcd594 100644 --- a/app/src/main/java/com/infomaniak/swisstransfer/ui/components/HighlightedText.kt +++ b/app/src/main/java/com/infomaniak/swisstransfer/ui/components/HighlightedText.kt @@ -98,7 +98,7 @@ fun HighlightedText( private fun TextLayoutResult.getArgumentBoundingBoxes(text: String, argument: String): List { val startIndex = text.indexOf(argument) - return getBoundingBoxesForRange(start = startIndex, end = startIndex + argument.count()) + return getBoundingBoxesForRange(start = startIndex, end = startIndex + argument.count() - 1) } private fun List.transformForHighlightedStyle( From 1ed3fee3886cd0759b4936ff51dc3d5624c55864 Mon Sep 17 00:00:00 2001 From: Gibran Chevalley Date: Wed, 27 Nov 2024 15:44:19 +0100 Subject: [PATCH 3/3] fix: Correctly compute the right coordinate of bounding boxes the right coordinate of the each returned bounding box should be defined based on the current bounding box's last character's rect. This wasn't properly handled when the last character of the current bounding box was in any line but the last one of the whole string vs when it was at the end of the whole string --- .../infomaniak/swisstransfer/ui/components/HighlightedText.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/com/infomaniak/swisstransfer/ui/components/HighlightedText.kt b/app/src/main/java/com/infomaniak/swisstransfer/ui/components/HighlightedText.kt index c1fbcd594..c24bdf4db 100644 --- a/app/src/main/java/com/infomaniak/swisstransfer/ui/components/HighlightedText.kt +++ b/app/src/main/java/com/infomaniak/swisstransfer/ui/components/HighlightedText.kt @@ -187,7 +187,8 @@ private fun TextLayoutResult.getBoundingBoxesForRange(start: Int, end: Int): Lis // Check if we reached the end of the current bounding box (i.e. if we reached a new line or the last character in the range) if (previousRect != null && (areOnDifferentLines(previousRect, currentRect) || isLastCharacter)) { - boundingBoxes.add(firstBoundingBoxRect.copy(right = currentRect.right)) + val lastBoundingBoxRect = if (isLastCharacter) currentRect else previousRect + boundingBoxes.add(firstBoundingBoxRect.copy(right = lastBoundingBoxRect.right)) // Start a new line (reset firstBoundingBoxRect to the current character's rect) firstBoundingBoxRect = currentRect