Review changes

src/activities/home/FileBrowserActivity.cpp

@@ -16,10 +16,6 @@
 #include "components/UITheme.h"
 #include "fontIds.h"
 
-namespace {
-constexpr unsigned long GO_HOME_MS = 1000;
-}  // namespace
-
 void sortFileList(std::vector<std::string>& strs) {
   std::sort(begin(strs), end(strs), [](const std::string& str1, const std::string& str2) {
     // Directories first

src/activities/reader/EpubReaderActivity.cpp

@@ -219,12 +219,14 @@ void EpubReaderActivity::loop() {
   }
 
   if (inputDrainGuard.shouldDrain(mappedInput)) {
+    buttonEvents.drain();
     return;
   }
 
   if (automaticPageTurnActive) {
     if (mappedInput.wasReleased(MappedInputManager::Button::Confirm) ||
         mappedInput.wasReleased(MappedInputManager::Button::Back)) {
+      buttonEvents.drain();
       automaticPageTurnActive = false;
       // updates chapter title space to indicate page turn disabled
       requestUpdate();
@@ -260,36 +262,7 @@ void EpubReaderActivity::loop() {
         return;
       }
       if (ev.type == ButtonEventManager::PressType::Short) {
-        const int currentPage = section ? section->currentPage + 1 : 0;
+        openReaderMenu();
-        const int totalPages = section ? section->pageCount : 0;
-        float bookProgress = 0.0f;
-        if (epub->getBookSize() > 0 && section && section->pageCount > 0) {
-          const float chapterProgress =
-              static_cast<float>(section->currentPage) / static_cast<float>(section->pageCount);
-          bookProgress = epub->calculateProgress(currentSpineIndex, chapterProgress) * 100.0f;
-        }
-        const int bookProgressPercent = clampPercent(static_cast<int>(bookProgress + 0.5f));
-        const bool isCurrentPageStarred = section && bookmarkStore.has(static_cast<uint16_t>(currentSpineIndex),
-                                                                       static_cast<uint16_t>(section->currentPage));
-        ReaderUtils::enforceExitFullRefresh(renderer);
-        startActivityForResult(
-            std::make_unique<EpubReaderMenuActivity>(
-                renderer, mappedInput, epub->getTitle(), currentPage, totalPages, bookProgressPercent,
-                SETTINGS.orientation, !currentPageFootnotes.empty(), bookEmbeddedStyleOverride,
-                bookImageRenderingOverride, bookFontFamilyOverride, bookFontSizeOverride, SETTINGS.textDarkness,
-                !bookmarkStore.isEmpty(), isCurrentPageStarred),
-            [this](const ActivityResult& result) {
-              // Always apply orientation/darkness change even if the menu was cancelled
-              const auto& menu = std::get<MenuResult>(result.data);
-              applyOrientation(menu.orientation);
-              applyTextDarkness(menu.textDarkness);
-              toggleAutoPageTurn(menu.pageTurnOption);
-              applyBookReaderOverrides(menu.embeddedStyleOverride, menu.imageRenderingOverride, menu.fontFamilyOverride,
-                                       menu.fontSizeOverride);
-              if (!result.isCancelled) {
-                onReaderMenuConfirm(static_cast<EpubReaderMenuActivity::MenuAction>(menu.action));
-              }
-            });
         return;
       }
     }
@@ -1741,6 +1714,36 @@ bool EpubReaderActivity::drawCurrentPageToBuffer(const std::string& filePath, Gf
   return true;
 }
 
+void EpubReaderActivity::openReaderMenu() {
+  const int currentPage = section ? section->currentPage + 1 : 0;
+  const int totalPages = section ? section->pageCount : 0;
+  float bookProgress = 0.0f;
+  if (epub->getBookSize() > 0 && section && section->pageCount > 0) {
+    const float chapterProgress = static_cast<float>(section->currentPage) / static_cast<float>(section->pageCount);
+    bookProgress = epub->calculateProgress(currentSpineIndex, chapterProgress) * 100.0f;
+  }
+  const int bookProgressPercent = clampPercent(static_cast<int>(bookProgress + 0.5f));
+  const bool isCurrentPageStarred = section && bookmarkStore.has(static_cast<uint16_t>(currentSpineIndex),
+                                                                 static_cast<uint16_t>(section->currentPage));
+  ReaderUtils::enforceExitFullRefresh(renderer);
+  startActivityForResult(
+      std::make_unique<EpubReaderMenuActivity>(
+          renderer, mappedInput, epub->getTitle(), currentPage, totalPages, bookProgressPercent, SETTINGS.orientation,
+          !currentPageFootnotes.empty(), bookEmbeddedStyleOverride, bookImageRenderingOverride, bookFontFamilyOverride,
+          bookFontSizeOverride, SETTINGS.textDarkness, !bookmarkStore.isEmpty(), isCurrentPageStarred),
+      [this](const ActivityResult& result) {
+        const auto& menu = std::get<MenuResult>(result.data);
+        applyOrientation(menu.orientation);
+        applyTextDarkness(menu.textDarkness);
+        toggleAutoPageTurn(menu.pageTurnOption);
+        applyBookReaderOverrides(menu.embeddedStyleOverride, menu.imageRenderingOverride, menu.fontFamilyOverride,
+                                 menu.fontSizeOverride);
+        if (!result.isCancelled) {
+          onReaderMenuConfirm(static_cast<EpubReaderMenuActivity::MenuAction>(menu.action));
+        }
+      });
+}
+
 void EpubReaderActivity::onButtonAction(const CrossPointSettings::BUTTON_ACTION action) {
   using BA = CrossPointSettings::BUTTON_ACTION;
   switch (action) {
@@ -1859,35 +1862,7 @@ void EpubReaderActivity::onButtonAction(const CrossPointSettings::BUTTON_ACTION
       break;
     case BA::BTN_READER_MENU:
       if (epub) {
-        const int currentPage = section ? section->currentPage + 1 : 0;
+        openReaderMenu();
-        const int totalPages = section ? section->pageCount : 0;
-        float bookProgress = 0.0f;
-        if (epub->getBookSize() > 0 && section && section->pageCount > 0) {
-          const float chapterProgress =
-              static_cast<float>(section->currentPage) / static_cast<float>(section->pageCount);
-          bookProgress = epub->calculateProgress(currentSpineIndex, chapterProgress) * 100.0f;
-        }
-        const int bookProgressPercent = clampPercent(static_cast<int>(bookProgress + 0.5f));
-        const bool isCurrentPageStarred = section && bookmarkStore.has(static_cast<uint16_t>(currentSpineIndex),
-                                                                       static_cast<uint16_t>(section->currentPage));
-        ReaderUtils::enforceExitFullRefresh(renderer);
-        startActivityForResult(
-            std::make_unique<EpubReaderMenuActivity>(
-                renderer, mappedInput, epub->getTitle(), currentPage, totalPages, bookProgressPercent,
-                SETTINGS.orientation, !currentPageFootnotes.empty(), bookEmbeddedStyleOverride,
-                bookImageRenderingOverride, bookFontFamilyOverride, bookFontSizeOverride, SETTINGS.textDarkness,
-                !bookmarkStore.isEmpty(), isCurrentPageStarred),
-            [this](const ActivityResult& result) {
-              const auto& menu = std::get<MenuResult>(result.data);
-              applyOrientation(menu.orientation);
-              applyTextDarkness(menu.textDarkness);
-              toggleAutoPageTurn(menu.pageTurnOption);
-              applyBookReaderOverrides(menu.embeddedStyleOverride, menu.imageRenderingOverride, menu.fontFamilyOverride,
-                                       menu.fontSizeOverride);
-              if (!result.isCancelled) {
-                onReaderMenuConfirm(static_cast<EpubReaderMenuActivity::MenuAction>(menu.action));
-              }
-            });
       }
       break;
     case BA::BTN_KOREADER_SYNC:

src/activities/reader/EpubReaderActivity.h

@@ -166,6 +166,7 @@ class EpubReaderActivity final : public Activity {
   void toggleAutoPageTurn(uint8_t selectedPageTurnOption);
   void applyBookReaderOverrides(int8_t embeddedStyleOverride, int8_t imageRenderingOverride, int8_t fontFamilyOverride,
                                 int8_t fontSizeOverride);
+  void openReaderMenu();
   bool getEffectiveEmbeddedStyle() const;
   uint8_t getEffectiveImageRendering() const;
   int getEffectiveReaderFontId() const;

src/activities/reader/EpubReaderFootnotesActivity.cpp

@@ -38,50 +38,33 @@ void EpubReaderFootnotesActivity::loop() {
 
     if ((ev.button == MappedInputManager::Button::PageBack || ev.button == MappedInputManager::Button::Left) &&
         ev.type == ButtonEventManager::PressType::Short) {
-      if (!footnotes.empty()) {
+      advanceSelection(-1);
-        selectedIndex = (selectedIndex - 1 + footnotes.size()) % footnotes.size();
-        requestUpdate();
-      }
       return;
     }
 
     if ((ev.button == MappedInputManager::Button::PageForward || ev.button == MappedInputManager::Button::Right) &&
         ev.type == ButtonEventManager::PressType::Short) {
-      if (!footnotes.empty()) {
+      advanceSelection(1);
-        selectedIndex = (selectedIndex + 1) % footnotes.size();
-        requestUpdate();
-      }
       return;
     }
   }
 
-  buttonNavigator.onNextRelease([this] {
+  buttonNavigator.onNextRelease([this] { advanceSelection(1); });
-    if (!footnotes.empty()) {
-      selectedIndex = (selectedIndex + 1) % footnotes.size();
-      requestUpdate();
-    }
-  });
 
-  buttonNavigator.onPreviousRelease([this] {
+  buttonNavigator.onPreviousRelease([this] { advanceSelection(-1); });
-    if (!footnotes.empty()) {
-      selectedIndex = (selectedIndex - 1 + footnotes.size()) % footnotes.size();
-      requestUpdate();
-    }
-  });
 
-  buttonNavigator.onNextContinuous([this] {
+  buttonNavigator.onNextContinuous([this] { advanceSelection(1); });
-    if (!footnotes.empty()) {
-      selectedIndex = (selectedIndex + 1) % footnotes.size();
-      requestUpdate();
-    }
-  });
 
-  buttonNavigator.onPreviousContinuous([this] {
+  buttonNavigator.onPreviousContinuous([this] { advanceSelection(-1); });
-    if (!footnotes.empty()) {
+}
-      selectedIndex = (selectedIndex - 1 + footnotes.size()) % footnotes.size();
+
-      requestUpdate();
+void EpubReaderFootnotesActivity::advanceSelection(int delta) {
-    }
+  if (footnotes.empty()) {
-  });
+    return;
+  }
+  const int n = static_cast<int>(footnotes.size());
+  selectedIndex = ((selectedIndex + delta) % n + n) % n;
+  requestUpdate();
 }
 
 void EpubReaderFootnotesActivity::render(RenderLock&&) {

src/activities/reader/EpubReaderFootnotesActivity.h

@@ -21,6 +21,7 @@ class EpubReaderFootnotesActivity final : public Activity {
   void render(RenderLock&&) override;
 
  private:
+  void advanceSelection(int delta);
   const std::vector<FootnoteEntry>& footnotes;
   int selectedIndex = 0;
   int scrollOffset = 0;

src/activities/reader/MdReaderActivity.cpp

@@ -38,6 +38,8 @@ static std::string flattenHeadingText(const MdParser::ParsedLine& parsed) {
 void MdReaderActivity::onEnter() {
   Activity::onEnter();
 
+  inputDrainGuard.arm();
+
   if (!txt) {
     return;
   }
@@ -221,6 +223,11 @@ void MdReaderActivity::onExit() {
 }
 
 void MdReaderActivity::loop() {
+  if (inputDrainGuard.shouldDrain(mappedInput)) {
+    buttonEvents.drain();
+    return;
+  }
+
   ButtonEventManager::ButtonEvent ev;
   while (buttonEvents.consumeEvent(ev)) {
     if (ev.button == MappedInputManager::Button::Back) {

src/activities/reader/MdReaderActivity.h

@@ -6,6 +6,7 @@
 #include <vector>
 
 #include "CrossPointSettings.h"
+#include "ReaderUtils.h"
 #include "activities/Activity.h"
 
 struct MdHeading {
@@ -72,6 +73,7 @@ class MdReaderActivity final : public Activity {
   void assignHeadingPageNumbers();
   int getHeadingIndexForOffset(size_t offset) const;
   void jumpToHeading(bool next);
+  ReaderUtils::InputDrainGuard inputDrainGuard;
 
   // Word-wrap a parsed markdown line into one or more RenderedLines.
   // Returns true if all content was emitted, false if truncated by maxLines.

src/activities/reader/StarredPagesActivity.cpp

@@ -109,20 +109,6 @@ void StarredPagesActivity::loop() {
       deleteSelected();
       return;
     }
-
-    if (totalItems > 0 && ev.button == MappedInputManager::Button::PageBack &&
-        ev.type == ButtonEventManager::PressType::Short) {
-      selectorIndex = ButtonNavigator::previousIndex(selectorIndex, totalItems);
-      requestUpdate();
-      return;
-    }
-
-    if (totalItems > 0 && ev.button == MappedInputManager::Button::PageForward &&
-        ev.type == ButtonEventManager::PressType::Short) {
-      selectorIndex = ButtonNavigator::nextIndex(selectorIndex, totalItems);
-      requestUpdate();
-      return;
-    }
   }
 
   if (totalItems == 0) return;

src/activities/reader/TxtReaderActivity.cpp

@@ -137,6 +137,7 @@ void TxtReaderActivity::onExit() {
 
 void TxtReaderActivity::loop() {
   if (inputDrainGuard.shouldDrain(mappedInput)) {
+    buttonEvents.drain();
     return;
   }
 

src/activities/reader/XtcReaderActivity.cpp

@@ -22,10 +22,6 @@
 #include "XtcReaderChapterSelectionActivity.h"
 #include "fontIds.h"
 
-namespace {
-constexpr unsigned long goHomeMs = 1000;
-}  // namespace
-
 void XtcReaderActivity::onEnter() {
   Activity::onEnter();
 
@@ -61,6 +57,7 @@ void XtcReaderActivity::onExit() {
 
 void XtcReaderActivity::loop() {
   if (inputDrainGuard.shouldDrain(mappedInput)) {
+    buttonEvents.drain();
     return;
   }
 
@@ -146,9 +143,6 @@ void XtcReaderActivity::loop() {
     }
   } else if (nextTriggered) {
     currentPage++;
-    if (currentPage >= xtc->getPageCount()) {
-      currentPage = xtc->getPageCount();  // Allow showing "End of book"
-    }
     requestUpdate();
   }
 }

src/activities/reader/XtcReaderChapterSelectionActivity.cpp

@@ -5,6 +5,7 @@
 
 #include <algorithm>
 
+#include "ButtonEventManager.h"
 #include "MappedInputManager.h"
 #include "components/UITheme.h"
 #include "fontIds.h"
@@ -47,6 +48,10 @@ void XtcReaderChapterSelectionActivity::onEnter() {
 void XtcReaderChapterSelectionActivity::onExit() { Activity::onExit(); }
 
 void XtcReaderChapterSelectionActivity::loop() {
+  if (!xtc) {
+    return;
+  }
+
   const int pageItems = getPageItems();
   const int totalItems = static_cast<int>(xtc->getChapters().size());
 
@@ -82,11 +87,19 @@ void XtcReaderChapterSelectionActivity::loop() {
   }
 
   buttonNavigator.onNextRelease([this, totalItems] {
+    if (ButtonEventManager::hasDoubleAction(MappedInputManager::Button::Right) ||
+        ButtonEventManager::hasDoubleAction(MappedInputManager::Button::PageForward)) {
+      return;
+    }
     selectorIndex = ButtonNavigator::nextIndex(selectorIndex, totalItems);
     requestUpdate();
   });
 
   buttonNavigator.onPreviousRelease([this, totalItems] {
+    if (ButtonEventManager::hasDoubleAction(MappedInputManager::Button::Left) ||
+        ButtonEventManager::hasDoubleAction(MappedInputManager::Button::PageBack)) {
+      return;
+    }
     selectorIndex = ButtonNavigator::previousIndex(selectorIndex, totalItems);
     requestUpdate();
   });

src/activities/util/BmpViewerActivity.cpp

@@ -388,7 +388,7 @@ void BmpViewerActivity::loop() {
   // Keep CPU awake/polling so 1st click works
   Activity::loop();
 
-  // Long press BACK (1s+) goes to home screen
+  const bool toggleSupported = isBmpFile(filePath) ? bmpHasGreyscale : true;
   ButtonEventManager::ButtonEvent ev;
   while (buttonEvents.consumeEvent(ev)) {
     if (ev.button == MappedInputManager::Button::Back) {
@@ -401,27 +401,25 @@ void BmpViewerActivity::loop() {
         return;
       }
     }
-  }
 
-  // Confirm: toggle between 1-bit B&W and 4-level grayscale display.
+    if (ev.button == MappedInputManager::Button::Confirm && ev.type == ButtonEventManager::PressType::Short) {
-  // For decoded images this always applies; for BMPs it only makes sense when the bitmap
+      if (toggleSupported) {
-  // actually carries greyscale data (1-bit BMPs have nothing to toggle).
+        toggleDisplayMode();
-  const bool toggleSupported = isBmpFile(filePath) ? bmpHasGreyscale : true;
+      }
-  if (toggleSupported && mappedInput.wasReleased(MappedInputManager::Button::Confirm)) {
+      return;
-    toggleDisplayMode();
+    }
-    return;
-  }
 
 #ifdef ENABLE_IMAGE_DITHERING_EXTENSION
-  if (!isBmpFile(filePath) && grayscaleDisplay && mappedInput.wasReleased(MappedInputManager::Button::Left)) {
+    if (ev.button == MappedInputManager::Button::Left && ev.type == ButtonEventManager::PressType::Short &&
-    cycleDitherMode();
+        !isBmpFile(filePath) && grayscaleDisplay) {
-    return;
+      cycleDitherMode();
-  }
+      return;
+    }
 #endif
 
-  // Next/Right button: set this image as the sleep screen
+    if (ev.button == MappedInputManager::Button::Right && ev.type == ButtonEventManager::PressType::Short) {
-  if (mappedInput.wasReleased(MappedInputManager::Button::Right)) {
+      setAsSleepScreen();
-    setAsSleepScreen();
+      return;
-    return;
+    }
   }
 }

src/main.cpp

@@ -388,90 +388,92 @@ void loop() {
   {
     using BA = CrossPointSettings::BUTTON_ACTION;
     using B = MappedInputManager::Button;
+    auto actionFor = [&](const ButtonEventManager::ButtonEvent& ev) -> uint8_t {
+      switch (ev.button) {
+        case B::Back:
+          switch (ev.type) {
+            case ButtonEventManager::PressType::Short:
+              return SETTINGS.btnShortBack;
+            case ButtonEventManager::PressType::Double:
+              return SETTINGS.btnDoubleBack;
+            case ButtonEventManager::PressType::Long:
+              return SETTINGS.btnLongBack;
+          }
+          break;
+        case B::Confirm:
+          switch (ev.type) {
+            case ButtonEventManager::PressType::Short:
+              return SETTINGS.btnShortConfirm;
+            case ButtonEventManager::PressType::Double:
+              return SETTINGS.btnDoubleConfirm;
+            case ButtonEventManager::PressType::Long:
+              return SETTINGS.btnLongConfirm;
+          }
+          break;
+        case B::Left:
+          switch (ev.type) {
+            case ButtonEventManager::PressType::Short:
+              return SETTINGS.btnShortLeft;
+            case ButtonEventManager::PressType::Double:
+              return SETTINGS.btnDoubleLeft;
+            case ButtonEventManager::PressType::Long:
+              return SETTINGS.btnLongLeft;
+          }
+          break;
+        case B::Right:
+          switch (ev.type) {
+            case ButtonEventManager::PressType::Short:
+              return SETTINGS.btnShortRight;
+            case ButtonEventManager::PressType::Double:
+              return SETTINGS.btnDoubleRight;
+            case ButtonEventManager::PressType::Long:
+              return SETTINGS.btnLongRight;
+          }
+          break;
+        case B::PageBack:
+          switch (ev.type) {
+            case ButtonEventManager::PressType::Short:
+              return SETTINGS.btnShortPageBack;
+            case ButtonEventManager::PressType::Double:
+              return SETTINGS.btnDoublePageBack;
+            case ButtonEventManager::PressType::Long:
+              return SETTINGS.btnLongPageBack;
+          }
+          break;
+        case B::PageForward:
+          switch (ev.type) {
+            case ButtonEventManager::PressType::Short:
+              return SETTINGS.btnShortPageForward;
+            case ButtonEventManager::PressType::Double:
+              return SETTINGS.btnDoublePageForward;
+            case ButtonEventManager::PressType::Long:
+              return SETTINGS.btnLongPageForward;
+          }
+          break;
+        case B::Power:
+          switch (ev.type) {
+            case ButtonEventManager::PressType::Short:
+              return SETTINGS.btnShortPower;
+            case ButtonEventManager::PressType::Double:
+              return SETTINGS.btnDoublePower;
+            case ButtonEventManager::PressType::Long:
+              return SETTINGS.btnLongPower;
+          }
+          break;
+        default:
+          break;  // Up/Down have no FSMs — ButtonEventManager never emits these
+      }
+      return BA::BTN_DEFAULT;
+    };
     ButtonEventManager::ButtonEvent ev;
     std::vector<ButtonEventManager::ButtonEvent> defaultEvents;
     defaultEvents.reserve(8);
     while (buttonEventManager.consumeEvent(ev)) {
-      auto actionFor = [&](B btn) -> uint8_t {
+      const uint8_t action = actionFor(ev);
-        switch (btn) {
-          case B::Back:
-            switch (ev.type) {
-              case ButtonEventManager::PressType::Short:
-                return SETTINGS.btnShortBack;
-              case ButtonEventManager::PressType::Double:
-                return SETTINGS.btnDoubleBack;
-              case ButtonEventManager::PressType::Long:
-                return SETTINGS.btnLongBack;
-            }
-            break;
-          case B::Confirm:
-            switch (ev.type) {
-              case ButtonEventManager::PressType::Short:
-                return SETTINGS.btnShortConfirm;
-              case ButtonEventManager::PressType::Double:
-                return SETTINGS.btnDoubleConfirm;
-              case ButtonEventManager::PressType::Long:
-                return SETTINGS.btnLongConfirm;
-            }
-            break;
-          case B::Left:
-            switch (ev.type) {
-              case ButtonEventManager::PressType::Short:
-                return SETTINGS.btnShortLeft;
-              case ButtonEventManager::PressType::Double:
-                return SETTINGS.btnDoubleLeft;
-              case ButtonEventManager::PressType::Long:
-                return SETTINGS.btnLongLeft;
-            }
-            break;
-          case B::Right:
-            switch (ev.type) {
-              case ButtonEventManager::PressType::Short:
-                return SETTINGS.btnShortRight;
-              case ButtonEventManager::PressType::Double:
-                return SETTINGS.btnDoubleRight;
-              case ButtonEventManager::PressType::Long:
-                return SETTINGS.btnLongRight;
-            }
-            break;
-          case B::PageBack:
-            switch (ev.type) {
-              case ButtonEventManager::PressType::Short:
-                return SETTINGS.btnShortPageBack;
-              case ButtonEventManager::PressType::Double:
-                return SETTINGS.btnDoublePageBack;
-              case ButtonEventManager::PressType::Long:
-                return SETTINGS.btnLongPageBack;
-            }
-            break;
-          case B::PageForward:
-            switch (ev.type) {
-              case ButtonEventManager::PressType::Short:
-                return SETTINGS.btnShortPageForward;
-              case ButtonEventManager::PressType::Double:
-                return SETTINGS.btnDoublePageForward;
-              case ButtonEventManager::PressType::Long:
-                return SETTINGS.btnLongPageForward;
-            }
-            break;
-          case B::Power:
-            switch (ev.type) {
-              case ButtonEventManager::PressType::Short:
-                return SETTINGS.btnShortPower;
-              case ButtonEventManager::PressType::Double:
-                return SETTINGS.btnDoublePower;
-              case ButtonEventManager::PressType::Long:
-                return SETTINGS.btnLongPower;
-            }
-            break;
-          default:
-            break;  // Up/Down have no FSMs — ButtonEventManager never emits these
-        }
-        return BA::BTN_DEFAULT;
-      };
-
-      const uint8_t action = actionFor(ev.button);
       if (action == BA::BTN_DEFAULT) {
+        if (ev.type == ButtonEventManager::PressType::Double) {
+          continue;
+        }
         defaultEvents.push_back(ev);
         continue;
       }