[BUG] when transform color by lv_color_rgb_to_hsv(..) and transform backward by lv_color_hsv_to_rgb(..), it can't display the same lv_color_t color

Description

On the color-depth 16bit system,
when lvgl transforms color by lv_color_rgb_to_hsv(...)
and transforms backward by lv_color_hsv_to_rgb(..),

The color is not same with the original color.

What MCU/Processor/Board and compiler are you using?

Code to reproduce

  lv_color_t color1  = LV_COLOR_RED;
  printf("color1 : 0x%04x\n", color1.full);

  lv_color_hsv_t hsv = lv_color_rgb_to_hsv(color1.ch.red, color1.ch.green, color1.ch.blue);

  lv_color_t color2 = lv_color_hsv_to_rgb (hsv.h, hsv.s, hsv.v);
  printf("color2 : 0x%04x\n", color2.full);

Result

color1 : 0xf800
color2 : 0x4924

What do you want to achieve?

color1 and color2 are the same color.

@paulpv recently discovered some bugs in the HSV implementation and is working to refactor it. You have probably found the same issue.

Here’s the most recent PR that was opened relating to the issue: https://github.com/littlevgl/lvgl/pull/1218

2 Likes

@TridentTD Can you try this patch and see if it works now? You’ll probably have to apply it manually if you are using 6.0.

https://github.com/littlevgl/lvgl/pull/1218/files

1 Like

Thank you.

I’ve just tested on 6.0.2 change at function lv_color_to32(lv_color_t color)
in lv_color.h to this code

ret.ch.red = ( color.ch.red * 8423 + 7 ) >> 10;
ret.ch.green = ( color.ch.green * 259 + 3 ) >> 6;
ret.ch.blue = ( color.ch.blue * 8423 + 7 ) >> 10;

The result when tranform by lv_color_rgb_to_hsv(...) and lv_color_hsv_to_rgb(...)
is still like the previous result.

Result

color1 : 0xf800
color2 : 0x4924

Okay, can we continue the discussion on the PR to keep everything in one place? https://github.com/littlevgl/lvgl/pull/1218

1 Like

OKay. at github : )

With the backing “rgb565” to “rgb888” issue hopefully fixed, I am next visiting the actual “rgb2hsv” problem.

I have created the following PR:

I am no coding novice, but my experience in the HSV color space isn’t the most educated.
I am mostly basing my contribution here based on how I see the existing lv_color_hsv_t struct used in lvgl (be that it may be flawed, but I’d like to start off assume it is not).
I am working on the cpicker control that is based on HSV, and when I create a control in a 16-bit RGB565 colordepth and assign it the value LV_COLOR_RED the color is definitely not red.
Furthermore, when I simply lv_color_rgb_to_hsv(0xF1, 0x00, 0x00) the code seems to be 888 specific and doesn’t understand that 0xF1 is red/0 in 565.
Curiously even lv_color_rgb_to_hsv(0xFF, 0x00, 0x00) didn’t convert to what I would expect to be HSV 0, 100, 100

Thus the PR.
Feel free to test out the PR’s branch and let me know if this is related to the issue(s) you were/are seeing.

Pv

My test code is at https://github.com/paulpv/lv_examples/blob/master/lv_tests/lv_test_misc/lv_test_color/lv_test_color.ino

typedef struct {
  lv_color_t rgb;
  lv_color_hsv_t hsv;
} test_rgb_to_hsv_t;

bool isClose(uint8_t a, uint8_t b, uint8_t d) {
  return abs(a - b) <= d;
}

