From 285717fe927d84cfa7e7df202656cc689b7d30ac Mon Sep 17 00:00:00 2001 From: jpirnay Date: Sun, 24 May 2026 22:04:16 +0200 Subject: [PATCH] Fix image display inside epubs --- lib/Epub/Epub/Page.cpp | 24 +++++-- lib/Epub/Epub/Page.h | 16 ++++- lib/Epub/Epub/blocks/ImageBlock.cpp | 28 ++++---- lib/Epub/Epub/blocks/ImageBlock.h | 6 +- src/activities/reader/EpubReaderActivity.cpp | 70 ++++++++++++-------- 5 files changed, 97 insertions(+), 47 deletions(-) diff --git a/lib/Epub/Epub/Page.cpp b/lib/Epub/Epub/Page.cpp index d72e7566..447f54b9 100644 --- a/lib/Epub/Epub/Page.cpp +++ b/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::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(*element).renderWithForceLoad(renderer, xOffset, yOffset, forceLoadLargeImages); + auto& pi = static_cast(*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(*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); diff --git a/lib/Epub/Epub/Page.h b/lib/Epub/Epub/Page.h index 822adb0a..eed23f84 100644 --- a/lib/Epub/Epub/Page.h +++ b/lib/Epub/Epub/Page.h @@ -58,7 +58,8 @@ class PageImage final : public PageElement { PageImage(std::shared_ptr 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 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. diff --git a/lib/Epub/Epub/blocks/ImageBlock.cpp b/lib/Epub/Epub/blocks/ImageBlock.cpp index cba35dbe..af5dc09a 100644 --- a/lib/Epub/Epub/blocks/ImageBlock.cpp +++ b/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) { diff --git a/lib/Epub/Epub/blocks/ImageBlock.h b/lib/Epub/Epub/blocks/ImageBlock.h index 7cf93213..fd4fb205 100644 --- a/lib/Epub/Epub/blocks/ImageBlock.h +++ b/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 deserialize(FsFile& file); diff --git a/src/activities/reader/EpubReaderActivity.cpp b/src/activities/reader/EpubReaderActivity.cpp index 3060aeb7..eadf0f91 100644 --- a/src/activities/reader/EpubReaderActivity.cpp +++ b/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, 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, 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, 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, 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, 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, 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; }