diff --git a/README.md b/README.md index c938b6df..ddf70792 100644 --- a/README.md +++ b/README.md @@ -8,6 +8,8 @@ CrossPoint is open-source e-reader firmware - community-built, fully hackable, f ![CrossPoint Reader running on Xteink device](./docs/images/cover.jpg) +> If you're planning to buy an Xteink device, consider purchasing an **X3/X4 Developer Edition** through https://crosspointreader.com. CrossPoint receives a small share of each sale, helping fund development costs. + ## What can CrossPoint do? - **Reader engine**: EPUB 2/3 rendering with embedded-style option, image handling, hyphenation, kerning, chapter navigation, footnotes, bookmarks, go-to-percent, auto page turn, orientation control, focus reading, KOReader progress sync and more. diff --git a/docs/file-formats.md b/docs/file-formats.md index eec9474f..4289f7e1 100644 --- a/docs/file-formats.md +++ b/docs/file-formats.md @@ -90,13 +90,13 @@ if (parsedSize != fileSize) { ## `section.bin` -### Version 28 +### Version 29 Each file in `sections/*.bin` stores one laid-out spine section. The header is also the cache-busting key: if any layout-affecting setting differs from the current reader settings, the section is discarded and rebuilt. -Version 28 includes: +Version 29 includes: - cache-busting fields for paragraph alignment, hyphenation, embedded CSS, image rendering mode, and Focus Reading @@ -107,6 +107,10 @@ Version 28 includes: - per-page footnote entries - serialized word style bits for underline, strikethrough, superscript, and subscript +- flat TextBlock word storage (v29): per-word arrays plus one shared + NUL-terminated text blob, replacing v28's length-prefixed word strings. The + on-disk order mirrors the in-RAM arena so the firmware reads a whole block + payload with a single allocation and a single SD read ImHex pattern: @@ -115,7 +119,7 @@ import std.mem; import std.string; import std.core; -#define EXPECTED_VERSION 28 +#define EXPECTED_VERSION 29 #define MAX_STRING_LENGTH 65535 #define FOOTNOTE_NUMBER_LEN 32 #define FOOTNOTE_HREF_LEN 96 @@ -176,14 +180,20 @@ struct BlockStyle { struct TextBlock { u16 wordCount; - String words[wordCount]; - s16 wordXPos[wordCount]; - WordStyle wordStyle[wordCount]; - u8 hasFocus; - if (hasFocus != 0) { - u8 wordFocusBoundary[wordCount] [[comment("UTF-8 byte boundary between bold prefix and suffix")]]; - u16 wordFocusSuffixX[wordCount] [[comment("Suffix x offset from word start")]]; + u16 textBytes [[comment("Total size of text[], including one NUL per word")]]; + + if (wordCount > 0) { + u16 textOff[wordCount] [[comment("Byte offset of word i's text within text[]")]]; + s16 wordXPos[wordCount]; + if (hasFocus != 0) { + u16 wordFocusSuffixX[wordCount] [[comment("Suffix x offset from word start")]]; + } + WordStyle wordStyle[wordCount]; + if (hasFocus != 0) { + u8 wordFocusBoundary[wordCount] [[comment("UTF-8 byte boundary between bold prefix and suffix")]]; + } + char text[textBytes] [[comment("All words back to back, each NUL-terminated")]]; } BlockStyle blockStyle; diff --git a/lib/EpdFont/FontDecompressor.cpp b/lib/EpdFont/FontDecompressor.cpp index adfbb628..786f7d3e 100644 --- a/lib/EpdFont/FontDecompressor.cpp +++ b/lib/EpdFont/FontDecompressor.cpp @@ -33,12 +33,24 @@ void FontDecompressor::freePageBuffer() { } void FontDecompressor::freeHotGroup() { - hotGroup.clear(); - hotGroup.shrink_to_fit(); + free(hotGroup); + hotGroup = nullptr; + hotGroupCapacity = 0; hotGroupFont = nullptr; hotGroupIndex = UINT16_MAX; - hotGlyphBuf.clear(); - hotGlyphBuf.shrink_to_fit(); + free(hotGlyphBuf); + hotGlyphBuf = nullptr; + hotGlyphBufCapacity = 0; +} + +bool FontDecompressor::ensureCapacity(uint8_t*& buf, uint32_t& capacity, uint32_t needed) { + if (capacity >= needed) return true; + // Grow-only, free-then-malloc: every caller fully rewrites the buffer after a grow, so the + // old contents are dead -- freeing first gives the allocator its best shot on a tight heap. + free(buf); + buf = static_cast(malloc(needed)); // owned by FontDecompressor, freed in freeHotGroup() + capacity = buf ? needed : 0; + return buf != nullptr; } uint16_t FontDecompressor::getGroupIndex(const EpdFontData* fontData, uint32_t glyphIndex) { @@ -170,24 +182,20 @@ const uint8_t* FontDecompressor::getBitmap(const EpdFontData* fontData, const Ep } // Check if hot group already has this group decompressed — if not, decompress it - if (!(!hotGroup.empty() && hotGroupFont == fontData && hotGroupIndex == groupIndex)) { + if (!(hotGroup != nullptr && hotGroupFont == fontData && hotGroupIndex == groupIndex)) { stats.cacheMisses++; const EpdFontGroup& group = fontData->groups[groupIndex]; - hotGroup.resize(group.uncompressedSize); - if (hotGroup.empty()) { + // ensureCapacity may free the buffer, so the cached-group identity dies with it either way. + hotGroupFont = nullptr; + hotGroupIndex = UINT16_MAX; + if (!ensureCapacity(hotGroup, hotGroupCapacity, group.uncompressedSize)) { LOG_ERR("FDC", "Failed to allocate %u bytes for hot group %u", group.uncompressedSize, groupIndex); - hotGroupFont = nullptr; - hotGroupIndex = UINT16_MAX; stats.getBitmapTimeUs += micros() - tStart; return nullptr; } - if (!decompressGroup(fontData, groupIndex, hotGroup.data(), group.uncompressedSize)) { - hotGroup.clear(); - hotGroup.shrink_to_fit(); - hotGroupFont = nullptr; - hotGroupIndex = UINT16_MAX; + if (!decompressGroup(fontData, groupIndex, hotGroup, group.uncompressedSize)) { stats.getBitmapTimeUs += micros() - tStart; return nullptr; } @@ -200,18 +208,16 @@ const uint8_t* FontDecompressor::getBitmap(const EpdFontData* fontData, const Ep } // Compact just the requested glyph from byte-aligned data into scratch buffer - if (glyph->dataLength > hotGlyphBuf.size()) { - hotGlyphBuf.resize(glyph->dataLength); - } - if (hotGlyphBuf.empty()) { + if (!ensureCapacity(hotGlyphBuf, hotGlyphBufCapacity, glyph->dataLength)) { + LOG_ERR("FDC", "Failed to allocate %u bytes for glyph scratch", (unsigned)glyph->dataLength); stats.getBitmapTimeUs += micros() - tStart; return nullptr; } uint32_t alignedOff = getAlignedOffset(fontData, groupIndex, glyphIndex); - compactSingleGlyph(&hotGroup[alignedOff], hotGlyphBuf.data(), glyph->width, glyph->height); + compactSingleGlyph(&hotGroup[alignedOff], hotGlyphBuf, glyph->width, glyph->height); stats.getBitmapTimeUs += micros() - tStart; - return hotGlyphBuf.data(); + return hotGlyphBuf; } // --- Prewarm: pre-decompress glyph bitmaps for a page of text --- diff --git a/lib/EpdFont/FontDecompressor.h b/lib/EpdFont/FontDecompressor.h index 54e75a86..6ac27647 100644 --- a/lib/EpdFont/FontDecompressor.h +++ b/lib/EpdFont/FontDecompressor.h @@ -2,8 +2,6 @@ #include -#include - #include "EpdFontData.h" class FontDecompressor { @@ -67,13 +65,22 @@ class FontDecompressor { // Hot group: last decompressed group (byte-aligned) for non-prewarmed fallback path. // Kept in byte-aligned format; individual glyphs are compacted on demand into hotGlyphBuf. + // Nothrow high-water malloc buffers, NOT std::vector: getBitmap() runs on the render path, + // and under -fno-exceptions a vector resize that hits OOM abort()s the firmware instead of + // failing (field crash: hotGroup.resize() -> std::bad_alloc -> abort with ~11 KB free). + // ensureCapacity() returns false on OOM so the caller can skip the glyph gracefully. const EpdFontData* hotGroupFont = nullptr; uint16_t hotGroupIndex = UINT16_MAX; - std::vector hotGroup; + uint8_t* hotGroup = nullptr; // owned; freed in freeHotGroup()/dtor + uint32_t hotGroupCapacity = 0; // Scratch buffer for compacting a single glyph from the hot group. - // Valid until the next getBitmap() call. - std::vector hotGlyphBuf; + // Valid until the next getBitmap() call. Same ownership/OOM contract as hotGroup. + uint8_t* hotGlyphBuf = nullptr; + uint32_t hotGlyphBufCapacity = 0; + + // Grow (never shrink) an owned buffer to at least `needed` bytes; false on OOM, buffer freed. + static bool ensureCapacity(uint8_t*& buf, uint32_t& capacity, uint32_t needed); void freePageBuffer(); void freeHotGroup(); diff --git a/lib/Epub/Epub/Page.cpp b/lib/Epub/Epub/Page.cpp index 261ea6a6..55eefadb 100644 --- a/lib/Epub/Epub/Page.cpp +++ b/lib/Epub/Epub/Page.cpp @@ -39,7 +39,17 @@ std::unique_ptr PageLine::deserialize(HalFile& file) { serialization::readPod(file, yPos); auto tb = TextBlock::deserialize(file); - return std::unique_ptr(new PageLine(std::move(tb), xPos, yPos)); + if (!tb) { + LOG_ERR("PGE", "Deserialization failed: null TextBlock"); + return nullptr; + } + + auto* line = new (std::nothrow) PageLine(std::move(tb), xPos, yPos); + if (!line) { + LOG_ERR("PGE", "Deserialization failed: could not allocate PageLine"); + return nullptr; + } + return std::unique_ptr(line); } void PageImage::render(GfxRenderer& renderer, const int fontId, const int xOffset, const int yOffset) { @@ -159,9 +169,15 @@ std::unique_ptr Page::deserialize(HalFile& file) { if (tag == TAG_PageLine) { auto pl = PageLine::deserialize(file); + if (!pl) { + return nullptr; + } page->elements.push_back(std::move(pl)); } else if (tag == TAG_PageImage) { auto pi = PageImage::deserialize(file); + if (!pi) { + return nullptr; + } page->elements.push_back(std::move(pi)); } else if (tag == TAG_PageHorizontalRule) { auto rule = PageHorizontalRule::deserialize(file); diff --git a/lib/Epub/Epub/ParsedText.cpp b/lib/Epub/Epub/ParsedText.cpp index ebe758db..2ae204a4 100644 --- a/lib/Epub/Epub/ParsedText.cpp +++ b/lib/Epub/Epub/ParsedText.cpp @@ -2,6 +2,7 @@ #include #include +#include #include #include @@ -1133,8 +1134,14 @@ void ParsedText::extractLine(const size_t breakIndex, const int pageWidth, const } if (!lineHasFocusSplit) { - processLine(std::make_shared(std::move(lineWords), std::move(lineXPos), std::move(lineWordStyles), - std::vector{}, std::vector{}, blockStyle)); + // TextBlock flattens the vectors into its arena; they stay owned here and die at return. + auto block = std::make_shared(lineWords, lineXPos, lineWordStyles, std::vector{}, + std::vector{}, blockStyle); + if (!block->valid()) { + LOG_ERR("PTX", "Dropping line: TextBlock arena allocation failed"); + return; + } + processLine(std::move(block)); return; } @@ -1179,6 +1186,10 @@ void ParsedText::extractLine(const size_t breakIndex, const int pageWidth, const } } - processLine(std::make_shared(std::move(outWords), std::move(outXPos), std::move(outStyles), - std::move(outBoundaries), std::move(outSuffixX), blockStyle)); + auto block = std::make_shared(outWords, outXPos, outStyles, outBoundaries, outSuffixX, blockStyle); + if (!block->valid()) { + LOG_ERR("PTX", "Dropping line: TextBlock arena allocation failed"); + return; + } + processLine(std::move(block)); } diff --git a/lib/Epub/Epub/Section.cpp b/lib/Epub/Epub/Section.cpp index 2de11f58..663d086b 100644 --- a/lib/Epub/Epub/Section.cpp +++ b/lib/Epub/Epub/Section.cpp @@ -11,8 +11,9 @@ #include "parsers/ChapterHtmlSlimParser.h" namespace { -// v28: text decoration bits now include line-through in serialized wordStyles. -constexpr uint8_t SECTION_FILE_VERSION = 28; +// v29: TextBlock word data stored as one flat arena (offset table + NUL-terminated +// text blob) instead of length-prefixed strings and per-field arrays. +constexpr uint8_t SECTION_FILE_VERSION = 29; // 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 @@ -25,7 +26,11 @@ constexpr uint8_t SECTION_FILE_INCOMPLETE_VERSION = 0; // rebuilding in the background. Uses the same header layout as SECTION_FILE_VERSION, // so finalized files are untouched by this feature; older firmware treats the sentinel // as an unknown version and rebuilds, which is a safe downgrade. -constexpr uint8_t SECTION_FILE_PARTIAL_VERSION = 0xFE; +// MUST change in lockstep with SECTION_FILE_VERSION: the sentinel IS the partial's +// format version, so a stale-format partial otherwise passes the header check and +// only fails (noisily, via the block-decode error path) when a page is loaded. +// Derived so the pairing can't be forgotten: 0xFE for v28, 0xFD for v29, ... +constexpr uint8_t SECTION_FILE_PARTIAL_VERSION = 0xFE - (SECTION_FILE_VERSION - 28); constexpr uint32_t HEADER_SIZE = sizeof(uint8_t) + sizeof(int) + sizeof(float) + sizeof(bool) + sizeof(uint8_t) + sizeof(uint16_t) + sizeof(uint16_t) + sizeof(uint16_t) + sizeof(bool) + sizeof(bool) + sizeof(uint8_t) + sizeof(bool) + sizeof(uint32_t) + sizeof(uint32_t) + @@ -733,10 +738,10 @@ std::string Section::getTextFromSectionFile() { if (el->getTag() == TAG_PageLine) { const auto& line = static_cast(*el); if (line.getBlock()) { - const auto& words = line.getBlock()->getWords(); - for (const auto& w : words) { + const auto& block = *line.getBlock(); + for (uint16_t i = 0; i < block.wordCount(); i++) { if (!fullText.empty()) fullText += " "; - fullText += w; + fullText += block.wordText(i); } } } diff --git a/lib/Epub/Epub/blocks/BlockStyle.h b/lib/Epub/Epub/blocks/BlockStyle.h index fbc18d42..08bcfd62 100644 --- a/lib/Epub/Epub/blocks/BlockStyle.h +++ b/lib/Epub/Epub/blocks/BlockStyle.h @@ -32,6 +32,11 @@ struct BlockStyle { bool isRtl = false; // true if resolved direction is RTL bool directionDefined = false; // true if direction was explicitly set in CSS/HTML + // Set when this block was created by a
element. Used by startNewTextBlock to inject + // a full line-height gap when the
block stays empty (section-break use case). + // NOT propagated through getCombinedBlockStyle so it can't leak into sibling blocks. + bool fromBrElement = false; + // Combined insets (margin + padding) [[nodiscard]] int16_t leftInset() const { return marginLeft + paddingLeft; } [[nodiscard]] int16_t rightInset() const { return marginRight + paddingRight; } @@ -92,6 +97,9 @@ struct BlockStyle { result.directionDefined = true; } + // fromBrElement is consumed by startNewTextBlock when an empty
block + // is merged with the following paragraph; never propagate it further. + result.fromBrElement = false; return result; } diff --git a/lib/Epub/Epub/blocks/TextBlock.cpp b/lib/Epub/Epub/blocks/TextBlock.cpp index 3d5f920b..c90467d6 100644 --- a/lib/Epub/Epub/blocks/TextBlock.cpp +++ b/lib/Epub/Epub/blocks/TextBlock.cpp @@ -3,19 +3,114 @@ #include #include #include +#include #include #include -void TextBlock::render(const GfxRenderer& renderer, const int fontId, const int x, const int y) const { +size_t TextBlock::arenaSize(const uint16_t wordCount, const bool hasFocus, const uint16_t textBytes) { + // Layout documented in TextBlock.h: 16-bit arrays first, then 8-bit arrays, then text. + size_t size = static_cast(wordCount) * (sizeof(uint16_t) + sizeof(int16_t) + sizeof(uint8_t)); + if (hasFocus) { + size += static_cast(wordCount) * (sizeof(uint16_t) + sizeof(uint8_t)); + } + return size + textBytes; +} + +void TextBlock::bindArenaPointers() { + uint8_t* base = arena.get(); + const size_t wc = numWords; + textOffArr = reinterpret_cast(base); + xposArr = reinterpret_cast(base + wc * 2); + size_t off = wc * 4; + if (focusPresent) { + focusSuffixXArr = reinterpret_cast(base + off); + off += wc * 2; + } + stylesArr = base + off; + off += wc; + if (focusPresent) { + focusBoundaryArr = base + off; + off += wc; + } + textArr = reinterpret_cast(base + off); +} + +TextBlock::TextBlock(const std::vector& words, const std::vector& wordXpos, + const std::vector& wordStyles, const std::vector& focusBoundary, + const std::vector& focusSuffixX, const BlockStyle& blockStyle) + : blockStyle(blockStyle) { // Focus annotations are optional: empty vectors mean no word in this block has a split. // When present, they must be sized in lockstep with words[]. - const bool hasFocus = !wordFocusBoundary.empty(); - if (words.size() != wordXpos.size() || words.size() != wordStyles.size() || - (hasFocus && (words.size() != wordFocusBoundary.size() || words.size() != wordFocusSuffixX.size()))) { - LOG_ERR("TXB", "Render skipped: size mismatch (words=%u, xpos=%u, styles=%u, boundary=%u, suffixX=%u)\n", - (uint32_t)words.size(), (uint32_t)wordXpos.size(), (uint32_t)wordStyles.size(), - (uint32_t)wordFocusBoundary.size(), (uint32_t)wordFocusSuffixX.size()); + const bool hasFocus = !focusBoundary.empty(); + if (words.size() != wordXpos.size() || words.size() != wordStyles.size() || words.size() > 10000 || + (hasFocus && (words.size() != focusBoundary.size() || words.size() != focusSuffixX.size()))) { + LOG_ERR("TXB", "Construction failed: size mismatch (words=%u, xpos=%u, styles=%u, boundary=%u, suffixX=%u)", + static_cast(words.size()), static_cast(wordXpos.size()), + static_cast(wordStyles.size()), static_cast(focusBoundary.size()), + static_cast(focusSuffixX.size())); + isValid = false; + return; + } + + numWords = static_cast(words.size()); + focusPresent = hasFocus; + if (numWords == 0) { + return; // valid empty block, no arena + } + + // Pass 1: total text size, one NUL per word. A line is at most a physical + // row of the page, so uint16_t offsets are ample; reject anything larger. + size_t totalText = 0; + for (const auto& w : words) totalText += w.size() + 1; + if (totalText > UINT16_MAX) { + LOG_ERR("TXB", "Construction failed: text size %u exceeds arena limit", static_cast(totalText)); + numWords = 0; + focusPresent = false; + isValid = false; + return; + } + textBytes = static_cast(totalText); + + const size_t size = arenaSize(numWords, focusPresent, textBytes); + arena = makeUniqueNoThrow(size); + if (!arena) { + LOG_ERR("TXB", "OOM: arena %u bytes", static_cast(size)); + numWords = 0; + textBytes = 0; + focusPresent = false; + isValid = false; + return; + } + bindArenaPointers(); + + // Pass 2: fill. Mutable aliases of the const views bound above. + auto* textOff = const_cast(textOffArr); + auto* xpos = const_cast(xposArr); + auto* styles = const_cast(stylesArr); + auto* text = const_cast(textArr); + uint16_t off = 0; + for (uint16_t i = 0; i < numWords; i++) { + textOff[i] = off; + xpos[i] = wordXpos[i]; + styles[i] = static_cast(wordStyles[i]); + memcpy(text + off, words[i].data(), words[i].size()); + off += static_cast(words[i].size()); + text[off++] = '\0'; + } + if (focusPresent) { + auto* suffixX = const_cast(focusSuffixXArr); + auto* boundary = const_cast(focusBoundaryArr); + for (uint16_t i = 0; i < numWords; i++) { + suffixX[i] = focusSuffixX[i]; + boundary[i] = focusBoundary[i]; + } + } +} + +void TextBlock::render(const GfxRenderer& renderer, const int fontId, const int x, const int y) const { + if (!isValid) { + LOG_ERR("TXB", "Render skipped: invalid block"); return; } @@ -54,12 +149,13 @@ void TextBlock::render(const GfxRenderer& renderer, const int fontId, const int } }; - for (size_t i = 0; i < words.size(); i++) { - const int wordX = wordXpos[i] + x; - const EpdFontFamily::Style currentStyle = wordStyles[i]; - const auto baseDir = static_cast( - BidiUtils::detectParagraphLevel(words[i].c_str(), blockStyle.isRtl ? 1 : 0)); - const uint8_t boundary = hasFocus ? wordFocusBoundary[i] : 0; + for (uint16_t i = 0; i < numWords; i++) { + const char* word = wordText(i); + const int wordX = xposArr[i] + x; + const EpdFontFamily::Style currentStyle = wordStyle(i); + const auto baseDir = + static_cast(BidiUtils::detectParagraphLevel(word, blockStyle.isRtl ? 1 : 0)); + const uint8_t boundary = focusBoundary(i); // SUP/SUB shift the baseline passed to drawText; the glyph is also scaled 50% inside // drawText, so these offsets are chosen relative to the full-size ascender: @@ -82,14 +178,15 @@ void TextBlock::render(const GfxRenderer& renderer, const int fontId, const int static_assert(sizeof(boldBuf) >= MAX_FOCUS_PREFIX_BYTES, "boldBuf too small for max focus prefix (9 codepoints * 4 UTF-8 bytes + null)"); const auto boldStyle = static_cast(currentStyle | EpdFontFamily::BOLD); - const size_t boldLen = std::min({static_cast(boundary), words[i].size(), sizeof(boldBuf) - 1}); - memcpy(boldBuf, words[i].c_str(), boldLen); + const size_t boldLen = + std::min({static_cast(boundary), static_cast(wordTextLen(i)), sizeof(boldBuf) - 1}); + memcpy(boldBuf, word, boldLen); boldBuf[boldLen] = '\0'; renderer.drawText(fontId, wordX, wordY, boldBuf, true, boldStyle, baseDir); - const int suffixX = wordX + wordFocusSuffixX[i]; - renderer.drawText(fontId, suffixX, wordY, words[i].c_str() + boldLen, true, currentStyle, baseDir); + const int suffixX = wordX + focusSuffixXArr[i]; + renderer.drawText(fontId, suffixX, wordY, word + boldLen, true, currentStyle, baseDir); } else { - renderer.drawText(fontId, wordX, wordY, words[i].c_str(), true, currentStyle, baseDir); + renderer.drawText(fontId, wordX, wordY, word, true, currentStyle, baseDir); } if (scanning) { @@ -97,18 +194,17 @@ void TextBlock::render(const GfxRenderer& renderer, const int fontId, const int } if (EpdFontFamily::hasTextDecoration(currentStyle)) { - const std::string& w = words[i]; int lineStartX = wordX; - int lineWidth = renderer.getTextWidth(fontId, w.c_str(), currentStyle, baseDir); + int lineWidth = renderer.getTextWidth(fontId, word, currentStyle, baseDir); if ((currentStyle & (EpdFontFamily::SUP | EpdFontFamily::SUB)) != 0) { lineWidth = (lineWidth + 1) / 2; } // Do not decorate the synthetic em-space used for paragraph indentation. - if (w.size() >= 3 && static_cast(w[0]) == 0xE2 && static_cast(w[1]) == 0x80 && - static_cast(w[2]) == 0x83) { - const char* visibleText = w.c_str() + 3; + if (wordTextLen(i) >= 3 && static_cast(word[0]) == 0xE2 && static_cast(word[1]) == 0x80 && + static_cast(word[2]) == 0x83) { + const char* visibleText = word + 3; lineStartX += renderer.getTextAdvanceX(fontId, "\xe2\x80\x83", currentStyle); lineWidth = renderer.getTextWidth(fontId, visibleText, currentStyle, baseDir); if ((currentStyle & (EpdFontFamily::SUP | EpdFontFamily::SUB)) != 0) { @@ -140,29 +236,23 @@ void TextBlock::render(const GfxRenderer& renderer, const int fontId, const int } bool TextBlock::serialize(HalFile& file) const { - // Focus annotations are optional; vectors are either empty (no splits in this block) - // or sized in lockstep with words[]. - const bool hasFocus = !wordFocusBoundary.empty(); - if (words.size() != wordXpos.size() || words.size() != wordStyles.size() || - (hasFocus && (words.size() != wordFocusBoundary.size() || words.size() != wordFocusSuffixX.size()))) { - LOG_ERR("TXB", "Serialization failed: size mismatch (words=%u, xpos=%u, styles=%u, boundary=%u, suffixX=%u)\n", - static_cast(words.size()), static_cast(wordXpos.size()), - static_cast(wordStyles.size()), static_cast(wordFocusBoundary.size()), - static_cast(wordFocusSuffixX.size())); + if (!isValid) { + LOG_ERR("TXB", "Serialization failed: invalid block"); return false; } - // Word data - serialization::writePod(file, static_cast(words.size())); - for (const auto& w : words) serialization::writeString(file, w); - for (auto x : wordXpos) serialization::writePod(file, x); - for (auto s : wordStyles) serialization::writePod(file, s); - // Focus block: 1-byte presence flag, followed by per-word vectors only when present. - // Saves 3 bytes/word when focus reading is disabled or no word on this line was split. - serialization::writePod(file, static_cast(hasFocus ? 1 : 0)); - if (hasFocus) { - for (auto b : wordFocusBoundary) serialization::writePod(file, b); - for (auto sx : wordFocusSuffixX) serialization::writePod(file, sx); + // Word data: scalars, then the arena verbatim -- its in-memory layout is + // exactly the on-disk layout (see TextBlock.h), so one write covers all + // per-word arrays and the text blob. + serialization::writePod(file, numWords); + serialization::writePod(file, static_cast(focusPresent ? 1 : 0)); + serialization::writePod(file, textBytes); + if (numWords > 0) { + const size_t size = arenaSize(numWords, focusPresent, textBytes); + if (file.write(arena.get(), size) != size) { + LOG_ERR("TXB", "Serialization failed: arena write (%u bytes)", static_cast(size)); + return false; + } } // Style (alignment + margins/padding/indent) @@ -186,41 +276,64 @@ bool TextBlock::serialize(HalFile& file) const { std::unique_ptr TextBlock::deserialize(HalFile& file) { uint16_t wc; - std::vector words; - std::vector wordXpos; - std::vector wordStyles; - std::vector wordFocusBoundary; - std::vector wordFocusSuffixX; - BlockStyle blockStyle; - - // Word count + uint8_t hasFocus; + uint16_t textBytes; serialization::readPod(file, wc); + serialization::readPod(file, hasFocus); + serialization::readPod(file, textBytes); - // Sanity check: prevent allocation of unreasonably large vectors (max 10000 words per block) + // Sanity checks: cap the arena allocation and reject impossible geometry + // (every word carries at least its NUL terminator). if (wc > 10000) { LOG_ERR("TXB", "Deserialization failed: word count %u exceeds maximum", wc); return nullptr; } + if ((wc == 0 && textBytes != 0) || (wc > 0 && textBytes < wc)) { + LOG_ERR("TXB", "Deserialization failed: bad text size %u for %u words", textBytes, wc); + return nullptr; + } - // Word data - words.resize(wc); - wordXpos.resize(wc); - wordStyles.resize(wc); - for (auto& w : words) serialization::readString(file, w); - for (auto& x : wordXpos) serialization::readPod(file, x); - for (auto& s : wordStyles) serialization::readPod(file, s); - // Focus block: presence flag, then vectors only if present. Empty vectors when absent - // signal "no splits in this block" to render() (zero per-word RAM cost). - uint8_t hasFocus; - serialization::readPod(file, hasFocus); - if (hasFocus) { - wordFocusBoundary.resize(wc); - wordFocusSuffixX.resize(wc); - for (auto& b : wordFocusBoundary) serialization::readPod(file, b); - for (auto& sx : wordFocusSuffixX) serialization::readPod(file, sx); + std::unique_ptr block(new (std::nothrow) TextBlock()); + if (!block) { + LOG_ERR("TXB", "OOM: TextBlock"); + return nullptr; + } + block->numWords = wc; + block->textBytes = textBytes; + block->focusPresent = hasFocus != 0; + + if (wc > 0) { + const size_t size = arenaSize(wc, block->focusPresent, textBytes); + block->arena = makeUniqueNoThrow(size); + if (!block->arena) { + LOG_ERR("TXB", "OOM: arena %u bytes", static_cast(size)); + return nullptr; + } + if (file.read(block->arena.get(), size) != size) { + LOG_ERR("TXB", "Deserialization failed: arena read (%u bytes)", static_cast(size)); + return nullptr; + } + block->bindArenaPointers(); + + // Validate offsets before anything dereferences wordText(): offset 0 first, + // strictly increasing, in bounds, and every word NUL-terminated (word i ends + // at the byte before offset i+1; the last word at the last text byte). + const uint16_t* textOff = block->textOffArr; + const char* text = block->textArr; + if (textOff[0] != 0 || text[textBytes - 1] != '\0') { + LOG_ERR("TXB", "Deserialization failed: corrupt text layout"); + return nullptr; + } + for (uint16_t i = 1; i < wc; i++) { + if (textOff[i] <= textOff[i - 1] || textOff[i] >= textBytes || text[textOff[i] - 1] != '\0') { + LOG_ERR("TXB", "Deserialization failed: corrupt word offset %u", i); + return nullptr; + } + } } // Style (alignment + margins/padding/indent) + BlockStyle& blockStyle = block->blockStyle; serialization::readPod(file, blockStyle.alignment); serialization::readPod(file, blockStyle.textAlignDefined); serialization::readPod(file, blockStyle.marginTop); @@ -236,7 +349,5 @@ std::unique_ptr TextBlock::deserialize(HalFile& file) { serialization::readPod(file, blockStyle.isRtl); serialization::readPod(file, blockStyle.directionDefined); - return std::unique_ptr(new TextBlock(std::move(words), std::move(wordXpos), std::move(wordStyles), - std::move(wordFocusBoundary), std::move(wordFocusSuffixX), - blockStyle)); + return block; } diff --git a/lib/Epub/Epub/blocks/TextBlock.h b/lib/Epub/Epub/blocks/TextBlock.h index 5f4bf80e..38f24f53 100644 --- a/lib/Epub/Epub/blocks/TextBlock.h +++ b/lib/Epub/Epub/blocks/TextBlock.h @@ -9,42 +9,83 @@ #include "Block.h" #include "BlockStyle.h" -// Represents a line of text on a page +// Represents a line of text on a page. +// +// All per-word data lives in ONE flat heap allocation (the arena) instead of +// six parallel vectors: a resident page holds ~25-30 of these blocks, and the +// vector-of-string layout cost ~250 throwing allocations per page load, which +// was the primary driver of heap fragmentation on the ESP32-C3. +// +// Arena layout, in order (2-byte alignment holds by construction: all 16-bit +// arrays come first and the arena base is allocator-aligned; RISC-V faults on +// unaligned multi-byte access): +// uint16_t textOff[wordCount] byte offset of word i's text in text[] +// int16_t xpos[wordCount] +// uint16_t focusSuffixX[wordCount] present only when focusPresent +// uint8_t styles[wordCount] +// uint8_t focusBoundary[wordCount] present only when focusPresent +// char text[textBytes] all words back to back, NUL-terminated +// +// Each word is stored NUL-terminated so render() can hand `text + textOff[i]` +// straight to C APIs (drawText) with no std::string materialization. +// +// Focus split semantics (unchanged from the vector layout): boundary N > 0 +// means the first N bytes of word i render bold, the remainder in the base +// style. N is bounded to 9 codepoints (<= 36 UTF-8 bytes) by the clamp in +// ParsedText::addWord. focusSuffixX is the pre-computed pixel offset from the +// word start to the regular suffix. Both arrays are omitted from the arena +// entirely when no word on the line has a split (zero per-word RAM cost when +// focus reading is disabled). class TextBlock final : public Block { private: - std::vector words; - std::vector wordXpos; - std::vector wordStyles; - // Per-word focus boundary: N > 0 means the first N bytes of words[i] are rendered bold, - // the remainder in the base style. 0 means no split (whole word uses wordStyles[i]). - // N encodes the bold PREFIX length only — bounded to 9 codepoints (≤36 UTF-8 bytes) by - // FOCUS_READING_PERCENT's 1..9 clamp in ParsedText::addWord, so it always fits in uint8_t. - // Vector is empty when no focus splits exist anywhere in the block (zero per-word RAM cost - // when focus reading is disabled, or on lines that happen to contain no splittable words). - std::vector wordFocusBoundary; - // Pre-computed pixel offset from word start to the regular suffix, stored when boundary > 0. - // Eliminates getTextAdvanceX from the render path. 0 when boundary == 0. - // Empty in lockstep with wordFocusBoundary. - std::vector wordFocusSuffixX; BlockStyle blockStyle; + uint16_t numWords = 0; + uint16_t textBytes = 0; // total size of the text region, including NULs + bool focusPresent = false; + bool isValid = true; + // The ONLY allocation: makeUniqueNoThrow, so OOM yields an invalid block + // instead of abort() (bare new is not nothrow with -fno-exceptions). + std::unique_ptr arena; + // Typed views into the arena, bound once after the arena is filled. All + // 16-bit bases sit at even offsets, so direct dereference is alignment-safe. + const uint16_t* textOffArr = nullptr; + const int16_t* xposArr = nullptr; + const uint16_t* focusSuffixXArr = nullptr; // null when !focusPresent + const uint8_t* stylesArr = nullptr; + const uint8_t* focusBoundaryArr = nullptr; // null when !focusPresent + const char* textArr = nullptr; + + TextBlock() = default; // deserialize() fills the fields directly + static size_t arenaSize(uint16_t wordCount, bool hasFocus, uint16_t textBytes); + void bindArenaPointers(); public: - explicit TextBlock(std::vector words, std::vector word_xpos, - std::vector word_styles, std::vector focus_boundary, - std::vector focus_suffix_x, const BlockStyle& blockStyle = BlockStyle()) - : words(std::move(words)), - wordXpos(std::move(word_xpos)), - wordStyles(std::move(word_styles)), - wordFocusBoundary(std::move(focus_boundary)), - wordFocusSuffixX(std::move(focus_suffix_x)), - blockStyle(blockStyle) {} + // Flatten-on-construct: copies the layout-time vectors into the arena; the + // vectors die with the caller. On arena OOM the block is empty and valid() + // is false -- callers must check and fail the line instead of using it. + explicit TextBlock(const std::vector& words, const std::vector& wordXpos, + const std::vector& wordStyles, const std::vector& focusBoundary, + const std::vector& focusSuffixX, const BlockStyle& blockStyle = BlockStyle()); ~TextBlock() override = default; + TextBlock(const TextBlock&) = delete; + TextBlock& operator=(const TextBlock&) = delete; + void setBlockStyle(const BlockStyle& blockStyle) { this->blockStyle = blockStyle; } const BlockStyle& getBlockStyle() const { return blockStyle; } - const std::vector& getWords() const { return words; } - bool isEmpty() override { return words.empty(); } - size_t wordCount() const { return words.size(); } - // given a renderer works out where to break the words into lines + bool isEmpty() override { return numWords == 0; } + bool valid() const { return isValid; } + uint16_t wordCount() const { return numWords; } + // NUL-terminated by construction; safe to pass to C APIs directly. + const char* wordText(const uint16_t i) const { return textArr + textOffArr[i]; } + uint16_t wordTextLen(const uint16_t i) const { + const uint16_t end = (i + 1 < numWords) ? textOffArr[i + 1] : textBytes; + return end - textOffArr[i] - 1; // exclude the NUL + } + int16_t wordXpos(const uint16_t i) const { return xposArr[i]; } + EpdFontFamily::Style wordStyle(const uint16_t i) const { return static_cast(stylesArr[i]); } + uint8_t focusBoundary(const uint16_t i) const { return focusPresent ? focusBoundaryArr[i] : 0; } + uint16_t focusSuffixX(const uint16_t i) const { return focusPresent ? focusSuffixXArr[i] : 0; } + void render(const GfxRenderer& renderer, int fontId, int x, int y) const; BlockType getType() override { return TEXT_BLOCK; } bool serialize(HalFile& file) const; diff --git a/lib/Epub/Epub/parsers/ChapterHtmlSlimParser.cpp b/lib/Epub/Epub/parsers/ChapterHtmlSlimParser.cpp index b04159e7..7263de4a 100644 --- a/lib/Epub/Epub/parsers/ChapterHtmlSlimParser.cpp +++ b/lib/Epub/Epub/parsers/ChapterHtmlSlimParser.cpp @@ -239,7 +239,16 @@ void ChapterHtmlSlimParser::startNewTextBlock(const BlockStyle& blockStyle) { // open. Merge those into the new style so the first child in a container inherits // the container's vertical spacing. const auto style = currentTextBlock->getBlockStyle(); - currentTextBlock->setBlockStyle(style.getCombinedBlockStyle(blockStyle, BlockStyle::CombineAxis::Vertical)); + BlockStyle incoming = blockStyle; + if (style.fromBrElement) { + // The empty block was created by a
section separator. Inject a full line of + // blank space before the following paragraph so the scene/section break is visible. + // This only fires when the
block stayed empty (i.e. no inline text was added). + const int16_t lineHeight = static_cast(renderer.getLineHeight(fontId) * lineCompression + 0.5f); + incoming.marginTop = static_cast(incoming.marginTop + lineHeight); + } + + currentTextBlock->setBlockStyle(style.getCombinedBlockStyle(incoming, BlockStyle::CombineAxis::Vertical)); flushPendingAnchor(); return; @@ -855,7 +864,14 @@ void XMLCALL ChapterHtmlSlimParser::startElement(void* userData, const XML_Char* // flush word preceding
to currentTextBlock before calling startNewTextBlock self->flushPartWordBuffer(); } - self->startNewTextBlock(self->blockStyleStack.back().withoutBottom()); + // Tag the new block so startNewTextBlock can inject a full line-height gap if + // the block remains empty (i.e.
is a section separator between paragraphs). + // If the block gets text added before the next block opens it becomes non-empty, + // goes through makePages() normally, and the flag has no effect (inline
case). + BlockStyle brStyle = + self->currentTextBlock ? self->currentTextBlock->getBlockStyle() : self->blockStyleStack.back(); + brStyle.fromBrElement = true; + self->startNewTextBlock(brStyle); } else { self->currentCssStyle = cssStyle; const auto accumulated = self->blockStyleStack.back().getCombinedBlockStyle(userAlignmentBlockStyle, diff --git a/lib/I18n/translations/english.yaml b/lib/I18n/translations/english.yaml index 1fe36220..ae9adfe2 100644 --- a/lib/I18n/translations/english.yaml +++ b/lib/I18n/translations/english.yaml @@ -18,6 +18,7 @@ STR_NO_CHAPTERS: "No chapters" STR_END_OF_BOOK: "End of book" STR_EMPTY_CHAPTER: "Empty chapter" STR_INDEXING: "Indexing" +STR_INDEX_FAILED: "Failed to index - invalid book" STR_MEMORY_ERROR: "Memory error" STR_PAGE_LOAD_ERROR: "Page load error" STR_EMPTY_FILE: "Empty file" diff --git a/lib/I18n/translations/italian.yaml b/lib/I18n/translations/italian.yaml index 70fc18dc..3ddbcb65 100644 --- a/lib/I18n/translations/italian.yaml +++ b/lib/I18n/translations/italian.yaml @@ -383,3 +383,6 @@ STR_DISABLED: "Disattivato" STR_BOOKMARK_OPTION: "Segnalibro" STR_KOSYNC: "KOSync" STR_PWR_BTN_FOOTNOTE_BACK: "Rientro rapido dalle note" +STR_EOB_CONTINUE_WITH: "Continua con" +STR_EOB_HOME: "Home" +STR_INDEX_FAILED: "Indicizzazione fallita - libro non valido" \ No newline at end of file diff --git a/lib/KOReaderSync/KOReaderCredentialStore.cpp b/lib/KOReaderSync/KOReaderCredentialStore.cpp index 3b96eb50..f418f602 100644 --- a/lib/KOReaderSync/KOReaderCredentialStore.cpp +++ b/lib/KOReaderSync/KOReaderCredentialStore.cpp @@ -1,118 +1,43 @@ #include "KOReaderCredentialStore.h" -#include #include #include #include -#include - -#include "KOReaderJsonIO.h" - -// Initialize the static instance -KOReaderCredentialStore KOReaderCredentialStore::instance; namespace { -// File format version (for binary migration) -constexpr uint8_t KOREADER_FILE_VERSION = 1; - -// File paths -constexpr char KOREADER_FILE_BIN[] = "/.crosspoint/koreader.bin"; -constexpr char KOREADER_FILE_JSON[] = "/.crosspoint/koreader.json"; -constexpr char KOREADER_FILE_BAK[] = "/.crosspoint/koreader.bin.bak"; - // Default sync server URL constexpr char DEFAULT_SERVER_URL[] = "https://sync.koreader.rocks:443"; - -// Legacy obfuscation key - "KOReader" in ASCII (only used for binary migration) -constexpr uint8_t LEGACY_OBFUSCATION_KEY[] = {0x4B, 0x4F, 0x52, 0x65, 0x61, 0x64, 0x65, 0x72}; -constexpr size_t LEGACY_KEY_LENGTH = sizeof(LEGACY_OBFUSCATION_KEY); - -void legacyDeobfuscate(std::string& data) { - for (size_t i = 0; i < data.size(); i++) { - data[i] ^= LEGACY_OBFUSCATION_KEY[i % LEGACY_KEY_LENGTH]; - } -} } // namespace -bool KOReaderCredentialStore::saveToFile() const { - Storage.mkdir("/.crosspoint"); - return KOReaderJsonIO::save(*this, KOREADER_FILE_JSON); +void KOReaderCredentialStore::toJson(JsonDocument& doc) const { + doc["username"] = getUsername(); + doc["password_obf"] = obfuscation::obfuscateToBase64(getPassword()); + doc["serverUrl"] = getServerUrl(); + doc["matchMethod"] = static_cast(getMatchMethod()); } -bool KOReaderCredentialStore::loadFromFile() { - // Try JSON first - if (Storage.exists(KOREADER_FILE_JSON)) { - String json = Storage.readFile(KOREADER_FILE_JSON); - if (!json.isEmpty()) { - bool resave = false; - bool result = KOReaderJsonIO::load(*this, json.c_str(), &resave); - if (result && resave) { - saveToFile(); - LOG_DBG("KRS", "Resaved KOReader credentials to update format"); - } - return result; - } - } +bool KOReaderCredentialStore::fromJson(JsonVariantConst doc) { + std::string user = doc["username"] | ""; - // Fall back to binary migration - if (Storage.exists(KOREADER_FILE_BIN)) { - if (loadFromBinaryFile()) { - if (saveToFile()) { - Storage.rename(KOREADER_FILE_BIN, KOREADER_FILE_BAK); - LOG_DBG("KRS", "Migrated koreader.bin to koreader.json"); - return true; - } else { - LOG_ERR("KRS", "Failed to save KOReader credentials during migration"); - return false; - } - } - } + bool needsResave = false; + std::string pass = extractPassword(doc, needsResave); - LOG_DBG("KRS", "No credentials file found"); - return false; -} + setCredentials(user, pass); + setServerUrl(doc["serverUrl"] | ""); -bool KOReaderCredentialStore::loadFromBinaryFile() { - HalFile file; - if (!Storage.openFileForRead("KRS", KOREADER_FILE_BIN, file)) { - return false; - } - - uint8_t version; - serialization::readPod(file, version); - if (version != KOREADER_FILE_VERSION) { - LOG_DBG("KRS", "Unknown file version: %u", version); - return false; - } - - if (file.available()) { - serialization::readString(file, username); + uint8_t method = doc["matchMethod"] | (uint8_t)0; + if (method <= static_cast(DocumentMatchMethod::BINARY)) { + setMatchMethod(static_cast(method)); } else { - username.clear(); + LOG_DBG("KRS", "Invalid matchMethod %u in JSON, resetting to FILENAME", method); + setMatchMethod(DocumentMatchMethod::FILENAME); } - if (file.available()) { - serialization::readString(file, password); - legacyDeobfuscate(password); - } else { - password.clear(); + if (needsResave) { + LOG_DBG("KRS", "Resaved KOReader credentials to update format"); + saveToFile(); } - if (file.available()) { - serialization::readString(file, serverUrl); - } else { - serverUrl.clear(); - } - - if (file.available()) { - uint8_t method; - serialization::readPod(file, method); - matchMethod = static_cast(method); - } else { - matchMethod = DocumentMatchMethod::FILENAME; - } - - LOG_DBG("KRS", "Loaded KOReader credentials from binary for user: %s", username.c_str()); return true; } diff --git a/lib/KOReaderSync/KOReaderCredentialStore.h b/lib/KOReaderSync/KOReaderCredentialStore.h index c2ea515d..1ac437a1 100644 --- a/lib/KOReaderSync/KOReaderCredentialStore.h +++ b/lib/KOReaderSync/KOReaderCredentialStore.h @@ -1,4 +1,7 @@ #pragma once +#include +#include + #include #include @@ -14,9 +17,9 @@ enum class DocumentMatchMethod : uint8_t { * and base64-encoded before writing to JSON (not cryptographically secure, * but prevents casual reading and ties credentials to the specific device). */ -class KOReaderCredentialStore { + +class KOReaderCredentialStore : public PersistableStore { private: - static KOReaderCredentialStore instance; std::string username; std::string password; std::string serverUrl; // Custom sync server URL (empty = default) @@ -24,20 +27,14 @@ class KOReaderCredentialStore { // Private constructor for singleton KOReaderCredentialStore() = default; + ~KOReaderCredentialStore() = default; - bool loadFromBinaryFile(); + friend class PersistableStore; public: - // Delete copy constructor and assignment - KOReaderCredentialStore(const KOReaderCredentialStore&) = delete; - KOReaderCredentialStore& operator=(const KOReaderCredentialStore&) = delete; - - // Get singleton instance - static KOReaderCredentialStore& getInstance() { return instance; } - - // Save/load from SD card - bool saveToFile() const; - bool loadFromFile(); + static const char* getFilePath() { return "/.crosspoint/koreader.json"; } + void toJson(JsonDocument& doc) const; + bool fromJson(JsonVariantConst doc); // Credential management void setCredentials(const std::string& user, const std::string& pass); diff --git a/lib/KOReaderSync/KOReaderJsonIO.cpp b/lib/KOReaderSync/KOReaderJsonIO.cpp deleted file mode 100644 index 05bfc119..00000000 --- a/lib/KOReaderSync/KOReaderJsonIO.cpp +++ /dev/null @@ -1,51 +0,0 @@ -#include "KOReaderJsonIO.h" - -#include -#include -#include -#include - -#include "KOReaderCredentialStore.h" - -namespace KOReaderJsonIO { - -bool save(const KOReaderCredentialStore& store, const char* path) { - JsonDocument doc; - doc["username"] = store.getUsername(); - doc["password_obf"] = obfuscation::obfuscateToBase64(store.getPassword()); - doc["serverUrl"] = store.getServerUrl(); - doc["matchMethod"] = static_cast(store.getMatchMethod()); - - String json; - serializeJson(doc, json); - return Storage.writeFile(path, json); -} - -bool load(KOReaderCredentialStore& store, const char* json, bool* needsResave) { - if (needsResave) *needsResave = false; - JsonDocument doc; - auto error = deserializeJson(doc, json); - if (error) { - LOG_ERR("KRS", "JSON parse error: %s", error.c_str()); - return false; - } - - std::string user = doc["username"] | std::string(""); - - bool ok = false; - std::string pass = obfuscation::deobfuscateFromBase64(doc["password_obf"] | "", &ok); - if (!ok || pass.empty()) { - pass = doc["password"] | std::string(""); - if (!pass.empty() && needsResave) *needsResave = true; - } - - store.setCredentials(user, pass); - store.setServerUrl(doc["serverUrl"] | std::string("")); - - uint8_t method = doc["matchMethod"] | (uint8_t)0; - store.setMatchMethod(static_cast(method)); - - return true; -} - -} // namespace KOReaderJsonIO diff --git a/lib/KOReaderSync/KOReaderJsonIO.h b/lib/KOReaderSync/KOReaderJsonIO.h deleted file mode 100644 index c8d8eb81..00000000 --- a/lib/KOReaderSync/KOReaderJsonIO.h +++ /dev/null @@ -1,8 +0,0 @@ -#pragma once - -class KOReaderCredentialStore; - -namespace KOReaderJsonIO { -bool save(const KOReaderCredentialStore& store, const char* path); -bool load(KOReaderCredentialStore& store, const char* json, bool* needsResave); -} // namespace KOReaderJsonIO diff --git a/lib/Serialization/PersistableStore.cpp b/lib/Serialization/PersistableStore.cpp new file mode 100644 index 00000000..b9a2514f --- /dev/null +++ b/lib/Serialization/PersistableStore.cpp @@ -0,0 +1,45 @@ +#include "PersistableStore.h" + +#include +#include +#include + +bool PersistableStoreBase::writeDocToFile(const char* path, const JsonDocument& doc) { + Storage.mkdir("/.crosspoint"); + String json; + serializeJson(doc, json); + if (!Storage.writeFile(path, json)) { + LOG_ERR("PERSIST", "Failed to write %s", path); + return false; + } + return true; +} + +bool PersistableStoreBase::readDocFromFile(const char* path, JsonDocument& doc) { + if (!Storage.exists(path)) { + return false; // Expected on first boot — not an error. + } + String json = Storage.readFile(path); + if (json.isEmpty()) { + LOG_ERR("PERSIST", "Failed to read %s (empty)", path); + return false; + } + auto error = deserializeJson(doc, json); + if (error) { + LOG_ERR("PERSIST", "JSON parse error in %s: %s", path, error.c_str()); + return false; + } + return true; +} + +std::string PersistableStoreBase::extractPassword(JsonVariantConst doc, bool& needsResave) { + bool ok = false; + std::string pass = obfuscation::deobfuscateFromBase64(doc["password_obf"] | "", &ok); + if (!ok) { + // Deobfuscation failed — fall back to legacy plaintext password. + pass = doc["password"] | ""; + if (!pass.empty()) needsResave = true; + } + // A successfully decoded empty string is a legitimate value; preserve as-is. + return pass; +} diff --git a/lib/Serialization/PersistableStore.h b/lib/Serialization/PersistableStore.h new file mode 100644 index 00000000..14c6020c --- /dev/null +++ b/lib/Serialization/PersistableStore.h @@ -0,0 +1,82 @@ +#pragma once + +#include +#include + +#include + +/** + * @brief Non-template core of PersistableStore. + * + * All ArduinoJson parse/serialize machinery is instantiated once here (in + * PersistableStore.cpp) instead of in every store's translation unit. GCC + * emits the JSON serializer/parser templates as local .isra clones per TU + * (~0.5KB each), so keeping serializeJson/deserializeJson out of the stores + * is what makes the abstraction flash-neutral. + */ +class PersistableStoreBase { + protected: + PersistableStoreBase() = default; + ~PersistableStoreBase() = default; + + // Serializes doc and writes it to path (ensures /.crosspoint exists). Logs on failure. + static bool writeDocToFile(const char* path, const JsonDocument& doc); + + // Reads path and parses it into doc. Returns false silently when the file + // does not exist (expected on first boot); logs on read/parse failure. + static bool readDocFromFile(const char* path, JsonDocument& doc); + + /** + * Helper function for extracting an obfuscated password from a JSON value. + * Accepts JsonVariantConst so callers can pass either a whole JsonDocument + * or a JsonObject element (e.g. inside an array iteration). + * If the decoded password requires a resave (e.g. from plaintext fallback), `needsResave` is set to true. + */ + static std::string extractPassword(JsonVariantConst doc, bool& needsResave); +}; + +/** + * @brief Base class for persistable singletons using CRTP. + * + * Derived classes must provide: + * - A private default constructor + * - friend class PersistableStore; + * - static const char* getFilePath(); + * - void toJson(JsonDocument& doc) const; + * - bool fromJson(JsonVariantConst doc); + * + * Note for implementers: read string values as `const char*` (e.g. + * `obj["name"] | ""`), never as `| std::string("")` — ArduinoJson's + * std::string converter drags a per-TU copy of the whole JSON serializer + * into flash via its serializeJson fallback. + */ +template +class PersistableStore : public PersistableStoreBase { + protected: + PersistableStore() = default; + ~PersistableStore() = default; + + public: + // Delete copy constructor and assignment + PersistableStore(const PersistableStore&) = delete; + PersistableStore& operator=(const PersistableStore&) = delete; + + static T& getInstance() { + static T instance; + return instance; + } + + bool saveToFile() const { + JsonDocument doc; + static_cast(this)->toJson(doc); + return writeDocToFile(T::getFilePath(), doc); + } + + bool loadFromFile() { + JsonDocument doc; + if (!readDocFromFile(T::getFilePath(), doc)) { + return false; + } + return static_cast(this)->fromJson(doc.as()); + } +}; diff --git a/scripts/generate_br_section_break_epub.py b/scripts/generate_br_section_break_epub.py new file mode 100644 index 00000000..a22d4462 --- /dev/null +++ b/scripts/generate_br_section_break_epub.py @@ -0,0 +1,210 @@ +#!/usr/bin/env python3 +""" +Generate a test EPUB for
section-break rendering. + +Tests that a bare
element between paragraphs produces a visible blank-line +gap (section separator), while a
inside a paragraph only produces a line +break with no extra spacing. + +Cases covered: + 1. Standalone
between paragraphs (section break — must show gap). + 2.
with a CSS class (calibre-style section break). + 3. Multiple consecutive
elements (each adds one line of spacing). + 4. Inline
inside a

(line break only — no extra gap). + 5.
at start of chapter (no gap before first paragraph). + 6.
following a heading. + +Visual verification instructions are embedded as the first paragraph of each +chapter so a human tester can confirm the expected result on device. +""" + +import os +import zipfile +from pathlib import Path + +OUTPUT_DIR = Path(__file__).parent.parent / "test" / "epubs" +OUTPUT_PATH = OUTPUT_DIR / "test_br_section_break.epub" + +FILLER = ( + "Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod " + "tempor incididunt ut labore et dolore magna aliqua." +) + +CSS = """\ +body { margin: 0; padding: 0; } +p { margin-top: 1pt; margin-bottom: 0; text-indent: 1em; text-align: justify; } +h1 { text-align: center; margin-top: 0.5em; margin-bottom: 0.5em; } +h2 { text-align: center; margin-top: 0.5em; margin-bottom: 0.5em; } +.section-br { display: block; } +""" + +def xhtml(title, body): + return f"""\ + + + + + {title} + + + +{body} + +""" + + +# --------------------------------------------------------------------------- +# Chapter 1 — standalone
between paragraphs +# --------------------------------------------------------------------------- +ch1 = xhtml("Ch1: Standalone br", f""" +

Ch 1: Standalone <br> Section Break

+

PASS: A visible blank-line gap should appear between the two sections below.

+

{FILLER}

+
+

{FILLER}

+

PASS: The gap above should be roughly one line tall (same as a blank line).

+""") + +# --------------------------------------------------------------------------- +# Chapter 2 —
CSS-classed section break (calibre style) +# --------------------------------------------------------------------------- +ch2 = xhtml("Ch2: Classed br", f""" +

Ch 2: <br class="section-br"/>

+

PASS: A blank-line gap should appear between the two sections below, identical +to Ch 1, even though the <br> carries a CSS class.

+

{FILLER}

+
+

{FILLER}

+""") + +# --------------------------------------------------------------------------- +# Chapter 3 — multiple consecutive
elements +# --------------------------------------------------------------------------- +ch3 = xhtml("Ch3: Multiple br", f""" +

Ch 3: Multiple Consecutive <br> Elements

+

PASS: Two blank lines should appear between the sections (one per <br>).

+

{FILLER}

+
+
+

{FILLER}

+

PASS: Three blank lines should appear below.

+

{FILLER}

+
+
+
+

{FILLER}

+""") + +# --------------------------------------------------------------------------- +# Chapter 4 — inline
inside a paragraph (line break, NOT a gap) +# --------------------------------------------------------------------------- +ch4 = xhtml("Ch4: Inline br", """ +

Ch 4: Inline <br> Inside a Paragraph

+

PASS: The two lines below should be adjacent with NO extra gap between them. +The <br> is inside the paragraph and must only break the line.

+

First line of the paragraph.
Second line of the paragraph — directly below, no gap.

+

PASS: Above should look like two closely-spaced lines, not like two paragraphs +separated by a blank line.

+""") + +# --------------------------------------------------------------------------- +# Chapter 5 —
following a heading +# --------------------------------------------------------------------------- +ch5 = xhtml("Ch5: br after heading", f""" +

Ch 5: <br> After a Heading

+
+

PASS: There should be a blank-line gap between the heading above and this paragraph.

+

{FILLER}

+

Section heading

+
+

PASS: There should be a blank-line gap between the section heading and this paragraph.

+""") + +# --------------------------------------------------------------------------- +# Chapter 6 —
at very start of chapter (no spurious leading gap) +# --------------------------------------------------------------------------- +ch6 = xhtml("Ch6: br at chapter start", f"""
+

Ch 6: <br> at Chapter Start

+

PASS: This heading should appear near the top of the page with no large blank +area above it despite the <br> being the very first element.

+

{FILLER}

+""") + +CHAPTERS = [ + ("ch1", "chapter1.xhtml", "Chapter 1: Standalone br", ch1), + ("ch2", "chapter2.xhtml", "Chapter 2: Classed br", ch2), + ("ch3", "chapter3.xhtml", "Chapter 3: Multiple br", ch3), + ("ch4", "chapter4.xhtml", "Chapter 4: Inline br", ch4), + ("ch5", "chapter5.xhtml", "Chapter 5: br after heading", ch5), + ("ch6", "chapter6.xhtml", "Chapter 6: br at start", ch6), +] + +def build_epub(path): + os.makedirs(os.path.dirname(path), exist_ok=True) + with zipfile.ZipFile(path, "w", zipfile.ZIP_DEFLATED) as epub: + # mimetype must be first and uncompressed + epub.writestr("mimetype", "application/epub+zip", + compress_type=zipfile.ZIP_STORED) + + epub.writestr("META-INF/container.xml", """\ + + + + + +""") + + epub.writestr("OEBPS/styles/test.css", CSS) + + manifest_items = [] + spine_items = [] + nav_items = [] + + for (chid, chfile, chtitle, chcontent) in CHAPTERS: + epub.writestr(f"OEBPS/{chfile}", chcontent) + manifest_items.append( + f' ') + spine_items.append(f' ') + nav_items.append(f'
  • {chtitle}
  • ') + + manifest_items.append( + ' ') + + content_opf = f"""\ + + + + test-epub-br-section-break + Test: br Section Break + en + + +{chr(10).join(manifest_items)} + + +{chr(10).join(spine_items)} + +""" + epub.writestr("OEBPS/content.opf", content_opf) + + nav_xhtml = f"""\ + + +Table of Contents + + + +""" + epub.writestr("OEBPS/nav.xhtml", nav_xhtml) + + print(f"Generated: {path}") + + +if __name__ == "__main__": + build_epub(OUTPUT_PATH) diff --git a/src/CrossPointSettings.cpp b/src/CrossPointSettings.cpp index 3b5b5dcb..f5501f13 100644 --- a/src/CrossPointSettings.cpp +++ b/src/CrossPointSettings.cpp @@ -24,7 +24,7 @@ void readAndValidate(HalFile& file, uint8_t& member, const uint8_t maxValue) { } namespace { -constexpr uint8_t SETTINGS_FILE_VERSION = 1; +constexpr uint8_t SETTINGS_FILE_VERSION = 2; constexpr char SETTINGS_FILE_BIN[] = "/.crosspoint/settings.bin"; constexpr char SETTINGS_FILE_JSON[] = "/.crosspoint/settings.json"; constexpr char SETTINGS_FILE_BAK[] = "/.crosspoint/settings.bin.bak"; @@ -229,13 +229,6 @@ bool CrossPointSettings::loadFromBinaryFile() { if (++settingsRead >= fileSettingsCount) break; readAndValidate(inputFile, sleepScreenCoverMode, SLEEP_SCREEN_COVER_MODE_COUNT); if (++settingsRead >= fileSettingsCount) break; - { - std::string urlStr; - serialization::readString(inputFile, urlStr); - strncpy(opdsServerUrl, urlStr.c_str(), sizeof(opdsServerUrl) - 1); - opdsServerUrl[sizeof(opdsServerUrl) - 1] = '\0'; - } - if (++settingsRead >= fileSettingsCount) break; serialization::readPod(inputFile, textAntiAliasing); if (++settingsRead >= fileSettingsCount) break; readAndValidate(inputFile, hideBatteryPercentage, HIDE_BATTERY_PERCENTAGE_COUNT); @@ -244,20 +237,6 @@ bool CrossPointSettings::loadFromBinaryFile() { if (++settingsRead >= fileSettingsCount) break; serialization::readPod(inputFile, hyphenationEnabled); if (++settingsRead >= fileSettingsCount) break; - { - std::string usernameStr; - serialization::readString(inputFile, usernameStr); - strncpy(opdsUsername, usernameStr.c_str(), sizeof(opdsUsername) - 1); - opdsUsername[sizeof(opdsUsername) - 1] = '\0'; - } - if (++settingsRead >= fileSettingsCount) break; - { - std::string passwordStr; - serialization::readString(inputFile, passwordStr); - strncpy(opdsPassword, passwordStr.c_str(), sizeof(opdsPassword) - 1); - opdsPassword[sizeof(opdsPassword) - 1] = '\0'; - } - if (++settingsRead >= fileSettingsCount) break; readAndValidate(inputFile, sleepScreenCoverFilter, SLEEP_SCREEN_COVER_FILTER_COUNT); if (++settingsRead >= fileSettingsCount) break; serialization::readPod(inputFile, uiTheme); diff --git a/src/CrossPointSettings.h b/src/CrossPointSettings.h index e21fc49b..5f4460f5 100644 --- a/src/CrossPointSettings.h +++ b/src/CrossPointSettings.h @@ -254,10 +254,6 @@ class CrossPointSettings { // Reader screen margin settings uint8_t screenMargin = 5; - // OPDS browser settings - char opdsServerUrl[128] = ""; - char opdsUsername[64] = ""; - char opdsPassword[64] = ""; // Hide battery percentage uint8_t hideBatteryPercentage = HIDE_NEVER; // Long-press page turn button behavior diff --git a/src/JsonSettingsIO.cpp b/src/JsonSettingsIO.cpp index c92f5c03..da6e0b1c 100644 --- a/src/JsonSettingsIO.cpp +++ b/src/JsonSettingsIO.cpp @@ -295,150 +295,6 @@ bool JsonSettingsIO::loadSettings(CrossPointSettings& s, const char* json, bool* return true; } -// ---- WifiCredentialStore ---- - -bool JsonSettingsIO::saveWifi(const WifiCredentialStore& store, const char* path) { - JsonDocument doc; - doc["lastConnectedSsid"] = store.getLastConnectedSsid(); - - JsonArray arr = doc["credentials"].to(); - for (const auto& cred : store.getCredentials()) { - JsonObject obj = arr.add(); - obj["ssid"] = cred.ssid; - obj["password_obf"] = obfuscation::obfuscateToBase64(cred.password); - } - - String json; - serializeJson(doc, json); - return Storage.writeFile(path, json); -} - -bool JsonSettingsIO::loadWifi(WifiCredentialStore& store, const char* json, bool* needsResave) { - if (needsResave) *needsResave = false; - JsonDocument doc; - auto error = deserializeJson(doc, json); - if (error) { - LOG_ERR("WCS", "JSON parse error: %s", error.c_str()); - return false; - } - - store.lastConnectedSsid = doc["lastConnectedSsid"] | std::string(""); - - store.credentials.clear(); - JsonArray arr = doc["credentials"].as(); - for (JsonObject obj : arr) { - if (store.credentials.size() >= store.MAX_NETWORKS) break; - WifiCredential cred; - cred.ssid = obj["ssid"] | std::string(""); - bool ok = false; - cred.password = obfuscation::deobfuscateFromBase64(obj["password_obf"] | "", &ok); - if (!ok || cred.password.empty()) { - cred.password = obj["password"] | std::string(""); - if (!cred.password.empty() && needsResave) *needsResave = true; - } - store.credentials.push_back(cred); - } - - LOG_DBG("WCS", "Loaded %zu WiFi credentials from file", store.credentials.size()); - return true; -} - -// ---- RecentBooksStore ---- - -bool JsonSettingsIO::saveRecentBooks(const RecentBooksStore& store, const char* path) { - JsonDocument doc; - JsonArray arr = doc["books"].to(); - for (const auto& book : store.getBooks()) { - JsonObject obj = arr.add(); - obj["path"] = book.path; - obj["title"] = book.title; - obj["author"] = book.author; - obj["coverBmpPath"] = book.coverBmpPath; - } - - String json; - serializeJson(doc, json); - return Storage.writeFile(path, json); -} - -bool JsonSettingsIO::loadRecentBooks(RecentBooksStore& store, const char* json) { - JsonDocument doc; - auto error = deserializeJson(doc, json); - if (error) { - LOG_ERR("RBS", "JSON parse error: %s", error.c_str()); - return false; - } - - store.recentBooks.clear(); - JsonArray arr = doc["books"].as(); - store.recentBooks.reserve(std::min(arr.size(), (size_t)10)); - for (JsonObject obj : arr) { - if (store.getCount() >= 10) break; - RecentBook book; - book.path = obj["path"] | std::string(""); - book.title = obj["title"] | std::string(""); - book.author = obj["author"] | std::string(""); - book.coverBmpPath = obj["coverBmpPath"] | std::string(""); - store.recentBooks.push_back(book); - } - - LOG_DBG("RBS", "Recent books loaded from file (%d entries)", store.getCount()); - return true; -} - -// ---- OpdsServerStore ---- -// Follows the same save/load pattern as WifiCredentialStore above. -// Passwords are XOR-obfuscated with the device MAC and base64-encoded ("password_obf" key). - -bool JsonSettingsIO::saveOpds(const OpdsServerStore& store, const char* path) { - JsonDocument doc; - - JsonArray arr = doc["servers"].to(); - for (const auto& server : store.getServers()) { - JsonObject obj = arr.add(); - obj["name"] = server.name; - obj["url"] = server.url; - obj["username"] = server.username; - obj["password_obf"] = obfuscation::obfuscateToBase64(server.password); - } - - String json; - serializeJson(doc, json); - return Storage.writeFile(path, json); -} - -bool JsonSettingsIO::loadOpds(OpdsServerStore& store, const char* json, bool* needsResave) { - if (needsResave) *needsResave = false; - JsonDocument doc; - auto error = deserializeJson(doc, json); - if (error) { - LOG_ERR("OPS", "JSON parse error: %s", error.c_str()); - return false; - } - - store.servers.clear(); - JsonArray arr = doc["servers"].as(); - for (JsonObject obj : arr) { - if (store.servers.size() >= OpdsServerStore::MAX_SERVERS) break; - OpdsServer server; - server.name = obj["name"] | std::string(""); - server.url = obj["url"] | std::string(""); - server.username = obj["username"] | std::string(""); - // Try the obfuscated key first; fall back to plaintext "password" for - // files written before obfuscation was added (or hand-edited JSON). - bool ok = false; - server.password = obfuscation::deobfuscateFromBase64(obj["password_obf"] | "", &ok); - if (!ok || server.password.empty()) { - server.password = obj["password"] | std::string(""); - if (!server.password.empty() && needsResave) *needsResave = true; - } - store.servers.push_back(std::move(server)); - } - - LOG_DBG("OPS", "Loaded %zu OPDS servers from file", store.servers.size()); - return true; -} - // ---- Bookmarks ---- bool JsonSettingsIO::saveBookmarks(const std::vector& bookmarks, const char* path) { diff --git a/src/JsonSettingsIO.h b/src/JsonSettingsIO.h index 45888b67..487d204a 100644 --- a/src/JsonSettingsIO.h +++ b/src/JsonSettingsIO.h @@ -19,18 +19,6 @@ bool loadSettings(CrossPointSettings& s, const char* json, bool* needsResave = n bool saveState(const CrossPointState& s, const char* path); bool loadState(CrossPointState& s, const char* json); -// WifiCredentialStore -bool saveWifi(const WifiCredentialStore& store, const char* path); -bool loadWifi(WifiCredentialStore& store, const char* json, bool* needsResave = nullptr); - -// RecentBooksStore -bool saveRecentBooks(const RecentBooksStore& store, const char* path); -bool loadRecentBooks(RecentBooksStore& store, const char* json); - -// OpdsServerStore -bool saveOpds(const OpdsServerStore& store, const char* path); -bool loadOpds(OpdsServerStore& store, const char* json, bool* needsResave = nullptr); - // Bookmarks bool saveBookmarks(const std::vector& bookmarks, const char* path); bool loadBookmarks(std::vector& bookmarks, const char* json); diff --git a/src/OpdsServerStore.cpp b/src/OpdsServerStore.cpp index b2151682..14ae9998 100644 --- a/src/OpdsServerStore.cpp +++ b/src/OpdsServerStore.cpp @@ -1,74 +1,48 @@ #include "OpdsServerStore.h" -#include -#include #include +#include +#include #include -#include "CrossPointSettings.h" - -OpdsServerStore OpdsServerStore::instance; - -namespace { -constexpr char OPDS_FILE_JSON[] = "/.crosspoint/opds.json"; -} // namespace - -bool OpdsServerStore::saveToFile() const { - Storage.mkdir("/.crosspoint"); - return JsonSettingsIO::saveOpds(*this, OPDS_FILE_JSON); +void OpdsServerStore::toJson(JsonDocument& doc) const { + JsonArray arr = doc["servers"].to(); + for (const auto& server : servers) { + JsonObject obj = arr.add(); + obj["name"] = server.name; + obj["url"] = server.url; + obj["username"] = server.username; + obj["password_obf"] = obfuscation::obfuscateToBase64(server.password); + } } -bool OpdsServerStore::loadFromFile() { - if (Storage.exists(OPDS_FILE_JSON)) { - String json = Storage.readFile(OPDS_FILE_JSON); - if (!json.isEmpty()) { - // resave flag is set when passwords were stored in plaintext and need re-obfuscation - bool resave = false; - bool result = JsonSettingsIO::loadOpds(*this, json.c_str(), &resave); - if (result && resave) { - LOG_DBG("OPS", "Resaving JSON with obfuscated passwords"); - saveToFile(); - } - return result; - } - } - - // No opds.json found — attempt one-time migration from the legacy single-server - // fields in CrossPointSettings (opdsServerUrl/opdsUsername/opdsPassword). - if (migrateFromSettings()) { - LOG_DBG("OPS", "Migrated legacy OPDS settings"); - return true; - } - - return false; -} - -bool OpdsServerStore::migrateFromSettings() { - if (strlen(SETTINGS.opdsServerUrl) == 0) { - return false; - } - - OpdsServer server; - server.name = "OPDS Server"; - server.url = SETTINGS.opdsServerUrl; - server.username = SETTINGS.opdsUsername; - server.password = SETTINGS.opdsPassword; - servers.push_back(std::move(server)); - - if (saveToFile()) { - // Clear legacy fields so migration won't run again on next boot - SETTINGS.opdsServerUrl[0] = '\0'; - SETTINGS.opdsUsername[0] = '\0'; - SETTINGS.opdsPassword[0] = '\0'; - SETTINGS.saveToFile(); - LOG_DBG("OPS", "Migrated single-server OPDS config to opds.json"); - return true; - } - - // Save failed — roll back in-memory state so we don't have a partial migration +bool OpdsServerStore::fromJson(JsonVariantConst doc) { + // Tolerate a missing/invalid 'servers' key (treat as empty list); only a + // JSON parse error is fatal. A null JsonArray iterates zero times. servers.clear(); - return false; + JsonArrayConst arr = doc["servers"].as(); + servers.reserve(std::min(arr.size(), MAX_SERVERS)); + bool needsResave = false; + + for (JsonObjectConst obj : arr) { + if (servers.size() >= OpdsServerStore::MAX_SERVERS) break; + OpdsServer server; + server.name = obj["name"] | ""; + server.url = obj["url"] | ""; + server.username = obj["username"] | ""; + server.password = extractPassword(obj, needsResave); + servers.push_back(std::move(server)); + } + + LOG_DBG("OPS", "Loaded %zu OPDS servers from file", servers.size()); + + if (needsResave) { + LOG_DBG("OPS", "Resaving JSON with obfuscated passwords"); + saveToFile(); + } + + return true; } bool OpdsServerStore::addServer(const OpdsServer& server) { diff --git a/src/OpdsServerStore.h b/src/OpdsServerStore.h index 87571f65..3845b1f3 100644 --- a/src/OpdsServerStore.h +++ b/src/OpdsServerStore.h @@ -1,4 +1,7 @@ #pragma once +#include +#include + #include #include @@ -9,37 +12,25 @@ struct OpdsServer { std::string password; // Plaintext in memory; obfuscated with hardware key on disk }; -class OpdsServerStore; -namespace JsonSettingsIO { -bool saveOpds(const OpdsServerStore& store, const char* path); -bool loadOpds(OpdsServerStore& store, const char* json, bool* needsResave); -} // namespace JsonSettingsIO - /** * Singleton class for storing OPDS server configurations on the SD card. * Passwords are XOR-obfuscated with the device's unique hardware MAC address * and base64-encoded before writing to JSON. */ -class OpdsServerStore { +class OpdsServerStore : public PersistableStore { private: - static OpdsServerStore instance; std::vector servers; static constexpr size_t MAX_SERVERS = 8; OpdsServerStore() = default; - friend bool JsonSettingsIO::saveOpds(const OpdsServerStore&, const char*); - friend bool JsonSettingsIO::loadOpds(OpdsServerStore&, const char*, bool*); + friend class PersistableStore; public: - OpdsServerStore(const OpdsServerStore&) = delete; - OpdsServerStore& operator=(const OpdsServerStore&) = delete; - - static OpdsServerStore& getInstance() { return instance; } - - bool saveToFile() const; - bool loadFromFile(); + static const char* getFilePath() { return "/.crosspoint/opds.json"; } + void toJson(JsonDocument& doc) const; + bool fromJson(JsonVariantConst doc); bool addServer(const OpdsServer& server); bool updateServer(size_t index, const OpdsServer& server); @@ -49,12 +40,6 @@ class OpdsServerStore { const OpdsServer* getServer(size_t index) const; size_t getCount() const { return servers.size(); } bool hasServers() const { return !servers.empty(); } - - /** - * Migrate from legacy single-server settings in CrossPointSettings. - * Called once during first load if no opds.json exists. - */ - bool migrateFromSettings(); }; #define OPDS_STORE OpdsServerStore::getInstance() diff --git a/src/RecentBooksStore.cpp b/src/RecentBooksStore.cpp index af6e9d4c..897e8a54 100644 --- a/src/RecentBooksStore.cpp +++ b/src/RecentBooksStore.cpp @@ -3,23 +3,42 @@ #include #include #include -#include #include -#include #include #include #include -namespace { -constexpr uint8_t RECENT_BOOKS_FILE_VERSION = 3; -constexpr char RECENT_BOOKS_FILE_BIN[] = "/.crosspoint/recent.bin"; -constexpr char RECENT_BOOKS_FILE_JSON[] = "/.crosspoint/recent.json"; -constexpr char RECENT_BOOKS_FILE_BAK[] = "/.crosspoint/recent.bin.bak"; -constexpr int MAX_RECENT_BOOKS = 10; -} // namespace +void RecentBooksStore::toJson(JsonDocument& doc) const { + JsonArray arr = doc["books"].to(); + for (const auto& book : recentBooks) { + JsonObject obj = arr.add(); + obj["path"] = book.path; + obj["title"] = book.title; + obj["author"] = book.author; + obj["coverBmpPath"] = book.coverBmpPath; + } +} -RecentBooksStore RecentBooksStore::instance; +bool RecentBooksStore::fromJson(JsonVariantConst doc) { + // Tolerate a missing/invalid 'books' key (treat as empty list); only a + // JSON parse error is fatal. A null JsonArray iterates zero times. + recentBooks.clear(); + JsonArrayConst arr = doc["books"].as(); + recentBooks.reserve(std::min(arr.size(), static_cast(MAX_RECENT_BOOKS))); + for (JsonObjectConst obj : arr) { + if (getCount() >= MAX_RECENT_BOOKS) break; + RecentBook book; + book.path = obj["path"] | ""; + book.title = obj["title"] | ""; + book.author = obj["author"] | ""; + book.coverBmpPath = obj["coverBmpPath"] | ""; + recentBooks.push_back(book); + } + + LOG_DBG("RBS", "Recent books loaded from file (%d entries)", getCount()); + return true; +} void RecentBooksStore::addBook(const std::string& path, const std::string& title, const std::string& author, const std::string& coverBmpPath) { @@ -92,11 +111,6 @@ bool RecentBooksStore::pruneMissing() { return recentBooks.size() != before; } -bool RecentBooksStore::saveToFile() const { - Storage.mkdir("/.crosspoint"); - return JsonSettingsIO::saveRecentBooks(*this, RECENT_BOOKS_FILE_JSON); -} - RecentBook RecentBooksStore::getDataFromBook(std::string path) const { std::string lastBookFileName = ""; const size_t lastSlash = path.find_last_of('/'); @@ -124,95 +138,3 @@ RecentBook RecentBooksStore::getDataFromBook(std::string path) const { } return RecentBook{path, "", "", ""}; } - -bool RecentBooksStore::loadFromFile() { - // Try JSON first - if (Storage.exists(RECENT_BOOKS_FILE_JSON)) { - String json = Storage.readFile(RECENT_BOOKS_FILE_JSON); - if (!json.isEmpty()) { - return JsonSettingsIO::loadRecentBooks(*this, json.c_str()); - } - } - - // Fall back to binary migration - if (Storage.exists(RECENT_BOOKS_FILE_BIN)) { - if (loadFromBinaryFile()) { - saveToFile(); - Storage.rename(RECENT_BOOKS_FILE_BIN, RECENT_BOOKS_FILE_BAK); - LOG_DBG("RBS", "Migrated recent.bin to recent.json"); - return true; - } - } - - return false; -} - -bool RecentBooksStore::loadFromBinaryFile() { - HalFile inputFile; - if (!Storage.openFileForRead("RBS", RECENT_BOOKS_FILE_BIN, inputFile)) { - return false; - } - - uint8_t version; - serialization::readPod(inputFile, version); - if (version == 1 || version == 2) { - // Old version, just read paths - uint8_t count; - serialization::readPod(inputFile, count); - recentBooks.clear(); - recentBooks.reserve(count); - for (uint8_t i = 0; i < count; i++) { - std::string path; - serialization::readString(inputFile, path); - - // load book to get missing data - RecentBook book = getDataFromBook(path); - if (book.title.empty() && book.author.empty() && version == 2) { - // Fall back to loading what we can from the store - std::string title, author; - serialization::readString(inputFile, title); - serialization::readString(inputFile, author); - recentBooks.push_back({path, title, author, ""}); - } else { - recentBooks.push_back(book); - } - } - } else if (version == 3) { - uint8_t count; - serialization::readPod(inputFile, count); - - recentBooks.clear(); - recentBooks.reserve(count); - uint8_t omitted = 0; - - for (uint8_t i = 0; i < count; i++) { - std::string path, title, author, coverBmpPath; - serialization::readString(inputFile, path); - serialization::readString(inputFile, title); - serialization::readString(inputFile, author); - serialization::readString(inputFile, coverBmpPath); - - // Omit books with missing title (e.g. saved before metadata was available) - if (title.empty()) { - omitted++; - continue; - } - - recentBooks.push_back({path, title, author, coverBmpPath}); - } - - if (omitted > 0) { - // Explicitly close() file before saveToFile() rewrites the same file - inputFile.close(); - saveToFile(); - LOG_DBG("RBS", "Omitted %u recent book(s) with missing title", omitted); - return true; - } - } else { - LOG_ERR("RBS", "Deserialization failed: Unknown version %u", version); - return false; - } - - LOG_DBG("RBS", "Recent books loaded from binary file (%d entries)", static_cast(recentBooks.size())); - return true; -} diff --git a/src/RecentBooksStore.h b/src/RecentBooksStore.h index f3a8391e..37f39047 100644 --- a/src/RecentBooksStore.h +++ b/src/RecentBooksStore.h @@ -1,4 +1,7 @@ #pragma once +#include +#include + #include #include @@ -11,24 +14,21 @@ struct RecentBook { bool operator==(const RecentBook& other) const { return path == other.path; } }; -class RecentBooksStore; -namespace JsonSettingsIO { -bool loadRecentBooks(RecentBooksStore& store, const char* json); -} // namespace JsonSettingsIO - -class RecentBooksStore { - // Static instance - static RecentBooksStore instance; - +class RecentBooksStore : public PersistableStore { + private: std::vector recentBooks; - friend bool JsonSettingsIO::loadRecentBooks(RecentBooksStore&, const char*); + static constexpr int MAX_RECENT_BOOKS = 10; - public: + RecentBooksStore() = default; ~RecentBooksStore() = default; - // Get singleton instance - static RecentBooksStore& getInstance() { return instance; } + friend class PersistableStore; + + public: + static const char* getFilePath() { return "/.crosspoint/recent.json"; } + void toJson(JsonDocument& doc) const; + bool fromJson(JsonVariantConst doc); // Add a book to the recent list (moves to front if already exists) void addBook(const std::string& path, const std::string& title, const std::string& author, @@ -61,13 +61,7 @@ class RecentBooksStore { // Get the count of recent books int getCount() const { return static_cast(recentBooks.size()); } - bool saveToFile() const; - - bool loadFromFile(); RecentBook getDataFromBook(std::string path) const; - - private: - bool loadFromBinaryFile(); }; // Helper macro to access recent books store diff --git a/src/WifiCredentialStore.cpp b/src/WifiCredentialStore.cpp index 6827ad5c..532b925e 100644 --- a/src/WifiCredentialStore.cpp +++ b/src/WifiCredentialStore.cpp @@ -1,106 +1,46 @@ #include "WifiCredentialStore.h" -#include -#include #include #include -#include #include -// Initialize the static instance -WifiCredentialStore WifiCredentialStore::instance; +void WifiCredentialStore::toJson(JsonDocument& doc) const { + doc["lastConnectedSsid"] = lastConnectedSsid; -namespace { -// File format version (for binary migration) -constexpr uint8_t WIFI_FILE_VERSION = 2; - -// File paths -constexpr char WIFI_FILE_BIN[] = "/.crosspoint/wifi.bin"; -constexpr char WIFI_FILE_JSON[] = "/.crosspoint/wifi.json"; -constexpr char WIFI_FILE_BAK[] = "/.crosspoint/wifi.bin.bak"; - -// Legacy obfuscation key - "CrossPoint" in ASCII (only used for binary migration) -constexpr uint8_t LEGACY_OBFUSCATION_KEY[] = {0x43, 0x72, 0x6F, 0x73, 0x73, 0x50, 0x6F, 0x69, 0x6E, 0x74}; -constexpr size_t LEGACY_KEY_LENGTH = sizeof(LEGACY_OBFUSCATION_KEY); - -void legacyDeobfuscate(std::string& data) { - for (size_t i = 0; i < data.size(); i++) { - data[i] ^= LEGACY_OBFUSCATION_KEY[i % LEGACY_KEY_LENGTH]; + JsonArray arr = doc["credentials"].to(); + for (const auto& cred : credentials) { + JsonObject obj = arr.add(); + obj["ssid"] = cred.ssid; + obj["password_obf"] = obfuscation::obfuscateToBase64(cred.password); } } -} // namespace -bool WifiCredentialStore::saveToFile() const { - Storage.mkdir("/.crosspoint"); - return JsonSettingsIO::saveWifi(*this, WIFI_FILE_JSON); -} - -bool WifiCredentialStore::loadFromFile() { - // Try JSON first - if (Storage.exists(WIFI_FILE_JSON)) { - String json = Storage.readFile(WIFI_FILE_JSON); - if (!json.isEmpty()) { - bool resave = false; - bool result = JsonSettingsIO::loadWifi(*this, json.c_str(), &resave); - if (result && resave) { - LOG_DBG("WCS", "Resaving JSON with obfuscated passwords"); - saveToFile(); - } - return result; - } - } - - // Fall back to binary migration - if (Storage.exists(WIFI_FILE_BIN)) { - if (loadFromBinaryFile()) { - if (saveToFile()) { - Storage.rename(WIFI_FILE_BIN, WIFI_FILE_BAK); - LOG_DBG("WCS", "Migrated wifi.bin to wifi.json"); - return true; - } else { - LOG_ERR("WCS", "Failed to save wifi during migration"); - return false; - } - } - } - - return false; -} - -bool WifiCredentialStore::loadFromBinaryFile() { - HalFile file; - if (!Storage.openFileForRead("WCS", WIFI_FILE_BIN, file)) { - return false; - } - - uint8_t version; - serialization::readPod(file, version); - if (version > WIFI_FILE_VERSION) { - LOG_DBG("WCS", "Unknown file version: %u", version); - return false; - } - - if (version >= 2) { - serialization::readString(file, lastConnectedSsid); - } else { - lastConnectedSsid.clear(); - } - - uint8_t count; - serialization::readPod(file, count); +bool WifiCredentialStore::fromJson(JsonVariantConst doc) { + lastConnectedSsid = doc["lastConnectedSsid"] | ""; + // Tolerate a missing/invalid 'credentials' key (treat as empty list); only + // a JSON parse error is fatal. A null JsonArray iterates zero times. credentials.clear(); - credentials.reserve(std::min(count, MAX_NETWORKS)); - for (uint8_t i = 0; i < count && i < MAX_NETWORKS; i++) { + JsonArrayConst arr = doc["credentials"].as(); + credentials.reserve(std::min(arr.size(), MAX_NETWORKS)); + bool needsResave = false; + + for (JsonObjectConst obj : arr) { + if (credentials.size() >= MAX_NETWORKS) break; WifiCredential cred; - serialization::readString(file, cred.ssid); - serialization::readString(file, cred.password); - legacyDeobfuscate(cred.password); + cred.ssid = obj["ssid"] | ""; + cred.password = extractPassword(obj, needsResave); credentials.push_back(cred); } - // LOG_DBG("WCS", "Loaded %zu WiFi credentials from binary file", credentials.size()); + LOG_DBG("WCS", "Loaded %zu WiFi credentials from file", credentials.size()); + + if (needsResave) { + LOG_DBG("WCS", "Resaving JSON with obfuscated passwords"); + saveToFile(); + } + return true; } diff --git a/src/WifiCredentialStore.h b/src/WifiCredentialStore.h index 46650954..8a95ebe9 100644 --- a/src/WifiCredentialStore.h +++ b/src/WifiCredentialStore.h @@ -1,4 +1,7 @@ #pragma once +#include +#include + #include #include @@ -7,21 +10,14 @@ struct WifiCredential { std::string password; // Plaintext in memory; obfuscated with hardware key on disk }; -class WifiCredentialStore; -namespace JsonSettingsIO { -bool saveWifi(const WifiCredentialStore& store, const char* path); -bool loadWifi(WifiCredentialStore& store, const char* json, bool* needsResave); -} // namespace JsonSettingsIO - /** * Singleton class for storing WiFi credentials on the SD card. * Passwords are XOR-obfuscated with the device's unique hardware MAC address * and base64-encoded before writing to JSON (not cryptographically secure, * but prevents casual reading and ties credentials to the specific device). */ -class WifiCredentialStore { +class WifiCredentialStore : public PersistableStore { private: - static WifiCredentialStore instance; std::vector credentials; std::string lastConnectedSsid; @@ -30,22 +26,12 @@ class WifiCredentialStore { // Private constructor for singleton WifiCredentialStore() = default; - bool loadFromBinaryFile(); - - friend bool JsonSettingsIO::saveWifi(const WifiCredentialStore&, const char*); - friend bool JsonSettingsIO::loadWifi(WifiCredentialStore&, const char*, bool*); + friend class PersistableStore; public: - // Delete copy constructor and assignment - WifiCredentialStore(const WifiCredentialStore&) = delete; - WifiCredentialStore& operator=(const WifiCredentialStore&) = delete; - - // Get singleton instance - static WifiCredentialStore& getInstance() { return instance; } - - // Save/load from SD card - bool saveToFile() const; - bool loadFromFile(); + static const char* getFilePath() { return "/.crosspoint/wifi.json"; } + void toJson(JsonDocument& doc) const; + bool fromJson(JsonVariantConst doc); // Credential management bool addCredential(const std::string& ssid, const std::string& password); diff --git a/src/activities/reader/EpubReaderActivity.cpp b/src/activities/reader/EpubReaderActivity.cpp index 605b730a..f1441491 100644 --- a/src/activities/reader/EpubReaderActivity.cpp +++ b/src/activities/reader/EpubReaderActivity.cpp @@ -939,6 +939,15 @@ void EpubReaderActivity::render(RenderLock&& lock) { GUI.drawPopup(renderer, tr(STR_SAVE_PROGRESS_FAILED)); }; + // A section build failure (e.g. an invalid/corrupt EPUB that fails XML parsing) leaves the + // "Indexing" popup on screen with no way forward. Surface an explicit error instead of hanging. + // clearScreen first so the error popup doesn't overlay the stale "Indexing" popup. + const auto showBuildError = [this]() { + renderer.clearScreen(); + GUI.drawPopup(renderer, tr(STR_INDEX_FAILED)); + automaticPageTurnActive = false; + }; + // edge case handling for sub-zero spine index if (currentSpineIndex < 0) { currentSpineIndex = 0; @@ -1086,7 +1095,7 @@ void EpubReaderActivity::render(RenderLock&& lock) { silentRestartDefrag(); LOG_ERR("ERS", "Failed to persist page data to SD"); section.reset(); - showPendingSyncSaveError(); + showBuildError(); return; } } else { @@ -1139,7 +1148,7 @@ void EpubReaderActivity::render(RenderLock&& lock) { silentRestartDefrag(); LOG_ERR("ERS", "Failed to start section build"); section.reset(); - showPendingSyncSaveError(); + showBuildError(); return; } while (!section->isBuildComplete() && @@ -1150,7 +1159,7 @@ void EpubReaderActivity::render(RenderLock&& lock) { if (!section->buildSomeMore(BUILD_PAGES_PER_CHUNK)) { LOG_ERR("ERS", "Failed during incremental section build"); section.reset(); - showPendingSyncSaveError(); + showBuildError(); return; } } @@ -1204,7 +1213,7 @@ void EpubReaderActivity::render(RenderLock&& lock) { SETTINGS.focusReadingEnabled)) { LOG_ERR("ERS", "Failed to start partial extension build"); section.reset(); - showPendingSyncSaveError(); + showBuildError(); return; } // Extend until either the target page exists or the build completes. @@ -1212,7 +1221,7 @@ void EpubReaderActivity::render(RenderLock&& lock) { if (!section->buildSomeMore(BUILD_PAGES_PER_CHUNK)) { LOG_ERR("ERS", "Failed during incremental section build"); section.reset(); - showPendingSyncSaveError(); + showBuildError(); return; } } @@ -1223,7 +1232,7 @@ void EpubReaderActivity::render(RenderLock&& lock) { if (!section->buildSomeMore(BUILD_PAGES_PER_CHUNK)) { LOG_ERR("ERS", "Failed during incremental section build"); section.reset(); - showPendingSyncSaveError(); + showBuildError(); return; } } @@ -1273,17 +1282,29 @@ void EpubReaderActivity::render(RenderLock&& lock) { auto p = section->loadPage(section->currentPage); if (!p) { LOG_ERR("ERS", "Failed to load page from SD - clearing section cache"); + automaticPageTurnActive = false; + // Retrying rebuilds a transiently corrupt section and usually recovers, but a page that keeps + // failing would loop forever on a blank screen, so bound the retries before giving up. + const bool giveUp = ++pageLoadRetryCount > MAX_PAGE_LOAD_RETRIES; // Abandon (not suspend) any active build BEFORE clearing: clearCache deletes the files, // and the destructor's suspend would otherwise commit tables into a deleted handle. section->abandonBuild(); section->clearCache(); section.reset(); + if (giveUp) { + LOG_ERR("ERS", "Page load retry limit reached, aborting"); + pageLoadRetryCount = 0; // Reset so a later user-initiated navigation can try afresh + renderer.clearScreen(); + renderer.drawCenteredText(UI_12_FONT_ID, 300, tr(STR_PAGE_LOAD_ERROR), true, EpdFontFamily::BOLD); + renderer.displayBuffer(); + showPendingSyncSaveError(); + return; + } requestUpdate(); // Try again after clearing cache - // TODO: prevent infinite loop if the page keeps failing to load for some reason - automaticPageTurnActive = false; showPendingSyncSaveError(); return; } + pageLoadRetryCount = 0; // Reset the retry counter once a page loads cleanly // Collect footnotes from the loaded page currentPageFootnotes = std::move(p->footnotes); diff --git a/src/activities/reader/EpubReaderActivity.h b/src/activities/reader/EpubReaderActivity.h index e40c9af1..e9ee40e2 100644 --- a/src/activities/reader/EpubReaderActivity.h +++ b/src/activities/reader/EpubReaderActivity.h @@ -32,6 +32,10 @@ class EpubReaderActivity final : public Activity { float pendingSpineProgress = 0.0f; bool pendingScreenshot = false; bool pendingSyncSaveError = false; + // Consecutive page-load failures. Each failure drops the section and rebuilds on the next render, + // which recovers a transiently corrupt cache; capped so a persistently bad page can't spin forever. + uint8_t pageLoadRetryCount = 0; + static constexpr uint8_t MAX_PAGE_LOAD_RETRIES = 3; bool skipNextButtonCheck = false; // Skip button processing for one frame after subactivity exit bool automaticPageTurnActive = false; bool showBookmarkMessage = false; diff --git a/src/network/html/FilesPage.html b/src/network/html/FilesPage.html index 8572b3e1..23881a7e 100644 --- a/src/network/html/FilesPage.html +++ b/src/network/html/FilesPage.html @@ -169,6 +169,27 @@ .modal.picker-mode { max-width: 920px; } + .modal.image-preview-mode { + max-width: 640px; + } + .image-preview-stage { + display: flex; + justify-content: center; + align-items: center; + overflow: auto; + margin-bottom: 15px; + border-radius: 6px; + } + .image-preview-stage img { + max-width: 100%; + max-height: 65vh; + object-fit: contain; + } + #imagePreviewDownload { + display: inline-block; + width: auto; + text-decoration: none; + } .picker-columns.picker-active { display: flex; flex-direction: row; @@ -1779,6 +1800,21 @@ + + +