From 8126b6e005c593d7492fa18ad18b6b4ca5bf6890 Mon Sep 17 00:00:00 2001 From: Usama <83345144+usamaiqb@users.noreply.github.com> Date: Wed, 19 Aug 2026 01:05:53 +0500 Subject: [PATCH 1/2] fix(viewer): stop scroll stutter on upward scroll MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ScrollView.onSizeChanged re-scrolls to keep a focused descendant visible. Code blocks are selectable TextViews and therefore focusable, and the chrome's expand/collapse animation resizes the viewport every frame — so that automatic scroll landed mid-drag, and the deltas it emitted flipped the chrome's own show/hide threshold straight back. Also stop routing the live scroll position through composition. It recomposed the whole screen, and the AndroidView update block, on every scroll frame, where an animated text colour then rebuilt the segment view tree and cancelled the gesture in flight. --- .../com/markreader/ui/screens/EditorScreen.kt | 5 +- .../com/markreader/ui/screens/ViewerScreen.kt | 151 +++++++--- .../ui/screens/ViewerTextInterop.kt | 271 ++++++++++++++---- .../markreader/ui/screens/ViewerViewModel.kt | 21 +- 4 files changed, 347 insertions(+), 101 deletions(-) diff --git a/app/src/main/java/com/markreader/ui/screens/EditorScreen.kt b/app/src/main/java/com/markreader/ui/screens/EditorScreen.kt index 769840d..6860735 100644 --- a/app/src/main/java/com/markreader/ui/screens/EditorScreen.kt +++ b/app/src/main/java/com/markreader/ui/screens/EditorScreen.kt @@ -295,14 +295,17 @@ fun EditorScreen( } else { val previewTextColor = MaterialTheme.colorScheme.onSurface.toArgb() val previewIsDark = isSystemInDarkTheme() + // The preview never restores a scroll position. + val previewScrollY = remember { mutableStateOf(0) } Box(modifier = Modifier.weight(1f).fillMaxWidth()) { RenderedTextView( text = previewText, textColor = previewTextColor, padding = PaddingValues(0.dp), - savedScrollY = 0, + savedScrollY = previewScrollY, scrollToOffset = null, onScrollChanged = { _, _ -> }, + onScrollExtentChanged = {}, onScrollConsumed = {}, headings = emptyList(), onActiveHeadingChanged = {}, diff --git a/app/src/main/java/com/markreader/ui/screens/ViewerScreen.kt b/app/src/main/java/com/markreader/ui/screens/ViewerScreen.kt index 647c121..74fb2ed 100644 --- a/app/src/main/java/com/markreader/ui/screens/ViewerScreen.kt +++ b/app/src/main/java/com/markreader/ui/screens/ViewerScreen.kt @@ -2,6 +2,7 @@ package com.markreader.ui.screens import android.app.Application import android.os.Build +import android.os.SystemClock import androidx.activity.compose.BackHandler import androidx.activity.compose.rememberLauncherForActivityResult import androidx.activity.result.contract.ActivityResultContracts @@ -79,10 +80,12 @@ import androidx.compose.material3.Surface import androidx.compose.material3.Text import androidx.compose.material3.TopAppBar import androidx.compose.material3.TopAppBarDefaults +import androidx.compose.ui.graphics.Color import androidx.compose.ui.graphics.toArgb import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.DisposableEffect +import androidx.compose.runtime.State import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember @@ -115,6 +118,12 @@ import com.markreader.ui.components.segmentShape import com.markreader.ui.export.ExportManager import kotlin.math.roundToInt +/** + * How long the chrome's visibility decision is held after it changes, covering the + * show/hide animation and the relayout it causes. + */ +private const val CHROME_SETTLE_MS = 350L + @OptIn(ExperimentalMaterial3Api::class) @Composable fun ViewerScreen( @@ -130,8 +139,13 @@ fun ViewerScreen( ) val uiState by viewModel.uiState.collectAsStateWithLifecycle() val scrollToOffset by viewModel.scrollToOffset.collectAsStateWithLifecycle() - val savedScrollY by viewModel.scrollY.collectAsStateWithLifecycle() - val scrollProgress by viewModel.scrollProgress.collectAsStateWithLifecycle() + // Deliberately not read with `by` here. Both change on every scroll frame, so + // reading them in this scope would recompose the whole screen — including the + // viewer's AndroidView update block — 60+ times a second while scrolling. + // They are read inside the leaf composables that actually display them, and + // inside snapshotFlow below, so the reads stay out of this scope. + val savedScrollY = viewModel.scrollY.collectAsStateWithLifecycle() + val scrollProgress = viewModel.scrollProgress.collectAsStateWithLifecycle() val prefs = uiState.userPreferences val isSystemDark = isSystemInDarkTheme() @@ -176,16 +190,32 @@ fun ViewerScreen( // Immersive reading: hide the chrome on downward scrolls, bring it back on // upward scrolls or at the top. Large deltas are programmatic jumps (TOC, // search match) where the user just used the chrome — keep it visible. + // Showing or hiding the bar changes the content's top inset, which resizes the + // ScrollView and can clamp its position — emitting a scroll delta that points + // the opposite way and immediately flips the decision back. Hold the decision + // briefly after each change so the bar can finish animating instead of + // stuttering against its own layout effect. LaunchedEffect(Unit) { var lastY = 0 - snapshotFlow { savedScrollY }.collect { y -> + var settleUntil = 0L + snapshotFlow { savedScrollY.value }.collect { y -> val delta = y - lastY - when { - y <= 0 -> isChromeVisible = true - delta < -8 -> isChromeVisible = true - delta in 9..1200 -> isChromeVisible = false - } lastY = y + if (y <= 0) { + isChromeVisible = true + settleUntil = 0L + return@collect + } + if (SystemClock.uptimeMillis() < settleUntil) return@collect + val target = when { + delta < -8 -> true + delta in 9..1200 -> false + else -> return@collect + } + if (target != isChromeVisible) { + isChromeVisible = target + settleUntil = SystemClock.uptimeMillis() + CHROME_SETTLE_MS + } } } LaunchedEffect(uiState.isSearchActive) { @@ -407,23 +437,11 @@ fun ViewerScreen( ) } } - scrollProgress?.let { progress -> - Surface( - shape = RoundedCornerShape(50), - color = chromeColors.tonalContainer.copy(alpha = 0.6f), - contentColor = chromeColors.muted - ) { - Text( - text = "${(progress * 100).roundToInt()}%", - style = MaterialTheme.typography.labelSmall, - maxLines = 1, - modifier = Modifier.padding( - horizontal = 8.dp, - vertical = 2.dp - ) - ) - } - } + ReadingProgressChip( + progress = scrollProgress, + containerColor = chromeColors.tonalContainer, + contentColor = chromeColors.muted + ) } } }, @@ -614,26 +632,11 @@ fun ViewerScreen( } } } - val animatedReadProgress by animateFloatAsState( - targetValue = scrollProgress ?: 0f, - animationSpec = spring(stiffness = Spring.StiffnessLow), - label = "readingProgress" + ReadingProgressBar( + progress = scrollProgress, + trackColor = chromeColors.tonalContainer, + barColor = chromeColors.content ) - if (scrollProgress != null) { - Box( - modifier = Modifier - .fillMaxWidth() - .height(3.dp) - .background(chromeColors.tonalContainer.copy(alpha = 0.5f)) - ) { - Box( - modifier = Modifier - .fillMaxWidth(animatedReadProgress.coerceIn(0f, 1f)) - .fillMaxHeight() - .background(chromeColors.content.copy(alpha = 0.7f)) - ) - } - } } } ) { paddingValues: PaddingValues -> @@ -712,6 +715,7 @@ fun ViewerScreen( savedScrollY = savedScrollY, scrollToOffset = scrollToOffset, onScrollChanged = viewModel::onScrollPositionChanged, + onScrollExtentChanged = viewModel::onScrollExtentChanged, onScrollConsumed = viewModel::onScrollConsumed, headings = uiState.headings, onActiveHeadingChanged = viewModel::onActiveHeadingChanged, @@ -891,6 +895,65 @@ fun ViewerScreen( } } +/** + * Reading-progress percentage chip. + * + * Takes progress as [State] and reads it here rather than in [ViewerScreen], so + * that a scroll frame recomposes only this chip instead of the entire screen. + */ +@Composable +private fun ReadingProgressChip( + progress: State, + containerColor: Color, + contentColor: Color +) { + val value = progress.value ?: return + Surface( + shape = RoundedCornerShape(50), + color = containerColor.copy(alpha = 0.6f), + contentColor = contentColor + ) { + Text( + text = "${(value * 100).roundToInt()}%", + style = MaterialTheme.typography.labelSmall, + maxLines = 1, + modifier = Modifier.padding(horizontal = 8.dp, vertical = 2.dp) + ) + } +} + +/** + * Reading-progress bar under the chrome. Reads progress in its own scope for the + * same reason as [ReadingProgressChip]. + */ +@Composable +private fun ReadingProgressBar( + progress: State, + trackColor: Color, + barColor: Color +) { + val value = progress.value + val animatedReadProgress by animateFloatAsState( + targetValue = value ?: 0f, + animationSpec = spring(stiffness = Spring.StiffnessLow), + label = "readingProgress" + ) + if (value == null) return + Box( + modifier = Modifier + .fillMaxWidth() + .height(3.dp) + .background(trackColor.copy(alpha = 0.5f)) + ) { + Box( + modifier = Modifier + .fillMaxWidth(animatedReadProgress.coerceIn(0f, 1f)) + .fillMaxHeight() + .background(barColor.copy(alpha = 0.7f)) + ) + } +} + @Composable private fun TocHeadingRow( heading: HeadingItem, diff --git a/app/src/main/java/com/markreader/ui/screens/ViewerTextInterop.kt b/app/src/main/java/com/markreader/ui/screens/ViewerTextInterop.kt index deec71e..bad87da 100644 --- a/app/src/main/java/com/markreader/ui/screens/ViewerTextInterop.kt +++ b/app/src/main/java/com/markreader/ui/screens/ViewerTextInterop.kt @@ -1,6 +1,7 @@ package com.markreader.ui.screens import android.content.Context +import android.graphics.Rect import android.graphics.Typeface import android.os.Build import android.graphics.text.LineBreaker @@ -21,11 +22,13 @@ import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.widthIn import androidx.compose.runtime.Composable +import androidx.compose.runtime.State import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember import androidx.compose.runtime.rememberUpdatedState import androidx.compose.runtime.setValue +import androidx.compose.runtime.snapshots.Snapshot import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.unit.dp @@ -41,11 +44,79 @@ import com.markreader.ui.zoom.ZoomableContentLayout import android.graphics.Color import android.graphics.drawable.GradientDrawable import android.view.Gravity +import android.view.MotionEvent +import android.view.ViewConfiguration import io.noties.markwon.ext.tables.TableRowSpan +import kotlin.math.abs private enum class SegmentType { Text, Code, Table } private data class Segment(val start: Int, val end: Int, val type: SegmentType) +private class DirectionalHorizontalScrollView(context: Context) : HorizontalScrollView(context) { + private val touchSlop = ViewConfiguration.get(context).scaledTouchSlop + private var downX = 0f + private var downY = 0f + private var directionDecided = false + + override fun onInterceptTouchEvent(ev: MotionEvent): Boolean { + updateParentIntercept(ev) + return super.onInterceptTouchEvent(ev) + } + + override fun onTouchEvent(ev: MotionEvent): Boolean { + updateParentIntercept(ev) + return super.onTouchEvent(ev) + } + + private fun updateParentIntercept(ev: MotionEvent) { + when (ev.actionMasked) { + MotionEvent.ACTION_DOWN -> { + downX = ev.x + downY = ev.y + directionDecided = false + parent?.requestDisallowInterceptTouchEvent(false) + } + MotionEvent.ACTION_MOVE -> { + if (!directionDecided) { + val dx = abs(ev.x - downX) + val dy = abs(ev.y - downY) + if (dx > touchSlop || dy > touchSlop) { + directionDecided = true + parent?.requestDisallowInterceptTouchEvent(dx > dy) + } + } + } + MotionEvent.ACTION_UP, + MotionEvent.ACTION_CANCEL -> { + directionDecided = false + parent?.requestDisallowInterceptTouchEvent(false) + } + } + } +} + +/** + * A [ScrollView] that never scrolls itself to keep a focused descendant on screen. + * + * Code blocks are selectable TextViews, and `setTextIsSelectable(true)` makes them + * focusable. Android's ScrollView reacts to a focused descendant in two ways, and + * both fight the user here: + * + * - `requestChildFocus()` scrolls the focused child into view. + * - `onSizeChanged()` re-scrolls to keep it on screen whenever the viewport + * height changes — and the chrome show/hide animation changes that height on + * every frame. + * + * The second one is the damaging one: the automatic `doScrollY` lands in the + * middle of the user's drag, and the scroll deltas it emits feed straight back + * into the chrome's show/hide threshold, so the bar re-hides itself as it is + * appearing. Both paths route through [computeScrollDeltaToGetChildRectOnScreen], + * so neutralising it disables both while leaving the text selectable. + */ +private class FocusStableScrollView(context: Context) : ScrollView(context) { + override fun computeScrollDeltaToGetChildRectOnScreen(rect: Rect?): Int = 0 +} + private data class ContentKey( val textHash: Int, val fontSizeSp: Float, @@ -56,14 +127,32 @@ private data class ContentKey( val textAlignment: TextAlignmentPreference ) +/** + * Identity of a scroll restore. [ContentKey] alone is not enough: toggling a + * wrap setting rebuilds the view tree without changing the content, so a + * restore for the new structure must not be confused with one already applied + * to the old structure. + */ +private data class RestoreKey( + val content: ContentKey, + val isWordWrapEnabled: Boolean, + val isCodeBlockWrapEnabled: Boolean +) + @Composable fun RenderedTextView( text: Any, textColor: Int, padding: PaddingValues, - savedScrollY: Int, + // Held as State, not Int: the live scroll position changes every frame while + // scrolling, and taking it as a value would recompose this composable — and + // re-run the AndroidView update block — on every one of those frames. It is + // only ever read to restore a position, so it is read without snapshot + // observation below. + savedScrollY: State, scrollToOffset: Int?, onScrollChanged: (scrollY: Int, maxScrollY: Int) -> Unit, + onScrollExtentChanged: (maxScrollY: Int) -> Unit, onScrollConsumed: () -> Unit, headings: List, onActiveHeadingChanged: (Int) -> Unit, @@ -89,7 +178,23 @@ fun RenderedTextView( textAlignment = textAlignment ) } - var lastRestoredKey by remember { mutableStateOf(null) } + // Span scans over the whole document — cached per text, never per frame. + val docHasCodeBlocks = remember(text) { text is Spanned && hasCodeBlocks(text) } + val docHasTables = remember(text) { text is Spanned && hasTables(text) } + // Split mode gives code blocks and tables their own horizontally scrollable + // views. Only worth the cost when the document actually contains one. + val isSplitMode = remember( + text, isWordWrapEnabled, isCodeBlockWrapEnabled, docHasCodeBlocks, docHasTables + ) { + text is Spanned && ( + (isWordWrapEnabled && ((!isCodeBlockWrapEnabled && docHasCodeBlocks) || docHasTables)) || + (!isWordWrapEnabled && docHasCodeBlocks) + ) + } + val restoreKey = remember(contentKey, isWordWrapEnabled, isCodeBlockWrapEnabled) { + RestoreKey(contentKey, isWordWrapEnabled, isCodeBlockWrapEnabled) + } + var lastRestoredKey by remember { mutableStateOf(null) } var lastWrapEnabled by remember { mutableStateOf(isWordWrapEnabled) } var lastCodeBlockWrapEnabled by remember { mutableStateOf(isCodeBlockWrapEnabled) } var pendingAnchorOffset by remember { mutableStateOf(null) } @@ -111,6 +216,7 @@ fun RenderedTextView( var lastTextColor by remember { mutableStateOf(textColor) } var lastWrapEnabledApplied by remember { mutableStateOf(isWordWrapEnabled) } var lastSelectionHighlightColor by remember { mutableStateOf(selectionHighlightColor) } + var lastCodeBlockBackgroundColor by remember { mutableStateOf(codeBlockBackgroundColor) } var splitBoundaries by remember { mutableStateOf>(emptyList()) } val currentHeadings by rememberUpdatedState(headings) val currentSplitBoundaries by rememberUpdatedState(splitBoundaries) @@ -129,13 +235,9 @@ fun RenderedTextView( factory = { context -> val density = context.resources.displayMetrics.density val paddingPx = (16 * density).toInt() - val isSplitMode = text is Spanned && ( - (isWordWrapEnabled && (!isCodeBlockWrapEnabled || hasTables(text))) || - (!isWordWrapEnabled && hasCodeBlocks(text)) - ) val useGlobalHorizontalScroll = !isWordWrapEnabled - val scrollView = ScrollView(context).apply { + val scrollView = FocusStableScrollView(context).apply { if (isSplitMode) { val container = LinearLayout(context).apply { orientation = LinearLayout.VERTICAL @@ -191,9 +293,13 @@ fun RenderedTextView( } } // Report scroll extent whenever content lays out so reading - // progress is available before the first scroll event. + // progress is available before the first scroll event. This + // reports the extent only — never a position. It fires on every + // window-wide layout, including every frame of the chrome's + // show/hide animation, and reporting a position from here would + // feed those frames into the chrome's scroll-delta threshold. viewTreeObserver.addOnGlobalLayoutListener { - onScrollChanged(scrollY, computeMaxScrollY(this)) + onScrollExtentChanged(computeMaxScrollY(this)) } } val rootView = if (useGlobalHorizontalScroll) { @@ -228,10 +334,6 @@ fun RenderedTextView( else -> rootView as ScrollView } val child = scrollView.getChildAt(0) ?: return@AndroidView - val isSplitMode = text is Spanned && ( - (isWordWrapEnabled && (!isCodeBlockWrapEnabled || hasTables(text))) || - (!isWordWrapEnabled && hasCodeBlocks(text)) - ) val useGlobalHorizontalScroll = !isWordWrapEnabled val wrapChanged = lastWrapEnabled != isWordWrapEnabled || lastCodeBlockWrapEnabled != isCodeBlockWrapEnabled @@ -239,6 +341,7 @@ fun RenderedTextView( val needsRestructure = wrapChanged || (isSplitMode != currentIsSplit) val density = scrollView.context.resources.displayMetrics.density val paddingPx = (16 * density).toInt() + var activeSplitBoundaries = currentSplitBoundaries if (needsRestructure) { val hasGlobalHorizontalScroll = rootView is HorizontalScrollView @@ -329,6 +432,7 @@ fun RenderedTextView( lastTextColor = textColor lastWrapEnabledApplied = isWordWrapEnabled lastSelectionHighlightColor = selectionHighlightColor + lastCodeBlockBackgroundColor = codeBlockBackgroundColor } // Text / style updates @@ -351,6 +455,7 @@ fun RenderedTextView( val splitTables = isWordWrapEnabled val segments = splitByMarkers(spanned, splitCode, splitTables) splitBoundaries = segments + activeSplitBoundaries = segments container.removeAllViews() for ((start, end, type) in segments) { val isCode = type == SegmentType.Code @@ -368,7 +473,7 @@ fun RenderedTextView( selectionHighlightColor, density ) val contentView = if (tableView != null) { - HorizontalScrollView(container.context).apply { + DirectionalHorizontalScrollView(container.context).apply { isHorizontalScrollBarEnabled = true addView( tableView, @@ -410,7 +515,7 @@ fun RenderedTextView( ) tv.text = segmentContent val contentView = if (needsCodeHScroll) { - HorizontalScrollView(container.context).apply { + DirectionalHorizontalScrollView(container.context).apply { isHorizontalScrollBarEnabled = true addView( tv, @@ -457,10 +562,11 @@ fun RenderedTextView( lastStyleKey = contentKey lastTextColor = textColor lastSelectionHighlightColor = selectionHighlightColor + lastCodeBlockBackgroundColor = codeBlockBackgroundColor } else if (textRefChanged) { // Same underlying text, different spans (search highlights) — // update existing TextViews in-place without rebuilding views - val boundaries = currentSplitBoundaries + val boundaries = activeSplitBoundaries for (i in 0 until container.childCount.coerceAtMost(boundaries.size)) { val (start, end, type) = boundaries[i] val segText = spanned.subSequence(start, end) as Spanned @@ -495,36 +601,47 @@ fun RenderedTextView( } } lastTextRef = spanned - } else if (lastStyleKey != contentKey || lastTextColor != textColor || - lastSelectionHighlightColor != selectionHighlightColor - ) { - // Rebuild segments to refresh table colors and code spans - lastTextHash = 0 } if (lastStyleKey != contentKey || lastTextColor != textColor || - lastSelectionHighlightColor != selectionHighlightColor + lastSelectionHighlightColor != selectionHighlightColor || + lastCodeBlockBackgroundColor != codeBlockBackgroundColor ) { + // Restyle in place. This must never fall back to a segment + // rebuild: textColor comes from an animated Color, so a + // rebuild here would tear down and recreate the whole view + // tree on alternating frames of that animation — which also + // cancels any touch gesture in flight. for (i in 0 until container.childCount) { val seg = container.getChildAt(i) val target = when (seg) { - is FrameLayout -> seg.getChildAt(0) + is FrameLayout -> { + // Code block wrapper — carries the block tint. + seg.setBackgroundColor(codeBlockBackgroundColor) + seg.getChildAt(0) + } else -> seg } - val tv = if (target is HorizontalScrollView) { - target.getChildAt(0) as? TextView + val inner = if (target is HorizontalScrollView) { + target.getChildAt(0) } else { - target as? TextView + target } - tv?.let { - applyStyleToTextView( - it, fontSizeSp, lineHeight, readingFont, codeFont, isSourceCode, - textAlignment, textColor, selectionHighlightColor + when (inner) { + is android.widget.TableLayout -> restyleTableLayout( + inner, textColor, fontSizeSp, lineHeight, readingFont, + selectionHighlightColor, density + ) + is TextView -> applyStyleToTextView( + inner, fontSizeSp, lineHeight, readingFont, codeFont, + isSourceCode, textAlignment, textColor, + selectionHighlightColor ) } } lastStyleKey = contentKey lastTextColor = textColor lastSelectionHighlightColor = selectionHighlightColor + lastCodeBlockBackgroundColor = codeBlockBackgroundColor } } else { // Single-TV mode @@ -574,31 +691,38 @@ fun RenderedTextView( } } - // Scroll handling - if (pendingAnchorOffset != null) { - val targetOffset = pendingAnchorOffset + // Scroll handling. Every branch claims restoreKey synchronously, + // before posting: the state writes above cause another pass through + // this block before the posted runnable gets to run, and a claim made + // inside the runnable would let that pass queue a second, conflicting + // restore behind this one. + val targetOffset = pendingAnchorOffset + if (targetOffset != null) { pendingAnchorOffset = null + lastRestoredKey = restoreKey scrollView.post { val y = resolveScrollY( - scrollView, currentSplitBoundaries, targetOffset ?: return@post + scrollView, activeSplitBoundaries, targetOffset ) - lastRestoredKey = contentKey scrollView.smoothScrollTo(0, y) } - } else if (scrollToOffset == null && savedScrollY > 0 && - lastRestoredKey != contentKey - ) { - lastRestoredKey = contentKey - scrollView.post { scrollView.scrollTo(0, savedScrollY) } + } else if (scrollToOffset == null && lastRestoredKey != restoreKey) { + // Restore-only read: observing it here would resubscribe this + // block to a value that changes on every scroll frame. + val restoreY = Snapshot.withoutReadObservation { savedScrollY.value } + if (restoreY > 0) { + lastRestoredKey = restoreKey + scrollView.post { scrollView.scrollTo(0, restoreY) } + } } if (scrollToOffset != null) { + lastRestoredKey = restoreKey scrollView.post { val y = resolveScrollY( - scrollView, currentSplitBoundaries, scrollToOffset + scrollView, activeSplitBoundaries, scrollToOffset ) val centeredY = (y - scrollView.height / 3).coerceAtLeast(0) - lastRestoredKey = contentKey scrollView.smoothScrollTo(0, centeredY) if (zoomLayout.currentScale > 1f) zoomLayout.resetPan() onScrollConsumed() @@ -611,7 +735,7 @@ fun RenderedTextView( if (curChild is LinearLayout) { onActiveHeadingChangedState( findActiveHeadingInSplit( - curChild, currentSplitBoundaries, + curChild, activeSplitBoundaries, currentHeadings, scrollView.scrollY ) ) @@ -846,10 +970,7 @@ private fun buildTableLayout( } catch (_: Exception) { return null } val cellPaddingPx = (8 * density).toInt() - val borderWidthPx = maxOf(1, density.toInt()) - // Markwon defaults: border = textColor at 75/255 alpha, odd row bg = textColor at 22/255 alpha - val borderColor = (textColor and 0x00FFFFFF) or (75 shl 24) - val oddRowBg = (textColor and 0x00FFFFFF) or (22 shl 24) + val borderWidthPx = tableBorderWidthPx(density) val typeface = resolveTypeface(context, false, readingFont, CodeFontPreference.JetBrainsMono) val tableLayout = android.widget.TableLayout(context) @@ -866,10 +987,7 @@ private fun buildTableLayout( val tableRow = android.widget.TableRow(context) for ((colIndex, cell) in cells.withIndex()) { - val cellBg = GradientDrawable().apply { - setStroke(borderWidthPx, borderColor) - setColor(if (isOdd) oddRowBg else Color.TRANSPARENT) - } + val cellBg = tableCellBackground(textColor, isOdd, borderWidthPx) val cellTv = SearchHighlightTextView(context).apply { this.text = cell.text() textSize = fontSizeSp @@ -906,6 +1024,57 @@ private fun buildTableLayout( return tableLayout } +private fun tableBorderWidthPx(density: Float): Int = maxOf(1, density.toInt()) + +// Markwon defaults: border = textColor at 75/255 alpha, odd row bg = textColor at 22/255 alpha +private fun tableCellBackground( + textColor: Int, + isOdd: Boolean, + borderWidthPx: Int +): GradientDrawable = GradientDrawable().apply { + setStroke(borderWidthPx, (textColor and 0x00FFFFFF) or (75 shl 24)) + setColor(if (isOdd) (textColor and 0x00FFFFFF) or (22 shl 24) else Color.TRANSPARENT) +} + +/** + * Reapply colors and text styling to an existing table without rebuilding it. + * Cell backgrounds bake in [textColor], so a theme change has to touch them — + * but tearing the table down to do it would also tear down the view tree the + * user may be mid-gesture on. + */ +private fun restyleTableLayout( + table: android.widget.TableLayout, + textColor: Int, + fontSizeSp: Float, + lineHeight: Float, + readingFont: ReadingFontPreference, + selectionHighlightColor: Int, + density: Float +) { + val borderWidthPx = tableBorderWidthPx(density) + val typeface = resolveTypeface( + table.context, false, readingFont, CodeFontPreference.JetBrainsMono + ) + for (rowIndex in 0 until table.childCount) { + val row = table.getChildAt(rowIndex) as? android.widget.TableRow ?: continue + val isHeader = rowIndex == 0 + val isOdd = !isHeader && rowIndex % 2 == 1 + for (colIndex in 0 until row.childCount) { + val cell = row.getChildAt(colIndex) as? TextView ?: continue + cell.textSize = fontSizeSp + cell.setLineSpacing(0f, lineHeight) + cell.setTextColor(textColor) + cell.highlightColor = selectionHighlightColor + cell.typeface = if (isHeader) { + Typeface.create(typeface, Typeface.BOLD) + } else { + typeface + } + cell.background = tableCellBackground(textColor, isOdd, borderWidthPx) + } + } +} + private fun stripBackgroundSpans(text: Spanned): Spanned { val mutable = SpannableStringBuilder(text) val lineSpans = mutable.getSpans(0, mutable.length, LineBackgroundSpan::class.java) diff --git a/app/src/main/java/com/markreader/ui/screens/ViewerViewModel.kt b/app/src/main/java/com/markreader/ui/screens/ViewerViewModel.kt index 0c70db2..b07ac55 100644 --- a/app/src/main/java/com/markreader/ui/screens/ViewerViewModel.kt +++ b/app/src/main/java/com/markreader/ui/screens/ViewerViewModel.kt @@ -351,11 +351,22 @@ class ViewerViewModel( fun onScrollPositionChanged(y: Int, maxY: Int) { _scrollY.value = y - _scrollProgress.value = if (maxY > 0) { - (y.toFloat() / maxY).coerceIn(0f, 1f) - } else { - null - } + _scrollProgress.value = progressFor(y, maxY) + } + + /** + * The scrollable extent changed without the position moving — a relayout + * rather than a scroll. Updates progress only: publishing a position here + * would make layout passes indistinguishable from user scrolling. + */ + fun onScrollExtentChanged(maxY: Int) { + _scrollProgress.value = progressFor(_scrollY.value, maxY) + } + + private fun progressFor(y: Int, maxY: Int): Float? = if (maxY > 0) { + (y.toFloat() / maxY).coerceIn(0f, 1f) + } else { + null } fun onScrollConsumed() { From bfff533e475604b385002bddf48e73671c47fb5d Mon Sep 17 00:00:00 2001 From: Usama <83345144+usamaiqb@users.noreply.github.com> Date: Wed, 19 Aug 2026 01:06:10 +0500 Subject: [PATCH 2/2] fix(viewer): keep scroll anchor across wrap toggles A table segment has no TextView, so resolving its anchor fell through to 0 and sent the viewer to the top of the document. Distinguish "unresolvable" from "position 0" and hold position instead. Also fix the double-counted container padding in the split path, narrow the focused-descendant scroll suppression to the two callers that actually misbehave so selection handles still auto-scroll, and drop the no-op DirectionalHorizontalScrollView. --- .../ui/screens/ViewerTextInterop.kt | 139 +++++++++--------- 1 file changed, 72 insertions(+), 67 deletions(-) diff --git a/app/src/main/java/com/markreader/ui/screens/ViewerTextInterop.kt b/app/src/main/java/com/markreader/ui/screens/ViewerTextInterop.kt index bad87da..4c2959c 100644 --- a/app/src/main/java/com/markreader/ui/screens/ViewerTextInterop.kt +++ b/app/src/main/java/com/markreader/ui/screens/ViewerTextInterop.kt @@ -44,57 +44,11 @@ import com.markreader.ui.zoom.ZoomableContentLayout import android.graphics.Color import android.graphics.drawable.GradientDrawable import android.view.Gravity -import android.view.MotionEvent -import android.view.ViewConfiguration import io.noties.markwon.ext.tables.TableRowSpan -import kotlin.math.abs private enum class SegmentType { Text, Code, Table } private data class Segment(val start: Int, val end: Int, val type: SegmentType) -private class DirectionalHorizontalScrollView(context: Context) : HorizontalScrollView(context) { - private val touchSlop = ViewConfiguration.get(context).scaledTouchSlop - private var downX = 0f - private var downY = 0f - private var directionDecided = false - - override fun onInterceptTouchEvent(ev: MotionEvent): Boolean { - updateParentIntercept(ev) - return super.onInterceptTouchEvent(ev) - } - - override fun onTouchEvent(ev: MotionEvent): Boolean { - updateParentIntercept(ev) - return super.onTouchEvent(ev) - } - - private fun updateParentIntercept(ev: MotionEvent) { - when (ev.actionMasked) { - MotionEvent.ACTION_DOWN -> { - downX = ev.x - downY = ev.y - directionDecided = false - parent?.requestDisallowInterceptTouchEvent(false) - } - MotionEvent.ACTION_MOVE -> { - if (!directionDecided) { - val dx = abs(ev.x - downX) - val dy = abs(ev.y - downY) - if (dx > touchSlop || dy > touchSlop) { - directionDecided = true - parent?.requestDisallowInterceptTouchEvent(dx > dy) - } - } - } - MotionEvent.ACTION_UP, - MotionEvent.ACTION_CANCEL -> { - directionDecided = false - parent?.requestDisallowInterceptTouchEvent(false) - } - } - } -} - /** * A [ScrollView] that never scrolls itself to keep a focused descendant on screen. * @@ -110,11 +64,37 @@ private class DirectionalHorizontalScrollView(context: Context) : HorizontalScro * The second one is the damaging one: the automatic `doScrollY` lands in the * middle of the user's drag, and the scroll deltas it emits feed straight back * into the chrome's show/hide threshold, so the bar re-hides itself as it is - * appearing. Both paths route through [computeScrollDeltaToGetChildRectOnScreen], - * so neutralising it disables both while leaving the text selectable. + * appearing. + * + * Both paths route through [computeScrollDeltaToGetChildRectOnScreen], but so + * does `requestChildRectangleOnScreen` — which is what scrolls when a selection + * handle is dragged past the edge of the viewport, and is worth keeping. So the + * suppression is scoped to the two callers that misbehave rather than applied to + * the method outright. */ private class FocusStableScrollView(context: Context) : ScrollView(context) { - override fun computeScrollDeltaToGetChildRectOnScreen(rect: Rect?): Int = 0 + private var suppressFocusScroll = false + + override fun onSizeChanged(w: Int, h: Int, oldw: Int, oldh: Int) { + suppressFocusScroll = true + try { + super.onSizeChanged(w, h, oldw, oldh) + } finally { + suppressFocusScroll = false + } + } + + override fun requestChildFocus(child: android.view.View?, focused: android.view.View?) { + suppressFocusScroll = true + try { + super.requestChildFocus(child, focused) + } finally { + suppressFocusScroll = false + } + } + + override fun computeScrollDeltaToGetChildRectOnScreen(rect: Rect?): Int = + if (suppressFocusScroll) 0 else super.computeScrollDeltaToGetChildRectOnScreen(rect) } private data class ContentKey( @@ -473,7 +453,7 @@ fun RenderedTextView( selectionHighlightColor, density ) val contentView = if (tableView != null) { - DirectionalHorizontalScrollView(container.context).apply { + HorizontalScrollView(container.context).apply { isHorizontalScrollBarEnabled = true addView( tableView, @@ -515,7 +495,7 @@ fun RenderedTextView( ) tv.text = segmentContent val contentView = if (needsCodeHScroll) { - DirectionalHorizontalScrollView(container.context).apply { + HorizontalScrollView(container.context).apply { isHorizontalScrollBarEnabled = true addView( tv, @@ -704,7 +684,8 @@ fun RenderedTextView( val y = resolveScrollY( scrollView, activeSplitBoundaries, targetOffset ) - scrollView.smoothScrollTo(0, y) + // Unresolvable anchor — hold position rather than jumping. + if (y != null) scrollView.smoothScrollTo(0, y) } } else if (scrollToOffset == null && lastRestoredKey != restoreKey) { // Restore-only read: observing it here would resubscribe this @@ -722,9 +703,13 @@ fun RenderedTextView( val y = resolveScrollY( scrollView, activeSplitBoundaries, scrollToOffset ) - val centeredY = (y - scrollView.height / 3).coerceAtLeast(0) - scrollView.smoothScrollTo(0, centeredY) - if (zoomLayout.currentScale > 1f) zoomLayout.resetPan() + if (y != null) { + val centeredY = (y - scrollView.height / 3).coerceAtLeast(0) + scrollView.smoothScrollTo(0, centeredY) + if (zoomLayout.currentScale > 1f) zoomLayout.resetPan() + } + // Consumed either way, so an unresolvable target does not + // leave the request pending forever. onScrollConsumed() } } @@ -841,12 +826,14 @@ private fun getAnchorFromView( is HorizontalScrollView -> { val tv = child.getChildAt(0) as? TextView val layout = tv?.layout ?: return null - val line = layout.getLineForVertical(sy) + // sy is in the ScrollView's space; the layout's line 0 starts below + // the TextView's top padding. + val line = layout.getLineForVertical((sy - tv.paddingTop).coerceAtLeast(0)) return layout.getLineStart(line) } is TextView -> { val layout = child.layout ?: return null - val line = layout.getLineForVertical(sy) + val line = layout.getLineForVertical((sy - child.paddingTop).coerceAtLeast(0)) return layout.getLineStart(line) } else -> return null @@ -859,12 +846,18 @@ private fun computeMaxScrollY(scrollView: ScrollView): Int { .coerceAtLeast(0) } +/** + * Scroll position that puts [offset] at the top of the viewport, or null if it + * cannot be resolved — the views may not be laid out yet. Null means "leave the + * scroll position alone"; returning 0 here would silently jump to the top of the + * document. + */ private fun resolveScrollY( scrollView: ScrollView, boundaries: List, offset: Int -): Int { - val child = scrollView.getChildAt(0) ?: return 0 +): Int? { + val child = scrollView.getChildAt(0) ?: return null if (child is LinearLayout && boundaries.isNotEmpty()) { return scrollToOffsetInSplit(child, boundaries, offset) } @@ -872,32 +865,44 @@ private fun resolveScrollY( is HorizontalScrollView -> child.getChildAt(0) as? TextView is TextView -> child else -> null - } ?: return 0 - val layout = tv.layout ?: return 0 + } ?: return null + val layout = tv.layout ?: return null val line = layout.getLineForOffset(offset) - return layout.getLineTop(line) + return layout.getLineTop(line) + tv.paddingTop } private fun scrollToOffsetInSplit( container: LinearLayout, boundaries: List, offset: Int -): Int { +): Int? { + // Child positions are meaningless until the container has been laid out. + if (!container.isLaidOut) return null for (i in boundaries.indices) { val (segStart, segEnd, _) = boundaries[i] if (offset < segStart || offset >= segEnd) continue if (i >= container.childCount) break val child = container.getChildAt(i) - val tv = extractTextView(child) ?: continue - val layout = tv.layout ?: continue + val tv = extractTextView(child) + // child.top is already measured from the container's padded origin, so the + // container's own padding must not be added again. + val layout = if (tv == null) { + // No TextView at all — a table. Still anchorable, just at segment + // granularity rather than line granularity. + return child.top + } else { + // A TextView whose layout has not been built yet is not the same thing: + // guessing here is what used to send the viewer to the top. + tv.layout ?: return null + } val localOffset = (offset - segStart).coerceIn( 0, layout.text.length.coerceAtLeast(1) - 1 ) val line = layout.getLineForOffset(localOffset) - return child.top + layout.getLineTop(line) + container.paddingTop + return child.top + layout.getLineTop(line) } - return 0 + return null } private fun extractTextView(view: android.view.View): TextView? {