void testRgbToHsvtoRgb() {
  /*
   * https://en.wikipedia.org/wiki/HSL_and_HSV#Examples
   */
  test_rgb_to_hsv_t expected[] = {     //  H, Shsv,   V      // https://www.htmlcsscolor.com
    { LV_COLOR_MAKE(0xFF, 0xFF, 0xFF), {   0,    0, 100 } }, // LV_COLOR_WHITE
    { LV_COLOR_MAKE(0x80, 0x80, 0x80), {   0,    0,  50 } }, // LV_COLOR_GRAY
    { LV_COLOR_MAKE(0x00, 0x00, 0x00), {   0,    0,   0 } }, // LV_COLOR_BLACK
    { LV_COLOR_MAKE(0xFF, 0x00, 0x00), {   0,  100, 100 } }, // LV_COLOR_RED
    { LV_COLOR_MAKE(0xBF, 0xBF, 0x00), {  60,  100,  75 } }, // "La Rioja"
    { LV_COLOR_MAKE(0x00, 0x80, 0x00), { 120,  100,  50 } }, // LV_COLOR_GREEN
    { LV_COLOR_MAKE(0x80, 0xFF, 0xFF), { 180,   50, 100 } }, // "Electric Blue"
    { LV_COLOR_MAKE(0x80, 0x80, 0xFF), { 240,   50, 100 } }, // "Light Slate Blue"
    { LV_COLOR_MAKE(0xBF, 0x40, 0xBF), { 300,   67,  75 } }, // "Fuchsia"
    { LV_COLOR_MAKE(0xA0, 0xA4, 0x24), {  62,   78,  64 } }, // "Lucky"
    { LV_COLOR_MAKE(0x41, 0x1B, 0xEA), { 251,   89,  92 } }, // "Han Purple"
    { LV_COLOR_MAKE(0x1E, 0xAC, 0x41), { 135,   83,  68 } }, // "Dark Pastel Green"
    { LV_COLOR_MAKE(0xF0, 0xC8, 0x0E), {  50,   94,  94 } }, // "Moon Yellow"
    { LV_COLOR_MAKE(0xB4, 0x30, 0xE5), { 284,   79,  90 } }, // "Dark Orchid"
    { LV_COLOR_MAKE(0xED, 0x76, 0x51), {  14,   66,  93 } }, // "Burnt Sienna"
    { LV_COLOR_MAKE(0xFE, 0xF8, 0x88), {  57,   47, 100 } }, // "Milan"
    { LV_COLOR_MAKE(0x19, 0xCB, 0x97), { 162,   88,  80 } }, // "Caribbean Green"
    { LV_COLOR_MAKE(0x36, 0x26, 0x98), { 248,   75,  60 } }, // "Dark Slate Blue"
    { LV_COLOR_MAKE(0x7E, 0x7E, 0xB8), { 241,   32,  72 } }, // "Moody Blue"
  };
  int test_count = sizeof(expected) / sizeof(expected[0]);
  for(int i=0; i < test_count; i++) {
    test_rgb_to_hsv_t rgb_to_hsv = expected[i];
    lv_color_t colorRgbExpected = rgb_to_hsv.rgb;
    lv_color_hsv_t colorHsvExpected = rgb_to_hsv.hsv;
    Serial.printf("testRgbToHsvtoRgb: colorRgbExpected=0x%08X, colorHsvExpected={ %d, %d, %d }\n", colorRgbExpected, colorHsvExpected.h, colorHsvExpected.s, colorHsvExpected.v);

    //
    // RGB to HSV
    //
    lv_color_hsv_t colorHsvActual = lv_color_to_hsv(colorRgbExpected);
    Serial.printf("testRgbToHsvtoRgb: colorHsvActual={ %d, %d, %d }\n", colorHsvActual.h, colorHsvActual.s, colorHsvActual.v);
    if (!isClose(colorHsvActual.h, colorHsvExpected.h, 3) || 
        !isClose(colorHsvActual.s, colorHsvExpected.s, 3) || 
        !isClose(colorHsvActual.v, colorHsvExpected.v, 3)) {
      Serial.println("testRgbToHsvtoRgb: FAIL!");
      break;
    }

    //
    // HSV to RGB
    //
    lv_color_t colorRgbActual = lv_color_hsv_to_rgb(colorHsvExpected.h, colorHsvExpected.s, colorHsvExpected.v);
    Serial.printf("testRgbToHsvtoRgb: colorRgbActual=0x%08X\n", colorRgbActual);
    if (!isClose(colorRgbActual.ch.red, colorRgbExpected.ch.red, 3) || 
        !isClose(colorRgbActual.ch.green, colorRgbExpected.ch.green, 3) || 
        !isClose(colorRgbActual.ch.blue, colorRgbExpected.ch.blue, 3)) {
      Serial.println("testRgbToHsvtoRgb: FAIL!");
      break;
    }
  }
  Serial.println("testRgbToHsvtoRgb: PASS!");
}
1 Like

