Repository navigation
Fix offset mapping for multi-character placeholders in SkikoParagraph - #3450
Draft
adamglin0 (adamglin0) wants to merge 1 commit into
Draft
adamglin0 (adamglin0) wants to merge 1 commit into
adamglin0 (adamglin0) wants to merge 1 commit into
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
adamglin0 (adamglin0)
force-pushed
the
fix/cmp-9009-skiko-placeholder-offsets
branch
from
September 24, 2026 05:57
73a269f to
d6dc488
Compare
SkikoParagraphBuilder never adds the characters covered by a placeholder to the Skia paragraph; Skia represents each placeholder as a single U+FFFC. When an inline content item uses an alternate text longer than one character (e.g. appendInlineContent(id, "[icon]")), every Skia offset after it is shifted by length - 1 relative to the Compose text. SkikoParagraph passed Compose offsets to Skia unconverted, so hit testing, cursor rects, selection paths, line start/end and word boundaries after an inline item were wrong on all Skiko platforms, and a link placed after an inline item could not be clicked. Introduce PlaceholderOffsetMapping, built once per paragraph from the placeholder ranges (null when every placeholder is one code unit, so the common case only pays a null check), and route every offset that crosses the Compose/Skia boundary in SkikoParagraph through it. Offsets inside a placeholder range map to the placeholder (caret before it, like Android's ReplacementSpan); a hit inside the placeholder resolves to its start or end depending on the half that was hit. Tests: DesktopParagraphPlaceholderOffsetTest (ui-text desktopTest) covers getOffsetForPosition, getCursorRect, getBoundingBox, getHorizontalPosition, getPathForRange, getWordBoundary, line APIs, accumulated deltas with two placeholders, the single-character identity path, mixed single- and multi-character placeholders, a multi-line paragraph and an RTL paragraph. BasicTextInlineContentOffsetTest (foundation skikoTest) reproduces the reported scenario: clicking a link after an inline item. Fixes https://youtrack.jetbrains.com/issue/CMP-9009
adamglin0 (adamglin0)
force-pushed
the
fix/cmp-9009-skiko-placeholder-offsets
branch
from
September 24, 2026 06:18
d6dc488 to
14eb699
Compare
Vladimir Mazunin (mazunin-v-jb)
requested review from
Ivan Matkov (MatkovIvan) and
Alexander Maryanovsky (m-sasha)
and removed request for
Ivan Matkov (MatkovIvan) and
Alexander Maryanovsky (m-sasha)
October 5, 2026 12:54
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
SkikoParagraphBuildernever adds the characters covered by a placeholder to the Skia paragraph; Skia represents each placeholder as a single U+FFFC. When an inline content item uses an alternate text longer than one character (e.g.appendInlineContent(id, "[icon]")), every Skia offset after it is shifted bylength - 1relative to the Compose text.SkikoParagraphpassed Compose offsets to Skia unconverted, so hit testing, cursor rects, selection paths, line start/end and word boundaries after an inline item were wrong on all Skiko platforms, and a link placed after an inline item could not be clicked.This change introduces
PlaceholderOffsetMapping, built once per paragraph from the placeholder ranges (nullwhen every placeholder is one code unit, so the common case only pays a null check), and routes every offset that crosses the Compose/Skia boundary inSkikoParagraphthrough it:getPathForRange,getCursorRect,getLineForOffset,getHorizontalPosition,getBidiRunDirection,getBoundingBox,getWordBoundary.getOffsetForPosition,getLineStart,getLineEnd,getWordBoundary.getBoxForwardByOffset/getBoxBackwardByOffset, trailing-whitespace handling) are bounded by the Skia text length and read characters through the mapping.Semantics: a Compose offset inside a placeholder range maps to the placeholder itself (caret before it, like Android's
ReplacementSpan); a hit inside the placeholder resolves to its start or end depending on which half was hit, matching Android. The builder's single-U+FFFC contract and the public API are unchanged.Fixes https://youtrack.jetbrains.com/issue/CMP-9009
Testing
DesktopParagraphPlaceholderOffsetTest(ui-textdesktopTest, 12 tests):getOffsetForPosition,getCursorRect,getBoundingBox,getHorizontalPosition,getPathForRange,getWordBoundary, line APIs, accumulated deltas with two placeholders, the single-character identity path, mixed single- and multi-character placeholders, a multi-line paragraph and an RTL paragraph. All positions are derived from the fixed-advance test font andplaceholderRects, not from the APIs under test.BasicTextInlineContentOffsetTest(foundationskikoTest, 2 tests): the reported scenario, clicking a link placed after an inline item with a multi-character alternate text.:compose:ui:ui-text:desktopTestand:compose:foundation:foundation:desktopTest --tests "androidx.compose.foundation.text.*"pass.Release Notes
Fixes - Multiple Platforms