Clean up flush callback.

esp/cpp/07_lcd-panel-i2c/main/panel.h

@@ -64,7 +64,7 @@ struct Panel {
    */
   inline void register_display_callbacks(lv_display_t *display_handle) const
   {
-    device_->register_draw_buffer(display_handle, esp_io_);
+    device_->register_rendering_data(display_handle, esp_io_);
     device_->register_lvgl_tick_timer();
   }
 

esp/cpp/07_lcd-panel-i2c/main/panel_device.cpp

@@ -4,9 +4,6 @@
 #include "scoped_lock.h"
 #include "time_keeper.h"
 
-// To use LV_COLOR_FORMAT_I1 we need an extra buffer to hold the converted data.
-uint8_t IPanelDevice::oled_buffer_[LCD_H_RES * LCD_V_RES / 8];
-
 bool IPanelDevice::lvgl_flush_ready_cb(esp_lcd_panel_io_handle_t,
                                        esp_lcd_panel_io_event_data_t *,
                                        void *user_ctx)
@@ -23,6 +20,7 @@ void IPanelDevice::lvgl_flush_cb(lv_display_t *display, const lv_area_t *area,
       (esp_lcd_panel_handle_t) lv_display_get_user_data(display);
 
   // Necessary because LVGL reserves 2x4 bytes in the buffer for a palette.
+  // Since we are monochrome, we don't need these additional bytes.
   // For more information about the monochrome, please refer to:
   // https://docs.lvgl.io/9.2/porting/display.html#monochrome-displays
   // Skip the palette here.
@@ -34,31 +32,40 @@ void IPanelDevice::lvgl_flush_cb(lv_display_t *display, const lv_area_t *area,
   int32_t y1 = area->y1;
   int32_t y2 = area->y2;
 
+
+  // As of 03/01/2025 master branch of LVGL contains this helper for the same.
+  // https://docs.lvgl.io/master/API/draw/sw/lv_draw_sw_utils.html
+  // lv_draw_sw_i1_convert_to_vtiled()
   for (int32_t y = y1; y <= y2; y++) {
     for (int32_t x = x1; x <= x2; x++) {
-      /* The order of bits is MSB first.
+      Pixel pixel(x, y, hor_res);
-                  MSB           LSB
-         bits      7 6 5 4 3 2 1 0
-         pixels    0 1 2 3 4 5 6 7
-                  Left         Right
-      */
-      bool chroma_color = (px_map[(hor_res >> 3) * y + (x >> 3)] &
-                           1 << (7 - x % 8));
 
-      // Write to the buffer as required for the display.
+      // True if the pixel is lit, else false.
-      // It writes only 1-bit for monochrome displays mapped vertically.
+      bool chroma_color =
-      uint8_t *buf = IPanelDevice::oled_buffer_ + hor_res * (y >> 3) + (x);
+          px_map[pixel.horizontal_byte_offset()] & Pixel::msb_mask(x);
+
+      // We need an additional buffer for transposing the pixel data to the
+      // vertical format required by the display driver.
+      uint8_t *buf = IPanelDevice::get_additional_draw_buffer();
+      // Move to the position in the auxillary buffer where the pixel is stored.
+      buf += pixel.vertical_byte_offset();
+
+      // Write the single bit to the monochrome display mapped vertically.
+      // Take the Least Significant Bit mask of the Y coordinate to select the
+      // bit representing a pixel at position y in a vertically-mapped display.
       if (chroma_color) {
-        (*buf) &= ~(1 << (y % 8));
+        // Set the vertically-mapped pixel to on.
+        *buf &= ~Pixel::lsb_mask(y);
       } else {
-        (*buf) |= (1 << (y % 8));
+        // Set the vertically-mapped pixel to off.
+        *buf |= Pixel::lsb_mask(y);
       }
     }
   }
   // Pass the draw buffer to the driver.
   ESP_ERROR_CHECK(
       esp_lcd_panel_draw_bitmap(panel_handle, x1, y1, x2 + 1, y2 + 1,
-                                IPanelDevice::oled_buffer_));
+                                IPanelDevice::get_additional_draw_buffer()));
 }
 
 void IPanelDevice::lvgl_increase_tick_cb(void *)
@@ -83,7 +90,7 @@ void IPanelDevice::lvgl_increase_tick_cb(void *)
   }
 }
 
