From 287457f7dd0219d21dd5543ca285cd52c64ace3d Mon Sep 17 00:00:00 2001 From: Leopoldo Pla Sempere Date: Sun, 12 Jul 2026 03:45:53 +0200 Subject: [PATCH] fix: handle low-bit-depth, upscaled, and SVG EPUB images (#2503) --- .../converters/PngToFramebufferConverter.cpp | 201 +++++++++++------- .../Epub/parsers/ChapterHtmlSlimParser.cpp | 28 +-- 2 files changed, 139 insertions(+), 90 deletions(-) diff --git a/lib/Epub/Epub/converters/PngToFramebufferConverter.cpp b/lib/Epub/Epub/converters/PngToFramebufferConverter.cpp index 24492e59..e6719b0f 100644 --- a/lib/Epub/Epub/converters/PngToFramebufferConverter.cpp +++ b/lib/Epub/Epub/converters/PngToFramebufferConverter.cpp @@ -99,24 +99,58 @@ int bytesPerPixelFromType(int pixelType) { } } -int requiredPngInternalBufferBytes(int srcWidth, int pixelType) { +int packedRowBytes(int srcWidth, int bitsPerSample) { return (srcWidth * bitsPerSample + 7) / 8; } + +int requiredPngInternalBufferBytes(int srcWidth, int pixelType, int bitsPerSample) { // +1 filter byte per scanline, *2 for current+previous lines, +32 for alignment margin. int pitch = srcWidth * bytesPerPixelFromType(pixelType); + if ((pixelType == PNG_PIXEL_GRAYSCALE || pixelType == PNG_PIXEL_INDEXED) && bitsPerSample < 8) { + pitch = packedRowBytes(srcWidth, bitsPerSample); + } return ((pitch + 1) * 2) + 32; } +bool isSupportedBitDepth(int pixelType, int bitsPerSample) { + if (bitsPerSample == 8) return true; + if (bitsPerSample != 1 && bitsPerSample != 2 && bitsPerSample != 4) return false; + return pixelType == PNG_PIXEL_GRAYSCALE || pixelType == PNG_PIXEL_INDEXED; +} + +uint8_t readPackedSample(const uint8_t* pixels, int x, int bitsPerSample) { + if (bitsPerSample == 8) return pixels[x]; + + const int bitOffset = x * bitsPerSample; + const int shift = 8 - bitsPerSample - (bitOffset & 7); + const uint8_t mask = (1U << bitsPerSample) - 1; + return (pixels[bitOffset >> 3] >> shift) & mask; +} + +uint8_t expandSampleToByte(uint8_t sample, int bitsPerSample) { + if (bitsPerSample == 8) return sample; + const uint8_t maxSample = (1U << bitsPerSample) - 1; + return static_cast((sample * 255U) / maxSample); +} + // Convert entire source line to grayscale with alpha blending to white background. +// Low-bit-depth grayscale/indexed scanlines are packed most-significant sample first. // For indexed PNGs with tRNS chunk, alpha values are stored at palette[768] onwards. // Processing the whole line at once improves cache locality and reduces per-pixel overhead. -void convertLineToGray(uint8_t* pPixels, uint8_t* grayLine, int width, int pixelType, uint8_t* palette, int hasAlpha) { +void convertLineToGray(const uint8_t* pPixels, uint8_t* grayLine, int width, int pixelType, int bitsPerSample, + uint8_t* palette, int hasAlpha) { switch (pixelType) { case PNG_PIXEL_GRAYSCALE: - memcpy(grayLine, pPixels, width); + if (bitsPerSample == 8) { + memcpy(grayLine, pPixels, width); + } else { + for (int x = 0; x < width; x++) { + grayLine[x] = expandSampleToByte(readPackedSample(pPixels, x, bitsPerSample), bitsPerSample); + } + } break; case PNG_PIXEL_TRUECOLOR: for (int x = 0; x < width; x++) { - uint8_t* p = &pPixels[x * 3]; + const uint8_t* p = &pPixels[x * 3]; grayLine[x] = (uint8_t)((p[0] * 77 + p[1] * 150 + p[2] * 29) >> 8); } break; @@ -125,7 +159,7 @@ void convertLineToGray(uint8_t* pPixels, uint8_t* grayLine, int width, int pixel if (palette) { if (hasAlpha) { for (int x = 0; x < width; x++) { - uint8_t idx = pPixels[x]; + uint8_t idx = readPackedSample(pPixels, x, bitsPerSample); uint8_t* p = &palette[idx * 3]; uint8_t gray = (uint8_t)((p[0] * 77 + p[1] * 150 + p[2] * 29) >> 8); uint8_t alpha = palette[768 + idx]; @@ -133,12 +167,15 @@ void convertLineToGray(uint8_t* pPixels, uint8_t* grayLine, int width, int pixel } } else { for (int x = 0; x < width; x++) { - uint8_t* p = &palette[pPixels[x] * 3]; + uint8_t idx = readPackedSample(pPixels, x, bitsPerSample); + uint8_t* p = &palette[idx * 3]; grayLine[x] = (uint8_t)((p[0] * 77 + p[1] * 150 + p[2] * 29) >> 8); } } } else { - memcpy(grayLine, pPixels, width); + for (int x = 0; x < width; x++) { + grayLine[x] = expandSampleToByte(readPackedSample(pPixels, x, bitsPerSample), bitsPerSample); + } } break; @@ -152,7 +189,7 @@ void convertLineToGray(uint8_t* pPixels, uint8_t* grayLine, int width, int pixel case PNG_PIXEL_TRUECOLOR_ALPHA: for (int x = 0; x < width; x++) { - uint8_t* p = &pPixels[x * 4]; + const uint8_t* p = &pPixels[x * 4]; uint8_t gray = (uint8_t)((p[0] * 77 + p[1] * 150 + p[2] * 29) >> 8); uint8_t alpha = p[3]; grayLine[x] = (uint8_t)((gray * alpha + 255 * (255 - alpha)) / 255); @@ -172,74 +209,83 @@ int pngDrawCallback(PNGDRAW* pDraw) { int srcY = pDraw->y; int srcWidth = ctx->srcWidth; - // Calculate destination Y with scaling - int dstY = (int)(srcY * ctx->scale); + // Map source rows with the exact output-height ratio. During downscaling, + // multiple source rows can select the same output row; during upscaling, one + // source row must be repeated across every output row in its range. Emitting + // only the first row of an upscale leaves zero-filled (black) gaps in the + // streamed pixel cache. + int firstDstY = (srcY * ctx->dstHeight) / ctx->srcHeight; + int endDstY = firstDstY + 1; + if (ctx->dstHeight > ctx->srcHeight) { + endDstY = ((srcY + 1) * ctx->dstHeight) / ctx->srcHeight; + } - // Skip if we already rendered this destination row (multiple source rows map to same dest) - if (dstY == ctx->lastDstY) return 1; - ctx->lastDstY = dstY; - - // Check bounds - if (dstY >= ctx->dstHeight) return 1; - - int outY = ctx->config->y + dstY; - if (outY >= ctx->screenHeight) return 1; + if (firstDstY <= ctx->lastDstY) firstDstY = ctx->lastDstY + 1; + if (firstDstY >= endDstY || firstDstY >= ctx->dstHeight) return 1; + if (endDstY > ctx->dstHeight) endDstY = ctx->dstHeight; // Convert entire source line to grayscale (improves cache locality) - convertLineToGray(pDraw->pPixels, ctx->grayLineBuffer, srcWidth, pDraw->iPixelType, pDraw->pPalette, + convertLineToGray(pDraw->pPixels, ctx->grayLineBuffer, srcWidth, pDraw->iPixelType, pDraw->iBpp, pDraw->pPalette, pDraw->iHasAlpha); - // Render scaled row using Bresenham-style integer stepping (no floating-point division) + // Render scaled rows using Bresenham-style integer stepping (no floating-point division) int dstWidth = ctx->dstWidth; int outXBase = ctx->config->x; int screenWidth = ctx->screenWidth; bool useDithering = ctx->config->useDithering; - bool caching = ctx->caching; - // Pre-compute orientation and render-mode state once per row + // Pre-compute orientation and render-mode state once per callback. DirectPixelWriter pw; pw.init(*ctx->renderer); - pw.beginRow(outY); - // The cache streams to disk one row at a time. Flushing rows below this one - // (PNGdec delivers scanlines top to bottom) repositions the single-row band. - // A flush failure stops caching for the rest of the decode so we never write - // past the band buffer; finalize() then drops the partial file. - DirectCacheWriter cw; - if (caching) { - if (!ctx->cache.advanceTo(dstY)) { - caching = false; - ctx->caching = false; - } else { - cw.init(ctx->cache.buffer, ctx->cache.bytesPerRow, ctx->cache.bandRows, ctx->cache.originX); - cw.beginRow(outY, ctx->config->y + ctx->cache.bandStart); - } - } + for (int dstY = firstDstY; dstY < endDstY; dstY++) { + ctx->lastDstY = dstY; + int outY = ctx->config->y + dstY; + if (outY >= ctx->screenHeight) continue; - int srcX = 0; - int error = 0; + pw.beginRow(outY); - for (int dstX = 0; dstX < dstWidth; dstX++) { - int outX = outXBase + dstX; - if (outX < screenWidth) { - uint8_t gray = ctx->grayLineBuffer[srcX]; - - uint8_t ditheredGray; - if (useDithering) { - ditheredGray = applyBayerDither4Level(gray, outX, outY); + // The cache streams to disk one row at a time. Flushing rows below this one + // (PNGdec delivers scanlines top to bottom) repositions the single-row band. + // A flush failure stops caching for the rest of the decode so we never write + // past the band buffer; finalize() then drops the partial file. + bool caching = ctx->caching; + DirectCacheWriter cw; + if (caching) { + if (!ctx->cache.advanceTo(dstY)) { + caching = false; + ctx->caching = false; } else { - ditheredGray = gray / 85; - if (ditheredGray > 3) ditheredGray = 3; + cw.init(ctx->cache.buffer, ctx->cache.bytesPerRow, ctx->cache.bandRows, ctx->cache.originX); + cw.beginRow(outY, ctx->config->y + ctx->cache.bandStart); } - pw.writePixel(outX, ditheredGray); - if (caching) cw.writePixel(outX, ditheredGray); } - // Bresenham-style stepping: advance srcX based on ratio srcWidth/dstWidth - error += srcWidth; - while (error >= dstWidth) { - error -= dstWidth; - srcX++; + int srcX = 0; + int error = 0; + + for (int dstX = 0; dstX < dstWidth; dstX++) { + int outX = outXBase + dstX; + if (outX < screenWidth) { + uint8_t gray = ctx->grayLineBuffer[srcX]; + + uint8_t ditheredGray; + if (useDithering) { + ditheredGray = applyBayerDither4Level(gray, outX, outY); + } else { + ditheredGray = gray / 85; + if (ditheredGray > 3) ditheredGray = 3; + } + pw.writePixel(outX, ditheredGray); + if (caching) cw.writePixel(outX, ditheredGray); + } + + // Bresenham-style stepping: advance srcX based on ratio srcWidth/dstWidth + error += srcWidth; + while (error >= dstWidth) { + error -= dstWidth; + srcX++; + } } } @@ -332,30 +378,44 @@ bool PngToFramebufferConverter::decodeToFramebuffer(const std::string& imagePath } ctx.lastDstY = -1; // Reset row tracking - LOG_DBG("PNG", "PNG %dx%d -> %dx%d (scale %.2f), bpp: %d", ctx.srcWidth, ctx.srcHeight, ctx.dstWidth, ctx.dstHeight, - ctx.scale, png->getBpp()); - const int pixelType = png->getPixelType(); - const int requiredInternal = requiredPngInternalBufferBytes(ctx.srcWidth, pixelType); + const int bitsPerSample = png->getBpp(); + LOG_DBG("PNG", "PNG %dx%d -> %dx%d (scale %.2f), type: %d, bpp: %d", ctx.srcWidth, ctx.srcHeight, ctx.dstWidth, + ctx.dstHeight, ctx.scale, pixelType, bitsPerSample); + + const int requiredInternal = requiredPngInternalBufferBytes(ctx.srcWidth, pixelType, bitsPerSample); if (requiredInternal > PNG_MAX_BUFFERED_PIXELS) { - LOG_ERR("PNG", - "PNG row buffer too small: need %d bytes for width=%d type=%d, configured PNG_MAX_BUFFERED_PIXELS=%d", - requiredInternal, ctx.srcWidth, pixelType, PNG_MAX_BUFFERED_PIXELS); + LOG_ERR( + "PNG", + "PNG row buffer too small: need %d bytes for width=%d type=%d bpp=%d, configured PNG_MAX_BUFFERED_PIXELS=%d", + requiredInternal, ctx.srcWidth, pixelType, bitsPerSample, PNG_MAX_BUFFERED_PIXELS); LOG_ERR("PNG", "Aborting decode to avoid PNGdec internal buffer overflow"); return false; } - if (png->getBpp() != 8) { - warnUnsupportedFeature("bit depth (" + std::to_string(png->getBpp()) + "bpp)", imagePath); + if (!isSupportedBitDepth(pixelType, bitsPerSample)) { + warnUnsupportedFeature( + "bit depth (" + std::to_string(bitsPerSample) + "bpp) for pixel type " + std::to_string(pixelType), imagePath); + return false; } - // Allocate grayscale line buffer on demand (~3.2 KB) - freed after decode - const size_t grayBufSize = PNG_MAX_BUFFERED_PIXELS / 2; - ctx.grayLineBuffer = static_cast(malloc(grayBufSize)); - if (!ctx.grayLineBuffer) { + // The converter expands each source row to 8-bit grayscale before dithering, + // so this scratch buffer is sized by source pixels even when PNGdec reads a + // packed 1/2/4-bit row internally. + constexpr size_t MAX_GRAY_LINE_BUFFER_BYTES = PNG_MAX_BUFFERED_PIXELS / 2; + const size_t grayBufSize = static_cast(ctx.srcWidth); + if (grayBufSize > MAX_GRAY_LINE_BUFFER_BYTES) { + LOG_ERR("PNG", "Expanded gray row too wide: need %u bytes for width=%d, max=%u", static_cast(grayBufSize), + ctx.srcWidth, static_cast(MAX_GRAY_LINE_BUFFER_BYTES)); + return false; + } + + auto grayLineBuffer = makeUniqueNoThrow(grayBufSize); + if (!grayLineBuffer) { LOG_ERR("PNG", "Failed to allocate gray line buffer"); return false; } + ctx.grayLineBuffer = grayLineBuffer.get(); // Stream the pixel cache to disk. PNGdec delivers source scanlines top to // bottom and we emit at most one (downscaled) output row per callback, so the @@ -375,7 +435,6 @@ bool PngToFramebufferConverter::decodeToFramebuffer(const std::string& imagePath rc = png->decode(&ctx, 0); unsigned long decodeTime = millis() - decodeStart; - free(ctx.grayLineBuffer); ctx.grayLineBuffer = nullptr; if (rc != PNG_SUCCESS) { diff --git a/lib/Epub/Epub/parsers/ChapterHtmlSlimParser.cpp b/lib/Epub/Epub/parsers/ChapterHtmlSlimParser.cpp index 7263de4a..865f7d96 100644 --- a/lib/Epub/Epub/parsers/ChapterHtmlSlimParser.cpp +++ b/lib/Epub/Epub/parsers/ChapterHtmlSlimParser.cpp @@ -35,7 +35,7 @@ constexpr const char* BOLD_TAGS[] = {"b", "strong"}; constexpr const char* ITALIC_TAGS[] = {"i", "em"}; constexpr const char* UNDERLINE_TAGS[] = {"u", "ins"}; constexpr const char* LINETHROUGH_TAGS[] = {"del", "s", "strike"}; -constexpr const char* IMAGE_TAGS[] = {"img"}; +constexpr const char* IMAGE_TAGS[] = {"img", "image"}; constexpr const char* SKIP_TAGS[] = {"head"}; bool isWhitespace(const char c) { return c == ' ' || c == '\r' || c == '\n' || c == '\t'; } @@ -495,11 +495,18 @@ void XMLCALL ChapterHtmlSlimParser::startElement(void* userData, const XML_Char* for (int i = 0; atts[i]; i += 2) { if (strcmp(atts[i], "src") == 0) { src = atts[i + 1]; + } else if (src.empty() && (strcmp(atts[i], "href") == 0 || strcmp(atts[i], "xlink:href") == 0)) { + src = atts[i + 1]; } else if (strcmp(atts[i], "alt") == 0) { alt = atts[i + 1]; } } + const size_t fragmentPos = src.find('#'); + if (fragmentPos != std::string::npos) { + src.resize(fragmentPos); + } + // imageRendering: 0=display, 1=placeholder (alt text only), 2=suppress entirely if (self->imageRendering == 2) { self->skipUntilDepth = self->depth; @@ -507,19 +514,6 @@ void XMLCALL ChapterHtmlSlimParser::startElement(void* userData, const XML_Char* return; } - // Skip image if CSS display:none - if (self->cssParser) { - CssStyle imgDisplayStyle = self->cssParser->resolveStyle("img", classAttr); - if (!styleAttr.empty()) { - imgDisplayStyle.applyOver(CssParser::parseInlineStyle(styleAttr)); - } - if (imgDisplayStyle.hasDisplay() && imgDisplayStyle.display == CssDisplay::None) { - self->skipUntilDepth = self->depth; - self->depth += 1; - return; - } - } - if (!src.empty() && self->imageRendering != 1) { LOG_DBG("EHP", "Found image: src=%s", src.c_str()); @@ -556,11 +550,7 @@ void XMLCALL ChapterHtmlSlimParser::startElement(void* userData, const XML_Char* int displayWidth = 0; int displayHeight = 0; const float emSize = static_cast(self->renderer.getFontAscenderSize(self->fontId)); - CssStyle imgStyle = self->cssParser ? self->cssParser->resolveStyle("img", classAttr) : CssStyle{}; - // Merge inline style (e.g. style="height: 2em") so it overrides stylesheet rules - if (!styleAttr.empty()) { - imgStyle.applyOver(CssParser::parseInlineStyle(styleAttr)); - } + const CssStyle& imgStyle = cssStyle; const bool hasCssHeight = imgStyle.hasImageHeight(); const bool hasCssWidth = imgStyle.hasImageWidth();