From f54f8c5fb3f713f50581fbe034949b7d27bcda0b Mon Sep 17 00:00:00 2001 From: William Byatt Date: Sun, 2 Aug 2026 18:17:03 -0400 Subject: [PATCH] video/rgbcolors: Fix RGBTO8 to use the high bits of each component. RGBTO8 shifted each component up before masking: (((uint8_t)(r) << 5) & 0xe0) The cast is promoted to int before the shift, so the mask keeps bits 5:7 of the shifted value, which are bits 0:2 of r. The macro therefore encoded the three least significant bits of red and green and the two least significant bits of blue, rather than the most significant. This disagrees with RGBTO16 in the same file, which correctly takes the high bits, and with RGB8RED/RGB8GREEN/RGB8BLUE immediately below it, which are documented as the inverse transformation but read the result as high bits. All in-tree callers pass full 8-bit components, so all were affected: RGBTO8(39, 64, 139) in apps/examples/nxterm, intended as midnight blue, evaluates to 0xe3 -- full red plus full blue, i.e. magenta. Take the high bits instead, so that RGBTO8 matches RGBTO16 and the RGB8xxx macros become its true inverse. Tested on a RISC-V LiteX/VexRiscv target with an 8bpp RGB332 frame buffer, and with a host round-trip check over all 256 representable colours. Assisted-by: Claude:claude-opus-5 Signed-off-by: William Byatt --- include/nuttx/video/rgbcolors.h | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/include/nuttx/video/rgbcolors.h b/include/nuttx/video/rgbcolors.h index 4fadbfcca2293..097f65db5c205 100644 --- a/include/nuttx/video/rgbcolors.h +++ b/include/nuttx/video/rgbcolors.h @@ -72,10 +72,15 @@ #define RGB16GREEN(rgb) (((rgb) >> 3) & 0xfc) #define RGB16BLUE(rgb) (((rgb) << 3) & 0xf8) -/* This macro creates RGB8 (3:3:2) from 8:8:8 RGB */ +/* This macro creates RGB8 (3:3:2) from 8:8:8 RGB: + * + * R[7:5] -> RGB[7:5] + * G[7:5] -> RGB[4:2] + * B[7:6] -> RGB[1:0] + */ #define RGBTO8(r,g,b) \ - ((((uint8_t)(r) << 5) & 0xe0) | (((uint8_t)(g) << 2) & 0x1c) | ((uint8_t)(b) & 0x03)) + (((uint8_t)(r) & 0xe0) | (((uint8_t)(g) & 0xe0) >> 3) | (((uint8_t)(b) & 0xc0) >> 6)) /* And these macros perform the inverse transformation */