cleanup for PR

Author: shadow578

Marlin/src/lcd/e3v2/common/dwin_api.cpp

@@ -192,51 +192,47 @@ void dwinFrameClear(const uint16_t color) {
       const uint16_t map_columns,
       const uint16_t map_rows,
       const uint8_t *map_data) {
-    // how many bytes can we write to the send buffer?
-    // one byte is used for F_HONE, so we can write up to len(dwinSendBuf) - 1 bytes.
-    constexpr size_t send_buffer_size = (COUNT(dwinSendBuf) - 1);
-    // at how many bytes should we flush the send buffer?
-    // one byte is used (hidden) for F_HONE, and we need 4 bytes when appending a point.
-    // so we should flush the send buffer when we have less than 5 bytes left.
+    // At how many bytes should we flush the send buffer?
+    // One byte is used (hidden) for F_HONE, and we need 4 bytes when appending a point.
+    // So we should flush the send buffer when we have less than 5 bytes left.
     constexpr size_t flush_send_buffer_at = (COUNT(dwinSendBuf) - 1 - 4);
 
-    // how long is the header of each draw command?
-    // 1B CMD, 2B COLOR, 1B WIDTH, 1B HEIGHT
+    // How long is the header of each draw command?
+    // => 1B CMD, 2B COLOR, 1B WIDTH, 1B HEIGHT
     constexpr size_t command_header_size = 5;
 
-    // draw the point map
     size_t i = 0;
     for (uint16_t row = 0; row < map_rows; row++) {
       for (uint16_t col = 0; col < map_columns; col++) {
         const uint8_t map_byte = map_data[(row * map_columns) + col];
         for (uint8_t bit = 0; bit < 8; bit++) {
-          // draw the bit of the byte if it's set
+          // Draw a point at this position?
           if (TEST(map_byte, bit)) {
-            // flush the send buffer and prepare next draw if either
-            // a) the buffer reached the 'should flush' state, or
-            // b) this is the first point to draw
+            // Flush the send buffer and prepare next draw if either
+            // a) The buffer reached the 'should flush' state, or
+            // b) This is the first point to draw
             if (i >= flush_send_buffer_at || i == 0)
             {
-              // dispatch the current draw command
+              // Dispatch the current draw command
               if (i > command_header_size) dwinSend(i);
 
-              // prepare the next draw command
+              // Prepare the next draw command
               i = 0;
               dwinByte(i, 0x02); // cmd: draw point(s)
               dwinWord(i, color);
               dwinByte(i, point_width);
               dwinByte(i, point_height);
             }
 
-            // append point coordinates to draw command
+            // Append point coordinates to draw command
             dwinWord(i, x + (point_width * ((8 * col) + (7 - bit)))); // x
             dwinWord(i, y + (point_height * (row)));                  // y
           }
         }
       }
     }
 
-    // dispatch final draw command if the buffer contains any points
+    // Dispatch final draw command if the buffer contains any points
     if (i > command_header_size) dwinSend(i);
   }
 #endif

Marlin/src/lcd/e3v2/marlinui/game.cpp

@@ -2,72 +2,90 @@
 
 #if IS_DWIN_MARLINUI && HAS_GAMES
 
-#define PERFORMANCE_COUNTERS 1
+// Enable performance counters (draw call count, frame timing) for debugging
+#define GAME_PERFORMANCE_COUNTERS 1
 
 #include "../../menu/game/types.h" // includes e3v2/marlinui/game.h
 #include "../../lcdprint.h"
 #include "lcdprint_dwin.h"
 #include "marlinui_dwin.h"
 
-#if PERFORMANCE_COUNTERS == 1
-  static uint32_t draw_call_cnt = 0;     // total number of draw calls in the current frame
-  static millis_t frame_draw_millis = 0, // time spent drawing the frame 
-                  frame_wait_millis = 0; // time spent waiting for the next frame
-
-  #define DRAW_CALL_INC(subcall_cnt) draw_call_cnt++
-#else
-  #define DRAW_CALL_INC(subcall_cnt)
+#if ENABLED(GAME_PERFORMANCE_COUNTERS)
+  typedef struct {
+    /**
+     * Number of draw calls sent to the LCD
+     */
+    uint32_t draw_calls;
+
+    /**
+     * millis() value at the start of the current frame
+     */
+    millis_t frame_draw_millis;
+
+    /**
+     * millis() value at the end of the previous frame (in frame_start)
+     * or time spend waiting for the next frame (in frame_end)
+     */
+    millis_t frame_wait_millis;
+  } dwin_game_perf_t;
+
+  static dwin_game_perf_t dwin_game_perf;  
 #endif
 