It’s look good, before-color16bit rgb is close to the after-color.
( rbg <-> hsv : delta red,green,blue <3 )

I have another question.
When I test color-picker, if not set any color at start, the result is the following…

capture_00025

But when I set LV_COLOR_BLUE at start to the color-picker
by lv_cpicker_set_color(...) the result is so strange.

capture_00024

How to fix this issue?

I’ve just tested again for all of color16bit by the following code.
and tested by color_diff = 3

bool isClose(uint8_t a, uint8_t b, uint8_t d) {
  return abs(a - b) <= d;
}

bool color16_checker(lv_color_t color1, lv_color_t color2, uint8_t color_diff){
  return (!isClose(color1.ch.red   , color2.ch.red  , color_diff) || 
          !isClose(color1.ch.green , color2.ch.green, color_diff) || 
          !isClose(color1.ch.blue  , color2.ch.blue , color_diff) ); 
}


void testColor16_to_HSV_to_16() {
  lv_color_t      color16Expected;
  lv_color_hsv_t  colorHSV;
  lv_color_t      color16Actual;
  
  for(uint32_t c = 0; c < 0xFFFF; c++) {
    color16Expected.full   = c;
    colorHSV          = lv_color_to_hsv(color16Expected);
    color16Actual     = lv_color_hsv_to_rgb(colorHSV.h, colorHSV.s, colorHSV.v);
    if (color16_checker(color16Actual,color16Expected, 3)) {
      Serial.printf("testColor16_to_HSV_to_16: FAIL! Color16 : Before 0x%04X --> After 0x%04X\n", color16Expected.full, color16Actual.full);
    }
    if( c % 1000 == 0) {
      Serial.printf("testColor16_to_HSV_to_16: OK : 0x%04X\n", c);
    }
  }
  Serial.println("testColor16_to_HSV_to_16: END!");
}

The result


