diff --git a/app/androidApp/src/main/kotlin/com/example/aiapp/TranscriptScroll.kt b/app/androidApp/src/main/kotlin/com/example/aiapp/TranscriptScroll.kt index 0fbc552..b62cf9e 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/TranscriptScroll.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/TranscriptScroll.kt @@ -22,6 +22,7 @@ import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember import androidx.compose.runtime.setValue import androidx.compose.runtime.snapshotFlow +import androidx.compose.runtime.withFrameNanos import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.draw.drawWithContent @@ -281,7 +282,35 @@ class TranscriptScroll(internal val scroll: ScrollState) { var retained: IntRange by mutableStateOf(IntRange.EMPTY) private set - fun retains(seq: Long): Boolean = rowIndex[seq]?.let { it in retained } ?: true + /** + * How tall rows [run] stand in for as a single spacer, the gaps between them included. + * + * The gaps have to be counted here because collapsing a run removes children from the column, + * and `Arrangement.spacedBy` puts a gap between children rather than after each one -- so a run + * of `k` rows drawn as one spacer is `k - 1` gaps shorter than the rows were unless it says so. + * Getting that wrong does not look like a spacing bug; it shortens the content, which moves + * everything the reader is looking at. + */ + fun runHeight(run: IntRange): Int { + if (run.isEmpty() || order.isEmpty()) return 0 + var total = 0 + for (index in run) total += assumed(order[index]) + return total + (run.last - run.first) * spacing + } + + /** + * [retained] clamped to a list of [count] rows, or empty if nothing should be built yet. + * + * The range is worked out against the order from the last layout, and a page of history landing + * changes that order in the same composition that reads this -- so an index from before it can + * be past the end. Clamping here rather than at the read sites keeps one answer to it. + */ + fun window(count: Int): IntRange { + if (count == 0 || retained.isEmpty()) return IntRange.EMPTY + val first = retained.first.coerceIn(0, count - 1) + val last = retained.last.coerceIn(first, count - 1) + return first..last + } /** * Recomputes [retained] if the view has moved far enough to be worth it. @@ -407,9 +436,6 @@ class TranscriptScroll(internal val scroll: ScrollState) { private var rangeVersion = -1 private var topsVersion = 0 - /** The height a row was last measured at, for the spacer that stands in for it. */ - fun heightOf(seq: Long): Int = assumed(seq) - /** The height of the visible area, 0 until the first measurement. */ val viewport: Int get() = scroll.viewportSize @@ -497,10 +523,25 @@ fun TranscriptColumn( layoutDirection } // One evaluation of the window a frame, for the whole list; see [TranscriptScroll.retained]. + // The row count is in here because a row arriving is the one case that does not move the view: + // without it a sent message falls outside the window and stands in as a spacer, which is a + // screen of blank between the last message and the box it was typed in. LaunchedEffect(state) { - snapshotFlow { state.scroll.value to state.scroll.maxValue } + snapshotFlow { Triple(state.scroll.value, state.scroll.maxValue, state.rowCount) } .collect { state.trackRetained() } } + // Walks the window towards its target a few rows a frame; see [TranscriptScroll.standUpSome]. + // Driven from a frame callback rather than a plain loop so the rows stand up between frames + // instead of all inside one, which is the entire point of doing it gradually. + LaunchedEffect(state) { + snapshotFlow { state.growing } + .collect { + while (state.growing) { + withFrameNanos {} + state.standUpSome() + } + } + } Column( modifier .verticalScroll(state.scroll, reverseScrolling = true) @@ -514,10 +555,22 @@ fun TranscriptColumn( // the whole transcript was being subcomposed again for each of those frames, which is // what made bringing the keyboard up cost more than anything else on the screen. Read // here it is a relayout, and the rows keep the measurements they already have. + // + // Timed in two halves because the frame's draw phase is where Compose's measurement + // lands, and "draw is high while nothing is being recorded" does not say which half. + // Measuring the transcript and placing it are different costs with different fixes: + // one is shaping text that has changed, the other is O(rows) whether or not anything + // has. .layout { measurable, constraints -> + val started = System.nanoTime() val placeable = measurable.measure(constraints.copy(minHeight = state.scroll.viewportSize)) - layout(placeable.width, placeable.height) { placeable.place(0, 0) } + DebugStats.record("measure: the whole transcript", System.nanoTime() - started) + layout(placeable.width, placeable.height) { + val placing = System.nanoTime() + placeable.place(0, 0) + DebugStats.record("place: the whole transcript", System.nanoTime() - placing) + } } .fillMaxWidth() // Once for the whole list, not once per row: this is where a saved position is put @@ -545,11 +598,25 @@ fun TranscriptColumn( }, verticalArrangement = Arrangement.spacedBy(spacing, Alignment.Bottom), ) { - rows.forEach { item -> + // Everything outside the window is two spacers rather than one per row, and that is what + // stops the cost of a frame growing with the conversation. A stood-down row was still a + // layout node, and the framework's own per-frame bookkeeping after a scroll -- the position + // dispatch, the cached screen rect each node keeps -- walks live nodes rather than visible + // ones. So the transcript got slower with every page loaded even though the extra rows drew + // nothing at all: on a Pixel 9 Pro XL, a flat 7.7ms in the frame's draw phase while our own + // recording accounted for a fortieth of it. Two spacers, because [retained] is a range: the + // rows that are not in it are always one run before it and one run after. + val window = state.window(rows.size) + val ahead = if (window.isEmpty()) rows.indices else 0.. Unit, ) { - // Derived, so a row hears about the scroll only when its own answer changes rather than on - // every frame; see [TranscriptScroll.retains]. - if (!state.retains(item.startSeq)) { - DebugStats.count("row stood down") - Spacer( - Modifier.fillMaxWidth() - .height(with(LocalDensity.current) { state.heightOf(item.startSeq).toDp() }) - ) - return - } Column( Modifier.fillMaxWidth() // A layer of its own, which is the piece of a lazy list this had not rebuilt.