Fix image display inside epubs

lib/Epub/Epub/Page.cpp

@@ -30,8 +30,9 @@ void PageImage::render(GfxRenderer& renderer, const int fontId, const int xOffse
   imageBlock->render(renderer, xPos + xOffset, yPos + yOffset);
 }
 
-void PageImage::renderWithForceLoad(GfxRenderer& renderer, const int xOffset, const int yOffset, const bool forceLoad) {
-  imageBlock->render(renderer, xPos + xOffset, yPos + yOffset, forceLoad);
+void PageImage::renderWithForceLoad(GfxRenderer& renderer, const int xOffset, const int yOffset, const bool forceLoad,
+                                    const bool monochromeOutput) {
+  imageBlock->render(renderer, xPos + xOffset, yPos + yOffset, forceLoad, monochromeOutput);
 }
 
 bool PageImage::serialize(FsFile& file) {
@@ -205,10 +206,12 @@ std::unique_ptr<PageTableFragment> PageTableFragment::deserialize(FsFile& file)
 }
 
 void Page::render(GfxRenderer& renderer, const int fontId, const int xOffset, const int yOffset,
-                  const bool forceLoadLargeImages) const {
+                  const bool forceLoadLargeImages, const bool skipDecodedImages, const bool monochromeImages) const {
   for (auto& element : elements) {
     if (element->getTag() == TAG_PageImage) {
-      static_cast<PageImage&>(*element).renderWithForceLoad(renderer, xOffset, yOffset, forceLoadLargeImages);
+      auto& pi = static_cast<PageImage&>(*element);
+      if (skipDecodedImages && !pi.getImageBlock().wouldShowPlaceholder(forceLoadLargeImages)) continue;
+      pi.renderWithForceLoad(renderer, xOffset, yOffset, forceLoadLargeImages, monochromeImages);
     } else {
       element->render(renderer, fontId, xOffset, yOffset);
     }
@@ -262,6 +265,19 @@ void Page::renderTextOnly(GfxRenderer& renderer, const int fontId, const int xOf
   }
 }
 
+void Page::renderImagesOnly(GfxRenderer& renderer, const int xOffset, const int yOffset,
+                            const bool forceLoadLargeImages) const {
+  for (auto& element : elements) {
+    if (element->getTag() == TAG_PageImage) {
+      auto& pi = static_cast<PageImage&>(*element);
+      // Placeholders already drew in the BW pass; the grayscale path is only
+      // for images with actual decoded pixel data.
+      if (pi.getImageBlock().wouldShowPlaceholder(forceLoadLargeImages)) continue;
+      pi.renderWithForceLoad(renderer, xOffset, yOffset, forceLoadLargeImages, false);
+    }
+  }
+}
+
 bool Page::serialize(FsFile& file) const {
   const uint16_t count = elements.size();
   serialization::writePod(file, count);

lib/Epub/Epub/Page.h

@@ -58,7 +58,8 @@ class PageImage final : public PageElement {
   PageImage(std::shared_ptr<ImageBlock> block, const int16_t xPos, const int16_t yPos)
       : PageElement(xPos, yPos), imageBlock(std::move(block)) {}
   void render(GfxRenderer& renderer, int fontId, int xOffset, int yOffset) override;
-  void renderWithForceLoad(GfxRenderer& renderer, int xOffset, int yOffset, bool forceLoad);
+  void renderWithForceLoad(GfxRenderer& renderer, int xOffset, int yOffset, bool forceLoad,
+                           bool monochromeOutput = false);
   bool serialize(FsFile& file) override;
   PageElementTag getTag() const override { return TAG_PageImage; }
   static std::unique_ptr<PageImage> deserialize(FsFile& file);
@@ -129,8 +130,19 @@ class Page {
     footnotes.push_back(entry);
   }
 
-  void render(GfxRenderer& renderer, int fontId, int xOffset, int yOffset, bool forceLoadLargeImages = true) const;
+  // skipDecodedImages=true: PageImages that would decode (not show as a placeholder)
+  // are skipped. Used by the BW pass when an AA grayscale-image pass will redraw
+  // them at full 4-level tonality. Placeholders still render in BW (they're just
+  // a box+text, not dithered pixels).
+  // monochromeImages=true: render images via the 1-bit Atkinson path. Used when
+  // no grayscale image pass will follow (AA off or low-mem) so the BW
+  // DirectPixelWriter `<3` rule yields clean black/white instead of muddy dark.
+  void render(GfxRenderer& renderer, int fontId, int xOffset, int yOffset, bool forceLoadLargeImages = true,
+              bool skipDecodedImages = false, bool monochromeImages = false) const;
   void renderTextOnly(GfxRenderer& renderer, int fontId, int xOffset, int yOffset) const;
+  // Renders only PageImages that have actual decoded pixels (skips placeholders,
+  // which already rendered in the BW pass as a box+text).
+  void renderImagesOnly(GfxRenderer& renderer, int xOffset, int yOffset, bool forceLoadLargeImages) const;
   // Decode any missing .pxc pixel caches for images on this page. Called before the
   // BW render so the large (~60 KB contiguous) PNG decoder allocation runs while heap
   // contig is at its peak — before font prewarm and BW backup chunks fragment it.

lib/Epub/Epub/blocks/ImageBlock.cpp

@@ -25,13 +25,16 @@ bool ImageBlock::imageExists() const { return Storage.exists(imagePath.c_str());
 
 namespace {
 
-std::string getCachePath(const std::string& imagePath, ImageDitherMode ditherMode) {
+std::string getCachePath(const std::string& imagePath, ImageDitherMode ditherMode, bool monochromeOutput) {
+  // The monochrome (1-bit Atkinson) cache only stores values 0/3, so it can't be
+  // reused for the 4-level grayscale render path — keep it in a separate file.
+  const char* monoSuffix = monochromeOutput ? "_mono" : "";
   // Replace extension with .pxc (pixel cache)
   size_t dotPos = imagePath.rfind('.');
   if (dotPos != std::string::npos) {
-    return imagePath.substr(0, dotPos) + getImageDitherCacheSuffix(ditherMode) + ".pxc";
+    return imagePath.substr(0, dotPos) + getImageDitherCacheSuffix(ditherMode) + monoSuffix + ".pxc";
   }
-  return imagePath + getImageDitherCacheSuffix(ditherMode) + ".pxc";
+  return imagePath + getImageDitherCacheSuffix(ditherMode) + monoSuffix + ".pxc";
 }
 
 bool renderFromCache(GfxRenderer& renderer, const std::string& cachePath, int x, int y, int expectedWidth,
@@ -118,7 +121,7 @@ bool ImageBlock::isLargeImage() const {
 
 bool ImageBlock::hasPixelCache() const {
   const ImageDitherMode ditherMode = imageDitherModeFromSetting(SETTINGS.imageDithering);
-  return Storage.exists(getCachePath(imagePath, ditherMode).c_str());
+  return Storage.exists(getCachePath(imagePath, ditherMode, false).c_str());
 }
 
 bool ImageBlock::wouldShowPlaceholder(bool forceLoad) const {
@@ -126,12 +129,7 @@ bool ImageBlock::wouldShowPlaceholder(bool forceLoad) const {
   if (!isLargeImage()) return false;
   // If the pixel cache already exists the render is instant — no placeholder needed
   const ImageDitherMode ditherMode = imageDitherModeFromSetting(SETTINGS.imageDithering);
-  const std::string pxcPath = [&] {
-    size_t dot = imagePath.rfind('.');
-    return (dot != std::string::npos ? imagePath.substr(0, dot) : imagePath) + getImageDitherCacheSuffix(ditherMode) +
-           ".pxc";
-  }();
-  return !Storage.exists(pxcPath.c_str());
+  return !Storage.exists(getCachePath(imagePath, ditherMode, false).c_str());
 }
 
 void ImageBlock::renderPlaceholder(GfxRenderer& renderer, const int x, const int y) const {
@@ -156,7 +154,8 @@ void ImageBlock::renderPlaceholder(GfxRenderer& renderer, const int x, const int
   }
 }
 
-void ImageBlock::render(GfxRenderer& renderer, const int x, const int y, const bool forceLoad) {
+void ImageBlock::render(GfxRenderer& renderer, const int x, const int y, const bool forceLoad,
+                        const bool monochromeOutput) {
   LOG_DBG("IMG", "Rendering image at %d,%d: %s (%dx%d)", x, y, imagePath.c_str(), width, height);
 
   const int screenWidth = renderer.getScreenWidth();
@@ -169,9 +168,11 @@ void ImageBlock::render(GfxRenderer& renderer, const int x, const int y, const b
     return;
   }
 
-  // Try to render from pixel cache first (always, regardless of forceLoad)
+  // Try to render from pixel cache first (always, regardless of forceLoad).
+  // Mono and 4-level caches are kept in separate files so each render path gets
+  // the dither output its DirectPixelWriter branch expects.
   const ImageDitherMode ditherMode = imageDitherModeFromSetting(SETTINGS.imageDithering);
-  std::string cachePath = getCachePath(imagePath, ditherMode);
+  std::string cachePath = getCachePath(imagePath, ditherMode, monochromeOutput);
   if (renderFromCache(renderer, cachePath, x, y, width, height)) {
     return;
   }
@@ -210,6 +211,7 @@ void ImageBlock::render(GfxRenderer& renderer, const int x, const int y, const b
   config.performanceMode = false;
   config.useExactDimensions = true;
   config.cachePath = cachePath;
+  config.monochromeOutput = monochromeOutput;
 
   ImageToFramebufferDecoder* decoder = ImageDecoderFactory::getDecoder(imagePath);
   if (!decoder) {

lib/Epub/Epub/blocks/ImageBlock.h

@@ -38,7 +38,11 @@ class ImageBlock final : public Block {
   BlockType getType() override { return IMAGE_BLOCK; }
   bool isEmpty() override { return false; }
 
-  void render(GfxRenderer& renderer, int x, int y, bool forceLoad = true);
+  // monochromeOutput=true switches the decode pipeline to a 1-bit Atkinson dither
+  // that emits only levels 0 and 3, so the BW DirectPixelWriter `<3` rule yields
+  // clean black/white instead of collapsing mid-greys to black. Used by the BW
+  // pass when the AA grayscale image pass isn't available (AA off or low-mem).
+  void render(GfxRenderer& renderer, int x, int y, bool forceLoad = true, bool monochromeOutput = false);
   bool serialize(FsFile& file);
   static std::unique_ptr<ImageBlock> deserialize(FsFile& file);
 

src/activities/reader/EpubReaderActivity.cpp

@@ -135,10 +135,12 @@ inline void logReaderMemSnapshot(const char*) {}
 //
 // The page is re-rendered ceil(panelHeight/stripRows) times per plane, but
 // renderCharImpl culls out-of-band glyphs before bitmap decode so the cost
-// stays close to one render. Only renderTextOnly() is called here, matching the
-// legacy AA pass — images and HRs do not participate in grayscale.
+// stays close to one render. Text always participates; images participate when
+// `includeImages` is set (caller decides — typically true when the page has any
+// decoded images, false otherwise to skip the per-strip image cost on text-only
+// pages).
 bool runTiledGrayscalePass(GfxRenderer& renderer, const Page& page, int fontId, int marginLeft, int contentTop,
-                           bool fastAA) {
+                           bool fastAA, bool includeImages, bool forceLoadLargeImages) {
   if (!renderer.supportsStripGrayscale()) return false;
 
   // Push the SETTINGS toggle into the SDK before the AA refresh. No-op on X4;
@@ -166,6 +168,11 @@ bool runTiledGrayscalePass(GfxRenderer& renderer, const Page& page, int fontId,
       renderer.beginStripTarget(scratch, y, rows);
       renderer.clearScreen(0x00);
       page.renderTextOnly(renderer, fontId, marginLeft, contentTop);
+      if (includeImages) {
+        // DirectPixelWriter clips against the active strip via originY/clipRows,
+        // so off-band image pixels are dropped automatically.
+        page.renderImagesOnly(renderer, marginLeft, contentTop, forceLoadLargeImages);
+      }
       renderer.endStripTarget();
       renderer.writeGrayscalePlaneStrip(lsbPlane, scratch, y, rows);
     }
@@ -2207,7 +2214,14 @@ void EpubReaderActivity::renderContents(std::unique_ptr<Page> page, const int or
   lastRenderStats.forcedHalfRefresh = forceHalfRefreshThisPage;
 
   logReaderMemSnapshot("before_bw_render");
-  page->render(renderer, getEffectiveReaderFontId(), orientedMarginLeft, contentTop, effectiveForceLoad);
+  // When AA + grayscale image pass will run, skip decoded images in BW so they
+  // don't get squashed by DirectPixelWriter's `<3` rule (3/4 dither levels → black).
+  // When AA isn't available, render images via the 1-bit Atkinson dither so the
+  // BW pass produces clean monochrome instead of muddy dark grays.
+  const bool grayscaleImagePassWillRun = imagePageWithAA;
+  page->render(renderer, getEffectiveReaderFontId(), orientedMarginLeft, contentTop, effectiveForceLoad,
+               /*skipDecodedImages=*/grayscaleImagePassWillRun,
+               /*monochromeImages=*/!grayscaleImagePassWillRun);
   renderStatusBar();
   if (showTruncatedSectionHintThisRender) {
     const int hintX = orientedMarginLeft + 4;
@@ -2231,27 +2245,11 @@ void EpubReaderActivity::renderContents(std::unique_ptr<Page> page, const int or
   logReaderMemSnapshot("after_bw_render");
 
   if (imagePageWithAA) {
-    // Double FAST_REFRESH with selective image blanking (pablohc's technique):
-    // HALF_REFRESH sets particles too firmly for the grayscale LUT to adjust.
-    // Instead, blank only the image area and do two fast refreshes.
-    // Step 1: Display page with image area blanked (text appears, image area white)
-    // Step 2: Re-render with images and display again (images appear clean)
-    int16_t imgX, imgY, imgW, imgH;
-    if (page->getImageBoundingBox(imgX, imgY, imgW, imgH)) {
-      renderer.fillRect(imgX + orientedMarginLeft, imgY + contentTop, imgW, imgH, false);
-      renderer.displayBuffer(HalDisplay::FAST_REFRESH);
-
-      // Re-render page content to restore images into the blanked area
-      // Status bar is not re-rendered here to avoid reading stale dynamic values (e.g. battery %)
-      page->render(renderer, getEffectiveReaderFontId(), orientedMarginLeft, contentTop, effectiveForceLoad);
-      renderer.displayBuffer(HalDisplay::FAST_REFRESH);
-    } else {
-      renderer.displayBuffer(HalDisplay::HALF_REFRESH);
-    }
-    // Double FAST_REFRESH handles ghosting for image pages; don't count toward full refresh cadence
-    if (forceHalfRefreshThisPage) {
-      pagesUntilFullRefresh = SETTINGS.getRefreshFrequency();
-    }
+    // The BW pass skipped decoded images, so the image area is currently blank
+    // in the framebuffer — no need for the legacy double-FAST_REFRESH blanking
+    // dance (pablohc's technique). The image will be painted by the subsequent
+    // grayscale image pass, which produces the cleaner 4-level result anyway.
+    ReaderUtils::displayWithRefreshCycle(renderer, pagesUntilFullRefresh);
   } else if (forceHalfRefreshThisPage) {
     renderer.displayBuffer(HalDisplay::HALF_REFRESH);
     pagesUntilFullRefresh = SETTINGS.getRefreshFrequency();
@@ -2278,7 +2276,8 @@ void EpubReaderActivity::renderContents(std::unique_ptr<Page> page, const int or
     logReaderMemSnapshot("tiled_gray_begin");
     const auto tTiledBegin = millis();
     grayscaleDone = runTiledGrayscalePass(renderer, *page, getEffectiveReaderFontId(), orientedMarginLeft, contentTop,
-                                          SETTINGS.fastAntiAliasing);
+                                          SETTINGS.fastAntiAliasing,
+                                          /*includeImages=*/imagePageWithAA, effectiveForceLoad);
     if (grayscaleDone) {
       tiledGrayMs = millis() - tTiledBegin;
       fcm->logStats("tiled_gray");
@@ -2342,6 +2341,9 @@ void EpubReaderActivity::renderContents(std::unique_ptr<Page> page, const int or
       renderer.clearScreen(0x00);
       renderer.setRenderMode(GfxRenderer::GRAYSCALE_LSB);
       page->renderTextOnly(renderer, getEffectiveReaderFontId(), orientedMarginLeft, contentTop);
+      if (imagePageWithAA) {
+        page->renderImagesOnly(renderer, orientedMarginLeft, contentTop, effectiveForceLoad);
+      }
       renderer.copyGrayscaleLsbBuffers();
       const auto tGrayLsb = millis();
       logReaderMemSnapshot("gray_lsb_end");
@@ -2351,6 +2353,9 @@ void EpubReaderActivity::renderContents(std::unique_ptr<Page> page, const int or
       renderer.clearScreen(0x00);
       renderer.setRenderMode(GfxRenderer::GRAYSCALE_MSB);
       page->renderTextOnly(renderer, getEffectiveReaderFontId(), orientedMarginLeft, contentTop);
+      if (imagePageWithAA) {
+        page->renderImagesOnly(renderer, orientedMarginLeft, contentTop, effectiveForceLoad);
+      }
       renderer.copyGrayscaleMsbBuffers();
       const auto tGrayMsb = millis();
       logReaderMemSnapshot("gray_msb_end");
@@ -2390,6 +2395,16 @@ void EpubReaderActivity::renderContents(std::unique_ptr<Page> page, const int or
         bwRestoreMs = tBwRestore - tBwStore;
       }
 
+      // Recovery: the BW pass deliberately skipped decoded images expecting the
+      // grayscale image pass to paint them, but no grayscale pass ran. Re-render
+      // the images in BW using the 1-bit Atkinson dither (monochromeImages=true)
+      // so the cover at least shows up as clean black/white instead of nothing.
+      if (imagePageWithAA) {
+        page->render(renderer, getEffectiveReaderFontId(), orientedMarginLeft, contentTop, effectiveForceLoad,
+                     /*skipDecodedImages=*/false, /*monochromeImages=*/true);
+        renderer.displayBuffer(HalDisplay::FAST_REFRESH);
+      }
+
       const auto tEnd = millis();
       lastRenderStats.usedGrayscale = false;
       lastRenderStats.phases = {
@@ -2459,8 +2474,9 @@ void EpubReaderActivity::displayPreRenderedPage(const Page& page, const int orie
   // support strip grayscale or when the strip scratch can't be allocated.
   const bool aaConfigured = getEffectiveTextAntiAliasing() && !antiAliasingSuspendedLowMemory;
   if (aaConfigured) {
+    // Pre-rendered pages are text-only — no image pass needed.
     if (runTiledGrayscalePass(renderer, page, getEffectiveReaderFontId(), orientedMarginLeft, contentTop,
-                              SETTINGS.fastAntiAliasing)) {
+                              SETTINGS.fastAntiAliasing, /*includeImages=*/false, /*forceLoadLargeImages=*/false)) {
       return;
     }