From 939014d996a488c4de7470e3c4f02a68849a566f Mon Sep 17 00:00:00 2001 From: WuTofu <5987870+WuTofu@users.noreply.github.com> Date: Wed, 6 May 2026 21:57:37 +0800 Subject: [PATCH] fix: incorrect y-axis scale factor in jpeg nearest-neighbor downscaler (#1807) ## Summary * **What is the goal of this PR?** Fixes a bug in the jpeg nearest-neighbor downscaler where source image rows containing visible content may be cropped. * **What changes are included?** Split the single `fineScaleFP`/`invScaleFP` scale pair in `JpegToFramebufferConverter.cpp` into separate X (`fineScaleFPX`/`invScaleFPX`) and Y (`fineScaleFPY`/`invScaleFPY`) pairs ## Additional Context The one case that I encountered in my epub: img_6_0 The image generated from pxc cache with the latest commit: img_6_0_master pxc With this fix, the bottom outline of each character is still there: img_6_0_fixed pxc --- ### AI Usage While CrossPoint doesn't have restrictions on AI tools in contributing, please be transparent about their usage as it helps set the right context for reviewers. Did you use AI tools to help write this code? _**PARTIALLY**_ --- .../converters/JpegToFramebufferConverter.cpp | 60 ++++++++++++------- 1 file changed, 39 insertions(+), 21 deletions(-) diff --git a/lib/Epub/Epub/converters/JpegToFramebufferConverter.cpp b/lib/Epub/Epub/converters/JpegToFramebufferConverter.cpp index 4cf55ae3..b0863bb5 100644 --- a/lib/Epub/Epub/converters/JpegToFramebufferConverter.cpp +++ b/lib/Epub/Epub/converters/JpegToFramebufferConverter.cpp @@ -32,9 +32,15 @@ struct JpegContext { int dstWidth{0}; int dstHeight{0}; - // Fine scale in 16.16 fixed-point (ESP32-C3 has no FPU) - int32_t fineScaleFP{1 << 16}; // src -> dst mapping - int32_t invScaleFP{1 << 16}; // dst -> src mapping + // Fine scale in 16.16 fixed-point (ESP32-C3 has no FPU). + // X and Y axes use separate scale factors: the aspect ratio of the output (dstWidth/dstHeight) + // may differ from the source (srcWidth/srcHeight) due to integer rounding of displayHeight. + // Using a single (X-based) scale for both axes causes the wrong srcRow to be skipped + // during nearest-neighbor downscaling, potentially losing critical image content. + int32_t fineScaleFPX{1 << 16}; // X: src -> dst column mapping + int32_t invScaleFPX{1 << 16}; // X: dst -> src column mapping + int32_t fineScaleFPY{1 << 16}; // Y: src -> dst row mapping + int32_t invScaleFPY{1 << 16}; // Y: dst -> src row mapping PixelCache cache; bool caching{false}; @@ -125,8 +131,10 @@ int jpegDrawCallback(JPEGDRAW* pDraw) { const bool useDithering = ctx->config->useDithering; const bool caching = ctx->caching; - const int32_t fineScaleFP = ctx->fineScaleFP; - const int32_t invScaleFP = ctx->invScaleFP; + const int32_t fineScaleFPX = ctx->fineScaleFPX; + const int32_t invScaleFPX = ctx->invScaleFPX; + const int32_t fineScaleFPY = ctx->fineScaleFPY; + const int32_t invScaleFPY = ctx->invScaleFPY; GfxRenderer& renderer = *ctx->renderer; const int cfgX = ctx->config->x; const int cfgY = ctx->config->y; @@ -137,10 +145,10 @@ int jpegDrawCallback(JPEGDRAW* pDraw) { const int srcYEnd = blockY + blockH; const int srcXEnd = blockX + validW; - int dstYStart = (int)((int64_t)blockY * fineScaleFP >> FP_SHIFT); - int dstYEnd = (srcYEnd >= ctx->scaledSrcHeight) ? ctx->dstHeight : (int)((int64_t)srcYEnd * fineScaleFP >> FP_SHIFT); - int dstXStart = (int)((int64_t)blockX * fineScaleFP >> FP_SHIFT); - int dstXEnd = (srcXEnd >= ctx->scaledSrcWidth) ? ctx->dstWidth : (int)((int64_t)srcXEnd * fineScaleFP >> FP_SHIFT); + int dstYStart = (int)((int64_t)blockY * fineScaleFPY >> FP_SHIFT); + int dstYEnd = (srcYEnd >= ctx->scaledSrcHeight) ? ctx->dstHeight : (int)((int64_t)srcYEnd * fineScaleFPY >> FP_SHIFT); + int dstXStart = (int)((int64_t)blockX * fineScaleFPX >> FP_SHIFT); + int dstXEnd = (srcXEnd >= ctx->scaledSrcWidth) ? ctx->dstWidth : (int)((int64_t)srcXEnd * fineScaleFPX >> FP_SHIFT); // Pre-clamp destination ranges to screen bounds (eliminates per-pixel screen checks) int clampYMax = ctx->dstHeight; @@ -165,7 +173,7 @@ int jpegDrawCallback(JPEGDRAW* pDraw) { } // === 1:1 fast path: no scaling math === - if (fineScaleFP == FP_ONE) { + if (fineScaleFPX == FP_ONE && fineScaleFPY == FP_ONE) { for (int dstY = dstYStart; dstY < dstYEnd; dstY++) { const int outY = cfgY + dstY; pw.beginRow(outY); @@ -191,11 +199,11 @@ int jpegDrawCallback(JPEGDRAW* pDraw) { // === Bilinear interpolation (upscale: fineScale > 1.0) === // Smooths block boundaries that would otherwise create visible banding // on progressive JPEG DC-only decode (1/8 resolution upscaled to target). - if (fineScaleFP > FP_ONE) { + if (fineScaleFPX > FP_ONE && fineScaleFPY > FP_ONE) { // Pre-compute safe X range where lx0 and lx0+1 are both in [0, validW-1]. // Only the left/right edge pixels (typically 0-2 and 1-8 respectively) need clamping. - int safeXStart = (int)(((int64_t)blockX * fineScaleFP + FP_MASK) >> FP_SHIFT); - int safeXEnd = (int)((int64_t)(blockX + validW - 1) * fineScaleFP >> FP_SHIFT); + int safeXStart = (int)(((int64_t)blockX * fineScaleFPX + FP_MASK) >> FP_SHIFT); + int safeXEnd = (int)((int64_t)(blockX + validW - 1) * fineScaleFPX >> FP_SHIFT); if (safeXStart < dstXStart) safeXStart = dstXStart; if (safeXEnd > dstXEnd) safeXEnd = dstXEnd; if (safeXStart > safeXEnd) safeXEnd = safeXStart; @@ -204,7 +212,7 @@ int jpegDrawCallback(JPEGDRAW* pDraw) { const int outY = cfgY + dstY; pw.beginRow(outY); if (caching) cw.beginRow(outY, ctx->config->y); - const int32_t srcFyFP = dstY * invScaleFP; + const int32_t srcFyFP = dstY * invScaleFPY; const int32_t fy = srcFyFP & FP_MASK; const int32_t fyInv = FP_ONE - fy; int ly0 = (srcFyFP >> FP_SHIFT) - blockY; @@ -219,7 +227,7 @@ int jpegDrawCallback(JPEGDRAW* pDraw) { // Left edge (with X boundary clamping) for (int dstX = dstXStart; dstX < safeXStart; dstX++) { const int outX = cfgX + dstX; - const int32_t srcFxFP = dstX * invScaleFP; + const int32_t srcFxFP = dstX * invScaleFPX; const int32_t fx = srcFxFP & FP_MASK; const int32_t fxInv = FP_ONE - fx; int lx0 = (srcFxFP >> FP_SHIFT) - blockX; @@ -247,7 +255,7 @@ int jpegDrawCallback(JPEGDRAW* pDraw) { // Interior (no X boundary checks — lx0 and lx0+1 guaranteed in bounds) for (int dstX = safeXStart; dstX < safeXEnd; dstX++) { const int outX = cfgX + dstX; - const int32_t srcFxFP = dstX * invScaleFP; + const int32_t srcFxFP = dstX * invScaleFPX; const int32_t fx = srcFxFP & FP_MASK; const int32_t fxInv = FP_ONE - fx; const int lx0 = (srcFxFP >> FP_SHIFT) - blockX; @@ -270,7 +278,7 @@ int jpegDrawCallback(JPEGDRAW* pDraw) { // Right edge (with X boundary clamping) for (int dstX = safeXEnd; dstX < dstXEnd; dstX++) { const int outX = cfgX + dstX; - const int32_t srcFxFP = dstX * invScaleFP; + const int32_t srcFxFP = dstX * invScaleFPX; const int32_t fx = srcFxFP & FP_MASK; const int32_t fxInv = FP_ONE - fx; int lx0 = (srcFxFP >> FP_SHIFT) - blockX; @@ -301,7 +309,7 @@ int jpegDrawCallback(JPEGDRAW* pDraw) { const int outY = cfgY + dstY; pw.beginRow(outY); if (caching) cw.beginRow(outY, ctx->config->y); - const int32_t srcFyFP = dstY * invScaleFP; + const int32_t srcFyFP = dstY * invScaleFPY; int ly = (srcFyFP >> FP_SHIFT) - blockY; if (ly < 0) ly = 0; if (ly >= blockH) ly = blockH - 1; @@ -309,7 +317,7 @@ int jpegDrawCallback(JPEGDRAW* pDraw) { for (int dstX = dstXStart; dstX < dstXEnd; dstX++) { const int outX = cfgX + dstX; - const int32_t srcFxFP = dstX * invScaleFP; + const int32_t srcFxFP = dstX * invScaleFPX; int lx = (srcFxFP >> FP_SHIFT) - blockX; if (lx < 0) lx = 0; if (lx >= validW) lx = validW - 1; @@ -442,12 +450,22 @@ bool JpegToFramebufferConverter::decodeToFramebuffer(const std::string& imagePat jpegScaleDenom = chooseJpegScale(targetScale, jpegScaleOption); } + if (destWidth <= 0 || destHeight <= 0) { + LOG_ERR("JPG", "Degenerate output dimensions %dx%d for %s, skipping render", destWidth, destHeight, + imagePath.c_str()); + jpeg->close(); + delete jpeg; + return false; + } + ctx.scaledSrcWidth = (srcWidth + jpegScaleDenom - 1) / jpegScaleDenom; ctx.scaledSrcHeight = (srcHeight + jpegScaleDenom - 1) / jpegScaleDenom; ctx.dstWidth = destWidth; ctx.dstHeight = destHeight; - ctx.fineScaleFP = (int32_t)((int64_t)destWidth * FP_ONE / ctx.scaledSrcWidth); - ctx.invScaleFP = (int32_t)((int64_t)ctx.scaledSrcWidth * FP_ONE / destWidth); + ctx.fineScaleFPX = (int32_t)((int64_t)destWidth * FP_ONE / ctx.scaledSrcWidth); + ctx.invScaleFPX = (int32_t)((int64_t)ctx.scaledSrcWidth * FP_ONE / destWidth); + ctx.fineScaleFPY = (int32_t)((int64_t)destHeight * FP_ONE / ctx.scaledSrcHeight); + ctx.invScaleFPY = (int32_t)((int64_t)ctx.scaledSrcHeight * FP_ONE / destHeight); LOG_DBG("JPG", "JPEG %dx%d -> %dx%d (scale %.2f, jpegScale 1/%d, fineScale %.2f)%s", srcWidth, srcHeight, destWidth, destHeight, targetScale, jpegScaleDenom, (float)destWidth / ctx.scaledSrcWidth,