From 76a1c30a87e9c441beb244ab2cb1f5b19a65c331 Mon Sep 17 00:00:00 2001 From: Eunchurn Park Date: Sun, 28 Dec 2025 20:57:10 +0900 Subject: [PATCH] fix(xtc): address PR review feedback for grayscale rendering - Use FsHelpers::openFileForRead for consistency (XtcParser.cpp) - Simplify file extension checking logic (FileSelectionActivity.cpp) - Fix grayscale rendering bugs (XtcReaderActivity.cpp): - Use drawPixel(x, y, false) for LSB/MSB passes (LUT: 0=apply effect) - Add cleanupGrayscaleWithFrameBuffer() after re-rendering BW - Use conditional HALF_REFRESH based on pagesUntilFullRefresh - Decrement refresh counter instead of resetting - Add cleanupGrayscaleWithFrameBuffer() helper (GfxRenderer) --- lib/GfxRenderer/GfxRenderer.cpp | 11 +++++++++ lib/GfxRenderer/GfxRenderer.h | 1 + lib/Xtc/Xtc/XtcParser.cpp | 5 ++-- .../reader/FileSelectionActivity.cpp | 15 ++++-------- src/activities/reader/XtcReaderActivity.cpp | 23 ++++++++++++------- 5 files changed, 33 insertions(+), 22 deletions(-) diff --git a/lib/GfxRenderer/GfxRenderer.cpp b/lib/GfxRenderer/GfxRenderer.cpp index 196fdfd6..f458b574 100644 --- a/lib/GfxRenderer/GfxRenderer.cpp +++ b/lib/GfxRenderer/GfxRenderer.cpp @@ -369,6 +369,17 @@ void GfxRenderer::restoreBwBuffer() { Serial.printf("[%lu] [GFX] Restored and freed BW buffer chunks\n", millis()); } +/** + * Cleanup grayscale buffers using the current frame buffer. + * Use this when BW buffer was re-rendered instead of stored/restored. + */ +void GfxRenderer::cleanupGrayscaleWithFrameBuffer() const { + uint8_t* frameBuffer = einkDisplay.getFrameBuffer(); + if (frameBuffer) { + einkDisplay.cleanupGrayscaleBuffers(frameBuffer); + } +} + void GfxRenderer::renderChar(const EpdFontFamily& fontFamily, const uint32_t cp, int* x, const int* y, const bool pixelState, const EpdFontStyle style) const { const EpdGlyph* glyph = fontFamily.getGlyph(cp, style); diff --git a/lib/GfxRenderer/GfxRenderer.h b/lib/GfxRenderer/GfxRenderer.h index 3893bc9c..22fb6194 100644 --- a/lib/GfxRenderer/GfxRenderer.h +++ b/lib/GfxRenderer/GfxRenderer.h @@ -67,6 +67,7 @@ class GfxRenderer { void displayGrayBuffer() const; bool storeBwBuffer(); // Returns true if buffer was stored successfully void restoreBwBuffer(); + void cleanupGrayscaleWithFrameBuffer() const; // Low level functions uint8_t* getFrameBuffer() const; diff --git a/lib/Xtc/Xtc/XtcParser.cpp b/lib/Xtc/Xtc/XtcParser.cpp index cb380f9a..a443f57b 100644 --- a/lib/Xtc/Xtc/XtcParser.cpp +++ b/lib/Xtc/Xtc/XtcParser.cpp @@ -7,6 +7,7 @@ #include "XtcParser.h" +#include #include #include @@ -31,9 +32,7 @@ XtcError XtcParser::open(const char* filepath) { } // Open file - m_file = SD.open(filepath, FILE_READ); - if (!m_file) { - Serial.printf("[%lu] [XTC] Failed to open file: %s\n", millis(), filepath); + if (!FsHelpers::openFileForRead("XTC", filepath, m_file)) { m_lastError = XtcError::FILE_NOT_FOUND; return m_lastError; } diff --git a/src/activities/reader/FileSelectionActivity.cpp b/src/activities/reader/FileSelectionActivity.cpp index a288e613..aab8df39 100644 --- a/src/activities/reader/FileSelectionActivity.cpp +++ b/src/activities/reader/FileSelectionActivity.cpp @@ -40,18 +40,11 @@ void FileSelectionActivity::loadFiles() { if (file.isDirectory()) { files.emplace_back(filename + "/"); - } else if (filename.length() >= 5 && filename.substr(filename.length() - 5) == ".epub") { - files.emplace_back(filename); - } else if (filename.length() >= 4) { - // Check for XTC format extensions (.xtc, .xtch) - std::string ext4 = filename.substr(filename.length() - 4); - if (ext4 == ".xtc") { + } else { + std::string ext4 = filename.length() >= 4 ? filename.substr(filename.length() - 4) : ""; + std::string ext5 = filename.length() >= 5 ? filename.substr(filename.length() - 5) : ""; + if (ext5 == ".epub" || ext5 == ".xtch" || ext4 == ".xtc") { files.emplace_back(filename); - } else if (filename.length() >= 5) { - std::string ext5 = filename.substr(filename.length() - 5); - if (ext5 == ".xtch") { - files.emplace_back(filename); - } } } file.close(); diff --git a/src/activities/reader/XtcReaderActivity.cpp b/src/activities/reader/XtcReaderActivity.cpp index c52454a8..0f3777f3 100644 --- a/src/activities/reader/XtcReaderActivity.cpp +++ b/src/activities/reader/XtcReaderActivity.cpp @@ -236,29 +236,33 @@ void XtcReaderActivity::renderPage() { } } - // Display BW first with half refresh (clean base for grayscale overlay) - renderer.displayBuffer(EInkDisplay::HALF_REFRESH); + // Display BW with conditional refresh based on pagesUntilFullRefresh + if (pagesUntilFullRefresh <= 1) { + renderer.displayBuffer(EInkDisplay::HALF_REFRESH); + } else { + renderer.displayBuffer(); + } // Pass 2: LSB buffer - mark DARK gray only (XTH value 1) - // README: "mark the **dark** grays pixels with `1`" + // In LUT: 0 bit = apply gray effect, 1 bit = untouched renderer.clearScreen(0x00); for (uint16_t y = 0; y < pageHeight; y++) { for (uint16_t x = 0; x < pageWidth; x++) { if (getPixelValue(x, y) == 1) { // Dark grey only - renderer.drawPixel(x, y, true); + renderer.drawPixel(x, y, false); } } } renderer.copyGrayscaleLsbBuffers(); // Pass 3: MSB buffer - mark LIGHT AND DARK gray (XTH value 1 or 2) - // README: "mark the **light and dark** grays pixels with `1`" + // In LUT: 0 bit = apply gray effect, 1 bit = untouched renderer.clearScreen(0x00); for (uint16_t y = 0; y < pageHeight; y++) { for (uint16_t x = 0; x < pageWidth; x++) { const uint8_t pv = getPixelValue(x, y); if (pv == 1 || pv == 2) { // Dark grey or Light grey - renderer.drawPixel(x, y, true); + renderer.drawPixel(x, y, false); } } } @@ -277,8 +281,11 @@ void XtcReaderActivity::renderPage() { } } - // Reset refresh counter (grayscale display is a full refresh) - pagesUntilFullRefresh = pagesPerRefresh; + // Cleanup grayscale buffers with current frame buffer + renderer.cleanupGrayscaleWithFrameBuffer(); + + // Decrement refresh counter + pagesUntilFullRefresh--; free(pageBuffer);