+#define COUNT_DRAW_CALLS(n) TERN_(GAME_PERFORMANCE_COUNTERS, dwin_game_perf.draw_calls += n)
+
 void MarlinGame::frame_start() {
-  // clear the screen before each frame
+  // Clear the screen before each frame
   //dwinFrameClear(CLEAR_COLOR);
 
-  // filling the play area should be faster than clearing the whole screen
+  // Instead of using dwinFrameClear, fill the play area with the background color
+  // This tends to be faster than clearing the whole screen
   const uint16_t fg = dwin_font.fg;
   dwin_font.fg = COLOR_BG_BLACK;
   draw_box(0, 0, GAME_WIDTH, GAME_HEIGHT);
   dwin_font.fg = fg;
 
+  // Ensure the correct font is selected
   dwin_font.index = DWIN_FONT_MENU;
 
-  // reset performance counters
-  #if PERFORMANCE_COUNTERS == 1
-    draw_call_cnt = 0;
-    frame_draw_millis = millis();
-    frame_wait_millis = frame_draw_millis - frame_wait_millis; 
+  #if ENABLED(GAME_PERFORMANCE_COUNTERS)
+    // Reset draw call counters
+    dwin_game_perf.draw_calls = 0;
+
+    // Update timing information
+    const millis_t now = millis();
+    dwin_game_perf.frame_draw_millis = now;
+    dwin_game_perf.frame_wait_millis = now - dwin_game_perf.frame_wait_millis;
   #endif
 }
 
 void MarlinGame::frame_end() {
-  #if PERFORMANCE_COUNTERS == 1
-    const millis_t frame_wait = frame_wait_millis;
-    frame_wait_millis = millis();
-    frame_draw_millis = frame_wait_millis - frame_draw_millis;
-
-    // calculate frames per deci-seconds (100 milliseconds)
-    const uint32_t fpds = 100 / (frame_draw_millis + frame_wait);
+  #if ENABLED(GAME_PERFORMANCE_COUNTERS)
+    const millis_t now = millis();
+    const millis_t frame_wait_millis = dwin_game_perf.frame_wait_millis;
+    const millis_t frame_draw_millis = now - dwin_game_perf.frame_draw_millis;
 
-    // format the performance counters as a string
-    char perf_str[64];
-    sprintf_P(
-      perf_str,
-      PSTR("d%04lu w%04lu c%04lu f%02lu "),
-      frame_draw_millis,
-      frame_wait,
-      draw_call_cnt,
-      fpds
-    );
+    dwin_game_perf.frame_wait_millis = now;
 
-    // draw the performance counters at the (physical) origin of the screen
+    // Save previous font settings and set new ones
     const uint16_t fg = dwin_font.fg;
     const bool solid = dwin_font.solid;
-    dwin_font.fg = RGB(0x1F, 0x3F, 0x00);
+    set_color(color::YELLOW);
     dwin_font.solid = true;
 
+    // Draw performance counters information
+    char perf_str[32];
+    sprintf_P(
+      perf_str,
+      PSTR("d%04lu w%04lu c%04lu"),
+      frame_draw_millis,
+      frame_wait_millis,
+      dwin_game_perf.draw_calls
+    );
     lcd_moveto_xy(0, 0);
     lcd_put_u8str(perf_str);
 
+    // Restore previous font settings
     dwin_font.fg = fg;
     dwin_font.solid = solid;
   #endif
@@ -107,17 +125,17 @@ void MarlinGame::set_color(const color color) {
 }
 
 void MarlinGame::draw_hline(const game_dim_t x, const game_dim_t y, const game_dim_t w) {
-  // draw lines as boxes, since DWIN lines are always 1px wide but we want to scale them
+  // Draw lines as boxes, since DWIN lines are always 1px wide but we want to scale them
   draw_box(x, y, w, 1);
 
-  DRAW_CALL_INC(1);
+  COUNT_DRAW_CALLS(1);
 }
 
 void MarlinGame::draw_vline(const game_dim_t x, const game_dim_t y, const game_dim_t h) {
-  // draw lines as boxes, since DWIN lines are always 1px wide but we want to scale them
+  // Draw lines as boxes, since DWIN lines are always 1px wide but we want to scale them
   draw_box(x, y, 1, h);
 
-  DRAW_CALL_INC(1);
+  COUNT_DRAW_CALLS(1);
 }
 
 void MarlinGame::draw_frame(const game_dim_t x, const game_dim_t y, const game_dim_t w, const game_dim_t h) {
@@ -130,7 +148,7 @@ void MarlinGame::draw_frame(const game_dim_t x, const game_dim_t y, const game_d
     dwin_game::game_to_screen(h)
   );
 
-  DRAW_CALL_INC(0);
+  COUNT_DRAW_CALLS(1);
 }
 
 void MarlinGame::draw_box(const game_dim_t x, const game_dim_t y, const game_dim_t w, const game_dim_t h) {
@@ -143,17 +161,15 @@ void MarlinGame::draw_box(const game_dim_t x, const game_dim_t y, const game_dim
     dwin_game::game_to_screen(h)
   );
 
-  DRAW_CALL_INC(0);
+  COUNT_DRAW_CALLS(1);
 }
 
 void MarlinGame::draw_pixel(const game_dim_t x, const game_dim_t y) {
-  // draw pixels using boxes.
-  // while DWIN protocol supports drawing points with different sizes, the
-  // 0x02 'draw point' command is way slower per pixel than 0x05 'fill rectangle'
+  // Draw pixels using boxes.
+  // While DWIN protocol supports drawing points with different sizes, the
+  // 0x02 'draw point' command is slower per pixel than 0x05 'fill rectangle'
   // (0.4 us vs 0.14 us per pixel)
   draw_box(x, y, 1, 1);
-
-  DRAW_CALL_INC(1);
 }
 
 void MarlinGame::draw_bitmap(const game_dim_t x, const game_dim_t y, const game_dim_t bytes_per_row, const game_dim_t rows, const pgm_bitmap_t bitmap) {
@@ -163,20 +179,19 @@ void MarlinGame::draw_bitmap(const game_dim_t x, const game_dim_t y, const game_
 
   #if DISABLED(TJC_DISPLAY)
     // DWIN T5UI actually supports drawing multiple points in one go using the 0x02 'draw point' command, ever since kernel 1.2.
-    // So we use that to draw the bitmap as a series of points, which should be faster than drawing rectangles using draw_pixel.
-    // This will be somewhat slow, but way faster than drawing rectangles one by one.
+    // So we use that to draw the bitmap as a series of points, which is faster than drawing rectangles using draw_pixel.
     dwinDrawPointMap(
-      dwin_font.fg, // color
-      dwin_game::game_to_screen(1), // point size
+      dwin_font.fg,
+      dwin_game::game_to_screen(1),
       dwin_game::game_to_screen(1),
-      dwin_game::game_to_screen(x) + dwin_game::x_offset, // x / y
+      dwin_game::game_to_screen(x) + dwin_game::x_offset,
       dwin_game::game_to_screen(y) + dwin_game::y_offset,
-      bytes_per_row, // bitmap dimensions
+      bytes_per_row,
       rows,
-      bitmap // U8G bitmap format is compatible to DrawPointMap format
+      bitmap
     );
 
-    DRAW_CALL_INC(0);
+    COUNT_DRAW_CALLS(1);
   #else
     // TJC displays don't seem to support the 0x02 'draw point' command, so instead we have to draw the bitmap
     // as a series of rectangles using draw_pixel.
@@ -185,10 +200,10 @@ void MarlinGame::draw_bitmap(const game_dim_t x, const game_dim_t y, const game_
       for (game_dim_t col = 0; col < bytes_per_row; col++) {
         const uint8_t byte = bitmap[(row * bytes_per_row) + col];
         for (uint8_t bit = 0; bit < 8; bit++) {
-          // assume that the screen area is cleared before drawing
+          // Assuming that the drawing area was cleared before drawing
           if (byte & (1 << bit)) {
             draw_pixel(x + (col * 8) + (7 - bit + 1), y + row);
-            DRAW_CALL_INC(1);
+            COUNT_DRAW_CALLS(1);
           }
         }
       }
@@ -197,7 +212,7 @@ void MarlinGame::draw_bitmap(const game_dim_t x, const game_dim_t y, const game_
 }
 
 int MarlinGame::draw_string(const game_dim_t x, const game_dim_t y, const char* str) {
-  DRAW_CALL_INC(0);
+  COUNT_DRAW_CALLS(1);
 
   lcd_moveto_xy(
     dwin_game::game_to_screen(x) + dwin_game::x_offset,
@@ -215,7 +230,7 @@ int MarlinGame::draw_string(const game_dim_t x, const game_dim_t y, FSTR_P const
 }
 
 void MarlinGame::draw_int(const game_dim_t x, const game_dim_t y, const int value) {
-  COUNT_DRAW_CALL(0);
+  COUNT_DRAW_CALLS(1);
 
   lcd_moveto_xy(
     dwin_game::game_to_screen(x) + dwin_game::x_offset,

Marlin/src/lcd/e3v2/marlinui/game.h

@@ -15,13 +15,13 @@ namespace dwin_game {
 
   constexpr int calculate_scale()
   {
-    // use whichever is smaller: the width or height scaling factor
+    // Use whichever is smaller: the width or height scaling factor
     float scaling_factor = _MIN(
       static_cast<float>(DWIN_WIDTH) / static_cast<float>(TARGET_WIDTH),
       static_cast<float>(DWIN_HEIGHT) / static_cast<float>(TARGET_HEIGHT)
     );
 
-    // round DOWN to closest integer
+    // Round DOWN to closest integer
     return static_cast<int>(scaling_factor);
   }
 
@@ -31,14 +31,14 @@ namespace dwin_game {
   constexpr int scale = calculate_scale();
 
   /**
-   * @brief scale a game dimension to screen dimensions
+   * @brief Scale a game dimension to screen dimensions
    */
   constexpr game_dim_t screen_to_game(const screen_dim_t x) {
     return x / scale;
   }
 
   /**
-   * @brief scale a screen dimension to game dimensions
+   * @brief Scale a screen dimension to game dimensions
    */
   constexpr screen_dim_t game_to_screen(const game_dim_t x) {
     return x * scale;
@@ -59,7 +59,7 @@ constexpr game_dim_t GAME_HEIGHT = dwin_game::screen_to_game(DWIN_HEIGHT - (dwin
 constexpr game_dim_t GAME_FONT_WIDTH = dwin_game::screen_to_game(MENU_FONT_WIDTH);
 constexpr game_dim_t GAME_FONT_ASCENT = dwin_game::screen_to_game(MENU_FONT_ASCENT);
 
-// not needed on DWIN
+// DWIN screens don't page, so these macros are always true
 #define PAGE_OVER(ya) true
 #define PAGE_UNDER(yb) true
 #define PAGE_CONTAINS(ya, yb) true

Marlin/src/lcd/menu/game/brickout.cpp

@@ -144,25 +144,22 @@ void BrickoutGame::game_screen() {
       const uint8_t yy = y * BRICK_H + BRICK_TOP;
       if (PAGE_CONTAINS(yy, yy + BRICK_H - 1)) {
         for (uint8_t x = 0; x < BRICK_COLS; ++x) {
-          // cycle through colors, even if the brick is gone
-          // otherwise, bricks would change color if their neighbor is hit
+          // Cycle through colors, even if the brick is gone.
+          // Otherwise, bricks would change color if their neighbor is hit
           set_color(brick_colors[color_index++ % COUNT(brick_colors)]);
 
-          // draw brick if it's still there
+          // Draw brick if it's still there
           if (TEST(bdat.bricks[y], x)) {
             const uint8_t xx = x * BRICK_W;
-            draw_box(xx, yy, BRICK_W - 1, BRICK_H - 1);
-            
-            //for (uint8_t v = 0; v < BRICK_H - 1; ++v)
-            //  if (PAGE_CONTAINS(yy + v, yy + v))
-            //    draw_hline(xx, yy + v, BRICK_W - 1);
+            if (PAGE_CONTAINS(yy, yy + BRICK_H - 1))
+              draw_box(xx, yy, BRICK_W - 1, BRICK_H - 1);
           }
         }
       }
     }
   }
 
-  // everything else is white
+  // Everything else is white
   set_color(color::WHITE);
 
   // Draw paddle