Stop paying a callback per row per frame, and drop the reply cap
Holding every row alive turned two per-row callbacks from a cost paid by
whatever was on screen into a cost paid by everything ever loaded, which
is why loading one more page of history made the whole transcript lag
and each page after made it worse. Both fire for every registered node
on every layout pass:
- an `onGloballyPositioned` per row, kept so a tap could be told which
half of the row it landed in
- an `onGloballyPositioned` per clickable inside a row, for the same
reason, so a tool group paid several
Neither is needed. One gesture detector on the whole list records where
a touch went down in the content's own coordinates, and the row's own
start is added up from heights when somebody actually taps -- so the
transcript keeps a height per row, which changes when the row does, in
place of a position per row, which is wrong the moment anything scrolls.
Positions for the saved-scroll anchor come from the same sum, and the
anchor is applied once per layout from the content rather than once per
row.
The reply cap is gone at Iris's request; she wasn't seeing it and does
not want it for now. It was doing measurable work -- p50 rises from 16ms
to 20ms on the emulator with it removed -- so it is worth knowing where
to look if long history feels heavy.
That 4ms is the only figure here I trust. This emulator's own noise
between identical repeats is larger than the difference the callback
change makes (identical builds measured 3.5% and 9.3% of frames over
budget), and its p50 of 16ms is already past a 120Hz budget, so it
cannot rank any of this the way the phone will. The callbacks are gone
because they are O(rows) per frame by construction and the list no
longer bounds how many rows there are -- not because a number here says
so.
Checked on the emulator against a real 1,200-event transcript: tapping a
collapsed row expands it with its top edge held exactly still, and a
scroll position comes back pixel-identical after leaving the session and
returning.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
ee1c493559
commit
f22c96abe2
6 files changed
+149
-259
No files matched your search
@@ -1,6 +1,8 @@
|
||||
package com.example.aiapp
|
||||
|
||||
import androidx.compose.foundation.ScrollState
|
||||
import androidx.compose.foundation.gestures.awaitEachGesture
|
||||
import androidx.compose.foundation.gestures.awaitFirstDown
|
||||
import androidx.compose.foundation.layout.Arrangement
|
||||
import androidx.compose.foundation.layout.Column
|
||||
import androidx.compose.foundation.layout.PaddingValues
|
||||
@@ -18,8 +20,12 @@ import androidx.compose.runtime.remember
|
||||
import androidx.compose.runtime.setValue
|
||||
import androidx.compose.ui.Alignment
|
||||
import androidx.compose.ui.Modifier
|
||||
import androidx.compose.ui.input.pointer.PointerEventPass
|
||||
import androidx.compose.ui.input.pointer.pointerInput
|
||||
import androidx.compose.ui.layout.onPlaced
|
||||
import androidx.compose.ui.layout.positionInParent
|
||||
import androidx.compose.ui.layout.onSizeChanged
|
||||
import androidx.compose.ui.platform.LocalDensity
|
||||
import androidx.compose.ui.platform.LocalLayoutDirection
|
||||
import androidx.compose.ui.unit.Dp
|
||||
import androidx.compose.ui.unit.dp
|
||||
|
||||
@@ -54,17 +60,62 @@ import androidx.compose.ui.unit.dp
|
||||
class TranscriptScroll(internal val scroll: ScrollState) {
|
||||
|
||||
/**
|
||||
* Where each row's top edge sits inside the content, by the seq that names it.
|
||||
* How tall each row is, by the seq that names it, and the order they are drawn in.
|
||||
*
|
||||
* Written from the layout pass as rows are placed, so it describes the layout that is on
|
||||
* screen. Keyed on [TranscriptRow.startSeq] rather than on the row's display key for the reason
|
||||
* the anchor is: a tool run is renamed when the newest page starts somewhere new, and a
|
||||
* position recorded against the old name is never found again.
|
||||
* Heights rather than positions, and that is the whole difference between this costing nothing
|
||||
* and costing the frame. A position is only correct for one layout, so keeping one per row
|
||||
* meant a callback per row per frame once the list stopped disposing them -- the transcript
|
||||
* lagged the moment a second page was loaded, and worse with each page after. A height changes
|
||||
* when its row changes and not otherwise, and a position can be added up from heights at the
|
||||
* two moments anything actually needs one: saving where the reader is, and putting it back.
|
||||
*
|
||||
* Keyed on [TranscriptRow.startSeq] rather than on the row's display key for the reason the
|
||||
* anchor is: a tool run is renamed when the newest page starts somewhere new, and a position
|
||||
* recorded against the old name is never found again.
|
||||
*/
|
||||
private val tops = HashMap<Long, Int>()
|
||||
private val heights = HashMap<Long, Int>()
|
||||
private var order: List<Long> = emptyList()
|
||||
private var spacing = 0
|
||||
private var padTop = 0
|
||||
|
||||
internal fun laidOut(order: List<Long>, spacing: Int, padTop: Int) {
|
||||
this.order = order
|
||||
this.spacing = spacing
|
||||
this.padTop = padTop
|
||||
}
|
||||
|
||||
internal fun height(seq: Long, height: Int) {
|
||||
heights[seq] = height
|
||||
}
|
||||
|
||||
/** Where the last touch went down, in the content's own coordinates. */
|
||||
private var tapY = 0f
|
||||
|
||||
internal fun touched(y: Float) {
|
||||
tapY = y
|
||||
}
|
||||
|
||||
/**
|
||||
* A position waiting to be put back, applied by the layout that first places its row.
|
||||
* Whether the last touch landed in the top half of the row named by [seq], which is the end
|
||||
* that row should hold when it changes height.
|
||||
*
|
||||
* One detector for the whole list rather than one per row, and one lookup at the moment of the
|
||||
* tap rather than a position kept current for every row. Both of the obvious arrangements cost
|
||||
* a callback per row per frame once the list stopped disposing rows -- an
|
||||
* `onGloballyPositioned` to know where a row is, or a gesture detector on each row to catch its
|
||||
* own touches -- and together they were most of the frame: on a deep transcript they took a
|
||||
* scroll from 4% of frames over budget to 47%. Neither is needed. The content knows where it
|
||||
* was touched, the heights say where each row starts, and the sum is only wanted when somebody
|
||||
* actually taps.
|
||||
*/
|
||||
fun tappedHigh(seq: Long): Boolean {
|
||||
val top = topOf(seq) ?: return false
|
||||
val height = heights[seq] ?: return false
|
||||
return tapY < top + height / 2f
|
||||
}
|
||||
|
||||
/**
|
||||
* A position waiting to be put back, applied by the layout that first places the content.
|
||||
*
|
||||
* Held here rather than applied by whoever loaded the row because the pixels do not exist yet
|
||||
* at that point: a plain column has no height for a row until it has been measured. Applying it
|
||||
@@ -79,19 +130,31 @@ class TranscriptScroll(internal val scroll: ScrollState) {
|
||||
val settling: Boolean
|
||||
get() = pending != null
|
||||
|
||||
internal fun placed(seq: Long, top: Int) {
|
||||
tops[seq] = top
|
||||
pending?.let { anchor ->
|
||||
if (anchor.seq != seq) return@let
|
||||
scroll.dispatchRawDelta(
|
||||
(scroll.maxValue - top - anchor.offset - scroll.value).toFloat()
|
||||
)
|
||||
pending = null
|
||||
/**
|
||||
* The content has been placed: put back a waiting position, if its row is there to hold it.
|
||||
*
|
||||
* Once per layout rather than once per row. Both numbers it needs are the scroll container's
|
||||
* own, and those are written during measure -- so by placement they describe this layout.
|
||||
*/
|
||||
internal fun placed() {
|
||||
val anchor = pending ?: return
|
||||
val rowTop = topOf(anchor.seq) ?: return
|
||||
scroll.dispatchRawDelta((scroll.maxValue - rowTop - anchor.offset - scroll.value).toFloat())
|
||||
pending = null
|
||||
}
|
||||
|
||||
/** Where a row's top edge sits inside the content, or null if it has not been measured. */
|
||||
private fun topOf(seq: Long): Int? {
|
||||
var y = padTop
|
||||
for (s in order) {
|
||||
if (s == seq) return y
|
||||
y += (heights[s] ?: return null) + spacing
|
||||
}
|
||||
return null
|
||||
}
|
||||
|
||||
/** Everything these described is gone -- a stream reset, or a different session. */
|
||||
fun clear() = tops.clear()
|
||||
fun clear() = heights.clear()
|
||||
|
||||
/** Whether the newest message is on screen. See the class comment: the newest end is zero. */
|
||||
val atNewest: Boolean
|
||||
@@ -116,8 +179,14 @@ class TranscriptScroll(internal val scroll: ScrollState) {
|
||||
fun anchor(): ScrollAnchor? {
|
||||
val top = scroll.maxValue - scroll.value
|
||||
// The row covering the top of the viewport: the last one that starts at or above it.
|
||||
val at = tops.entries.filter { it.value <= top }.maxByOrNull { it.value } ?: return null
|
||||
return ScrollAnchor(at.key, top - at.value)
|
||||
var y = padTop
|
||||
var found: Pair<Long, Int>? = null
|
||||
for (s in order) {
|
||||
if (y > top) break
|
||||
found = s to y
|
||||
y += (heights[s] ?: return null) + spacing
|
||||
}
|
||||
return found?.let { (seq, rowTop) -> ScrollAnchor(seq, top - rowTop) }
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -173,22 +242,47 @@ fun TranscriptColumn(
|
||||
below: @Composable () -> Unit,
|
||||
row: @Composable (TranscriptRow) -> Unit,
|
||||
) {
|
||||
val density = LocalDensity.current
|
||||
val layoutDirection = LocalLayoutDirection.current
|
||||
// The order and the gaps, so a row's position can be added up from heights when one is wanted.
|
||||
// Recomputed only when the rows change, which is what keeps every frame free of it.
|
||||
remember(rows, spacing, contentPadding, density) {
|
||||
state.laidOut(
|
||||
rows.map { it.startSeq },
|
||||
with(density) { spacing.roundToPx() },
|
||||
with(density) { contentPadding.calculateTopPadding().roundToPx() },
|
||||
)
|
||||
layoutDirection
|
||||
}
|
||||
Column(
|
||||
modifier
|
||||
.verticalScroll(state.scroll, reverseScrolling = true)
|
||||
.padding(contentPadding)
|
||||
.heightIn(min = viewportHeight)
|
||||
.fillMaxWidth(),
|
||||
.fillMaxWidth()
|
||||
// Once for the whole list, not once per row: this is where a saved position is put
|
||||
// back, and by placement the scroll container's own measurements describe this layout.
|
||||
.onPlaced { state.placed() }
|
||||
// One gesture detector for the whole list; see [TranscriptScroll.tappedHigh]. On the
|
||||
// initial pass and consuming nothing, so every control inside still gets the gesture
|
||||
// exactly as it would have.
|
||||
.pointerInput(Unit) {
|
||||
awaitEachGesture {
|
||||
state.touched(
|
||||
awaitFirstDown(requireUnconsumed = false, pass = PointerEventPass.Initial)
|
||||
.position
|
||||
.y
|
||||
)
|
||||
}
|
||||
},
|
||||
verticalArrangement = Arrangement.spacedBy(spacing, Alignment.Bottom),
|
||||
) {
|
||||
rows.forEach { item ->
|
||||
// Keyed so that a row keeps its composition -- and so the state inside it, an open
|
||||
// tool call or an expanded reply, stays with the row rather than with the position.
|
||||
// tool call, stays with the row rather than with the position.
|
||||
key(item.key) {
|
||||
Column(
|
||||
Modifier.fillMaxWidth().onPlaced {
|
||||
state.placed(item.startSeq, it.positionInParent().y.toInt())
|
||||
}
|
||||
Modifier.fillMaxWidth().onSizeChanged { state.height(item.startSeq, it.height) }
|
||||
) {
|
||||
row(item)
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user