-void IPanelDevice::register_draw_buffer(lv_display_t *display_handle,
+void IPanelDevice::register_rendering_data(lv_display_t *display_handle,
                                            esp_lcd_panel_io_handle_t io_handle)
 {
   // Create draw buffer.

esp/cpp/07_lcd-panel-i2c/main/panel_device.h

@@ -20,6 +20,102 @@
 #define LCD_H_RES              128
 #define LCD_V_RES              64
 
+/**
+ * Wraps some foundational operations performed on pixel coordinates when
+ * dealing with pointer arithmetic. Most of these could be done ad-hoc as needed
+ * but using this helper reduces the risk of errors.
+ */
+struct Pixel {
+  Pixel(int32_t x, int32_t y, uint16_t horizontal_res) :
+      x_(x), y_(y), hor_res_(horizontal_res) { }
+
+  /**
+   * Calculate byte offset for the pixel at [x,y] within a horizontally-mapped
+   * monochrome uint8 draw buffer, using the initialized horizontal resolution.
+   *
+   * We use `>> 3` because each pixel requires 1 bit, but each uint8 in the draw
+   * buffer can hold 8 bits. To find the uint8 value in our draw buffer that
+   * stores this pixel's value we must compensate for this when using pixel
+   * coordinates in byte math.
+   *
+   * Therefore, each uint8 in the draw buffer stores the state of 8 pixels.
+   * Below is an example of calculating for [x, y] pixel coordinates [20, 10].
+   *
+   * For the horizontal case, each row (y_) of the image is represented by
+   * `hor_res_ >> 3` bytes (16). The byte-offset of the first pixel in the 10th
+   * row for example is `16 * 10` = 160.
+   *
+   * Since the pixels are stored horizontally we must calculate the 20th pixel
+   * column (x_) as `160 + (20 >> 3)`, or `160 + (20 / 8)` to get a final offset
+   * of 162.
+   *
+   * @return byte offset for a single-byte monochrome pixel at [x,y].
+   */
+  [[nodiscard]] ptrdiff_t horizontal_byte_offset() const
+  {
+    // Convert pixel (bit) coordinates to byte coordinates in the draw buffer.
+    return (hor_res_ >> 3) * y_ + (x_ >> 3);
+  }
+
+  /**
+   * Calculate byte offset for the pixel at [x,y] within a vertically-mapped
+   * monochrome uint8 draw buffer, using the initialized horizontal resolution.
+   *
+   * We use `>> 3` because each pixel requires 1 bit, but each uint8 in the draw
+   * buffer can hold 8 bits. To find the uint8 value in our draw buffer that
+   * stores this pixel's value we must compensate for this when using pixel
+   * coordinates in byte math.
+   *
+   * Therefore, each uint8 in the draw buffer stores the state of 8 pixels.
+   * Below is an example of calculating for [x, y] pixel coordinates [20, 10].
+   *
+   * For the vertical case, each row (y_) of the image is represented by
+   * `hor_res_` bytes (128) - one for each column (x_). Because the pixels are
+   * stored vertically, the byte-offset of the first pixel in the 10th row is
+   * `128 * (10 >> 3)` or * `128 * (10 / 8)` = 128.
+   *
+   * From this location we can simply calculate the 20th pixel column (x_) as
+   * `128 + 20` to get a final offset of 148, because the pixels are stored in a
+   * columnar format.
+   *
+   */
+  [[nodiscard]] ptrdiff_t vertical_byte_offset() const
+  {
+    // Convert pixel (bit) coordinates to byte coordinates in the draw buffer.
+    return hor_res_ * (y_ >> 3) + x_;
+  }
+
+  /**
+   * Finds the Most Significant Bit location of bit `i` in a byte.
+   *
+   *          MSB           LSB
+   * bits      7 6 5 4 3 2 1 0
+   * data      8 7 6 5 4 3 2 1
+   *          Left         Right
+   *
+   * @return bitmask for MSB location of `i`.
+   */
+  [[maybe_unused]] [[nodiscard]] static uint8_t
+  msb_mask(const int32_t &i) { return 1 << (7 - i % 8); }
+
+  /**
+   * Finds the Least Significant Bit location of bit `i` in a byte.
+   *
+   *          LSB           MSB
+   * bits      0 1 2 3 4 5 6 7
+   * data      1 2 3 4 5 6 7 8
+   *          Left         Right
+   *
+   * @return bitmask for LSB location of `i`.
+   */
+  [[maybe_unused]] [[nodiscard]] static uint8_t
+  lsb_mask(const int32_t &i) { return 1 << (i % 8); }
+
+  int32_t x_, y_;
+  uint16_t hor_res_;
+};
+
+
 /**
  * Encapsulates vendor specific ESP LCD panel initialization logic.
  * This pure virtual interface can be inherited from for using new LCD devices.
@@ -146,7 +242,7 @@ public:
   esp_lcd_panel_io_i2c_config_t esp_io_config_;
 
   /**
-   * Registers LVGL draw buffers for this display.
+   * Registers LVGL draw buffers and callbacks for this display.
    *
    * An implementation of the interface can optionally override this method to
    * provide custom LVGL callbacks and display configurations.
@@ -154,7 +250,7 @@ public:
    * @param display_handle LVGL display handle to use for rendering.
    * @param io_handle IO handle for the ESP LCD panel.
    */
-  virtual void register_draw_buffer(lv_display_t *display_handle,
+  virtual void register_rendering_data(lv_display_t *display_handle,
                                        esp_lcd_panel_io_handle_t io_handle);
 
   /**
@@ -165,6 +261,30 @@ public:
    */
   virtual void register_lvgl_tick_timer();
 
+protected:
+  /**
+   * Static accessor to a static buffer to store draw buffer data for the panel.
+   *
+   * This method is protected to allow an implementation to provide a custom
+   * callback method similar to IPanelDevice::lvgl_flush_cb.
+   *
+   * The buffer is allocated statically within the scope of this function to
+   * allow creating multiple panels that _each_ manage their own statically
+   * allocated draw buffer data. This simplifies implementing the interface by
+   * taking this responsibility off of the implementor. The buffer will only be
+   * allocated if this method is called, so the memory is only used if required.
+   *
+   * @return Pointer to uint8 draw buffer data.
+   * @sa register_rendering_data for overriding LVGL rendering callbacks.
+   */
+  static uint8_t *get_additional_draw_buffer()
+  {
+    // Static to the scope of this function, not the compilation unit.
+    // For LV_COLOR_FORMAT_I1 we need an extra buffer to hold converted data.
+    static uint8_t oled_buffer[LCD_H_RES * LCD_V_RES / 8];
+    return oled_buffer;
+  }
+
 private:
 
   //
@@ -206,10 +326,44 @@ private:
    * `px_map` contains the rendered image as raw pixel map and it should be
    *  copied to `area` on the display.
    *
+   * The following details are crucial for understanding the logic surrounding
+   * flushing to the display in this example.
+   *
+   * The order of bits within the px_map from _LVGL_ is MSB first.
+   *              MSB           LSB
+   *     bits      7 6 5 4 3 2 1 0
+   *     pixels    0 1 2 3 4 5 6 7
+   *              Left         Right
+   *
+   * The bytes from _LVGL_ are mapped to pixel rows of the display
+   * [0, 0, 0, 0]
+   * [0, 0, 0, 0]
+   * [0, 0, 0, 0]
+   *
+   * The order of bits expected by the _display driver_ is LSB first.
+   * We must preserve pairing of each bit and pixel when writing to the display.
+   *              LSB           MSB
+   *     bits      0 1 2 3 4 5 6 7
+   *     pixels    7 6 5 4 3 2 1 0
+   *              Left         Right
+   *
+   * Bytes expected by the _display driver_ map to pixel columns of the display.
+   * [0, [0, [0, [0,
+   *  0,  0,  0,  0,
+   *  0]  0]  0]  0]
+   *
+   * For the LV_COLOR_FORMAT_I1 color format we are using, an additional buffer
+   * is needed for transposing the bits to the vertical arrangement required by
+   * the display driver that is outlined above.
+   *
+   * This callback implementation is an example of handling this transposition
+   * and flushing the data to the display in the expected format.
+   *
    * @param display LVGL display handle to use for rendering.
    * @param area Area of the display being flushed.
    * @param px_map Rendered image data for writing to the display area.
    * @sa register_rendering_data for overriding this callback.
+   * @sa get_additional_draw_buffer
    */
   static void lvgl_flush_cb(lv_display_t *display,
                             const lv_area_t *area,
@@ -241,12 +395,6 @@ private:
    */
   [[noreturn]] static void lvgl_port_task(void *data);
 
-  /**
-   * Draw buffer for this panel device.
-   * For LV_COLOR_FORMAT_I1 we need an extra buffer to hold the converted data.
-   */
-  static uint8_t oled_buffer_[LCD_H_RES * LCD_V_RES / 8];
-
   //
   // PRIVATE MEMBERS