From b8e61130f29e8b975f4fffdcdcd5f45a8fa003e5 Mon Sep 17 00:00:00 2001
From: Martin Brook <martin.brook100@googlemail.com>
Date: Fri, 30 Jan 2026 17:47:15 +0000
Subject: [PATCH] refactor: extract shared DitherUtils.h and PixelCache.h

Address review comments #2, #3, and #10:
- Extract duplicated bayer4x4 matrix, applyBayerDither4Level(), and
  drawPixelWithRenderMode() into shared DitherUtils.h
- Extract duplicated PixelCache struct into shared PixelCache.h so both
  JPEG and PNG decoders use the same implementation
- Add MAX_CACHE_BYTES (256KB) size limit to PixelCache::allocate() to
  proactively guard against oversized allocations on embedded targets
---
 lib/Epub/Epub/converters/DitherUtils.h | 40 +++++++++++++
 lib/Epub/Epub/converters/PixelCache.h  | 83 ++++++++++++++++++++++++++
 2 files changed, 123 insertions(+)
 create mode 100644 lib/Epub/Epub/converters/DitherUtils.h
 create mode 100644 lib/Epub/Epub/converters/PixelCache.h

diff --git a/lib/Epub/Epub/converters/DitherUtils.h b/lib/Epub/Epub/converters/DitherUtils.h
new file mode 100644
index 00000000..ec14a332
--- /dev/null
+++ b/lib/Epub/Epub/converters/DitherUtils.h
@@ -0,0 +1,40 @@
+#pragma once
+
+#include <GfxRenderer.h>
+#include <stdint.h>
+
+// 4x4 Bayer matrix for ordered dithering
+inline 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
+inline 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
+inline 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);
+  }
+}
diff --git a/lib/Epub/Epub/converters/PixelCache.h b/lib/Epub/Epub/converters/PixelCache.h
new file mode 100644
index 00000000..9a20b0ff
--- /dev/null
+++ b/lib/Epub/Epub/converters/PixelCache.h
@@ -0,0 +1,83 @@
+#pragma once
+
+#include <HardwareSerial.h>
+#include <SDCardManager.h>
+#include <SdFat.h>
+#include <stdint.h>
+
+#include <cstring>
+#include <string>
+
+// Cache buffer for storing 2-bit pixels (4 levels) during decode.
+// Packs 4 pixels per byte, MSB first.
+struct PixelCache {
+  uint8_t* buffer;
+  int width;
+  int height;
+  int bytesPerRow;
+  int originX;  // config.x - to convert screen coords to cache coords
+  int originY;  // config.y
+
+  PixelCache() : buffer(nullptr), width(0), height(0), bytesPerRow(0), originX(0), originY(0) {}
+
+  static constexpr size_t MAX_CACHE_BYTES = 256 * 1024;  // 256KB limit for embedded targets
+
+  bool allocate(int w, int h, int ox, int oy) {
+    width = w;
+    height = h;
+    originX = ox;
+    originY = oy;
+    bytesPerRow = (w + 3) / 4;  // 2 bits per pixel, 4 pixels per byte
+    size_t bufferSize = (size_t)bytesPerRow * h;
+    if (bufferSize > MAX_CACHE_BYTES) {
+      Serial.printf("[%lu] [IMG] Cache buffer too large: %d bytes for %dx%d (limit %d)\n", millis(), bufferSize, w, h,
+                    MAX_CACHE_BYTES);
+      return false;
+    }
+    buffer = (uint8_t*)malloc(bufferSize);
+    if (buffer) {
+      memset(buffer, 0, bufferSize);
+      Serial.printf("[%lu] [IMG] Allocated cache buffer: %d bytes for %dx%d\n", millis(), bufferSize, w, h);
+    }
+    return buffer != nullptr;
+  }
+
+  void setPixel(int screenX, int screenY, uint8_t value) {
+    if (!buffer) return;
+    int localX = screenX - originX;
+    int localY = screenY - originY;
+    if (localX < 0 || localX >= width || localY < 0 || localY >= height) return;
+
+    int byteIdx = localY * bytesPerRow + localX / 4;
+    int bitShift = 6 - (localX % 4) * 2;  // MSB first: pixel 0 at bits 6-7
+    buffer[byteIdx] = (buffer[byteIdx] & ~(0x03 << bitShift)) | ((value & 0x03) << bitShift);
+  }
+
+  bool writeToFile(const std::string& cachePath) {
+    if (!buffer) return false;
+
+    FsFile cacheFile;
+    if (!SdMan.openFileForWrite("IMG", cachePath, cacheFile)) {
+      Serial.printf("[%lu] [IMG] Failed to open cache file for writing: %s\n", millis(), cachePath.c_str());
+      return false;
+    }
+
+    uint16_t w = width;
+    uint16_t h = height;
+    cacheFile.write(&w, 2);
+    cacheFile.write(&h, 2);
+    cacheFile.write(buffer, bytesPerRow * height);
+    cacheFile.close();
+
+    Serial.printf("[%lu] [IMG] Cache written: %s (%dx%d, %d bytes)\n", millis(), cachePath.c_str(), width, height,
+                  4 + bytesPerRow * height);
+    return true;
+  }
+
+  ~PixelCache() {
+    if (buffer) {
+      free(buffer);
+      buffer = nullptr;
+    }
+  }
+};