Refine graphical effective columns

src/renderer/GraphicalDisplayRenderer.cpp

@@ -304,10 +304,11 @@ void GraphicalDisplayRenderer::drawItem(const char* text, const char* value, boo
         }
     }
 
-    gDisplay->setDrawColor(1);
     if (cursorRow == 0) {
         drawScrollBar();
     }
+
+    gDisplay->setDrawColor(1);
 }
 
 void GraphicalDisplayRenderer::clearBlinker() {}
@@ -382,18 +383,45 @@ uint8_t GraphicalDisplayRenderer::getMaxRows() const {
 }
 
 uint8_t GraphicalDisplayRenderer::getMaxCols() const {
-    uint8_t w = maxFontWidth == 0 ? 1 : maxFontWidth;
+    uint8_t w = maxFontWidth;
+    if (w == 0) {
+        w = gDisplay->getFontWidth();
+    }
+    if (w == 0) {
+        w = 1;
+    }
     return gDisplay->getDisplayWidth() / w;
 }
 
 uint8_t GraphicalDisplayRenderer::getEffectiveCols() const {
-    uint8_t w = gDisplay->getFontWidth();
-    if (w == 0) {
-        w = 1;
+    uint8_t charW = gDisplay->getFontWidth();
+    if (charW == 0) {
+        charW = 1;
     }
+
     uint8_t rightInset = totalItems > getMaxRows() ? scrollbarWidth + scrollbarGap : 0;
-    uint8_t usable = gDisplay->getDisplayWidth() > rightInset ? gDisplay->getDisplayWidth() - rightInset : 0;
-    return usable / w;
+    uint8_t usable = gDisplay->getDisplayWidth();
+    if (usable <= rightInset + leftPadding) {
+        return 0;
+    }
+    usable -= rightInset + leftPadding;
+
+    uint8_t cols = usable / charW;
+
+    uint8_t iconWidth = measureText(cursorIcon);
+    uint8_t editIconWidth = measureText(editCursorIcon);
+    if (editIconWidth > iconWidth) {
+        iconWidth = editIconWidth;
+    }
+
+    uint8_t iconCols = static_cast<uint8_t>((iconWidth + charW - 1) / charW);
+    if (cols > iconCols) {
+        cols -= iconCols;
+    } else {
+        cols = 0;
+    }
+
+    return cols;
 }
 
 void GraphicalDisplayRenderer::drawScrollBar() {

src/renderer/GraphicalDisplayRenderer.h

@@ -19,6 +19,8 @@ class GraphicalDisplayRenderer : public MenuRenderer,
                                  public GraphicalValueSelectionRenderer,
                                  public GraphicalRendererContext {
   private:
+    friend class FocusableGraphicalDisplayRenderer;
+
     GraphicalDisplayInterface* gDisplay;
     const uint8_t* defaultFont = NULL;
 

test/GraphicalDisplayRenderer.cpp

@@ -57,21 +57,46 @@ class StubGraphicalDisplay : public GraphicalDisplayInterface {
 
 class FocusableGraphicalDisplayRenderer : public GraphicalDisplayRenderer {
   public:
+    using GraphicalDisplayRenderer::getEffectiveCols;
+    using GraphicalDisplayRenderer::getMaxCols;
     using GraphicalDisplayRenderer::GraphicalDisplayRenderer;
 
     void setFocusForTest(bool focused) {
         hasFocus = focused;
     }
 };
 
+unittest(graphical_renderer_reports_effective_cols_with_cursor_icons) {
+    StubGraphicalDisplay display;
+
+    FocusableGraphicalDisplayRenderer plain(&display);
+    FocusableGraphicalDisplayRenderer iconed(&display, NULL, "[]", "[e]");
+
+    assertEqual(21, plain.getEffectiveCols());
+    assertEqual(18, iconed.getEffectiveCols());
+
+    iconed.setViewportContext(0, 9);
+    assertEqual(17, iconed.getEffectiveCols());
+}
+
+unittest(graphical_renderer_reports_max_cols_from_font_width) {
+    StubGraphicalDisplay display;
+    FocusableGraphicalDisplayRenderer renderer(&display);
+
+    renderer.begin();
+
+    assertEqual(21, renderer.getMaxCols());
+}
+
 unittest(graphical_renderer_exposes_value_selection_extension) {
     StubGraphicalDisplay display;
     GraphicalDisplayRenderer renderer(&display);
 
     void* extension = renderer.queryExtension(GraphicalValueSelectionRenderer::extensionId());
     assertTrue(extension != NULL);
 
-    GraphicalValueSelectionRenderer* selection = static_cast<GraphicalValueSelectionRenderer*>(extension);
+    GraphicalValueSelectionRenderer* selection =
+        static_cast<GraphicalValueSelectionRenderer*>(extension);
     selection->setValueSelection(1, 2);
     selection->clearValueSelection();
 }