From a4adbb9dfe026988f34764afc7deebc826801f6f Mon Sep 17 00:00:00 2001 From: cottongin Date: Thu, 29 Jan 2026 11:39:49 -0500 Subject: [PATCH] fix(dictionary): comprehensive dictionary fixes for stability and UX This commit completes a series of fixes addressing dictionary crashes, memory issues, and UI/UX improvements. Memory & Stability (from previous checkpoints): - Add uncompressed dictionary (.dict) support to avoid decompression memory issues with large dictzip chunks (58KB -> direct read) - Implement chunked on-demand HTML parsing for large definitions, parsing pages as user navigates rather than all at once - Refactor TextBlock/ParsedText from std::list to std::vector, reducing heap allocations by ~12x per TextBlock and eliminating crashes from repeated page navigation due to heap fragmentation - Limit cached pages to MAX_CACHED_PAGES (4) with re-parse capability for backward navigation beyond the cache window UI/Layout Fixes (this commit): - Restore DictionaryMargins.h for proper orientation-aware button hint space (front buttons: 45px, side buttons: 50px) - Add side button hints to definition screen with proper "<" / ">" labels for page navigation - Add side button hints to word selection screen ("UP"/"DOWN" labels, borderless, small font, 2px edge margin) - Add side button hints to dictionary menu ("< Prev", "Next >") - Fix double-button press bug when loading new chunks by checking forward navigation availability after parsing instead of page count - Add drawSideButtonHints() drawBorder parameter for minimal hints - Add drawTextRotated90CCW() for LandscapeCCW text orientation - Move page indicator up to avoid bezel cutoff --- lib/GfxRenderer/GfxRenderer.cpp | 142 +++++++++++++++--- lib/GfxRenderer/GfxRenderer.h | 5 +- .../dictionary/DictionaryMenuActivity.cpp | 16 +- .../dictionary/DictionaryResultActivity.cpp | 43 +++--- .../dictionary/DictionarySearchActivity.cpp | 6 +- .../dictionary/EpubWordSelectionActivity.cpp | 19 ++- src/activities/util/KeyboardEntryActivity.cpp | 4 +- 7 files changed, 178 insertions(+), 57 deletions(-) diff --git a/lib/GfxRenderer/GfxRenderer.cpp b/lib/GfxRenderer/GfxRenderer.cpp index f98807c..d24ad3b 100644 --- a/lib/GfxRenderer/GfxRenderer.cpp +++ b/lib/GfxRenderer/GfxRenderer.cpp @@ -650,7 +650,8 @@ void GfxRenderer::drawButtonHints(const int fontId, const char* btn1, const char setOrientation(orig_orientation); } -void GfxRenderer::drawSideButtonHints(const int fontId, const char* topBtn, const char* bottomBtn) { +void GfxRenderer::drawSideButtonHints(const int fontId, const char* topBtn, const char* bottomBtn, + const bool drawBorder) { const Orientation orig_orientation = getOrientation(); setOrientation(Orientation::Portrait); @@ -671,27 +672,32 @@ void GfxRenderer::drawSideButtonHints(const int fontId, const char* topBtn, cons // Draw the shared border for both buttons as one unit const int x = screenWidth - buttonX - buttonWidth; - // Draw top button outline (3 sides, bottom open) - if (topBtn != nullptr && topBtn[0] != '\0') { - drawLine(x, topButtonY, x + buttonWidth - 1, topButtonY); // Top - drawLine(x, topButtonY, x, topButtonY + buttonHeight - 1); // Left - drawLine(x + buttonWidth - 1, topButtonY, x + buttonWidth - 1, topButtonY + buttonHeight - 1); // Right - } + if (drawBorder) { + // Draw top button outline (3 sides, bottom open) + if (topBtn != nullptr && topBtn[0] != '\0') { + drawLine(x, topButtonY, x + buttonWidth - 1, topButtonY); // Top + drawLine(x, topButtonY, x, topButtonY + buttonHeight - 1); // Left + drawLine(x + buttonWidth - 1, topButtonY, x + buttonWidth - 1, topButtonY + buttonHeight - 1); // Right + } - // Draw shared middle border - if ((topBtn != nullptr && topBtn[0] != '\0') || (bottomBtn != nullptr && bottomBtn[0] != '\0')) { - drawLine(x, topButtonY + buttonHeight, x + buttonWidth - 1, topButtonY + buttonHeight); // Shared border - } + // Draw shared middle border + if ((topBtn != nullptr && topBtn[0] != '\0') || (bottomBtn != nullptr && bottomBtn[0] != '\0')) { + drawLine(x, topButtonY + buttonHeight, x + buttonWidth - 1, topButtonY + buttonHeight); // Shared border + } - // Draw bottom button outline (3 sides, top is shared) - if (bottomBtn != nullptr && bottomBtn[0] != '\0') { - drawLine(x, topButtonY + buttonHeight, x, topButtonY + 2 * buttonHeight - 1); // Left - drawLine(x + buttonWidth - 1, topButtonY + buttonHeight, x + buttonWidth - 1, - topButtonY + 2 * buttonHeight - 1); // Right - drawLine(x, topButtonY + 2 * buttonHeight - 1, x + buttonWidth - 1, topButtonY + 2 * buttonHeight - 1); // Bottom + // Draw bottom button outline (3 sides, top is shared) + if (bottomBtn != nullptr && bottomBtn[0] != '\0') { + drawLine(x, topButtonY + buttonHeight, x, topButtonY + 2 * buttonHeight - 1); // Left + drawLine(x + buttonWidth - 1, topButtonY + buttonHeight, x + buttonWidth - 1, + topButtonY + 2 * buttonHeight - 1); // Right + drawLine(x, topButtonY + 2 * buttonHeight - 1, x + buttonWidth - 1, topButtonY + 2 * buttonHeight - 1); // Bottom + } } // Draw text for each button + // Use CCW rotation for LandscapeCCW so text reads in same direction as screen content + const bool useCCW = (orig_orientation == Orientation::LandscapeCounterClockwise); + for (int i = 0; i < 2; i++) { if (labels[i] != nullptr && labels[i][0] != '\0') { const int y = topButtonY + i * buttonHeight; @@ -700,11 +706,22 @@ void GfxRenderer::drawSideButtonHints(const int fontId, const char* topBtn, cons const int textWidth = getTextWidth(fontId, labels[i]); const int textHeight = getTextHeight(fontId); - // Center the rotated text in the button - const int textX = x + (buttonWidth - textHeight) / 2; - const int textY = y + (buttonHeight + textWidth) / 2; + int textX, textY; + if (drawBorder) { + // Center the rotated text in the button + textX = x + (buttonWidth - textHeight) / 2; + textY = useCCW ? y + (buttonHeight - textWidth) / 2 : y + (buttonHeight + textWidth) / 2; + } else { + // Position at edge with 2px margin (no border mode) + textX = screenWidth - bezelRight - textHeight - 2; + textY = useCCW ? y + (buttonHeight - textWidth) / 2 : y + (buttonHeight + textWidth) / 2; + } - drawTextRotated90CW(fontId, textX, textY, labels[i]); + if (useCCW) { + drawTextRotated90CCW(fontId, textX, textY, labels[i]); + } else { + drawTextRotated90CW(fontId, textX, textY, labels[i]); + } } } @@ -802,6 +819,89 @@ void GfxRenderer::drawTextRotated90CW(const int fontId, const int x, const int y } } +void GfxRenderer::drawTextRotated90CCW(const int fontId, const int x, const int y, const char* text, const bool black, + const EpdFontFamily::Style style) const { + // Cannot draw a NULL / empty string + if (text == nullptr || *text == '\0') { + return; + } + + if (fontMap.count(fontId) == 0) { + Serial.printf("[%lu] [GFX] Font %d not found\n", millis(), fontId); + return; + } + const auto font = fontMap.at(fontId); + + // No printable characters + if (!font.hasPrintableChars(text, style)) { + return; + } + + // For 90° counter-clockwise rotation: + // Original (glyphX, glyphY) -> Rotated (-glyphY, glyphX) + // Text reads from top to bottom + + int yPos = y; // Current Y position (increases as we draw characters) + + uint32_t cp; + while ((cp = utf8NextCodepoint(reinterpret_cast(&text)))) { + const EpdGlyph* glyph = font.getGlyph(cp, style); + if (!glyph) { + glyph = font.getGlyph(REPLACEMENT_GLYPH, style); + } + if (!glyph) { + continue; + } + + const int is2Bit = font.getData(style)->is2Bit; + const uint32_t offset = glyph->dataOffset; + const uint8_t width = glyph->width; + const uint8_t height = glyph->height; + const int left = glyph->left; + const int top = glyph->top; + + const uint8_t* bitmap = &font.getData(style)->bitmap[offset]; + + if (bitmap != nullptr) { + for (int glyphY = 0; glyphY < height; glyphY++) { + for (int glyphX = 0; glyphX < width; glyphX++) { + const int pixelPosition = glyphY * width + glyphX; + + // 90° counter-clockwise rotation transformation: + // screenX = x + (top - glyphY) + // screenY = yPos + (left + glyphX) + const int screenX = x + (top - glyphY); + const int screenY = yPos + left + glyphX; + + if (is2Bit) { + const uint8_t byte = bitmap[pixelPosition / 4]; + const uint8_t bit_index = (3 - pixelPosition % 4) * 2; + const uint8_t bmpVal = 3 - (byte >> bit_index) & 0x3; + + if (renderMode == BW && bmpVal < 3) { + drawPixel(screenX, screenY, black); + } else if (renderMode == GRAYSCALE_MSB && (bmpVal == 1 || bmpVal == 2)) { + drawPixel(screenX, screenY, false); + } else if (renderMode == GRAYSCALE_LSB && bmpVal == 1) { + drawPixel(screenX, screenY, false); + } + } else { + const uint8_t byte = bitmap[pixelPosition / 8]; + const uint8_t bit_index = 7 - (pixelPosition % 8); + + if ((byte >> bit_index) & 1) { + drawPixel(screenX, screenY, black); + } + } + } + } + } + + // Move to next character position (going down, so increase Y) + yPos += glyph->advanceX; + } +} + uint8_t* GfxRenderer::getFrameBuffer() const { return einkDisplay.getFrameBuffer(); } size_t GfxRenderer::getBufferSize() { return EInkDisplay::BUFFER_SIZE; } diff --git a/lib/GfxRenderer/GfxRenderer.h b/lib/GfxRenderer/GfxRenderer.h index 9b82ec9..d25e05e 100644 --- a/lib/GfxRenderer/GfxRenderer.h +++ b/lib/GfxRenderer/GfxRenderer.h @@ -116,12 +116,15 @@ class GfxRenderer { // UI Components void drawButtonHints(int fontId, const char* btn1, const char* btn2, const char* btn3, const char* btn4); - void drawSideButtonHints(int fontId, const char* topBtn, const char* bottomBtn); + void drawSideButtonHints(int fontId, const char* topBtn, const char* bottomBtn, bool drawBorder = true); private: // Helper for drawing rotated text (90 degrees clockwise, for side buttons) void drawTextRotated90CW(int fontId, int x, int y, const char* text, bool black = true, EpdFontFamily::Style style = EpdFontFamily::REGULAR) const; + // Helper for drawing rotated text (90 degrees counter-clockwise, for LandscapeCCW orientation) + void drawTextRotated90CCW(int fontId, int x, int y, const char* text, bool black = true, + EpdFontFamily::Style style = EpdFontFamily::REGULAR) const; int getTextHeight(int fontId) const; public: diff --git a/src/activities/dictionary/DictionaryMenuActivity.cpp b/src/activities/dictionary/DictionaryMenuActivity.cpp index f783ef8..b5c5a56 100644 --- a/src/activities/dictionary/DictionaryMenuActivity.cpp +++ b/src/activities/dictionary/DictionaryMenuActivity.cpp @@ -107,7 +107,7 @@ void DictionaryMenuActivity::displayTaskLoop() { void DictionaryMenuActivity::render() const { renderer.clearScreen(); - // Get margins using same pattern as reader + button hint space + // Get margins with button hint space for all orientations int marginTop, marginRight, marginBottom, marginLeft; getDictionaryContentMargins(renderer, &marginTop, &marginRight, &marginBottom, &marginLeft); @@ -119,11 +119,11 @@ void DictionaryMenuActivity::render() const { const int contentWidth = pageWidth - marginLeft - marginRight; const int contentHeight = pageHeight - marginTop - marginBottom; - // Draw header with top margin - renderer.drawCenteredText(UI_12_FONT_ID, marginTop + 15, "Dictionary", true, EpdFontFamily::BOLD); + // Draw header + renderer.drawCenteredText(UI_12_FONT_ID, marginTop + 5, "Dictionary", true, EpdFontFamily::BOLD); // Draw subtitle - renderer.drawCenteredText(UI_10_FONT_ID, marginTop + 50, "Look up a word"); + renderer.drawCenteredText(UI_10_FONT_ID, marginTop + 30, "Look up a word"); // Draw menu items centered in content area constexpr int itemHeight = 50; // Height for each menu item (including description) @@ -144,9 +144,13 @@ void DictionaryMenuActivity::render() const { renderer.drawText(SMALL_FONT_ID, marginLeft + 20, itemY + 22, MENU_DESCRIPTIONS[i], /*black=*/!isSelected); } - // Draw help text at bottom - const auto labels = mappedInput.mapLabels("\xc2\xab Back", "Select", "", ""); + // Draw front button hints (Prev/Next for list navigation) + const auto labels = mappedInput.mapLabels("\xc2\xab Back", "Select", "< Prev", "Next >"); renderer.drawButtonHints(UI_10_FONT_ID, labels.btn1, labels.btn2, labels.btn3, labels.btn4); + // Draw side button hints for up/down navigation (standard style with borders, always shown since list wraps) + // Top button = up (prev), Bottom button = down (next) + renderer.drawSideButtonHints(UI_10_FONT_ID, "<", ">"); + renderer.displayBuffer(); } diff --git a/src/activities/dictionary/DictionaryResultActivity.cpp b/src/activities/dictionary/DictionaryResultActivity.cpp index d43ab03..9db2ca6 100644 --- a/src/activities/dictionary/DictionaryResultActivity.cpp +++ b/src/activities/dictionary/DictionaryResultActivity.cpp @@ -97,11 +97,11 @@ void DictionaryResultActivity::loop() { } else if (hasMoreContent) { // At end of cached pages but more content available - parse next chunk Serial.printf("[DICT-DBG] Parsing next chunk on navigation (page %d)\n", currentPage); - const size_t pagesBefore = pages.size(); parseNextChunk(); - // If new pages were added, navigate to the next one - if (pages.size() > pagesBefore) { + // After parsing (and possible page trimming), check if we can advance + // Note: Don't compare page counts - trimming may keep size the same while adding new content + if (currentPage < static_cast(pages.size()) - 1) { currentPage++; updateRequired = true; } @@ -122,7 +122,7 @@ void DictionaryResultActivity::paginateDefinition() { return; } - // Get margins using same pattern as reader + button hint space + // Get margins with button hint space for all orientations int marginTop, marginRight, marginBottom, marginLeft; getDictionaryContentMargins(renderer, &marginTop, &marginRight, &marginBottom, &marginLeft); @@ -130,8 +130,8 @@ void DictionaryResultActivity::paginateDefinition() { const auto pageHeight = renderer.getScreenHeight(); // Calculate available area for text (must match render() layout) - constexpr int headerHeight = 80; // Space for word and header (relative to marginTop) - constexpr int footerHeight = 30; // Space for page indicator + constexpr int headerHeight = 55; // Space for "Dictionary" + lookup word + constexpr int footerHeight = 20; // Space for page indicator const int textMargin = marginLeft + 10; const int textWidth = pageWidth - textMargin - marginRight - 10; const int textHeight = pageHeight - marginTop - marginBottom - headerHeight - footerHeight; @@ -272,7 +272,7 @@ void DictionaryResultActivity::parseNextChunk() { Serial.printf("[DICT-DBG] parseNextChunk starting at position %u of %u\n", parsePosition, rawDefinition.length()); - // Get margins for calculating page dimensions + // Get margins with button hint space for all orientations int marginTop, marginRight, marginBottom, marginLeft; getDictionaryContentMargins(renderer, &marginTop, &marginRight, &marginBottom, &marginLeft); @@ -280,8 +280,8 @@ void DictionaryResultActivity::parseNextChunk() { const auto pageHeight = renderer.getScreenHeight(); // Calculate text area dimensions (must match paginateDefinition and render) - constexpr int headerHeight = 80; - constexpr int footerHeight = 30; + constexpr int headerHeight = 55; // Space for "Dictionary" + lookup word + constexpr int footerHeight = 20; // Space for page indicator const int textMargin = marginLeft + 10; const int textWidth = pageWidth - textMargin - marginRight - 10; const int textHeight = pageHeight - marginTop - marginBottom - headerHeight - footerHeight; @@ -409,17 +409,15 @@ void DictionaryResultActivity::displayTaskLoop() { void DictionaryResultActivity::render() const { renderer.clearScreen(); - // Get margins using same pattern as reader + button hint space + // Get margins with button hint space for all orientations int marginTop, marginRight, marginBottom, marginLeft; getDictionaryContentMargins(renderer, &marginTop, &marginRight, &marginBottom, &marginLeft); const auto pageHeight = renderer.getScreenHeight(); - // Draw header with top margin - renderer.drawCenteredText(UI_12_FONT_ID, marginTop + 15, "Dictionary", true, EpdFontFamily::BOLD); - - // Draw word being looked up (bold) - renderer.drawCenteredText(UI_12_FONT_ID, marginTop + 50, lookupWord.c_str(), true, EpdFontFamily::BOLD); + // Draw header - "Dictionary" title and lookup word + renderer.drawCenteredText(UI_12_FONT_ID, marginTop + 5, "Dictionary", true, EpdFontFamily::BOLD); + renderer.drawCenteredText(UI_12_FONT_ID, marginTop + 30, lookupWord.c_str(), true, EpdFontFamily::BOLD); if (notFound) { // Show not found message (centered in content area) @@ -427,10 +425,12 @@ void DictionaryResultActivity::render() const { renderer.drawCenteredText(UI_10_FONT_ID, centerY, "Word not found"); } else if (!pages.empty()) { // Draw definition text using TextBlocks with rich formatting - const int textStartY = marginTop + 80; + constexpr int headerHeight = 55; // Space for "Dictionary" + lookup word + constexpr int footerHeight = 20; // Space for page indicator + const int textStartY = marginTop + headerHeight; const int textMargin = marginLeft + 10; const int lineHeight = renderer.getLineHeight(UI_10_FONT_ID); - const int bottomLimit = pageHeight - marginBottom - 25; // Leave space for page indicator + const int bottomLimit = pageHeight - marginBottom - footerHeight; const auto& pageBlocks = pages[currentPage]; int y = textStartY; @@ -449,12 +449,11 @@ void DictionaryResultActivity::render() const { const int displayPageNum = firstPageNumber + currentPage; const int lastKnownPage = firstPageNumber + static_cast(pages.size()) - 1; if (hasMoreContent) { - // More content to load - show "Page X of Y+" to indicate more pages coming snprintf(pageIndicator, sizeof(pageIndicator), "Page %d of %d+", displayPageNum, lastKnownPage); } else { snprintf(pageIndicator, sizeof(pageIndicator), "Page %d of %d", displayPageNum, lastKnownPage); } - renderer.drawCenteredText(SMALL_FONT_ID, pageHeight - marginBottom - 5, pageIndicator); + renderer.drawCenteredText(SMALL_FONT_ID, pageHeight - marginBottom - 15, pageIndicator); } } @@ -468,5 +467,11 @@ void DictionaryResultActivity::render() const { const auto labels = mappedInput.mapLabels("\xc2\xab Back", "Search", leftHint, rightHint); renderer.drawButtonHints(UI_10_FONT_ID, labels.btn1, labels.btn2, labels.btn3, labels.btn4); + // Draw side button hints for page navigation (rotated 90° CW: ">" appears as "^", "<" as "v") + // Top physical button = PageBack (prev), Bottom physical button = PageForward (next) + const char* sideTopHint = canGoBack ? "<" : ""; + const char* sideBottomHint = canGoForward ? ">" : ""; + renderer.drawSideButtonHints(UI_10_FONT_ID, sideTopHint, sideBottomHint); + renderer.displayBuffer(); } diff --git a/src/activities/dictionary/DictionarySearchActivity.cpp b/src/activities/dictionary/DictionarySearchActivity.cpp index 8fe6b5d..d2b998f 100644 --- a/src/activities/dictionary/DictionarySearchActivity.cpp +++ b/src/activities/dictionary/DictionarySearchActivity.cpp @@ -235,14 +235,14 @@ void DictionarySearchActivity::displayTaskLoop() { void DictionarySearchActivity::render() const { renderer.clearScreen(); - // Get margins using same pattern as reader + button hint space + // Get margins with button hint space for all orientations int marginTop, marginRight, marginBottom, marginLeft; getDictionaryContentMargins(renderer, &marginTop, &marginRight, &marginBottom, &marginLeft); const auto pageHeight = renderer.getScreenHeight(); - // Draw header with top margin - renderer.drawCenteredText(UI_12_FONT_ID, marginTop + 15, "Dictionary", true, EpdFontFamily::BOLD); + // Draw header + renderer.drawCenteredText(UI_12_FONT_ID, marginTop + 5, "Dictionary", true, EpdFontFamily::BOLD); if (isSearching) { // Show searching status with word and animated ellipsis diff --git a/src/activities/dictionary/EpubWordSelectionActivity.cpp b/src/activities/dictionary/EpubWordSelectionActivity.cpp index bd48b0a..1bd8c17 100644 --- a/src/activities/dictionary/EpubWordSelectionActivity.cpp +++ b/src/activities/dictionary/EpubWordSelectionActivity.cpp @@ -223,10 +223,13 @@ void EpubWordSelectionActivity::displayTaskLoop() { void EpubWordSelectionActivity::render() const { renderer.clearScreen(); - // Get margins using same pattern as reader + button hint space + // Get margins with button hint space for all orientations int marginTop, marginRight, marginBottom, marginLeft; getDictionaryContentMargins(renderer, &marginTop, &marginRight, &marginBottom, &marginLeft); + const auto screenWidth = renderer.getScreenWidth(); + const auto screenHeight = renderer.getScreenHeight(); + // Draw the page content (uses pre-calculated offsets from reader) // The page already has proper offsets, so render as-is if (page) { @@ -246,14 +249,20 @@ void EpubWordSelectionActivity::render() const { renderer.drawText(fontId, selected.x, selected.y, selected.text.c_str(), false, selected.style); } - // Draw instruction text - position it just above the front button area - const auto screenHeight = renderer.getScreenHeight(); + // Draw instruction text - always show, positioned just above the front button area renderer.drawCenteredText(SMALL_FONT_ID, screenHeight - marginBottom - 10, "Navigate with arrows, select with confirm"); - // Draw button hints - const auto labels = mappedInput.mapLabels("\xc2\xab Cancel", "Select", "< >", ""); + // Draw button hints with proper left/right navigation labels + const auto labels = mappedInput.mapLabels("\xc2\xab Cancel", "Select", "< Prev", "Next >"); renderer.drawButtonHints(UI_10_FONT_ID, labels.btn1, labels.btn2, labels.btn3, labels.btn4); + // Draw side button hints for up/down line navigation (no border, small font) + // Top physical button = Up (prev line), Bottom physical button = Down (next line) + const int lastLine = findLineForWordIndex(static_cast(allWords.size()) - 1); + const char* sideTopHint = (currentLineIndex > 0) ? "UP" : ""; + const char* sideBottomHint = (currentLineIndex < lastLine) ? "DOWN" : ""; + renderer.drawSideButtonHints(SMALL_FONT_ID, sideTopHint, sideBottomHint, false); // No border + renderer.displayBuffer(EInkDisplay::FAST_REFRESH); } diff --git a/src/activities/util/KeyboardEntryActivity.cpp b/src/activities/util/KeyboardEntryActivity.cpp index 67775b3..446fe6c 100644 --- a/src/activities/util/KeyboardEntryActivity.cpp +++ b/src/activities/util/KeyboardEntryActivity.cpp @@ -1,7 +1,7 @@ #include "KeyboardEntryActivity.h" -#include "MappedInputManager.h" #include "activities/dictionary/DictionaryMargins.h" +#include "MappedInputManager.h" #include "fontIds.h" // Keyboard layouts - lowercase @@ -249,7 +249,7 @@ void KeyboardEntryActivity::loop() { } void KeyboardEntryActivity::render() const { - // Get margins using same pattern as reader + button hint space + // Get margins with button hint space for all orientations int marginTop, marginRight, marginBottom, marginLeft; getDictionaryContentMargins(renderer, &marginTop, &marginRight, &marginBottom, &marginLeft);