Planck EZ: LED colors 'washed out' compared to before

[phogy]

I'm not sure what is right and not but I found that theLED colors on my Planck EZ looked far more pale than before when I moved from 0.6.402 to 0.7.52. I guess the former was the base when I received my Planck and I discovered a feature I wanted from master and decided to move to latest basically.
Anyway, to the point, I have found the PR that "breaks" it for me:
#6764

#ifdef USE_CIE1931_CURVE	
    rgb.r = pgm_read_byte(&CIE1931_CURVE[rgb.r]);	
    rgb.g = pgm_read_byte(&CIE1931_CURVE[rgb.g]);	
    rgb.b = pgm_read_byte(&CIE1931_CURVE[rgb.b]);	
#endif

With this change, the CIE1931 curve is applied to the luminance of HSV and not the resulting RGB and this causes my RGB values to be lighter and all colors look less saturated and brighter using the same HSV values as input before and after.
Not a big deal now that I've found it - all I need to do on my side is to use a copy of the old implementation in my keymap.c - but then again I did discover that the Plank EZ configurator nowadays adds a constant to all three RGB values from its UI tool. Was that an attempt to remedy this? Either way, it doesn't do the trick...
My question is if this change in color saturation was the intent of #6764 or if in fact something went wrong?
Thanks for your input.

[yanfali]

@XScorpion2 want to weigh in here? Sounds like a bug.

[phogy]

but then again I did discover that the Plank EZ configurator nowadays adds a constant to all three RGB values from its UI tool. Was that an attempt to remedy this? Either way, it doesn't do the trick...

My bad. That (what the configurator adds nowadays) is just to get the RGB matrix lightness setting into play.

[XScorpion2]

@yanfali I can't find the previous discussion. The change was intentional as I was aiming for behavioral accuracy (linear brightness, color remaining constant, etc), not color accuracy. Easy enough to switch back in color.c to the previous method.

[yanfali]

@XScorpion2 I don't suppose there's a way to have both?

[drashna]

but then again I did discover that the Plank EZ configurator nowadays adds a constant to all three RGB values from its UI tool. Was that an attempt to remedy this? Either way, it doesn't do the trick...

Nope. The Oryx Configurator use RGB instead of HSV, because there is no conversion that needs to be done.

Specifically, it calls rgb_matrix_set_color, which takes an RGB value. Otherwise, we'd have to take the HSV value, convert it, and then send the rgb value.

So by using RGB directly, it cuts out a step, and reduces overhead for the custom LED stuff.

[drashna]

@XScorpion2 you mean this one?
#6910

[phogy]

@drashna Sorry for the confusion, I meant this construct in set_layer_color:

    if (!hsv.h && !hsv.s && !hsv.v) {
        rgb_matrix_set_color( i, 0, 0, 0 );
    } else {
        RGB rgb = hsv_to_rgb( hsv );
        float f = (float)rgb_matrix_config.hsv.v / UINT8_MAX;
        rgb_matrix_set_color( i, f * rgb.r, f * rgb.g, f * rgb.b );
    }

Which btw works great if you want to dim the layer LEDs which I do now :)

[stale]

This issue has been automatically marked as stale because it has not had activity in the last 90 days. It will be closed in the next 30 days unless it is tagged properly or other activity occurs.
For maintainers: Please label with bug, in progress, on hold, discussion or to do to prevent the issue from being re-flagged.

[jpinnix]

I am experiencing washed out/pale colors as well on my Ergodox-EZ. They pointed me to https://docs.qmk.fm/#/feature_rgb_matrix but I figured someone else here had the same problem. If I forego custom colors created with the configurator, and just use the built-in color effects and such, colors are natural in color. But once I use a config file generated by the configurator, then the colors are completely off.

[stale]

This issue has been automatically marked as stale because it has not had activity in the last 90 days. It will be closed in the next 30 days unless it is tagged properly or other activity occurs.
For maintainers: Please label with bug, in progress, on hold, discussion or to do to prevent the issue from being re-flagged.

[stale]

This issue has been automatically closed because it has not had activity in the last 30 days. If this issue is still valid, re-open the issue and let us know.