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 width="480" height="37" alt="img_6_0" src="https://github.com/user-attachments/assets/0de3778c-cf3d-42dc-a1ba-4ba729f6b1c4" /> The image generated from pxc cache with the latest commit: <img width="464" height="36" alt="img_6_0_master pxc" src="https://github.com/user-attachments/assets/92ba06e5-be15-4db6-840e-882bdddb1baf" /> With this fix, the bottom outline of each character is still there: <img width="464" height="36" alt="img_6_0_fixed pxc" src="https://github.com/user-attachments/assets/4246ce55-5a20-480f-862f-7988f804c1fb" /> --- ### 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**_
This commit is contained in:
@@ -32,9 +32,15 @@ struct JpegContext {
|
|||||||
int dstWidth{0};
|
int dstWidth{0};
|
||||||
int dstHeight{0};
|
int dstHeight{0};
|
||||||
|
|
||||||
// Fine scale in 16.16 fixed-point (ESP32-C3 has no FPU)
|
// Fine scale in 16.16 fixed-point (ESP32-C3 has no FPU).
|
||||||
int32_t fineScaleFP{1 << 16}; // src -> dst mapping
|
// X and Y axes use separate scale factors: the aspect ratio of the output (dstWidth/dstHeight)
|
||||||
int32_t invScaleFP{1 << 16}; // dst -> src mapping
|
// 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;
|
PixelCache cache;
|
||||||
bool caching{false};
|
bool caching{false};
|
||||||
@@ -125,8 +131,10 @@ int jpegDrawCallback(JPEGDRAW* pDraw) {
|
|||||||
|
|
||||||
const bool useDithering = ctx->config->useDithering;
|
const bool useDithering = ctx->config->useDithering;
|
||||||
const bool caching = ctx->caching;
|
const bool caching = ctx->caching;
|
||||||
const int32_t fineScaleFP = ctx->fineScaleFP;
|
const int32_t fineScaleFPX = ctx->fineScaleFPX;
|
||||||
const int32_t invScaleFP = ctx->invScaleFP;
|
const int32_t invScaleFPX = ctx->invScaleFPX;
|
||||||
|
const int32_t fineScaleFPY = ctx->fineScaleFPY;
|
||||||
|
const int32_t invScaleFPY = ctx->invScaleFPY;
|
||||||
GfxRenderer& renderer = *ctx->renderer;
|
GfxRenderer& renderer = *ctx->renderer;
|
||||||
const int cfgX = ctx->config->x;
|
const int cfgX = ctx->config->x;
|
||||||
const int cfgY = ctx->config->y;
|
const int cfgY = ctx->config->y;
|
||||||
@@ -137,10 +145,10 @@ int jpegDrawCallback(JPEGDRAW* pDraw) {
|
|||||||
const int srcYEnd = blockY + blockH;
|
const int srcYEnd = blockY + blockH;
|
||||||
const int srcXEnd = blockX + validW;
|
const int srcXEnd = blockX + validW;
|
||||||
|
|
||||||
int dstYStart = (int)((int64_t)blockY * fineScaleFP >> FP_SHIFT);
|
int dstYStart = (int)((int64_t)blockY * fineScaleFPY >> FP_SHIFT);
|
||||||
int dstYEnd = (srcYEnd >= ctx->scaledSrcHeight) ? ctx->dstHeight : (int)((int64_t)srcYEnd * fineScaleFP >> FP_SHIFT);
|
int dstYEnd = (srcYEnd >= ctx->scaledSrcHeight) ? ctx->dstHeight : (int)((int64_t)srcYEnd * fineScaleFPY >> FP_SHIFT);
|
||||||
int dstXStart = (int)((int64_t)blockX * fineScaleFP >> FP_SHIFT);
|
int dstXStart = (int)((int64_t)blockX * fineScaleFPX >> FP_SHIFT);
|
||||||
int dstXEnd = (srcXEnd >= ctx->scaledSrcWidth) ? ctx->dstWidth : (int)((int64_t)srcXEnd * fineScaleFP >> 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)
|
// Pre-clamp destination ranges to screen bounds (eliminates per-pixel screen checks)
|
||||||
int clampYMax = ctx->dstHeight;
|
int clampYMax = ctx->dstHeight;
|
||||||
@@ -165,7 +173,7 @@ int jpegDrawCallback(JPEGDRAW* pDraw) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// === 1:1 fast path: no scaling math ===
|
// === 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++) {
|
for (int dstY = dstYStart; dstY < dstYEnd; dstY++) {
|
||||||
const int outY = cfgY + dstY;
|
const int outY = cfgY + dstY;
|
||||||
pw.beginRow(outY);
|
pw.beginRow(outY);
|
||||||
@@ -191,11 +199,11 @@ int jpegDrawCallback(JPEGDRAW* pDraw) {
|
|||||||
// === Bilinear interpolation (upscale: fineScale > 1.0) ===
|
// === Bilinear interpolation (upscale: fineScale > 1.0) ===
|
||||||
// Smooths block boundaries that would otherwise create visible banding
|
// Smooths block boundaries that would otherwise create visible banding
|
||||||
// on progressive JPEG DC-only decode (1/8 resolution upscaled to target).
|
// 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].
|
// 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.
|
// 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 safeXStart = (int)(((int64_t)blockX * fineScaleFPX + FP_MASK) >> FP_SHIFT);
|
||||||
int safeXEnd = (int)((int64_t)(blockX + validW - 1) * fineScaleFP >> FP_SHIFT);
|
int safeXEnd = (int)((int64_t)(blockX + validW - 1) * fineScaleFPX >> FP_SHIFT);
|
||||||
if (safeXStart < dstXStart) safeXStart = dstXStart;
|
if (safeXStart < dstXStart) safeXStart = dstXStart;
|
||||||
if (safeXEnd > dstXEnd) safeXEnd = dstXEnd;
|
if (safeXEnd > dstXEnd) safeXEnd = dstXEnd;
|
||||||
if (safeXStart > safeXEnd) safeXEnd = safeXStart;
|
if (safeXStart > safeXEnd) safeXEnd = safeXStart;
|
||||||
@@ -204,7 +212,7 @@ int jpegDrawCallback(JPEGDRAW* pDraw) {
|
|||||||
const int outY = cfgY + dstY;
|
const int outY = cfgY + dstY;
|
||||||
pw.beginRow(outY);
|
pw.beginRow(outY);
|
||||||
if (caching) cw.beginRow(outY, ctx->config->y);
|
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 fy = srcFyFP & FP_MASK;
|
||||||
const int32_t fyInv = FP_ONE - fy;
|
const int32_t fyInv = FP_ONE - fy;
|
||||||
int ly0 = (srcFyFP >> FP_SHIFT) - blockY;
|
int ly0 = (srcFyFP >> FP_SHIFT) - blockY;
|
||||||
@@ -219,7 +227,7 @@ int jpegDrawCallback(JPEGDRAW* pDraw) {
|
|||||||
// Left edge (with X boundary clamping)
|
// Left edge (with X boundary clamping)
|
||||||
for (int dstX = dstXStart; dstX < safeXStart; dstX++) {
|
for (int dstX = dstXStart; dstX < safeXStart; dstX++) {
|
||||||
const int outX = cfgX + 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 fx = srcFxFP & FP_MASK;
|
||||||
const int32_t fxInv = FP_ONE - fx;
|
const int32_t fxInv = FP_ONE - fx;
|
||||||
int lx0 = (srcFxFP >> FP_SHIFT) - blockX;
|
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)
|
// Interior (no X boundary checks — lx0 and lx0+1 guaranteed in bounds)
|
||||||
for (int dstX = safeXStart; dstX < safeXEnd; dstX++) {
|
for (int dstX = safeXStart; dstX < safeXEnd; dstX++) {
|
||||||
const int outX = cfgX + 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 fx = srcFxFP & FP_MASK;
|
||||||
const int32_t fxInv = FP_ONE - fx;
|
const int32_t fxInv = FP_ONE - fx;
|
||||||
const int lx0 = (srcFxFP >> FP_SHIFT) - blockX;
|
const int lx0 = (srcFxFP >> FP_SHIFT) - blockX;
|
||||||
@@ -270,7 +278,7 @@ int jpegDrawCallback(JPEGDRAW* pDraw) {
|
|||||||
// Right edge (with X boundary clamping)
|
// Right edge (with X boundary clamping)
|
||||||
for (int dstX = safeXEnd; dstX < dstXEnd; dstX++) {
|
for (int dstX = safeXEnd; dstX < dstXEnd; dstX++) {
|
||||||
const int outX = cfgX + 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 fx = srcFxFP & FP_MASK;
|
||||||
const int32_t fxInv = FP_ONE - fx;
|
const int32_t fxInv = FP_ONE - fx;
|
||||||
int lx0 = (srcFxFP >> FP_SHIFT) - blockX;
|
int lx0 = (srcFxFP >> FP_SHIFT) - blockX;
|
||||||
@@ -301,7 +309,7 @@ int jpegDrawCallback(JPEGDRAW* pDraw) {
|
|||||||
const int outY = cfgY + dstY;
|
const int outY = cfgY + dstY;
|
||||||
pw.beginRow(outY);
|
pw.beginRow(outY);
|
||||||
if (caching) cw.beginRow(outY, ctx->config->y);
|
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;
|
int ly = (srcFyFP >> FP_SHIFT) - blockY;
|
||||||
if (ly < 0) ly = 0;
|
if (ly < 0) ly = 0;
|
||||||
if (ly >= blockH) ly = blockH - 1;
|
if (ly >= blockH) ly = blockH - 1;
|
||||||
@@ -309,7 +317,7 @@ int jpegDrawCallback(JPEGDRAW* pDraw) {
|
|||||||
|
|
||||||
for (int dstX = dstXStart; dstX < dstXEnd; dstX++) {
|
for (int dstX = dstXStart; dstX < dstXEnd; dstX++) {
|
||||||
const int outX = cfgX + dstX;
|
const int outX = cfgX + dstX;
|
||||||
const int32_t srcFxFP = dstX * invScaleFP;
|
const int32_t srcFxFP = dstX * invScaleFPX;
|
||||||
int lx = (srcFxFP >> FP_SHIFT) - blockX;
|
int lx = (srcFxFP >> FP_SHIFT) - blockX;
|
||||||
if (lx < 0) lx = 0;
|
if (lx < 0) lx = 0;
|
||||||
if (lx >= validW) lx = validW - 1;
|
if (lx >= validW) lx = validW - 1;
|
||||||
@@ -442,12 +450,22 @@ bool JpegToFramebufferConverter::decodeToFramebuffer(const std::string& imagePat
|
|||||||
jpegScaleDenom = chooseJpegScale(targetScale, jpegScaleOption);
|
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.scaledSrcWidth = (srcWidth + jpegScaleDenom - 1) / jpegScaleDenom;
|
||||||
ctx.scaledSrcHeight = (srcHeight + jpegScaleDenom - 1) / jpegScaleDenom;
|
ctx.scaledSrcHeight = (srcHeight + jpegScaleDenom - 1) / jpegScaleDenom;
|
||||||
ctx.dstWidth = destWidth;
|
ctx.dstWidth = destWidth;
|
||||||
ctx.dstHeight = destHeight;
|
ctx.dstHeight = destHeight;
|
||||||
ctx.fineScaleFP = (int32_t)((int64_t)destWidth * FP_ONE / ctx.scaledSrcWidth);
|
ctx.fineScaleFPX = (int32_t)((int64_t)destWidth * FP_ONE / ctx.scaledSrcWidth);
|
||||||
ctx.invScaleFP = (int32_t)((int64_t)ctx.scaledSrcWidth * FP_ONE / destWidth);
|
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,
|
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,
|
destHeight, targetScale, jpegScaleDenom, (float)destWidth / ctx.scaledSrcWidth,
|
||||||
|
|||||||
Reference in New Issue
Block a user