wbyatt opened a new pull request, #19685:
URL: https://github.com/apache/nuttx/pull/19685
## Summary
RGBTO8 shifted each channel 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.
## Impact
No impact on build process, hardware, documentation, security, or
compatibility.
Users of the RGBTO8 macro will get new values, potentially changing RGB332
rendering given a 24 bpp source. However, this change aligns the code with the
other macros and generates outputs that are much more correct.
## Testing
Tested on a RISC-V LiteX/VexRiscv target on an Altera DE2-115 with an 8bpp
RGB332 frame buffer, and with a host round-trip check over all 256
representable colors.
Ran the following code to test it:
```c
printf("Old vs new macro across 32 sample colours "
"(8 hues x dark/mid/light/muted):\n");
{
static const unsigned hues[8] = {0, 45, 90, 135, 180, 225, 270,
315};
static const double vs[4] = {1.00, 1.00, 1.00, 0.35};
static const double vl[4] = {0.25, 0.50, 0.75, 0.50};
static const char * const vname[4] = {"dark", "mid", "light", "muted"};
for (i = 0; i < 32; i++)
{
unsigned hue = hues[i / 4];
unsigned variant = i % 4;
unsigned r8;
unsigned g8;
unsigned b8;
unsigned old;
unsigned new;
hsl_to_rgb((double)hue, vs[variant], vl[variant], &r8, &g8, &b8);
old = RGBTO8_OLD(r8, g8, b8);
new = RGBTO8_NEW(r8, g8, b8);
printf(" h=%3u %-5s (%3u,%3u,%3u): old 0x%02x -> (%3u,%3u,%3u) "
"new 0x%02x -> (%3u,%3u,%3u)\n",
hue, vname[variant], r8, g8, b8,
old, RGB8RED(old), RGB8GREEN(old), RGB8BLUE(old),
new, RGB8RED(new), RGB8GREEN(new), RGB8BLUE(new));
}
}
```
With this output:
```
Old vs new macro across 32 sample colours (8 hues x dark/mid/light/muted):
h= 0 dark (128, 0, 0): old 0x00 -> ( 0, 0, 0) new 0x80 -> (128,
0, 0)
h= 0 mid (255, 0, 0): old 0xe0 -> (224, 0, 0) new 0xe0 -> (224,
0, 0)
h= 0 light (255,128,128): old 0xe0 -> (224, 0, 0) new 0xf2 ->
(224,128,128)
h= 0 muted (172, 83, 83): old 0x8f -> (128, 96,192) new 0xa9 -> (160,
64, 64)
h= 45 dark (128, 96, 0): old 0x00 -> ( 0, 0, 0) new 0x8c -> (128,
96, 0)
h= 45 mid (255,191, 0): old 0xfc -> (224,224, 0) new 0xf4 ->
(224,160, 0)
h= 45 light (255,223,128): old 0xfc -> (224,224, 0) new 0xfa ->
(224,192,128)
h= 45 muted (172,150, 83): old 0x9b -> (128,192,192) new 0xb1 ->
(160,128, 64)
h= 90 dark ( 64,128, 0): old 0x00 -> ( 0, 0, 0) new 0x50 -> (
64,128, 0)
h= 90 mid (128,255, 0): old 0x1c -> ( 0,224, 0) new 0x9c ->
(128,224, 0)
h= 90 light (191,255,128): old 0xfc -> (224,224, 0) new 0xbe ->
(160,224,128)
h= 90 muted (128,172, 83): old 0x13 -> ( 0,128,192) new 0x95 ->
(128,160, 64)
h=135 dark ( 0,128, 32): old 0x00 -> ( 0, 0, 0) new 0x10 -> (
0,128, 0)
h=135 mid ( 0,255, 64): old 0x1c -> ( 0,224, 0) new 0x1d -> (
0,224, 64)
h=135 light (128,255,159): old 0x1f -> ( 0,224,192) new 0x9e ->
(128,224,128)
h=135 muted ( 83,172,105): old 0x71 -> ( 96,128, 64) new 0x55 -> (
64,160, 64)
h=180 dark ( 0,128,128): old 0x00 -> ( 0, 0, 0) new 0x12 -> (
0,128,128)
h=180 mid ( 0,255,255): old 0x1f -> ( 0,224,192) new 0x1f -> (
0,224,192)
h=180 light (128,255,255): old 0x1f -> ( 0,224,192) new 0x9f ->
(128,224,192)
h=180 muted ( 83,172,172): old 0x70 -> ( 96,128, 0) new 0x56 -> (
64,160,128)
h=225 dark ( 0, 32,128): old 0x00 -> ( 0, 0, 0) new 0x06 -> ( 0,
32,128)
h=225 mid ( 0, 64,255): old 0x03 -> ( 0, 0,192) new 0x0b -> ( 0,
64,192)
h=225 light (128,159,255): old 0x1f -> ( 0,224,192) new 0x93 ->
(128,128,192)
h=225 muted ( 83,105,172): old 0x64 -> ( 96, 32, 0) new 0x4e -> ( 64,
96,128)
h=270 dark ( 64, 0,128): old 0x00 -> ( 0, 0, 0) new 0x42 -> ( 64,
0,128)
h=270 mid (128, 0,255): old 0x03 -> ( 0, 0,192) new 0x83 -> (128,
0,192)
h=270 light (191,128,255): old 0xe3 -> (224, 0,192) new 0xb3 ->
(160,128,192)
h=270 muted (128, 83,172): old 0x0c -> ( 0, 96, 0) new 0x8a -> (128,
64,128)
h=315 dark (128, 0, 96): old 0x00 -> ( 0, 0, 0) new 0x81 -> (128,
0, 64)
h=315 mid (255, 0,191): old 0xe3 -> (224, 0,192) new 0xe2 -> (224,
0,128)
h=315 light (255,128,223): old 0xe3 -> (224, 0,192) new 0xf3 ->
(224,128,192)
h=315 muted (172, 83,150): old 0x8e -> (128, 96,128) new 0xaa -> (160,
64,128)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
