snark13/Xteink-X4-crosspoint-reader
1
0
Fork 0
mirror of https://github.com/daveallie/crosspoint-reader.git synced 2026-02-04 14:47:37 +03:00
Code Issues Actions 1 Packages Projects Releases Wiki Activity

refactor: replace PNG global state with PngContext struct

Browse Source 
Address review comments #1 and #7:
- Replace all file-scope global variables (gRenderer, gConfig, gScale,
  gCacheBuffer, etc.) with a PngContext struct passed through pDraw->pUser
- Unify file I/O callbacks to use pFile->fHandle instead of global FsFile*
- Remove the unused FsFile opened at the start of decodeToFramebuffer()
  that was never used for actual decoding (duplicate open)
- Use shared PixelCache from PixelCache.h instead of hand-rolled globals
This commit is contained in:
Martin Brook 2026-01-30 17:47:54 +00:00
parent b8e61130f2
commit 8a31d05c38
1 changed files with 105 additions and 204 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

309
lib/Epub/Epub/converters/PngToFramebufferConverter.cpp
View File

@ -6,20 +6,73 @@
#include <SDCardManager.h> #include <SDCardManager.h>
#include <SdFat.h> #include <SdFat.h>
static FsFile* gPngFile = nullptr; #include "DitherUtils.h"
#include "PixelCache.h"
static void* pngOpenForDims(const char* filename, int32_t* size) { return gPngFile; } // Context struct passed through PNGdec callbacks to avoid global mutable state.
// The draw callback receives this via pDraw->pUser (set by png.decode()).
// The file I/O callbacks receive the FsFile* via pFile->fHandle (set by pngOpen()).
struct PngContext {
GfxRenderer* renderer;
const RenderConfig* config;
int screenWidth;
int screenHeight;
static void pngCloseForDims(void* handle) {} // Scaling state
float scale;
int srcWidth;
int srcHeight;
int dstWidth;
int dstHeight;
int lastDstY; // Track last rendered destination Y to avoid duplicates
static int32_t pngReadForDims(PNGFILE* pFile, uint8_t* pBuf, int32_t len) { PixelCache cache;
if (!gPngFile) return 0; bool caching;
return gPngFile->read(pBuf, len);
PngContext()
: renderer(nullptr),
config(nullptr),
screenWidth(0),
screenHeight(0),
scale(1.0f),
srcWidth(0),
srcHeight(0),
dstWidth(0),
dstHeight(0),
lastDstY(-1),
caching(false) {}
};
// File I/O callbacks use pFile->fHandle to access the FsFile*,
// avoiding the need for global file state.
static void* pngOpenWithHandle(const char* filename, int32_t* size) {
FsFile* f = new FsFile();
if (!SdMan.openFileForRead("PNG", std::string(filename), *f)) {
delete f;
return nullptr;
}
*size = f->size();
return f;
} }
static int32_t pngSeekForDims(PNGFILE* pFile, int32_t pos) { static void pngCloseWithHandle(void* handle) {
if (!gPngFile) return -1; FsFile* f = reinterpret_cast<FsFile*>(handle);
return gPngFile->seek(pos); if (f) {
f->close();
delete f;
}
}
static int32_t pngReadWithHandle(PNGFILE* pFile, uint8_t* pBuf, int32_t len) {
FsFile* f = reinterpret_cast<FsFile*>(pFile->fHandle);
if (!f) return 0;
return f->read(pBuf, len);
}
static int32_t pngSeekWithHandle(PNGFILE* pFile, int32_t pos) {
FsFile* f = reinterpret_cast<FsFile*>(pFile->fHandle);
if (!f) return -1;
return f->seek(pos);
} }
// Single static PNG object shared between getDimensions and decode // Single static PNG object shared between getDimensions and decode
@ -27,20 +80,11 @@ static int32_t pngSeekForDims(PNGFILE* pFile, int32_t pos) {
static PNG png; static PNG png;
bool PngToFramebufferConverter::getDimensionsStatic(const std::string& imagePath, ImageDimensions& out) { bool PngToFramebufferConverter::getDimensionsStatic(const std::string& imagePath, ImageDimensions& out) {
FsFile file; int rc = png.open(imagePath.c_str(), pngOpenWithHandle, pngCloseWithHandle, pngReadWithHandle, pngSeekWithHandle,
if (!SdMan.openFileForRead("PNG", imagePath, file)) { nullptr);
Serial.printf("[%lu] [PNG] Failed to open file for dimensions: %s\n", millis(), imagePath.c_str());
return false;
}
gPngFile = &file;
int rc = png.open(imagePath.c_str(), pngOpenForDims, pngCloseForDims, pngReadForDims, pngSeekForDims, nullptr);
if (rc != 0) { if (rc != 0) {
Serial.printf("[%lu] [PNG] Failed to open PNG for dimensions: %d\n", millis(), rc); Serial.printf("[%lu] [PNG] Failed to open PNG for dimensions: %d\n", millis(), rc);
file.close();
gPngFile = nullptr;
return false; return false;
} }
@ -48,107 +92,8 @@ bool PngToFramebufferConverter::getDimensionsStatic(const std::string& imagePath
out.height = png.getHeight(); out.height = png.getHeight();
png.close(); png.close();
file.close();
gPngFile = nullptr;
return true; return true;
} }
static GfxRenderer* gRenderer = nullptr;
static const RenderConfig* gConfig = nullptr;
static int gScreenWidth = 0;
static int gScreenHeight = 0;
static FsFile* pngFile = nullptr;
// Scaling state for PNG
static float gScale = 1.0f;
static int gSrcWidth = 0;
static int gSrcHeight = 0;
static int gDstWidth = 0;
static int gDstHeight = 0;
static int gLastDstY = -1; // Track last rendered destination Y to avoid duplicates
// Pixel cache for PNG (uses scaled dimensions)
static uint8_t* gCacheBuffer = nullptr;
static int gCacheWidth = 0;
static int gCacheHeight = 0;
static int gCacheBytesPerRow = 0;
static int gCacheOriginX = 0;
static int gCacheOriginY = 0;
static void cacheSetPixel(int screenX, int screenY, uint8_t value) {
if (!gCacheBuffer) return;
int localX = screenX - gCacheOriginX;
int localY = screenY - gCacheOriginY;
if (localX < 0 || localX >= gCacheWidth || localY < 0 || localY >= gCacheHeight) return;
int byteIdx = localY * gCacheBytesPerRow + localX / 4;
int bitShift = 6 - (localX % 4) * 2; // MSB first: pixel 0 at bits 6-7
gCacheBuffer[byteIdx] = (gCacheBuffer[byteIdx] & ~(0x03 << bitShift)) | ((value & 0x03) << bitShift);
}
// 4x4 Bayer matrix for ordered dithering
static const uint8_t bayer4x4[4][4] = {
{0, 8, 2, 10},
{12, 4, 14, 6},
{3, 11, 1, 9},
{15, 7, 13, 5},
};
// Apply Bayer dithering and quantize to 4 levels (0-3)
// Stateless - works correctly with any pixel processing order
static uint8_t applyBayerDither4Level(uint8_t gray, int x, int y) {
int bayer = bayer4x4[y & 3][x & 3];
int dither = (bayer - 8) * 5; // Scale to ±40 (half of quantization step 85)
int adjusted = gray + dither;
if (adjusted < 0) adjusted = 0;
if (adjusted > 255) adjusted = 255;
if (adjusted < 64) return 0;
if (adjusted < 128) return 1;
if (adjusted < 192) return 2;
return 3;
}
// Draw a pixel respecting the current render mode for grayscale support
static void drawPixelWithRenderMode(GfxRenderer* renderer, int x, int y, uint8_t pixelValue) {
GfxRenderer::RenderMode renderMode = renderer->getRenderMode();
if (renderMode == GfxRenderer::BW && pixelValue < 3) {
renderer->drawPixel(x, y, true);
} else if (renderMode == GfxRenderer::GRAYSCALE_MSB && (pixelValue == 1 || pixelValue == 2)) {
renderer->drawPixel(x, y, false);
} else if (renderMode == GfxRenderer::GRAYSCALE_LSB && pixelValue == 1) {
renderer->drawPixel(x, y, false);
}
}
void* pngOpen(const char* filename, int32_t* size) {
pngFile = new FsFile();
if (!SdMan.openFileForRead("PNG", std::string(filename), *pngFile)) {
delete pngFile;
pngFile = nullptr;
return nullptr;
}
*size = pngFile->size();
return pngFile;
}
void pngClose(void* handle) {
if (pngFile) {
pngFile->close();
delete pngFile;
pngFile = nullptr;
}
}
int32_t pngRead(PNGFILE* pFile, uint8_t* pBuf, int32_t len) {
if (!pngFile) return 0;
return pngFile->read(pBuf, len);
}
int32_t pngSeek(PNGFILE* pFile, int32_t pos) {
if (!pngFile) return -1;
return pngFile->seek(pos);
}
// Helper to get grayscale from PNG pixel data // Helper to get grayscale from PNG pixel data
static uint8_t getGrayFromPixel(uint8_t* pPixels, int x, int pixelType, uint8_t* palette) { static uint8_t getGrayFromPixel(uint8_t* pPixels, int x, int pixelType, uint8_t* palette) {
@ -184,45 +129,46 @@ static uint8_t getGrayFromPixel(uint8_t* pPixels, int x, int pixelType, uint8_t*
} }
int pngDrawCallback(PNGDRAW* pDraw) { int pngDrawCallback(PNGDRAW* pDraw) {
if (!gConfig || !gRenderer) return 0; PngContext* ctx = reinterpret_cast<PngContext*>(pDraw->pUser);
if (!ctx || !ctx->config || !ctx->renderer) return 0;
int srcY = pDraw->y; int srcY = pDraw->y;
uint8_t* pPixels = pDraw->pPixels; uint8_t* pPixels = pDraw->pPixels;
int pixelType = pDraw->iPixelType; int pixelType = pDraw->iPixelType;
// Calculate destination Y with scaling // Calculate destination Y with scaling
int dstY = (int)(srcY * gScale); int dstY = (int)(srcY * ctx->scale);
// Skip if we already rendered this destination row (multiple source rows map to same dest) // Skip if we already rendered this destination row (multiple source rows map to same dest)
if (dstY == gLastDstY) return 1; if (dstY == ctx->lastDstY) return 1;
gLastDstY = dstY; ctx->lastDstY = dstY;
// Check bounds // Check bounds
if (dstY >= gDstHeight) return 1; if (dstY >= ctx->dstHeight) return 1;
int outY = gConfig->y + dstY; int outY = ctx->config->y + dstY;
if (outY >= gScreenHeight) return 1; if (outY >= ctx->screenHeight) return 1;
// Render scaled row using nearest-neighbor sampling // Render scaled row using nearest-neighbor sampling
for (int dstX = 0; dstX < gDstWidth; dstX++) { for (int dstX = 0; dstX < ctx->dstWidth; dstX++) {
int outX = gConfig->x + dstX; int outX = ctx->config->x + dstX;
if (outX >= gScreenWidth) continue; if (outX >= ctx->screenWidth) continue;
// Map destination X back to source X // Map destination X back to source X
int srcX = (int)(dstX / gScale); int srcX = (int)(dstX / ctx->scale);
if (srcX >= gSrcWidth) srcX = gSrcWidth - 1; if (srcX >= ctx->srcWidth) srcX = ctx->srcWidth - 1;
uint8_t gray = getGrayFromPixel(pPixels, srcX, pixelType, pDraw->pPalette); uint8_t gray = getGrayFromPixel(pPixels, srcX, pixelType, pDraw->pPalette);
uint8_t ditheredGray; uint8_t ditheredGray;
if (gConfig->useDithering) { if (ctx->config->useDithering) {
ditheredGray = applyBayerDither4Level(gray, outX, outY); ditheredGray = applyBayerDither4Level(gray, outX, outY);
} else { } else {
ditheredGray = gray / 85; ditheredGray = gray / 85;
if (ditheredGray > 3) ditheredGray = 3; if (ditheredGray > 3) ditheredGray = 3;
} }
drawPixelWithRenderMode(gRenderer, outX, outY, ditheredGray); drawPixelWithRenderMode(*ctx->renderer, outX, outY, ditheredGray);
cacheSetPixel(outX, outY, ditheredGray); if (ctx->caching) ctx->cache.setPixel(outX, outY, ditheredGray);
} }
return 1; return 1;
@ -232,48 +178,38 @@ bool PngToFramebufferConverter::decodeToFramebuffer(const std::string& imagePath
const RenderConfig& config) { const RenderConfig& config) {
Serial.printf("[%lu] [PNG] Decoding PNG: %s\n", millis(), imagePath.c_str()); Serial.printf("[%lu] [PNG] Decoding PNG: %s\n", millis(), imagePath.c_str());
FsFile file; PngContext ctx;
if (!SdMan.openFileForRead("PNG", imagePath, file)) { ctx.renderer = &renderer;
Serial.printf("[%lu] [PNG] Failed to open file: %s\n", millis(), imagePath.c_str()); ctx.config = &config;
return false; ctx.screenWidth = renderer.getScreenWidth();
} ctx.screenHeight = renderer.getScreenHeight();
gRenderer = &renderer; int rc = png.open(imagePath.c_str(), pngOpenWithHandle, pngCloseWithHandle, pngReadWithHandle, pngSeekWithHandle,
gConfig = &config; pngDrawCallback);
gScreenWidth = renderer.getScreenWidth();
gScreenHeight = renderer.getScreenHeight();
int rc = png.open(imagePath.c_str(), pngOpen, pngClose, pngRead, pngSeek, pngDrawCallback);
if (rc != PNG_SUCCESS) { if (rc != PNG_SUCCESS) {
Serial.printf("[%lu] [PNG] Failed to open PNG: %d\n", millis(), rc); Serial.printf("[%lu] [PNG] Failed to open PNG: %d\n", millis(), rc);
file.close();
gRenderer = nullptr;
gConfig = nullptr;
return false; return false;
} }
if (!validateImageDimensions(png.getWidth(), png.getHeight(), "PNG")) { if (!validateImageDimensions(png.getWidth(), png.getHeight(), "PNG")) {
png.close(); png.close();
file.close();
gRenderer = nullptr;
gConfig = nullptr;
return false; return false;
} }
// Calculate scale factor to fit within maxWidth x maxHeight // Calculate scale factor to fit within maxWidth x maxHeight
gSrcWidth = png.getWidth(); ctx.srcWidth = png.getWidth();
gSrcHeight = png.getHeight(); ctx.srcHeight = png.getHeight();
float scaleX = (float)config.maxWidth / gSrcWidth; float scaleX = (float)config.maxWidth / ctx.srcWidth;
float scaleY = (float)config.maxHeight / gSrcHeight; float scaleY = (float)config.maxHeight / ctx.srcHeight;
gScale = (scaleX < scaleY) ? scaleX : scaleY; ctx.scale = (scaleX < scaleY) ? scaleX : scaleY;
if (gScale > 1.0f) gScale = 1.0f; // Don't upscale if (ctx.scale > 1.0f) ctx.scale = 1.0f; // Don't upscale
gDstWidth = (int)(gSrcWidth * gScale); ctx.dstWidth = (int)(ctx.srcWidth * ctx.scale);
gDstHeight = (int)(gSrcHeight * gScale); ctx.dstHeight = (int)(ctx.srcHeight * ctx.scale);
gLastDstY = -1; // Reset row tracking ctx.lastDstY = -1; // Reset row tracking
Serial.printf("[%lu] [PNG] PNG %dx%d -> %dx%d (scale %.2f), bpp: %d\n", millis(), gSrcWidth, gSrcHeight, gDstWidth, Serial.printf("[%lu] [PNG] PNG %dx%d -> %dx%d (scale %.2f), bpp: %d\n", millis(), ctx.srcWidth, ctx.srcHeight,
gDstHeight, gScale, png.getBpp()); ctx.dstWidth, ctx.dstHeight, ctx.scale, png.getBpp());
if (png.getBpp() != 8) { if (png.getBpp() != 8) {
warnUnsupportedFeature("bit depth (" + std::to_string(png.getBpp()) + "bpp)", imagePath); warnUnsupportedFeature("bit depth (" + std::to_string(png.getBpp()) + "bpp)", imagePath);
@ -284,64 +220,29 @@ bool PngToFramebufferConverter::decodeToFramebuffer(const std::string& imagePath
} }
// Allocate cache buffer using SCALED dimensions // Allocate cache buffer using SCALED dimensions
bool caching = !config.cachePath.empty(); ctx.caching = !config.cachePath.empty();
if (caching) { if (ctx.caching) {
gCacheWidth = gDstWidth; if (!ctx.cache.allocate(ctx.dstWidth, ctx.dstHeight, config.x, config.y)) {
gCacheHeight = gDstHeight;
gCacheBytesPerRow = (gCacheWidth + 3) / 4;
gCacheOriginX = config.x;
gCacheOriginY = config.y;
size_t bufferSize = gCacheBytesPerRow * gCacheHeight;
gCacheBuffer = (uint8_t*)malloc(bufferSize);
if (gCacheBuffer) {
memset(gCacheBuffer, 0, bufferSize);
Serial.printf("[%lu] [PNG] Allocated cache buffer: %d bytes for %dx%d\n", millis(), bufferSize, gCacheWidth,
gCacheHeight);
} else {
Serial.printf("[%lu] [PNG] Failed to allocate cache buffer, continuing without caching\n", millis()); Serial.printf("[%lu] [PNG] Failed to allocate cache buffer, continuing without caching\n", millis());
caching = false; ctx.caching = false;
} }
} }
rc = png.decode(nullptr, 0); rc = png.decode(&ctx, 0);
if (rc != PNG_SUCCESS) { if (rc != PNG_SUCCESS) {
Serial.printf("[%lu] [PNG] Decode failed: %d\n", millis(), rc); Serial.printf("[%lu] [PNG] Decode failed: %d\n", millis(), rc);
png.close(); png.close();
file.close();
gRenderer = nullptr;
gConfig = nullptr;
if (gCacheBuffer) {
free(gCacheBuffer);
gCacheBuffer = nullptr;
}
return false; return false;
} }
png.close(); png.close();
file.close();
Serial.printf("[%lu] [PNG] PNG decoding complete\n", millis()); Serial.printf("[%lu] [PNG] PNG decoding complete\n", millis());
// Write cache file if caching was enabled and buffer was allocated // Write cache file if caching was enabled and buffer was allocated
if (caching && gCacheBuffer) { if (ctx.caching) {
FsFile cacheFile; ctx.cache.writeToFile(config.cachePath);
if (SdMan.openFileForWrite("IMG", config.cachePath, cacheFile)) {
uint16_t w = gCacheWidth;
uint16_t h = gCacheHeight;
cacheFile.write(&w, 2);
cacheFile.write(&h, 2);
cacheFile.write(gCacheBuffer, gCacheBytesPerRow * gCacheHeight);
cacheFile.close();
Serial.printf("[%lu] [PNG] Cache written: %s (%dx%d, %d bytes)\n", millis(), config.cachePath.c_str(),
gCacheWidth, gCacheHeight, 4 + gCacheBytesPerRow * gCacheHeight);
} else {
Serial.printf("[%lu] [PNG] Failed to open cache file for writing: %s\n", millis(), config.cachePath.c_str());
}
free(gCacheBuffer);
gCacheBuffer = nullptr;
} }
gRenderer = nullptr;
gConfig = nullptr;
return true; return true;
} }