Draw an inline code chip behind the text instead of under it
The chip was the renderer's span background, and a span's background is part of the text's own drawing: the text node paints the selection first and the glyphs over it, so an opaque chip covered the selection and selecting a sentence highlighted every word of it except the ones in backticks. The previous fix let the selection show through by taking the chip to 60% alpha, which is a compromise on both sides -- the chip is a weaker step down from the page, and selected it reached #3C344F where the words around it reached #776394. There is a place that is under both, and a fenced block was already in it: a modifier on the text rather than a style inside it. So `appendCodeChip` takes the code span from the renderer's inline builder, keeps its style and its space of padding either side but drops the background, and marks the range; `LinkedText` draws those ranges in a `drawBehind`. The chip is back to the full `rawSurface` fill (measured #11111B against a #1E1E2E page) and a selection over it now lands at #776394, the same as the rest of the sentence -- the fenced block's numbers exactly. The geometry is one box per line, from the bounding boxes of the run's first and last characters, taken as far as the line's `visibleEnd`. Not `getPathForRange`: that is the shape of a *selection*, which runs to the right edge of every line but the last, and a code span that wrapped left a full-width empty chip behind on the line above -- twice in one fixture. `visibleEnd` is the same rule the selection rectangle obeys, so the chip stops where the selection stops instead of sticking its padding space out past the end of a selected line. Checked on the emulator against a fixture with chips in a heading, three kinds of list item, a quote, a table cell and a link label, unselected and under Select All, and a link with a chip in its label still opens. Cost, against the same build without the change, streaming sixty paragraphs of three chips each: measure 755ms against 776ms, record 327ms against 321ms, transcript draw 0.22ms in both.
This commit is contained in:
1 parent
9c4df43951
commit
3bb178363d
4 files changed
+119
-24
No files matched your search
@@ -307,6 +307,10 @@ first if a remote spawn ever mangles an argument.
|
|||||||
reply into the tool output under it -- and a container per row leaves
|
reply into the tool output under it -- and a container per row leaves
|
||||||
whatever was drawn without one silently unselectable, which nothing on
|
whatever was drawn without one silently unselectable, which nothing on
|
||||||
screen reports. Rows keep their tap handlers; selection is a long press.
|
screen reports. Rows keep their tap handlers; selection is a long press.
|
||||||
|
**An inline code chip is drawn behind the text** rather than as the
|
||||||
|
renderer's span background, because a span background is part of the
|
||||||
|
text's own drawing and hid the selection under it -- see
|
||||||
|
`appendCodeChip` in `MarkdownLinks.kt` and TRANSCRIPT_RENDERING.md.
|
||||||
- **A session can be moved to another directory** from the settings dialog
|
- **A session can be moved to another directory** from the settings dialog
|
||||||
(`POST /sessions/{id}/cwd`). It stops the process, because a working
|
(`POST /sessions/{id}/cwd`). It stops the process, because a working
|
||||||
directory is settled at spawn; the next message starts it in the new one.
|
directory is settled at spawn; the next message starts it in the new one.
|
||||||
|
|||||||
@@ -72,6 +72,26 @@ the inline builder draws nothing for a node type it does not know (a week
|
|||||||
of blank headings). Tables go through `LinkedTable`/`LinkedTableRow` so
|
of blank headings). Tables go through `LinkedTable`/`LinkedTableRow` so
|
||||||
cells get the same treatment.
|
cells get the same treatment.
|
||||||
|
|
||||||
|
**An inline code chip is drawn behind the text, not as a span
|
||||||
|
background.** A `SpanStyle` background is part of the text's own drawing
|
||||||
|
and the text node draws the selection *under* the glyphs, so an opaque
|
||||||
|
chip hid the selection: selecting a sentence highlighted every word of it
|
||||||
|
except the ones in backticks, and there is no way to reorder that -- the
|
||||||
|
order is the node's. `appendCodeChip` therefore takes the code span from
|
||||||
|
the renderer's builder, keeps its style and its space of padding either
|
||||||
|
side but drops the background, and marks the range; `LinkedText` draws
|
||||||
|
those ranges in a `drawBehind`, which is under both the selection and the
|
||||||
|
glyphs -- the same place a fenced block's box already was, which is why
|
||||||
|
one of those always looked right. Geometry is one box per line, from the
|
||||||
|
bounding boxes of the run's first and last characters, taken as far as the
|
||||||
|
line's `visibleEnd`: `getPathForRange` is a *selection* shape and runs to
|
||||||
|
the right edge of every line but the last, which left a full-width empty
|
||||||
|
chip behind whenever the code wrapped, and `visibleEnd` is what makes the
|
||||||
|
chip and the selection rectangle stop in the same place. Measured against
|
||||||
|
the same build without it, streaming 60 paragraphs of three chips each:
|
||||||
|
measure 755ms against 776ms, record 327ms against 321ms, transcript draw
|
||||||
|
0.22ms in both -- noise.
|
||||||
|
|
||||||
**Text draws on the platform directly.** A paragraph without an image
|
**Text draws on the platform directly.** A paragraph without an image
|
||||||
skips the renderer's `MarkdownText`, which charges every paragraph for the
|
skips the renderer's `MarkdownText`, which charges every paragraph for the
|
||||||
possibility of inline images (placement callback, derived inline-content
|
possibility of inline images (placement callback, derived inline-content
|
||||||
|
|||||||
@@ -368,15 +368,10 @@ private fun MarkdownRoot(
|
|||||||
// exactly a card's own fill, so a fenced block inside a tool call had no
|
// exactly a card's own fill, so a fenced block inside a tool call had no
|
||||||
// background at all and one in a reply read as a step *up* out of the page.
|
// background at all and one in a reply read as a step *up* out of the page.
|
||||||
codeBackground = rawSurface,
|
codeBackground = rawSurface,
|
||||||
// The same colour, but let through. An inline span's background is part of the
|
// The same colour. Not drawn by the renderer as a span background but by
|
||||||
// *text's* own drawing and the selection rectangle is drawn underneath it, so an
|
// [LinkedText] behind the text, so a selection lands on top of it as it does on a
|
||||||
// opaque chip hides the selection completely: selecting a sentence highlighted
|
// fenced block -- see `appendCodeChip`.
|
||||||
// every word of it except the ones in backticks, which is a difference in
|
inlineCodeBackground = rawSurface,
|
||||||
// appearance the reader has no way to account for. Translucent, the selection
|
|
||||||
// shows through and the chip still reads as one step down from the page -- there
|
|
||||||
// is no way to draw it over the selection instead, since the order is the text
|
|
||||||
// node's.
|
|
||||||
inlineCodeBackground = rawSurface.copy(alpha = INLINE_CODE_ALPHA),
|
|
||||||
// The same tint a code block gets, rather than the renderer's 2%-alpha default:
|
// The same tint a code block gets, rather than the renderer's 2%-alpha default:
|
||||||
// two adjacent tints that differ by a fiftieth read as one flat block on a phone,
|
// two adjacent tints that differ by a fiftieth read as one flat block on a phone,
|
||||||
// so the table would have had a border-less grid and nothing saying where it began.
|
// so the table would have had a border-less grid and nothing saying where it began.
|
||||||
@@ -719,12 +714,3 @@ class ParsedReplies {
|
|||||||
ready.clear()
|
ready.clear()
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
|
||||||
* How much of the inline-code chip's fill is its own colour, the rest being whatever it sits on.
|
|
||||||
*
|
|
||||||
* High enough that the chip is still a clear step down from the page, low enough that a selection
|
|
||||||
* under it changes what the chip looks like. Both halves are the point: at 1.0 the chip was the
|
|
||||||
* only part of a selected sentence that did not look selected.
|
|
||||||
*/
|
|
||||||
private const val INLINE_CODE_ALPHA = 0.6f
|
|
||||||
@@ -9,7 +9,10 @@ import androidx.compose.runtime.compositionLocalOf
|
|||||||
import androidx.compose.runtime.remember
|
import androidx.compose.runtime.remember
|
||||||
import androidx.compose.runtime.rememberUpdatedState
|
import androidx.compose.runtime.rememberUpdatedState
|
||||||
import androidx.compose.ui.Modifier
|
import androidx.compose.ui.Modifier
|
||||||
|
import androidx.compose.ui.draw.drawBehind
|
||||||
import androidx.compose.ui.geometry.Offset
|
import androidx.compose.ui.geometry.Offset
|
||||||
|
import androidx.compose.ui.geometry.Rect
|
||||||
|
import androidx.compose.ui.graphics.Color
|
||||||
import androidx.compose.ui.graphics.isSpecified
|
import androidx.compose.ui.graphics.isSpecified
|
||||||
import androidx.compose.ui.input.pointer.pointerInput
|
import androidx.compose.ui.input.pointer.pointerInput
|
||||||
import androidx.compose.ui.node.Ref
|
import androidx.compose.ui.node.Ref
|
||||||
@@ -96,12 +99,23 @@ fun LinkedText(content: String, node: ASTNode, style: TextStyle, modifier: Modif
|
|||||||
val layout = remember { Ref<TextLayoutResult>() }
|
val layout = remember { Ref<TextLayoutResult>() }
|
||||||
// The renderer's own rule for a style that names no colour: the theme's text colour.
|
// The renderer's own rule for a style that names no colour: the theme's text colour.
|
||||||
val color = if (style.color.isSpecified) style.color else LocalMarkdownColors.current.text
|
val color = if (style.color.isSpecified) style.color else LocalMarkdownColors.current.text
|
||||||
|
val chips = remember(text) { text.getStringAnnotations(CODE_CHIP, 0, text.length) }
|
||||||
|
val chipColor = LocalMarkdownColors.current.inlineCodeBackground
|
||||||
|
// Filled in by `onTextLayout`, which runs in the layout phase, so the draw of the same frame
|
||||||
|
// finds it set -- no state needed, and a relayout redraws the node anyway.
|
||||||
|
val chipFills = remember { Ref<List<Rect>>() }
|
||||||
|
val chipFill =
|
||||||
|
if (chips.isEmpty()) Modifier
|
||||||
|
else
|
||||||
|
Modifier.drawBehind {
|
||||||
|
chipFills.value?.forEach { drawRect(chipColor, it.topLeft, it.size) }
|
||||||
|
}
|
||||||
BasicText(
|
BasicText(
|
||||||
text = text,
|
text = text,
|
||||||
modifier =
|
modifier =
|
||||||
// A tap here is either a link or the card's; see [LocalMarkdownTap] for why the
|
// A tap here is either a link or the card's; see [LocalMarkdownTap] for why the
|
||||||
// second one has to be answered from inside the text rather than left to the card.
|
// second one has to be answered from inside the text rather than left to the card.
|
||||||
modifier.pointerInput(text, onPlainTap) {
|
modifier.then(chipFill).pointerInput(text, onPlainTap) {
|
||||||
awaitEachGesture {
|
awaitEachGesture {
|
||||||
// Unconsumed is not required: something outside may already be tracking this
|
// Unconsumed is not required: something outside may already be tracking this
|
||||||
// press, and it is still the press that may land on a link.
|
// press, and it is still the press that may land on a link.
|
||||||
@@ -130,7 +144,10 @@ fun LinkedText(content: String, node: ASTNode, style: TextStyle, modifier: Modif
|
|||||||
},
|
},
|
||||||
style = style,
|
style = style,
|
||||||
color = { color },
|
color = { color },
|
||||||
onTextLayout = { layout.value = it },
|
onTextLayout = {
|
||||||
|
layout.value = it
|
||||||
|
chipFills.value = chips.flatMap { chip -> it.chipRects(chip.start, chip.end) }
|
||||||
|
},
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -188,15 +205,83 @@ private fun AnnotatedString.linkAt(layout: TextLayoutResult?, position: Offset):
|
|||||||
private const val LINK_URL = "url"
|
private const val LINK_URL = "url"
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* The renderer's annotator settings with [appendPlainLink] answering for links. The annotator needs
|
* Appends [node] as inline code -- the renderer's own span, padded by a space each side as it does,
|
||||||
* the settings to draw a link's label, and the settings hold the annotator, so the reference goes
|
* but with no background of its own -- if it is a code span; false leaves anything else to the
|
||||||
* through a cell filled in once both exist.
|
* renderer.
|
||||||
|
*
|
||||||
|
* The chip's fill is drawn by [LinkedText] from the layout instead, behind the text. A span's
|
||||||
|
* background is part of the text's own drawing, and the text node draws the selection first and the
|
||||||
|
* glyphs over it, so a chip painted as a span background covered the selection: selecting a
|
||||||
|
* sentence highlighted every word of it except the ones in backticks. Anything drawn by a modifier
|
||||||
|
* on the text is under both, which is where a fenced block's box already is and why one of those
|
||||||
|
* always looked right. The [CODE_CHIP] annotation is what says where the fill goes.
|
||||||
|
*/
|
||||||
|
private fun appendCodeChip(
|
||||||
|
builder: AnnotatedString.Builder,
|
||||||
|
content: String,
|
||||||
|
node: ASTNode,
|
||||||
|
settings: AnnotatorSettings,
|
||||||
|
): Boolean {
|
||||||
|
if (node.type != MarkdownElementTypes.CODE_SPAN) return false
|
||||||
|
builder.pushStringAnnotation(CODE_CHIP, "")
|
||||||
|
builder.pushStyle(settings.codeSpanStyle.copy(background = Color.Unspecified))
|
||||||
|
builder.append(' ')
|
||||||
|
// The backticks are the first and last children.
|
||||||
|
builder.buildMarkdownAnnotatedString(content, node.children.drop(1).dropLast(1), settings)
|
||||||
|
builder.append(' ')
|
||||||
|
builder.pop()
|
||||||
|
builder.pop()
|
||||||
|
return true
|
||||||
|
}
|
||||||
|
|
||||||
|
private const val CODE_CHIP = "code"
|
||||||
|
|
||||||
|
/**
|
||||||
|
* One box per line of the text [start] until [end] covers, in the layout's own coordinates.
|
||||||
|
*
|
||||||
|
* Not `getPathForRange`, which is the geometry of a *selection* and runs to the right edge of every
|
||||||
|
* line but the last, so a chip whose code wrapped left a full-width empty box behind on the line
|
||||||
|
* above. Each line is taken as far as `visibleEnd`, which is where that line's own trailing space
|
||||||
|
* stops being drawn: the same rule the selection rectangle obeys, so the two agree rather than the
|
||||||
|
* chip sticking a space out past the end of a selected line. It is also what leaves nothing behind
|
||||||
|
* when the only thing to reach a line is the space a chip is padded with.
|
||||||
|
*
|
||||||
|
* A run's extent is taken from the boxes of its first and last characters, which is exact while a
|
||||||
|
* line reads in one direction; mixed directions inside a code span would draw one box across the
|
||||||
|
* whole run rather than one per direction, and code spans are code.
|
||||||
|
*/
|
||||||
|
private fun TextLayoutResult.chipRects(start: Int, end: Int): List<Rect> {
|
||||||
|
val rects = mutableListOf<Rect>()
|
||||||
|
for (line in getLineForOffset(start)..getLineForOffset(end - 1)) {
|
||||||
|
val from = maxOf(start, getLineStart(line))
|
||||||
|
val to = minOf(end, getLineEnd(line, visibleEnd = true))
|
||||||
|
if (from >= to) continue
|
||||||
|
val head = getBoundingBox(from)
|
||||||
|
val tail = getBoundingBox(to - 1)
|
||||||
|
rects +=
|
||||||
|
Rect(
|
||||||
|
left = minOf(head.left, tail.left),
|
||||||
|
top = minOf(head.top, tail.top),
|
||||||
|
right = maxOf(head.right, tail.right),
|
||||||
|
bottom = maxOf(head.bottom, tail.bottom),
|
||||||
|
)
|
||||||
|
}
|
||||||
|
return rects
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The renderer's annotator settings with [appendPlainLink] answering for links and [appendCodeChip]
|
||||||
|
* for inline code. The annotator needs the settings to draw a link's label, and the settings hold
|
||||||
|
* the annotator, so the reference goes through a cell filled in once both exist.
|
||||||
*/
|
*/
|
||||||
@Composable
|
@Composable
|
||||||
private fun plainLinkSettings(): AnnotatorSettings {
|
private fun plainLinkSettings(): AnnotatorSettings {
|
||||||
val cell = remember { Ref<AnnotatorSettings>() }
|
val cell = remember { Ref<AnnotatorSettings>() }
|
||||||
val annotator = remember {
|
val annotator = remember {
|
||||||
markdownAnnotator { content, node -> appendPlainLink(this, content, node, cell.value!!) }
|
markdownAnnotator { content, node ->
|
||||||
|
appendPlainLink(this, content, node, cell.value!!) ||
|
||||||
|
appendCodeChip(this, content, node, cell.value!!)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
return annotatorSettings(annotator = annotator).also { cell.value = it }
|
return annotatorSettings(annotator = annotator).also { cell.value = it }
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in new issue
Block a user