diff --git a/PLAN.md b/PLAN.md index c18c209..a6c4d6e 100644 --- a/PLAN.md +++ b/PLAN.md @@ -1189,28 +1189,34 @@ dev-updater (Kotlin 2.4.x, CMP 1.11.x, JDK 21). question *was*, and "Deny" alone does not say Allow was the alternative. One rule in two places (`AskedQuestion` and `PermissionAsk`). An answer typed into **Other** matches no option, so that one is still written out. - - **Opening a row keeps still the end nearest the tap, and closing one - keeps still the point touched** (2026-09-16). A row about to open is - small, so both its edges are within a heading's height of the finger and - the wanted one is the edge pressed: touch the upper half and the top - edge holds, so it opens downwards; touch the lower half and the bottom - edge holds, as the list does by default. A row about to close leaves - almost nothing of itself, and the only place the closed card can - sensibly appear is under the finger that closed it — the case that - settles it is a card taller than the screen, where holding either edge - throws the card a screen's length from the hand. One number carries - both: the share of the row's height above the touch, spent as it is on a - close and rounded to the nearer edge on an open. Which half, rather than - which control, so everything behaves alike whether or not it has a - control at each end. The transcript is laid out from the bottom, so a - bottom edge is anchored for free and anything else has to be arranged: - `Modifier.holdTouchedPoint` asks the list to shift during the *layout* - phase, before anything is drawn. From an effect instead, the wrong - position is drawn once first, which reads as a flick. The scroll offset - that asks for it **goes negative on a close** — that is the list being - asked for the rows below what it has composed — and clamping it at zero, - which is what the growing-only version did, leaves every close - uncorrected. + - **Opening a row keeps still the end nearest the tap; closing one lands + the closed row centred on the tap itself** (2026-09-16). A row about to + open is small, so both its edges are within a heading's height of the + finger and the wanted one is the edge pressed: touch the upper half and + the top edge holds, so it opens downwards; touch the lower half and the + bottom edge holds, as the list does by default. A row about to close + leaves a heading where a screenful of card was, and the place that + heading belongs is under the finger that shut it — a card taller than the + screen settles it, since both of its edges can be a screen's length from + the hand, and holding a *share* of the card's height was the same miss in + miniature: tap away from the middle of a long card and the heading landed + a card's height off. Where there is not enough conversation on the far + side to scroll, the row lands as close as the list can put it. + **The correction is measured, never predicted, and it is asked for from + inside the list's own measure** (`settleHold`, called from + `TranscriptList`'s `afterMeasure`). Both halves of that were paid for: + working the scroll out from the change in height needs the list to behave + the way the arithmetic assumed, and it does not — which item it holds + still across a resize, and how much of a scroll it can honour, depend on + what it has composed, and a row taller than the screen is anchored on + *itself*. And a scroll asked for during the *placement* phase is never + picked up by another measure, so it does nothing at all; from a coroutine + it arrives a frame late, which is the flick this mechanism exists to + avoid. So each pass asks for the error it can see, and the pass that + answers is where the rest becomes askable — three passes is the worst + case seen, including the one where the shut card has left the viewport + entirely and has to be asked back to the bottom edge before it can be + measured at all. - **The full-screen image lives on the screen, not in the row that drew the thumbnail** (`SessionImageViewer`). A `Read` whose result is an image is a row of one call until the next call arrives and makes it a group — a diff --git a/app/androidApp/src/main/kotlin/com/example/aiapp/SessionScreen.kt b/app/androidApp/src/main/kotlin/com/example/aiapp/SessionScreen.kt index 9db63b1..d4c2620 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/SessionScreen.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/SessionScreen.kt @@ -82,7 +82,7 @@ import androidx.lifecycle.Lifecycle import androidx.lifecycle.compose.LocalLifecycleOwner import androidx.lifecycle.repeatOnLifecycle import java.util.concurrent.atomic.AtomicLong -import kotlin.math.roundToInt +import kotlin.math.abs import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.awaitCancellation import kotlinx.coroutines.cancelAndJoin @@ -169,34 +169,61 @@ private data class HistoryLoadSignal( ) /** - * Which row was asked to hold a point of itself still, and what share of its height change that - * point is worth; see [holdTouchedPoint]. + * Where a row has been asked to put itself, and how close it has got; see [holdTouchedPoint] and + * `SessionScreen`'s `settleHold`. * - * Deliberately *not* snapshot state, which is the point of the class. Both fields are written from + * Positions here are the list's own: the distance of an edge *above* the bottom of the viewport, + * which is what [LazyListItemInfo.offset] measures and the coordinate the whole correction is done + * in. [target] is the point the row has to arrive at -- the finger for a row shutting, its own top + * edge as it was for one opening -- and [onCentre] says which part of the row arrives there: the + * middle of it for a close, since what is left of a shut card is a heading and the finger should be + * on it wherever down the card it pressed, and the top edge for an open, where the row is a heading + * already and the edge pressed is what the reader wants held. + * + * Deliberately *not* snapshot state, which is the point of the class. The fields are written from * the layout phase; a snapshot write there that composition reads would schedule another * recomposition, and the correction has to land inside the frame already being laid out. [key] is - * cleared by the resize it was set for, so it cannot be spent on an unrelated one. + * cleared once the row has arrived, so a hold cannot be spent on an unrelated resize. */ private class TouchHold { var key: Any? = null + var target = 0 + var onCentre = false - /** - * 1 holds the row's top edge, 0 its bottom, and anything between the point that share names. - */ - var share = 0f -} + /** Where the row was in the list, for the pass where its own resize has hidden it. */ + var index = 0 -/** One row's height between layouts, so a change in it can be noticed. See [holdTouchedPoint]. */ -private class LastHeight { - var value: Int? = null + /** The row's height as this pass measured it, which its own modifier reports. */ + var height = 0 + + /** How far out the last pass was, so a pass that cannot improve on it is the last one. */ + var error = Int.MAX_VALUE + var passes = 0 } /** - * Which row the last touch landed in, and where down that row it landed -- as the share of the - * row's height that sits above the touch; see [toggleAnchored]. + * How many measure passes one hold may ask for. * - * A share rather than a distance because that is what both readings of the touch want: a close - * spends it as it is, and an open rounds it to the nearer edge. + * More than one because a single request is not always granted in full: a card giving back more + * than a screenful asks the list for rows below what it had composed, and it can only scroll as far + * as it has. The pass that answers is also the pass that composes them, so the next can ask for the + * rest. The cap is what stops a row the list will not move from asking forever. + */ +private const val HOLD_PASSES = 4 + +/** + * Reports this row's height to a hold on it, which is the one thing the hold cannot measure itself. + * + * In the measure phase, so the height is this pass's before the list's own measure hook reads it -- + * see `settleHold`, which is where the correction is made and why it is not made here. + */ +@Composable +private fun Modifier.holdTouchedPoint(key: Any, held: TouchHold): Modifier = onSizeChanged { size -> + if (held.key == key) held.height = size.height +} + +/** + * Which row the last touch landed in, and where in that row it landed. * * One slot rather than a map, because only the touch about to toggle something matters: * [toggleAnchored] reads it in the same gesture that wrote it. Written from a detector on each @@ -204,34 +231,10 @@ private class LastHeight { */ private class LastTouch { var key: Any? = null - var share = 0f -} -/** - * Keeps a point of this row still when the row changes height, if it was asked to. - * - * This runs in the *layout* phase, from the measurement that discovers the new height, and that is - * why it is a modifier rather than an effect. A correction posted to a coroutine arrives a frame or - * more after the layout it is correcting, so the wrong position is drawn once first -- visible as a - * flick, and worse the faster the screen refreshes. - * - * [hold] is given how far the list has to scroll. The row's bottom edge is held by the list and all - * growth goes upward, so scrolling by the whole change keeps the top edge where it was, by none of - * it the bottom edge, and by [TouchHold.share] of it the point that share names. - */ -@Composable -private fun Modifier.holdTouchedPoint(key: Any, held: TouchHold, hold: (Int) -> Unit): Modifier { - val last = remember { LastHeight() } - return onSizeChanged { size -> - val previous = last.value - last.value = size.height - // A first measurement has no previous height to have moved from, and a row that came back - // after being scrolled away is a first measurement again. - if (previous == null || previous == size.height || held.key != key) return@onSizeChanged - val share = held.share - held.key = null - hold(((size.height - previous) * share).roundToInt()) - } + /** How far down the row the finger landed, and how tall the row was when it did. */ + var y = 0 + var height = 1 } // The transcript's data model -- TranscriptItem, foldEvent, joinPages, warm -- lives in @@ -600,31 +603,104 @@ fun SessionScreen( * shrink therefore travels, and how far it travels depends on where down the row it landed. * * [closing] is the difference between the two ends of that, and they want different answers. - * Shutting a row leaves almost nothing of it, so the point under the finger is the only place - * the closed row can sensibly appear -- a row whose top is far off the screen is the case that - * settles it, since holding either edge throws the card the length of the screen away from the - * hand that shut it. Opening one is the opposite: the row is small, every point in it is within - * a heading's height of both edges, and what the reader wants held is the edge they pressed -- - * so the touch is rounded to the nearer of the two rather than splitting the difference and - * carrying the heading part of the way up an unbounded expansion. + * Shutting a row leaves a heading where a screenful of card was, and the place that heading + * belongs is under the finger that shut it -- centred on it, wherever down the card the tap + * landed. A card taller than the screen is the case that settles it: both of its edges can be a + * screen's length from the hand. Opening one is the opposite: the row is small, every point in + * it is within a heading's height of both edges, and what the reader wants held is the edge + * they pressed -- so the touch picks the nearer of the two rather than carrying the heading + * part of the way up an unbounded expansion. * * Where the touch landed decides it, rather than which control was pressed, so everything * behaves the same way whether or not it has a control at each end. * - * The correction itself belongs to the measurement -- see [holdTouchedPoint]. + * Asking for it is `settleHold`'s, which walks the row there once its new height is known. */ fun toggleAnchored(row: TranscriptRow, closing: Boolean, toggle: () -> Unit) = expanding { - if (lastTouch.key == row.key) { - val share = if (closing) lastTouch.share else if (lastTouch.share > 0.5f) 1f else 0f - // Nothing to correct at zero: the bottom edge is what the list holds on its own. - if (share > 0f) { - touchHeld.key = row.key - touchHeld.share = share - } + val item = listState.layoutInfo.visibleItemsInfo.firstOrNull { it.key == row.key } + // An open in the row's lower half wants its bottom edge held, which is what the list does + // on its own -- there is nothing to ask for and nothing to correct. + val wanted = closing || lastTouch.y < lastTouch.height / 2 + if (lastTouch.key == row.key && item != null && wanted) { + // An item's offset is its bottom edge, counted up from the bottom of the viewport, and + // the row sits on that edge with its gap above it -- so the touch is that far up, less + // how far down the row the finger landed. The finger for a close; for an open the edge + // nearest it, which is the row's top because the other half of `wanted` is the other + // edge. + val top = item.offset + lastTouch.height + touchHeld.key = row.key + touchHeld.index = item.index + touchHeld.onCentre = closing + touchHeld.target = if (closing) top - lastTouch.y else top + touchHeld.height = 0 + touchHeld.error = Int.MAX_VALUE + touchHeld.passes = 0 } toggle() } + /** + * Walks the held row to where the touch on it asked for, one measure pass at a time. + * + * Called from inside the list's own measure ([TranscriptList]'s `afterMeasure`), which is the + * only place a correction lands: a scroll asked for from the placement phase is not picked up + * by another measure and so does nothing at all, and one posted to a coroutine arrives a frame + * late, which is the flick this whole mechanism exists to avoid. + * + * It measures rather than predicts, and that is the point. Working out the scroll from the + * change in height needs the list to behave the way the arithmetic assumed, and it does not: + * which item it holds still across a resize, and how much of a scroll it can honour, both + * depend on what it has composed -- a row taller than the screen is anchored on *itself*, and a + * close asks for the rows below one that has been giving the screen its whole height. Asking + * for the error this pass can see, and asking again on the pass that answers, is what does not + * have to know any of that. A row the list will not move gets one pass that changes nothing and + * is then let go. + */ + fun settleHold() { + val key = touchHeld.key ?: return + val height = touchHeld.height + val info = listState.layoutInfo + val item = info.visibleItemsInfo.firstOrNull { it.key == key } + touchHeld.passes++ + // Measured before its own row was, or out of passes: nothing to do, and a correction aimed + // at a guess is worse than none. + if (height == 0 || touchHeld.passes > HOLD_PASSES) { + touchHeld.key = null + return + } + // A row shut while it reached past the bottom of the screen has just left it: 2,600px of + // card became a heading, and the heading is below the viewport with the rest of the + // conversation above it. It is not in this pass's layout to be measured against, so this + // pass spends itself asking for it back -- against the bottom edge, which is the one + // position the list can always be asked for -- and the next one, which has it, corrects + // the rest. + if (item == null) { + Snapshot.withoutReadObservation { listState.requestScrollToItem(touchHeld.index, 0) } + return + } + // Where the row's bottom edge has to be for the part that matters to land on the target. + val wanted = touchHeld.target - if (touchHeld.onCentre) height / 2 else height + val error = wanted - item.offset + // Arrived, or as far as the list will go -- a request it cannot honour leaves the error + // where it was, and asking again would only spend passes. + if (error == 0 || abs(error) >= abs(touchHeld.error)) { + touchHeld.key = null + return + } + touchHeld.error = error + // Reads unobserved: this runs inside the list's measure, and observing the scroll position + // here would make every frame's measure depend on it. + Snapshot.withoutReadObservation { + // A scroll offset counts *down* from the viewport's bottom edge, so moving the row up + // by the error means taking the error off it. Negative is allowed and is what a close + // needs: it is the list being asked for the rows below what it has composed. + listState.requestScrollToItem( + listState.firstVisibleItemIndex, + listState.firstVisibleItemScrollOffset - error, + ) + } + } + /** * Whether the row holding transcript position [seq] is loaded, with older history behind it. * @@ -1562,6 +1638,7 @@ fun SessionScreen( selection = selection, modifier = Modifier.fillMaxSize().drawWithContent { if (settled) drawContent() }, + afterMeasure = ::settleHold, below = { // The last thing in the transcript, because that is where they are in // the session's reading of events: after everything it has taken in, @@ -1628,32 +1705,7 @@ fun SessionScreen( is TranscriptUnit.Whole -> { val row = unit.row Box( - Modifier.holdTouchedPoint(row.key, touchHeld) { by -> - // A *request*, not a raw scroll delta: this runs - // inside the measure pass that discovered the new - // height, and a raw delta forces a synchronous - // remeasure from within measure, which is fatal. The - // request is applied by the same frame's next - // remeasure, so the correction still lands before - // anything is drawn. Reads unobserved, or this row's - // measure would inherit the scroll position as a - // dependency and remeasure on every frame. - // - // The offset goes negative on a close, and has to: - // that is the list being asked for room below what it - // has composed, which is where the newer rows come - // from when a card gives a screenful back. Clamping it - // at zero -- which is what this did while every - // correction was a row *growing* -- leaves a close - // uncorrected, and the card lands wherever the list - // felt like putting it. - Snapshot.withoutReadObservation { - listState.requestScrollToItem( - listState.firstVisibleItemIndex, - listState.firstVisibleItemScrollOffset + by, - ) - } - } + Modifier.holdTouchedPoint(row.key, touchHeld) // Where down this row the touch landed, for // [toggleAnchored]. On the initial pass and consuming // nothing, so every control inside still gets the gesture; @@ -1667,12 +1719,9 @@ fun SessionScreen( pass = PointerEventPass.Initial, ) lastTouch.key = row.key - lastTouch.share = - 1f - - (down.position.y / size.height).coerceIn( - 0f, - 1f, - ) + lastTouch.height = size.height + lastTouch.y = + down.position.y.toInt().coerceIn(0, size.height) } } ) { diff --git a/app/androidApp/src/main/kotlin/com/example/aiapp/TranscriptList.kt b/app/androidApp/src/main/kotlin/com/example/aiapp/TranscriptList.kt index 5da4a9c..ec3cd97 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/TranscriptList.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/TranscriptList.kt @@ -57,6 +57,16 @@ fun TranscriptList( onRetryHistory: () -> Unit, selection: SelectionState, modifier: Modifier = Modifier, + /** + * Called inside the list's own measure, once its items have been measured and its + * [LazyListState.layoutInfo] describes this pass rather than the last one. + * + * It is where a correction to the scroll position belongs, and the only place one lands: a + * scroll asked for during the *placement* phase is not picked up by another measure, so it does + * nothing at all until something else moves the list. Asked for here it is served by this + * frame's next pass, before anything is drawn. See `SessionScreen`'s hold. + */ + afterMeasure: () -> Unit = {}, below: @Composable () -> Unit, unit: @Composable (TranscriptUnit) -> Unit, ) { @@ -73,6 +83,7 @@ fun TranscriptList( .layout { measurable, constraints -> val started = System.nanoTime() val placeable = measurable.measure(constraints) + afterMeasure() DebugStats.record( "measure: the whole transcript", System.nanoTime() - started,