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 cc1cacc..081ca63 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/TranscriptScroll.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/TranscriptScroll.kt @@ -21,7 +21,6 @@ import androidx.compose.runtime.remember import androidx.compose.runtime.setValue import androidx.compose.runtime.snapshotFlow import androidx.compose.runtime.snapshots.Snapshot -import androidx.compose.runtime.withFrameNanos import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.draw.drawWithContent @@ -119,7 +118,6 @@ class TranscriptScroll(internal val scroll: ScrollState) { if (first != null && last != null && first <= last) first..last else IntRange.EMPTY // Whatever it was walking towards was named in the old indices too. target = retained - growing = false // Something has to be built before the first frame, or the first frame is blank. The window // is worked out from the scroll position, and there is no scroll position until this has // been laid out once -- so on the composition that introduces the rows, every one of them @@ -176,16 +174,29 @@ class TranscriptScroll(internal val scroll: ScrollState) { /** The window's height, for the frames before the scroll container has measured one. */ private var screen = 0 + /** The tallest the viewport has been, which is what the window is sized against. */ + private var widest = 0 + /** - * How tall to treat the visible area as. + * How tall to treat the visible area as, for deciding how much to keep built. * - * The scroll container's own measurement once there is one, and the window's height before - * that. There is a first composition in which rows exist and no layout has happened, and - * answering it with zero there builds a window of nothing -- so the transcript draws blank for - * a frame, which is the flicker when a session opens. + * The tallest it has been rather than what it is now, and that is what makes opening the + * keyboard cheap. The IME animates over some thirty frames and shrinks the viewport on every + * one of them, so a window measured in screens gets smaller on every frame too -- which trims + * the far edge, disposes rows, and recomposes the list, thirty times, for a keyboard. How much + * to keep alive is not a question the keyboard has any business changing: it is about how far + * the reader might scroll, and they can scroll just as far with it open. + * + * The window's own height stands in before the scroll container has measured one. There is a + * first composition in which rows exist and no layout has happened, and answering it with zero + * there builds a window of nothing -- a blank frame when a session opens. */ private val visible: Int - get() = scroll.viewportSize.takeIf { it > 0 } ?: screen + get() { + val now = scroll.viewportSize + if (now > widest) widest = now + return if (widest > 0) widest else screen + } /** * Builds a screenful of rows around [index], for the frames before there is a window. @@ -512,97 +523,45 @@ class TranscriptScroll(internal val scroll: ScrollState) { /** Where the window is heading. [retained] walks towards it rather than jumping; see below. */ private var target: IntRange = IntRange.EMPTY - /** Whether [retained] is still short of [target], so somebody should keep stepping it. */ - var growing: Boolean by mutableStateOf(false) - private set - - /** - * Widens the built range towards its target, a few rows at a time. False when it has arrived. - * - * Standing rows up is not free and its cost is not recomposition -- it is laying the text out, - * which means shaping every glyph, and that is on the thread drawing the frame. Moving the - * window in one go meant two screens of markdown shaped inside a single frame at each step, and - * seventeen screens of it in the frame a session opens in. The platform files that under the - * frame's draw phase, which is why it never showed up in the counters here: nothing is being - * *recorded*, it is being measured. - * - * Spreading it over frames does not make it cheaper and is not meant to. It stops it arriving - * all at once, which is the difference between a frame that is late and a frame that is missed - * by ten. - */ - internal fun standUpSome(): Boolean { - if (target.isEmpty() || retained == target) { - growing = false - return false - } - var first = retained.first - var last = retained.last - var budget = STAND_UP_PER_FRAME - while (budget > 0 && (first > target.first || last < target.last)) { - if (first > target.first) { - first-- - budget-- - } - if (budget > 0 && last < target.last) { - last++ - budget-- - } - } - retained = first..last - growing = retained != target - return growing - } - internal fun trackRetained(from: Int? = null) = // Without subscribing whoever called it to the scroll position. This runs from the - // composition that lays the rows out as well as from the flow that watches scrolling, and - // a composition that reads `scroll.value` recomposes on every frame of every fling -- the - // O(rows)-per-frame mistake this whole file exists to undo, arriving by the back door. + // composition that lays the rows out as well as from the layout that places them, and a + // composition that reads `scroll.value` recomposes on every frame of every fling. Snapshot.withoutReadObservation { refreshTops() // [from] when the caller knows better than the scroll container does; see `laidOut`. val viewportTop = from ?: (scroll.maxValue - scroll.value) - // The outer bound moves lazily, because every row reads the window and moving it - // recomposes all of them -- affordable every couple of screens, and not at every row - // boundary a fling crosses. - val step = (visible * RETAIN_STEP_SCREENS).coerceAtLeast(1) - val moved = viewportTop - rangeAt - if (target.isEmpty() || topsVersion != rangeVersion || moved > step || moved < -step) { - rangeAt = viewportTop - rangeVersion = topsVersion - target = retainedRange(viewportTop, RETAIN_SCREENS) - } - // What is near the screen, every frame rather than only when the bound moves. This is - // the half that cannot be lazy and the half that was: a fling crosses a screen in a - // frame or two, so a window last widened two screens ago has already been outrun and - // the row arriving at the edge is drawn as the spacer it still is. Standing rows up a - // few at a time made it worse rather than causing it -- after a seed or a restore the - // built window is a couple of screens wide and grows two rows a frame, which a fling - // beats easily. Cheap enough to do always: one scan of the row list, and the write - // below is skipped when the answer has not changed, so nothing recomposes. - // Widened by a fixed number of rows as well as by screens, because a screen is a - // number of pixels and the rows it covers are only *estimated* until they have been - // measured -- and nothing has been measured on the frame a session opens, which is - // where this matters. Rows shorter than the running average make a two-screen window - // cover fewer rows than the screen actually shows, and the reader sees the difference - // as blank. + // Both edges every time, and the bound is a *limit* rather than a destination. + // + // It used to be a destination: rows were stood up towards it a couple at a time until + // the window was the full sixteen screens, whether or not anybody was going to read + // them. Two rows is not a bounded amount of work -- a row here runs to twenty-five + // screens of markdown -- so the transcript was quietly laying text out forever, and + // that is what the 70ms measurements were. + // + // And it used to move in two-screen steps, which is what made the boundaries the + // reader could feel. Crossing one moved the limit by two screens at once, which cut a + // chunk off the trailing edge in a single frame; coming back the other way needed all + // of it again, so an oscillation around a boundary rebuilt the same rows over and + // over. It was measured at twenty-two rows dropped and immediately wanted back. + // Recomputed every time, the trailing edge retreats a row at a time as the reader + // moves, and there is no boundary left to cross. The step existed because this was a + // scan of every row; it is a binary search now. + target = retainedRange(viewportTop, RETAIN_SCREENS) + // What is near the screen has to be built in the frame it is on screen, so this half + // is never deferred and never clamped away. val screens = retainedRange(viewportTop, RETAIN_NOW_SCREENS) val near = (screens.first - RETAIN_NOW_ROWS).coerceAtLeast(0)..(screens.last + RETAIN_NOW_ROWS) .coerceAtMost(order.lastIndex) val first = if (retained.isEmpty()) near.first else minOf(retained.first, near.first) val last = if (retained.isEmpty()) near.last else maxOf(retained.last, near.last) - // Clamped to the bound, but never past what is on screen: the bound is allowed to be a - // couple of screens out of date and this is not. val next = minOf(first.coerceAtLeast(target.first), near.first)..maxOf( last.coerceAtMost(target.last), near.last, ) - if (next != retained) { - retained = next - growing = retained != target - } + if (next != retained) retained = next } /** @@ -681,8 +640,6 @@ class TranscriptScroll(internal val scroll: ScrollState) { false } - private var rangeAt = Int.MIN_VALUE - private var rangeVersion = -1 private var topsVersion = 0 /** The height of the visible area, 0 until the first measurement. */ @@ -792,18 +749,6 @@ fun TranscriptColumn( snapshotFlow { state.scroll.value to state.scroll.maxValue } .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) @@ -961,10 +906,7 @@ private fun RetainedRow( } /** How far either side of the screen a row stays built; see [TranscriptScroll.retains]. */ -private const val RETAIN_SCREENS = 8 - -/** How far the view moves before the outer bound is worked out again; see `trackRetained`. */ -private const val RETAIN_STEP_SCREENS = 2 +private const val RETAIN_SCREENS = 5 /** * How much either side of the screen is built at once rather than a few rows at a time. @@ -980,11 +922,6 @@ private const val RETAIN_NOW_SCREENS = 3 /** The same margin counted in rows, for when no height is known yet; see `trackRetained`. */ private const val RETAIN_NOW_ROWS = 12 -/** - * How many rows may be laid out in one frame while the window is catching up; see `standUpSome`. - */ -private const val STAND_UP_PER_FRAME = 2 - /** * How much is built before there is a scroll position to work a window out from; see `seedAround`. */