[QP] Minor cleanup and support for RGB888 surface (#25706)

Co-authored-by: Drashna Jaelre <drashna@live.com>

8 files changed, 34 insertions, 46 deletions

diff --git a/quantum/painter/qff.h b/quantum/painter/qff.h
index ed88508d73..f5c59b315b 100644
--- a/quantum/painter/qff.h
+++ b/quantum/painter/qff.h
@@ -22,7 +22,7 @@
 
 #define QFF_FONT_DESCRIPTOR_TYPEID 0x00
 
-typedef struct QP_PACKED qff_font_descriptor_v1_t {
+typedef struct PACKED qff_font_descriptor_v1_t {
     qgf_block_header_v1_t header;              // = { .type_id = 0x00, .neg_type_id = (~0x00), .length = 20 }
     uint32_t              magic : 24;          // constant, equal to 0x464651 ("QFF")
     uint8_t               qff_version;         // constant, equal to 0x01
@@ -51,13 +51,13 @@ STATIC_ASSERT(sizeof(qff_font_descriptor_v1_t) == (sizeof(qgf_block_header_v1_t)
 #define QFF_GLYPH_OFFSET_BITS 18
 #define QFF_GLYPH_OFFSET_MASK (((1 << QFF_GLYPH_OFFSET_BITS) - 1) << QFF_GLYPH_WIDTH_BITS)
 
-typedef struct QP_PACKED qff_ascii_glyph_v1_t {
+typedef struct PACKED qff_ascii_glyph_v1_t {
     uint32_t value : 24; // Uses QFF_GLYPH_*_(BITS|MASK) as bitfield ordering is compiler-defined
 } qff_ascii_glyph_v1_t;
 
 STATIC_ASSERT(sizeof(qff_ascii_glyph_v1_t) == 3, "qff_ascii_glyph_v1_t must be 3 bytes in v1 of QFF");
 
-typedef struct QP_PACKED qff_ascii_glyph_table_v1_t {
+typedef struct PACKED qff_ascii_glyph_table_v1_t {
     qgf_block_header_v1_t header;    // = { .type_id = 0x01, .neg_type_id = (~0x01), .length = 285 }
     qff_ascii_glyph_v1_t  glyph[95]; // 95 glyphs, 0x20..0x7E
 } qff_ascii_glyph_table_v1_t;
@@ -69,14 +69,14 @@ STATIC_ASSERT(sizeof(qff_ascii_glyph_table_v1_t) == (sizeof(qgf_block_header_v1_
 
 #define QFF_UNICODE_GLYPH_DESCRIPTOR_TYPEID 0x02
 
-typedef struct QP_PACKED qff_unicode_glyph_v1_t {
+typedef struct PACKED qff_unicode_glyph_v1_t {
     uint32_t code_point : 24;
     uint32_t value : 24; // Uses QFF_GLYPH_*_(BITS|MASK) as bitfield ordering is compiler-defined
 } qff_unicode_glyph_v1_t;
 
 STATIC_ASSERT(sizeof(qff_unicode_glyph_v1_t) == 6, "qff_unicode_glyph_v1_t must be 6 bytes in v1 of QFF");
 
-typedef struct QP_PACKED qff_unicode_glyph_table_v1_t {
+typedef struct PACKED qff_unicode_glyph_table_v1_t {
     qgf_block_header_v1_t  header;   // = { .type_id = 0x02, .neg_type_id = (~0x02), .length = (N * 6) }
     qff_unicode_glyph_v1_t glyph[0]; // Extent of '0' signifies that this struct is immediately followed by the glyph data
 } qff_unicode_glyph_table_v1_t;
diff --git a/quantum/painter/qgf.c b/quantum/painter/qgf.c
index 07c3f80314..e5a1895b74 100644
--- a/quantum/painter/qgf.c
+++ b/quantum/painter/qgf.c
@@ -26,7 +26,7 @@ bool qgf_validate_block_header(qgf_block_header_v1_t *desc, uint8_t expected_typ
 
 bool qgf_parse_format(qp_image_format_t format, uint8_t *bpp, bool *has_palette, bool *is_panel_native) {
     // clang-format off
-    static const struct QP_PACKED {
+    static const struct PACKED {
         uint8_t bpp;
         bool    has_palette;
         bool is_panel_native;
diff --git a/quantum/painter/qgf.h b/quantum/painter/qgf.h
index a1e245f15d..87a9124f27 100644
--- a/quantum/painter/qgf.h
+++ b/quantum/painter/qgf.h
@@ -19,7 +19,7 @@
 /////////////////////////////////////////
 // Common block header
 
-typedef struct QP_PACKED qgf_block_header_v1_t {
+typedef struct PACKED qgf_block_header_v1_t {
     uint8_t  type_id;     // See each respective block type below.
     uint8_t  neg_type_id; // Negated type ID, used for detecting parsing errors.
     uint32_t length : 24; // 24-bit blob length, allowing for block sizes of a maximum of 16MB.
@@ -32,7 +32,7 @@ STATIC_ASSERT(sizeof(qgf_block_header_v1_t) == 5, "qgf_block_header_v1_t must be
 
 #define QGF_GRAPHICS_DESCRIPTOR_TYPEID 0x00
 
-typedef struct QP_PACKED qgf_graphics_descriptor_v1_t {
+typedef struct PACKED qgf_graphics_descriptor_v1_t {
     qgf_block_header_v1_t header;              // = { .type_id = 0x00, .neg_type_id = (~0x00), .length = 18 }
     uint32_t              magic : 24;          // constant, equal to 0x464751 ("QGF")
     uint8_t               qgf_version;         // constant, equal to 0x01
@@ -52,7 +52,7 @@ STATIC_ASSERT(sizeof(qgf_graphics_descriptor_v1_t) == (sizeof(qgf_block_header_v
 
 #define QGF_FRAME_OFFSET_DESCRIPTOR_TYPEID 0x01
 
-typedef struct QP_PACKED qgf_frame_offsets_v1_t {
+typedef struct PACKED qgf_frame_offsets_v1_t {
     qgf_block_header_v1_t header;    // = { .type_id = 0x01, .neg_type_id = (~0x01), .length = (N * sizeof(uint32_t)) }
     uint32_t              offset[0]; // '0' signifies that this struct is immediately followed by the frame offsets
 } qgf_frame_offsets_v1_t;
@@ -64,7 +64,7 @@ STATIC_ASSERT(sizeof(qgf_frame_offsets_v1_t) == sizeof(qgf_block_header_v1_t), "
 
 #define QGF_FRAME_DESCRIPTOR_TYPEID 0x02
 
-typedef struct QP_PACKED qgf_frame_v1_t {
+typedef struct PACKED qgf_frame_v1_t {
     qgf_block_header_v1_t header;                 // = { .type_id = 0x02, .neg_type_id = (~0x02), .length = 6 }
     qp_image_format_t     format : 8;             // Frame format, see qp_internal_formats.h.
     uint8_t               flags;                  // Frame flags, see below.
@@ -83,7 +83,7 @@ STATIC_ASSERT(sizeof(qgf_frame_v1_t) == (sizeof(qgf_block_header_v1_t) + 6), "qg
 
 #define QGF_FRAME_PALETTE_DESCRIPTOR_TYPEID 0x03
 
-typedef struct QP_PACKED qgf_palette_entry_v1_t {
+typedef struct PACKED qgf_palette_entry_v1_t {
     uint8_t h; // hue component: `[0,360)` degrees is mapped to `[0,255]` uint8_t.
     uint8_t s; // saturation component: `[0,1]` is mapped to `[0,255]` uint8_t.
     uint8_t v; // value component: `[0,1]` is mapped to `[0,255]` uint8_t.
@@ -91,7 +91,7 @@ typedef struct QP_PACKED qgf_palette_entry_v1_t {
 
 STATIC_ASSERT(sizeof(qgf_palette_entry_v1_t) == 3, "Palette entry is not 3 bytes in size");
 
-typedef struct QP_PACKED qgf_palette_v1_t {
+typedef struct PACKED qgf_palette_v1_t {
     qgf_block_header_v1_t  header; // = { .type_id = 0x03, .neg_type_id = (~0x03), .length = (N * 3 * sizeof(uint8_t)) }
     qgf_palette_entry_v1_t hsv[0]; // N * hsv, where N is the number of palette entries depending on the frame format in the descriptor
 } qgf_palette_v1_t;
@@ -103,7 +103,7 @@ STATIC_ASSERT(sizeof(qgf_palette_v1_t) == sizeof(qgf_block_header_v1_t), "qgf_pa
 
 #define QGF_FRAME_DELTA_DESCRIPTOR_TYPEID 0x04
 
-typedef struct QP_PACKED qgf_delta_v1_t {
+typedef struct PACKED qgf_delta_v1_t {
     qgf_block_header_v1_t header; // = { .type_id = 0x04, .neg_type_id = (~0x04), .length = 8 }
     uint16_t              left;   // The left pixel location to draw the delta image
     uint16_t              top;    // The top pixel location to draw the delta image
@@ -118,7 +118,7 @@ STATIC_ASSERT(sizeof(qgf_delta_v1_t) == (sizeof(qgf_block_header_v1_t) + 8), "qg
 
 #define QGF_FRAME_DATA_DESCRIPTOR_TYPEID 0x05
 
-typedef struct QP_PACKED qgf_data_v1_t {
+typedef struct PACKED qgf_data_v1_t {
     qgf_block_header_v1_t header;  // = { .type_id = 0x05, .neg_type_id = (~0x05), .length = N }
     uint8_t               data[0]; // 0 signifies that this struct is immediately followed by the length of data specified in the header
 } qgf_data_v1_t;
diff --git a/quantum/painter/qp_draw_core.c b/quantum/painter/qp_draw_core.c
index 852abb19e8..3958045943 100644
--- a/quantum/painter/qp_draw_core.c
+++ b/quantum/painter/qp_draw_core.c
@@ -52,7 +52,7 @@ bool qp_internal_setpixel_impl(painter_device_t device, uint16_t x, uint16_t y)
 void qp_internal_fill_pixdata(painter_device_t device, uint32_t num_pixels, uint8_t hue, uint8_t sat, uint8_t val) {
     painter_driver_t *driver            = (painter_driver_t *)device;
     uint32_t          pixels_in_pixdata = qp_internal_num_pixels_in_buffer(device);
-    num_pixels                          = QP_MIN(pixels_in_pixdata, num_pixels);
+    num_pixels                          = MIN(pixels_in_pixdata, num_pixels);
 
     // Convert the color to native pixel format
     qp_pixel_t color = {.hsv888 = {.h = hue, .s = sat, .v = val}};
@@ -232,17 +232,17 @@ bool qp_internal_fillrect_helper_impl(painter_device_t device, uint16_t left, ui
     uint32_t          pixels_in_pixdata = qp_internal_num_pixels_in_buffer(device);
     painter_driver_t *driver            = (painter_driver_t *)device;
 
-    uint16_t l = QP_MIN(left, right);
-    uint16_t r = QP_MAX(left, right);
-    uint16_t t = QP_MIN(top, bottom);
-    uint16_t b = QP_MAX(top, bottom);
+    uint16_t l = MIN(left, right);
+    uint16_t r = MAX(left, right);
+    uint16_t t = MIN(top, bottom);
+    uint16_t b = MAX(top, bottom);
     uint16_t w = r - l + 1;
     uint16_t h = b - t + 1;
 
     uint32_t remaining = w * h;
     driver->driver_vtable->viewport(device, l, t, r, b);
     while (remaining > 0) {
-        uint32_t transmit = QP_MIN(remaining, pixels_in_pixdata);
+        uint32_t transmit = MIN(remaining, pixels_in_pixdata);
         if (!driver->driver_vtable->pixdata(device, qp_internal_global_pixdata_buffer, transmit)) {
             return false;
         }
@@ -260,10 +260,10 @@ bool qp_rect(painter_device_t device, uint16_t left, uint16_t top, uint16_t righ
     }
 
     // Cater for cases where people have submitted the coordinates backwards
-    uint16_t l = QP_MIN(left, right);
-    uint16_t r = QP_MAX(left, right);
-    uint16_t t = QP_MIN(top, bottom);
-    uint16_t b = QP_MAX(top, bottom);
+    uint16_t l = MIN(left, right);
+    uint16_t r = MAX(left, right);
+    uint16_t t = MIN(top, bottom);
+    uint16_t b = MAX(top, bottom);
     uint16_t w = r - l + 1;
     uint16_t h = b - t + 1;
 
@@ -281,7 +281,7 @@ bool qp_rect(painter_device_t device, uint16_t left, uint16_t top, uint16_t righ
         ret = qp_internal_fillrect_helper_impl(device, l, t, r, b);
     } else {
         // Fill up the pixdata buffer with the required number of native pixels
-        qp_internal_fill_pixdata(device, QP_MAX(w, h), hue, sat, val);
+        qp_internal_fill_pixdata(device, MAX(w, h), hue, sat, val);
 
         // Draw 4x filled single-width rects to create an outline
         if (!qp_internal_fillrect_helper_impl(device, l, t, r, t) || !qp_internal_fillrect_helper_impl(device, l, b, r, b) || !qp_internal_fillrect_helper_impl(device, l, t + 1, l, b - 1) || !qp_internal_fillrect_helper_impl(device, r, t + 1, r, b - 1)) {
diff --git a/quantum/painter/qp_draw_ellipse.c b/quantum/painter/qp_draw_ellipse.c
index 9e77bca8b0..22f019d179 100644
--- a/quantum/painter/qp_draw_ellipse.c
+++ b/quantum/painter/qp_draw_ellipse.c
@@ -75,7 +75,7 @@ bool qp_ellipse(painter_device_t device, uint16_t x, uint16_t y, uint16_t sizex,
     int16_t dx = 0;
     int16_t dy = ((int16_t)sizey);
 
-    qp_internal_fill_pixdata(device, QP_MAX(sizex, sizey), hue, sat, val);
+    qp_internal_fill_pixdata(device, MAX(sizex, sizey), hue, sat, val);
 
     if (!qp_comms_start(device)) {
         qp_dprintf("qp_ellipse: fail (could not start comms)\n");
diff --git a/quantum/painter/qp_internal.h b/quantum/painter/qp_internal.h
index e7a6d113c5..52b1b5c69d 100644
--- a/quantum/painter/qp_internal.h
+++ b/quantum/painter/qp_internal.h
@@ -5,17 +5,11 @@
 
 #include "quantum.h"
 #include "qp.h"
+#include "util.h" // PACKED/MIN/MAX
 
 ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
 // Helpers
 
-// Mark certain types that there should be no padding bytes between members.
-#define QP_PACKED __attribute__((packed))
-
-// Min/max defines
-#define QP_MIN(X, Y) (((X) < (Y)) ? (X) : (Y))
-#define QP_MAX(X, Y) (((X) > (Y)) ? (X) : (Y))
-
 #ifdef QUANTUM_PAINTER_DEBUG
 #    include <debug.h>
 #    include <print.h>
diff --git a/quantum/painter/qp_internal_formats.h b/quantum/painter/qp_internal_formats.h
index bd7105cab2..28cff0cd61 100644
--- a/quantum/painter/qp_internal_formats.h
+++ b/quantum/painter/qp_internal_formats.h
@@ -3,6 +3,7 @@
 
 #pragma once
 
+#include "color.h"
 #include "compiler_support.h"
 #include "qp_internal.h"
 
@@ -10,21 +11,13 @@
 // Quantum Painter pixel formats
 
 // Datatype containing a pixel's color. The internal member used is dependent on the external context.
-typedef union QP_PACKED qp_pixel_t {
+typedef union PACKED qp_pixel_t {
     uint8_t mono;
     uint8_t palette_idx;
 
-    struct QP_PACKED {
-        uint8_t h;
-        uint8_t s;
-        uint8_t v;
-    } hsv888;
-
-    struct QP_PACKED {
-        uint8_t r;
-        uint8_t g;
-        uint8_t b;
-    } rgb888;
+    hsv_t hsv888;
+
+    rgb_t rgb888;
 
     uint16_t rgb565;
 
diff --git a/quantum/painter/rules.mk b/quantum/painter/rules.mk
index 10c2698092..2c5bc17eed 100644
--- a/quantum/painter/rules.mk
+++ b/quantum/painter/rules.mk
@@ -247,7 +247,8 @@ ifeq ($(strip $(QUANTUM_PAINTER_NEEDS_SURFACE)), yes)
     SRC += \
         $(DRIVER_PATH)/painter/generic/qp_surface_common.c \
         $(DRIVER_PATH)/painter/generic/qp_surface_mono1bpp.c \
-        $(DRIVER_PATH)/painter/generic/qp_surface_rgb565.c
+        $(DRIVER_PATH)/painter/generic/qp_surface_rgb565.c \
+        $(DRIVER_PATH)/painter/generic/qp_surface_rgb888.c
 endif
 
 # If dummy comms is needed, set up the required files