Compare commits

...
Author SHA1 Message Date
Justin Mitchell bf9fea5923 Fix image decode overflow and viewport clipping issues
Prevent integer overflow when validating large image dimensions by using 64-bit arithmetic. Add periodic yielding during JPEG/PNG decode to prevent watchdog timeouts on multi-second operations. Clamp image top margins to prevent full-height images from overflowing page bottom and failing bounds checks. Increase max source pixels from 3MP to 8MP to support high-resolution ebook covers while keeping decode times reasonable.
2026-08-10 14:48:47 -04:00
9 changed files with 83 additions and 50 deletions

No files matched your search

+4 -1
View File
@@ -33,7 +33,10 @@ namespace {
// v35: Persist a uint32_t visible-text start offset for every page.
// v36: Ruby and CJK justification layout changes invalidate cached word positions.
// v37: Footnote href records grew from 96 to 256 bytes.
constexpr uint8_t SECTION_FILE_VERSION = 37;
// v38: Image top margin is clamped so a full-viewport-height image cannot
// overflow the page bottom; older caches can hold placements that panels
// with no bottom inset refuse to draw.
constexpr uint8_t SECTION_FILE_VERSION = 38;
// Written into the version field while a build is in progress; patched to
// SECTION_FILE_VERSION only when the build is finalized. An abandoned /
// crash-interrupted .bin therefore carries version 0, which loadSectionFile rejects
@@ -1,16 +1,28 @@
#include "ImageToFramebufferDecoder.h"
#include <Arduino.h>
#include <Logging.h>
bool ImageToFramebufferDecoder::validateImageDimensions(int width, int height, const std::string& format) {
if (width * height > MAX_SOURCE_PIXELS) {
LOG_ERR("IMG", "Image too large (%dx%d = %d pixels %s), max supported: %d pixels", width, height, width * height,
format.c_str(), MAX_SOURCE_PIXELS);
// 64-bit multiply: a malformed header (e.g. 40000x60000) overflows a plain
// int product before the comparison can reject it.
const int64_t pixels = static_cast<int64_t>(width) * static_cast<int64_t>(height);
if (pixels > MAX_SOURCE_PIXELS) {
LOG_ERR("IMG", "Image too large (%dx%d = %lld pixels %s), max supported: %lld pixels", width, height,
static_cast<long long>(pixels), format.c_str(), static_cast<long long>(MAX_SOURCE_PIXELS));
return false;
}
return true;
}
void ImageToFramebufferDecoder::yieldDuringDecode(uint32_t& lastYieldMs) {
const uint32_t now = millis();
if (now - lastYieldMs >= 250) {
lastYieldMs = now;
vTaskDelay(1);
}
}
void ImageToFramebufferDecoder::warnUnsupportedFeature(const std::string& feature, const std::string& imagePath) {
LOG_ERR("IMG", "Warning: Unsupported feature '%s' in image '%s'. Image may not display correctly.", feature.c_str(),
imagePath.c_str());
@@ -31,9 +31,21 @@ class ImageToFramebufferDecoder {
virtual const char* getFormatName() const = 0;
// Call from per-row/per-MCU decode callbacks (free functions, hence public):
// yields one tick at most every 250 ms so multi-second decodes keep the idle
// task (and its watchdog) fed. `lastYieldMs` is caller-held state,
// initialized to the decode start time.
static void yieldDuringDecode(uint32_t& lastYieldMs);
protected:
// Size validation helpers
static constexpr int MAX_SOURCE_PIXELS = 3145728; // 2048 * 1536
// Size validation helpers. The cap bounds decode TIME, not memory: both decoders
// stream (JPEG in MCU bands at 1/2..1/8 coarse scale, PNG scanline-by-scanline
// with its own width-based row-buffer guard), so RAM never scales with source
// area. 8 MP admits real-world ebook covers (KDP recommends 1600x2560 and
// 2000x3000) while keeping a worst-case single decode in single-digit seconds;
// the row callbacks yield periodically so a long decode cannot starve the idle
// task's watchdog.
static constexpr int64_t MAX_SOURCE_PIXELS = 8388608; // 8 MP (e.g. 2048 * 4096)
bool validateImageDimensions(int width, int height, const std::string& format);
void warnUnsupportedFeature(const std::string& feature, const std::string& imagePath);
@@ -46,6 +46,8 @@ struct JpegContext {
PixelCache cache;
bool caching{false};
uint32_t lastYieldMs{0}; // throttle state for yieldDuringDecode()
};
// File I/O callbacks use pFile->fHandle to access the HalFile*,
@@ -122,6 +124,8 @@ int jpegDrawCallback(JPEGDRAW* pDraw) {
JpegContext* ctx = reinterpret_cast<JpegContext*>(pDraw->pUser);
if (!ctx || !ctx->config || !ctx->renderer) return 0;
ImageToFramebufferDecoder::yieldDuringDecode(ctx->lastYieldMs);
// In EIGHT_BIT_GRAYSCALE mode, pPixels contains 8-bit grayscale values
// Buffer is densely packed: stride = pDraw->iWidth, valid columns = pDraw->iWidthUsed
uint8_t* pixels = reinterpret_cast<uint8_t*>(pDraw->pPixels);
@@ -494,6 +498,7 @@ bool JpegToFramebufferConverter::decodeToFramebuffer(const std::string& imagePat
}
unsigned long decodeStart = millis();
ctx.lastYieldMs = decodeStart;
rc = jpeg->decode(0, 0, jpegScaleOption);
unsigned long decodeTime = millis() - decodeStart;
@@ -38,6 +38,8 @@ struct PngContext {
bool caching{false};
uint8_t* grayLineBuffer{nullptr};
uint32_t lastYieldMs{0}; // throttle state for yieldDuringDecode()
};
// File I/O callbacks use pFile->fHandle to access the HalFile*,
@@ -206,6 +208,8 @@ int pngDrawCallback(PNGDRAW* pDraw) {
PngContext* ctx = reinterpret_cast<PngContext*>(pDraw->pUser);
if (!ctx || !ctx->config || !ctx->renderer || !ctx->grayLineBuffer) return 0;
ImageToFramebufferDecoder::yieldDuringDecode(ctx->lastYieldMs);
int srcY = pDraw->y;
int srcWidth = ctx->srcWidth;
@@ -432,6 +436,7 @@ bool PngToFramebufferConverter::decodeToFramebuffer(const std::string& imagePath
}
unsigned long decodeStart = millis();
ctx.lastYieldMs = decodeStart;
rc = png->decode(&ctx, 0);
unsigned long decodeTime = millis() - decodeStart;
@@ -793,7 +793,18 @@ void XMLCALL ChapterHtmlSlimParser::startElement(void* userData, const XML_Char*
self->currentPageVisibleOffsetSet = false;
}
// Apply top margin from container block
// Apply top margin from container block. Clamp it so the image never
// overflows the page bottom: a full-viewport-height image leaves no room
// for the margin, and the break above only fires on non-empty pages, so a
// fresh page would otherwise place the image at y=marginTop and run
// marginTop pixels past viewportHeight. A large bottom reserve (status
// bar / big screen margin) absorbs that overflow silently, but with a
// thin reserve it crosses the physical screen edge and fails
// ImageBlock::render's bounds check, dropping the image entirely.
if (self->currentPageNextY + imageMarginTop + displayHeight > self->viewportHeight) {
const int room = self->viewportHeight - displayHeight - self->currentPageNextY;
imageMarginTop = static_cast<int16_t>(room > 0 ? room : 0);
}
self->currentPageNextY += imageMarginTop;
// Create ImageBlock and add to page
+20 -16
View File
@@ -1,6 +1,7 @@
#include "GfxRenderer.h"
#include <BidiUtils.h>
#include <BoardConfig.h>
#include <BuildScratch.h>
#include <FontDecompressor.h>
#include <HalGPIO.h>
@@ -2220,30 +2221,33 @@ void GfxRenderer::cleanupGrayscaleWithFrameBuffer() const {
}
void GfxRenderer::getOrientedViewableTRBL(int* outTop, int* outRight, int* outBottom, int* outLeft) const {
// Board truth: the bezel insets live in the active profile (panel-native
// portrait frame); this only rotates them into the current orientation.
const BoardConfig::ViewableInsets& vi = BoardConfig::ACTIVE.viewableInsets;
switch (orientation) {
case Portrait:
*outTop = VIEWABLE_MARGIN_TOP;
*outRight = VIEWABLE_MARGIN_RIGHT;
*outBottom = VIEWABLE_MARGIN_BOTTOM;
*outLeft = VIEWABLE_MARGIN_LEFT;
*outTop = vi.top;
*outRight = vi.right;
*outBottom = vi.bottom;
*outLeft = vi.left;
break;
case LandscapeClockwise:
*outTop = VIEWABLE_MARGIN_LEFT;
*outRight = VIEWABLE_MARGIN_TOP;
*outBottom = VIEWABLE_MARGIN_RIGHT;
*outLeft = VIEWABLE_MARGIN_BOTTOM;
*outTop = vi.left;
*outRight = vi.top;
*outBottom = vi.right;
*outLeft = vi.bottom;
break;
case PortraitInverted:
*outTop = VIEWABLE_MARGIN_BOTTOM;
*outRight = VIEWABLE_MARGIN_LEFT;
*outBottom = VIEWABLE_MARGIN_TOP;
*outLeft = VIEWABLE_MARGIN_RIGHT;
*outTop = vi.bottom;
*outRight = vi.left;
*outBottom = vi.top;
*outLeft = vi.right;
break;
case LandscapeCounterClockwise:
*outTop = VIEWABLE_MARGIN_RIGHT;
*outRight = VIEWABLE_MARGIN_BOTTOM;
*outBottom = VIEWABLE_MARGIN_LEFT;
*outLeft = VIEWABLE_MARGIN_TOP;
*outTop = vi.right;
*outRight = vi.bottom;
*outBottom = vi.left;
*outLeft = vi.top;
break;
}
}
-5
View File
@@ -109,11 +109,6 @@ class GfxRenderer {
: display(halDisplay), renderMode(BW), orientation(Portrait), fadingFix(false) {}
~GfxRenderer() { freeBwBufferChunks(); }
static constexpr int VIEWABLE_MARGIN_TOP = 9;
static constexpr int VIEWABLE_MARGIN_RIGHT = 3;
static constexpr int VIEWABLE_MARGIN_BOTTOM = 3;
static constexpr int VIEWABLE_MARGIN_LEFT = 3;
// Setup
void begin(); // must be called right after display.begin()
void insertFont(int fontId, EpdFontFamily font);
+8 -22
View File
@@ -1619,28 +1619,14 @@ void EpubReaderActivity::renderContents(std::unique_ptr<Page> page, const int or
const auto tBwRender = millis();
if (pageHasImages) {
// 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)) {
// Image pages intentionally bypass the regular refresh cadence. Preserve
// a pending clean base before their double-FAST grayscale pipeline.
if (cleanImageBasePending) {
renderer.displayBuffer(HalDisplay::HALF_REFRESH);
}
renderer.fillRect(imgX + orientedMarginLeft, imgY + orientedMarginTop, 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, fontId, orientedMarginLeft, orientedMarginTop);
renderer.displayBuffer(HalDisplay::FAST_REFRESH);
} else {
renderer.displayBuffer(HalDisplay::HALF_REFRESH);
}
// Single FAST refresh with the image included, straight into the gray pass.
// FAST (not HALF) because HALF sets particles too firmly for the grayscale
// LUT to adjust (#1011). If large light-gray images wash out, the fix to
// restore is #957's prep: blank the image bounding box, FAST, re-render,
// FAST again, so the region reaches the gray pass from a uniform white.
// Image pages intentionally bypass the regular refresh cadence; preserve a
// pending clean base (silent restart / manual refresh) before the pipeline.
renderer.displayBuffer(cleanImageBasePending ? HalDisplay::HALF_REFRESH : HalDisplay::FAST_REFRESH);
// The image's own page is handled above and doesn't count toward the full
// refresh cadence. But the grayscale pass below leaves gray charge in the
// image region that a plain fast diff on the *next* page can't clear, so