testColor16_to_HSV_to_16: OK : 0x0000
testColor16_to_HSV_to_16: FAIL! Color16 : Before 0x039F --> After 0x041F
testColor16_to_HSV_to_16: OK : 0x03E8
testColor16_to_HSV_to_16: FAIL! Color16 : Before 0x043D --> After 0x04BD
testColor16_to_HSV_to_16: FAIL! Color16 : Before 0x057F --> After 0x05FF
testColor16_to_HSV_to_16: FAIL! Color16 : Before 0x05DF --> After 0x065F
testColor16_to_HSV_to_16: FAIL! Color16 : Before 0x063F --> After 0x06BF
testColor16_to_HSV_to_16: OK : 0x07D0
testColor16_to_HSV_to_16: FAIL! Color16 : Before 0x0A7F --> After 0x02FF
testColor16_to_HSV_to_16: FAIL! Color16 : Before 0x0ADF --> After 0x035F
testColor16_to_HSV_to_16: FAIL! Color16 : Before 0x0B3F --> After 0x03BF
testColor16_to_HSV_to_16: OK : 0x0BB8
testColor16_to_HSV_to_16: OK : 0x0FA0
testColor16_to_HSV_to_16: OK : 0x1388
testColor16_to_HSV_to_16: FAIL! Color16 : Before 0x159F --> After 0x0E1F
testColor16_to_HSV_to_16: FAIL! Color16 : Before 0x15FF --> After 0x0E7F
testColor16_to_HSV_to_16: FAIL! Color16 : Before 0x16DF --> After 0x0F5F
testColor16_to_HSV_to_16: OK : 0x1770
testColor16_to_HSV_to_16: OK : 0x1B58
testColor16_to_HSV_to_16: FAIL! Color16 : Before 0x1F3F --> After 0x17BF
testColor16_to_HSV_to_16: OK : 0x1F40
testColor16_to_HSV_to_16: OK : 0x2328
testColor16_to_HSV_to_16: FAIL! Color16 : Before 0x24FF --> After 0x257F
testColor16_to_HSV_to_16: FAIL! Color16 : Before 0x261F --> After 0x269F
testColor16_to_HSV_to_16: OK : 0x2710
testColor16_to_HSV_to_16: OK : 0x2AF8
testColor16_to_HSV_to_16: OK : 0x2EE0
testColor16_to_HSV_to_16: OK : 0x32C8
testColor16_to_HSV_to_16: OK : 0x36B0
testColor16_to_HSV_to_16: OK : 0x3A98
testColor16_to_HSV_to_16: OK : 0x3E80
testColor16_to_HSV_to_16: OK : 0x4268
testColor16_to_HSV_to_16: OK : 0x4650
testColor16_to_HSV_to_16: OK : 0x4A38
testColor16_to_HSV_to_16: OK : 0x4E20
testColor16_to_HSV_to_16: OK : 0x5208
testColor16_to_HSV_to_16: OK : 0x55F0
testColor16_to_HSV_to_16: OK : 0x59D8
testColor16_to_HSV_to_16: OK : 0x5DC0
testColor16_to_HSV_to_16: OK : 0x61A8
testColor16_to_HSV_to_16: OK : 0x6590
testColor16_to_HSV_to_16: OK : 0x6978
testColor16_to_HSV_to_16: OK : 0x6D60
testColor16_to_HSV_to_16: OK : 0x7148
testColor16_to_HSV_to_16: OK : 0x7530
testColor16_to_HSV_to_16: OK : 0x7918
testColor16_to_HSV_to_16: OK : 0x7D00
testColor16_to_HSV_to_16: OK : 0x80E8
testColor16_to_HSV_to_16: OK : 0x84D0
testColor16_to_HSV_to_16: OK : 0x88B8
testColor16_to_HSV_to_16: OK : 0x8CA0
testColor16_to_HSV_to_16: OK : 0x9088
testColor16_to_HSV_to_16: OK : 0x9470
testColor16_to_HSV_to_16: OK : 0x9858
testColor16_to_HSV_to_16: OK : 0x9C40
testColor16_to_HSV_to_16: OK : 0xA028
testColor16_to_HSV_to_16: OK : 0xA410
testColor16_to_HSV_to_16: OK : 0xA7F8
testColor16_to_HSV_to_16: OK : 0xABE0
testColor16_to_HSV_to_16: OK : 0xAFC8
testColor16_to_HSV_to_16: OK : 0xB3B0
testColor16_to_HSV_to_16: OK : 0xB798
testColor16_to_HSV_to_16: OK : 0xBB80
testColor16_to_HSV_to_16: OK : 0xBF68
testColor16_to_HSV_to_16: OK : 0xC350
testColor16_to_HSV_to_16: OK : 0xC738
testColor16_to_HSV_to_16: OK : 0xCB20
testColor16_to_HSV_to_16: OK : 0xCF08
testColor16_to_HSV_to_16: OK : 0xD2F0
testColor16_to_HSV_to_16: OK : 0xD6D8
testColor16_to_HSV_to_16: OK : 0xDAC0
testColor16_to_HSV_to_16: OK : 0xDEA8
testColor16_to_HSV_to_16: OK : 0xE290
testColor16_to_HSV_to_16: OK : 0xE678
testColor16_to_HSV_to_16: OK : 0xEA60
testColor16_to_HSV_to_16: OK : 0xEE48
testColor16_to_HSV_to_16: OK : 0xF230
testColor16_to_HSV_to_16: OK : 0xF618
testColor16_to_HSV_to_16: OK : 0xFA00
testColor16_to_HSV_to_16: OK : 0xFDE8
testColor16_to_HSV_to_16: END!

I suggest continuing this discussion on the related GitHub pull request:

1 Like