Merge fix/image-rendering: draw a chapter's image once, at the right size

Author: rephlex00

CHANGELOG.md

@@ -4,6 +4,27 @@ Notable changes, newest first. Versions are `YYYY.MM.build` — the year and mon
 plus a counter within that month. Each release's APK is on the
 [Releases page](https://github.com/rephlex00/sn-reader/releases).
 
+## 2026.07.6
+
+Pictures. A book that drew a small illustration at the head of each chapter was drawing it
+twice, and both copies too small.
+
+### Fixed
+
+* **Chapter illustrations appeared twice, at two different sizes.** Books converted for the
+  Kindle routinely carry two copies of the same picture — one for each of Amazon's two formats
+  — and hide the spare one. Reader was not reading the instruction to hide it, so it drew both.
+  This also stops Reader reading the things those books hide on purpose, like print-edition page
+  numbers and a second, redundant table of contents, as though they were part of the story.
+* **Every picture in a book was drawn at roughly half the size it was meant to be.** Book
+  illustrations are sized for an ordinary screen, and Reader was taking those sizes literally on
+  a much finer one — so a chapter ornament covered a fifth of the page instead of a third.
+  Pictures are now drawn at their intended size, still centered, and still shrunk to fit when
+  they are larger than the page. A full-page illustration looks exactly as it did; it is the
+  small ones that were wrong.
+
+Page previews are rebuilt once per book after this update, since where the pages fall has moved.
+
 ## 2026.07.5
 
 Mostly a repair release, from an audit of the whole app: a library that could forget your books, a

README.md

@@ -48,7 +48,7 @@ archive does not open yet. No PDF.
 **2. Install it over USB.**
 
 ```
-adb install -r sn-reader-2026.07.5.apk
+adb install -r sn-reader-2026.07.6.apk
 ```
 
 Debug mode needs to be on first. It lives in the Supernote's own Settings under security and

app/src/main/kotlin/dev/reader/ui/PreviewStrip.kt

@@ -98,8 +98,14 @@ private fun sha256Hex(text: String): String {
  * "strip-v3": expandSelfClosingTags stopped page anchors from eating the following footnote
  * marker's sup span, so those markers now measure 0.75x — line breaks and pagination shift in
  * every endnote-heavy book with print-edition page anchors.
+ *
+ * "strip-v4": the parser started honoring `display: none` (content the publisher hid no longer
+ * paginates at all) and images are now sized in CSS pixels rather than device pixels (so every
+ * image below the content box grew by the panel density). Both change how many lines a chapter
+ * occupies. The density half IS visible to [configHash] — it is a new [RenderConfig] field — but
+ * the display:none half is a pure renderer change, so the version bump is what covers it.
  */
-private const val INDEX_VERSION = "strip-v3"
+private const val INDEX_VERSION = "strip-v4"
 
 /** Flat text, no JSON dependency: a version line, a header line, then one line per entry. */
 fun StripIndex.serialize(): String = buildString {

app/src/main/kotlin/dev/reader/ui/ReaderPrefs.kt

@@ -28,6 +28,15 @@ class ReaderPrefs(context: Context) {
 
     private val prefs = context.getSharedPreferences("reader_prefs", Context.MODE_PRIVATE)
 
+    /**
+     * The panel's device-pixels-per-CSS-pixel, read once here rather than at each render:
+     * it cannot change without the process being recreated. Not a stored preference — like
+     * the viewport it is a property of the screen, not a choice — but unlike the viewport it
+     * needs no argument, since it does not change with rotation. Feeds [RenderConfig.density],
+     * which sizes a book's images; see its KDoc.
+     */
+    private val density: Float = context.resources.displayMetrics.density
+
     /** A bundled reader face — one of [BundledTypefaceProvider.FAMILIES] ("literata"/"bitter"/
      * "atkinson"). A legacy or unknown value resolves to the default face at render time (see
      * [BundledTypefaceProvider.fontResFor]); the store itself is a dumb string. */
@@ -160,6 +169,7 @@ class ReaderPrefs(context: Context) {
             // with the panel. See columnCountFor.
             columnCount = columns,
             columnGapPx = gap,
+            density = density,
         )
     }
 

engine/src/main/kotlin/dev/reader/engine/RenderConfig.kt

@@ -79,6 +79,19 @@ data class RenderConfig(
     val columnCount: Int = 1,
     /** Whitespace between columns; ignored when [columnCount] is 1. See [COLUMN_GAP_PX]. */
     val columnGapPx: Int = COLUMN_GAP_PX,
+    /**
+     * The panel's display density — device pixels per CSS pixel. Everything else in this
+     * class is already in raw device pixels, so this exists for the one input that is NOT:
+     * an EPUB's image dimensions, which (like all CSS lengths) are authored in CSS pixels
+     * on the assumption of a ~1x screen. Drawing such an image at its raw pixel count on a
+     * 1.875x panel renders it at barely half the size the publisher drew it — the same
+     * class of mistake the running foot made before 2026.07.2. See the image branch of
+     * `SpannedChapterBuilder`.
+     *
+     * Defaults to 1.0 (raw pixels are CSS pixels), which is both the correct value for a
+     * 1x display and the behavior every caller had before this field existed.
+     */
+    val density: Float = 1f,
 ) {
     /**
      * The width of one column of text — NOT the full width between the margins. With
@@ -97,6 +110,7 @@ data class RenderConfig(
         require(marginPx >= 0) { "marginPx must be non-negative, was $marginPx" }
         require(columnCount >= 1) { "columnCount must be at least 1, was $columnCount" }
         require(columnGapPx >= 0) { "columnGapPx must be non-negative, was $columnGapPx" }
+        require(density > 0f) { "density must be positive, was $density" }
         require(contentWidthPx > 0) { "margins leave no content width: $this" }
         require(contentHeightPx > 0) { "margins leave no content height: $this" }
     }

formats/src/main/kotlin/dev/reader/formats/epub/XhtmlBlockParser.kt

@@ -6,6 +6,7 @@ import dev.reader.engine.InlineStyle
 import dev.reader.engine.StyleSpan
 import dev.reader.engine.StyledText
 import dev.reader.engine.TextAlign
+import dev.reader.engine.isSeparatorLine
 import org.jsoup.Jsoup
 import org.jsoup.nodes.Document
 import org.jsoup.nodes.Element
@@ -22,7 +23,9 @@ import kotlin.math.abs
  * color → grayLevel, text-decoration → underline/strikethrough, letter-spacing, and the
  * block-level text-align/margins/text-indent/line-height. Box-layout properties (float,
  * position, border, background, width/height, padding, `display` beyond the block/inline
- * split already inferred) are ignored, never half-honored.
+ * split already inferred) are ignored, never half-honored — with one exception:
+ * `display: none` removes an element and its subtree, because that is a statement about
+ * what the book *contains*, not about how it looks. See [isHidden].
  *
  * Font-size is resolved to a **ratio** against the document's own baseline, never an
  * absolute size: `em`/`rem`/`%`/keywords compose into [ComputedStyle.fontSizeRatio], and a
@@ -255,6 +258,7 @@ class XhtmlBlockParser {
         inferHeadings: Boolean,
     ) {
         el.childNodes().forEach { node ->
+            if (node is Element && isHidden(node, chain + ctxOf(node), css)) return@forEach
             when (node) {
                 is TextNode -> builder.appendText(node.wholeText)
                 is Element -> when (val tag = node.tagName().lowercase()) {
@@ -349,6 +353,7 @@ class XhtmlBlockParser {
         var ordinal = 0
         fun walk(container: Element, containerChain: List<ElementCtx>) {
             container.childNodes().forEach { node ->
+                if (node is Element && isHidden(node, containerChain + ctxOf(node), css)) return@forEach
                 when (node) {
                     is TextNode -> {
                         val builder = InlineBuilder()
@@ -381,6 +386,43 @@ class XhtmlBlockParser {
         return declarations["page-break-before"] == "always" || declarations["break-before"] == "page"
     }
 
+    /**
+     * Whether [el] (the element ending [chain]) is removed from the rendered document by
+     * `display: none`, resolved through the full cascade (so a descendant selector and an
+     * inline `style` attribute both count). Its whole subtree goes with it: `display: none`
+     * generates no box for the element *or* its descendants, so the walk simply never
+     * enters it — which is also why this needs no separate inherit-down flag.
+     *
+     * This is the one box-layout property the parser honors, and it earns the exception by
+     * being about *existence* rather than appearance: honoring `width`/`float`/`border`
+     * would change how content looks, but ignoring `display: none` renders content the
+     * publisher said is not part of this book at all. Dual-target exports make that routine
+     * — an Amazon-converted book ships both the KF8 and the mobi7 rendering of a chapter
+     * ornament in the markup and hides one in CSS, so ignoring the rule drew every chapter
+     * ornament twice, at two different sizes (device-observed in Dungeon Crawler Carl).
+     * The same books hide print-only page numbers, non-printing chapter titles and a
+     * duplicate text table of contents, all of which were being read as body prose.
+     *
+     * **A hidden scene-break line is kept.** Those same exports write an ornamental break as
+     * an image with a hidden `* * *` beside it as the text fallback, and their image is an
+     * SVG — which [android.graphics.BitmapFactory] cannot decode, so it renders as nothing.
+     * Honoring the rule literally therefore deleted the scene break outright: 313 of them
+     * across four Dungeon Crawler Carl books, reopening exactly the bug 2026.07.2 closed
+     * (blank space at a page boundary is indistinguishable from the page simply ending).
+     * Keeping the fallback is the conservative direction — the reader renders its own mark
+     * for it either way, so a book whose ornament image DOES decode shows the mark under the
+     * ornament rather than losing the break. [isSeparatorLine] is a tight predicate
+     * (punctuation only, no letters or digits), and an `<img>` has no text at all, so the
+     * exception cannot rescue the duplicate images this rule exists to remove.
+     *
+     * Any other `display` value is ignored, exactly as before: the block/inline split is
+     * already inferred from the tag, and half-honoring `display` would be worse than not.
+     */
+    private fun isHidden(el: Element, chain: List<ElementCtx>, css: CssRules): Boolean {
+        if (css.resolve(chain, baselinePx)["display"] != "none") return false
+        return !isSeparatorLine(el.text())
+    }
+
     private fun emitImage(el: Element, chapterPath: String, out: MutableList<Block>) {
         val src = el.attr("src")
         if (src.isBlank()) return
@@ -454,6 +496,7 @@ class XhtmlBlockParser {
         chain: List<ElementCtx>,
         css: CssRules,
     ) {
+        if (node is Element && isHidden(node, chain + ctxOf(node), css)) return
         when (node) {
             is TextNode -> builder.appendText(node.wholeText)
 

formats/src/main/kotlin/dev/reader/formats/render/SpannedChapterBuilder.kt

@@ -318,14 +318,15 @@ class SpannedChapterBuilder {
      * Never decodes a full-resolution image into memory: [BitmapFactory.Options.inJustDecodeBounds]
      * reads the header first, [sampleSizeFor] picks a power-of-two downsample factor, and only
      * then is the image decoded for real — the same protection the cover extractor uses (a 16 MB
-     * JPEG decodes to hundreds of MB otherwise). The decoded bitmap is bounded to the content box
-     * ([RenderConfig.contentWidthPx] x [contentHeightPx], aspect preserved, only ever shrunk) and
-     * rendered gray via a saturation-0 colour filter on the drawable — the e-ink panel has no
-     * colour, and a filter avoids allocating a second bitmap just to drop the colour.
+     * JPEG decodes to hundreds of MB otherwise). The image is rendered gray via a saturation-0
+     * colour filter on the drawable — the e-ink panel has no colour, and a filter avoids
+     * allocating a second bitmap just to drop the colour.
      */
     private fun appendImage(sb: SpannableStringBuilder, block: Block.Image, config: RenderConfig) {
         val bytes = block.bytes ?: return
-        val drawable = decodeGrayscaleDrawable(bytes, config.contentWidthPx, config.contentHeightPx) ?: return
+        val drawable = decodeGrayscaleDrawable(
+            bytes, config.contentWidthPx, config.contentHeightPx, config.density,
+        ) ?: return
         val start = sb.length
         sb.append(IMAGE_PLACEHOLDER)
         sb.setSpan(ImageSpan(drawable, ImageSpan.ALIGN_BASELINE), start, sb.length, Spanned.SPAN_EXCLUSIVE_EXCLUSIVE)
@@ -337,15 +338,46 @@ class SpannedChapterBuilder {
         )
     }
 
-    private fun decodeGrayscaleDrawable(bytes: ByteArray, maxWidthPx: Int, maxHeightPx: Int): BitmapDrawable? {
+    /**
+     * The drawable for [bytes], sized for display and no larger in memory than that.
+     *
+     * **Display size** is the source's dimensions read as CSS pixels — what they are, an EPUB
+     * being CSS all the way down — so they scale by [density] before being fitted within the
+     * content box ([maxWidthPx] x [maxHeightPx], aspect preserved). Reading them as device
+     * pixels instead, as this did before, drew every image at 1/density of its intended size:
+     * a 264px chapter ornament covered 19% of a 1404px Nomad panel rather than the ~35% the
+     * publisher drew. The content box is still the hard ceiling, so a full-page illustration
+     * is unaffected — density only ever changes images that had room to spare.
+     *
+     * **Memory** is bounded independently, and deliberately does not follow the display size
+     * upward: [sampleSizeFor] downsamples the decode toward the display size, the result is
+     * trimmed to it if the power-of-two floor overshot (the same two-step the cover extractor
+     * uses), and a source *smaller* than the display size is kept at its own resolution and
+     * stretched by the drawable's bounds at draw time. Allocating an upscaled bitmap would
+     * cost several times the memory per cached chapter to invent detail the source never had;
+     * [BitmapDrawable] filters as it draws, which is the same picture for free.
+     */
+    private fun decodeGrayscaleDrawable(
+        bytes: ByteArray,
+        maxWidthPx: Int,
+        maxHeightPx: Int,
+        density: Float,
+    ): BitmapDrawable? {
         val bounds = BitmapFactory.Options().apply { inJustDecodeBounds = true }
         BitmapFactory.decodeByteArray(bytes, 0, bytes.size, bounds)
         if (bounds.outWidth <= 0 || bounds.outHeight <= 0) {
             return null // not decodable as an image — BitmapFactory reports this via -1, not a throw
         }
 
+        val (displayWidth, displayHeight) = fitWithin(
+            (bounds.outWidth * density).roundToInt().coerceAtLeast(1),
+            (bounds.outHeight * density).roundToInt().coerceAtLeast(1),
+            maxWidthPx,
+            maxHeightPx,
+        )
+
         val options = BitmapFactory.Options().apply {
-            inSampleSize = sampleSizeFor(bounds.outWidth, bounds.outHeight, maxWidthPx, maxHeightPx)
+            inSampleSize = sampleSizeFor(bounds.outWidth, bounds.outHeight, displayWidth, displayHeight)
         }
         val sampled = try {
             BitmapFactory.decodeByteArray(bytes, 0, bytes.size, options)
@@ -357,19 +389,23 @@ class SpannedChapterBuilder {
         } ?: return null
 
         // inSampleSize floors to a power of two >= the request, so the sampled bitmap can still
-        // overshoot the content box by nearly 2x per axis. Trim it to the fit dimensions and drop
-        // the oversized sampled bitmap — the same two-step the cover extractor uses — so what stays
-        // resident in the chapter cache is the displayed size, not up to ~4x its pixel area. On a
-        // memory-constrained e-ink device that difference matters across cached chapters.
-        val (targetWidth, targetHeight) = fitWithin(sampled.width, sampled.height, maxWidthPx, maxHeightPx)
-        val display = if (sampled.width == targetWidth && sampled.height == targetHeight) {
+        // overshoot the display size by nearly 2x per axis. Trim it down — the same two-step the
+        // cover extractor uses — so what stays resident in the chapter cache is the displayed
+        // size, not up to ~4x its pixel area. On a memory-constrained e-ink device that
+        // difference matters across cached chapters. fitWithin only ever shrinks, so a source
+        // already smaller than the display size passes through untouched and is upscaled by the
+        // bounds below instead of by an allocation.
+        val (keepWidth, keepHeight) = fitWithin(sampled.width, sampled.height, displayWidth, displayHeight)
+        val bitmap = if (sampled.width == keepWidth && sampled.height == keepHeight) {
             sampled
         } else {
-            Bitmap.createScaledBitmap(sampled, targetWidth, targetHeight, true)
+            Bitmap.createScaledBitmap(sampled, keepWidth, keepHeight, true)
                 .also { if (it !== sampled) sampled.recycle() }
         }
-        return BitmapDrawable(Resources.getSystem(), display).apply {
-            setBounds(0, 0, display.width, display.height)
+        return BitmapDrawable(Resources.getSystem(), bitmap).apply {
+            // The bounds, NOT the bitmap's own size, are what the ImageSpan lays out and draws
+            // to — so this is where an image smaller than its display size gets its upscale.
+            setBounds(0, 0, displayWidth, displayHeight)
             // Saturation 0 renders the image gray; the panel is grayscale.
             colorFilter = ColorMatrixColorFilter(ColorMatrix().apply { setSaturation(0f) })
         }