heronarts / LX

Core library for 3D LED lighting engines
https://chromatik.co/
Other
41 stars 25 forks source link

Improve handling of black color by gradients and palette transitions #83

Closed jkbelcher closed 1 year ago

jkbelcher commented 1 year ago

Attempting to improve an issue with gradients pointed out by @jvyduna.

To reproduce:

This PR produces expected behavior for the Gradient pattern and also palette transitions. It treats a color with zero brightness as hsb(other color's hue, 0, 0) in the blends.

Will not be surprise if this is constrained by other things. Although creating a gradient out of black seems like a common enough case to deserve improvement.

mcslee commented 1 year ago

Yeah this seems sensible enough. It kills the ability to completely specify the transition (e.g. hue/saturation can still be set via the color picker even if brightness is 0) - but I agree this is going to better match expected behavior in the bulk of cases.

The one change I'd make here is have saturation match the saturation of the alternate color rather than 0. Setting saturation to 0 will cause a "graying out" of the color as it fades.

In fact, I'm seeing a bug now actually somewhere in GradientPattern that I'll chase down. In HSV/HSV2 mode, it is doing that "graying out" when fading to black, vs. fading out properly when brightness is set to 1% instead of pure black. I'll fix that separately.

These images illustrate the "graying" problem of a 0-saturation fade:

1_pct_bright 0_pct_bright_graying
mcslee commented 1 year ago

FYI - you will need to quickly pull/merge this fix for the other issue I spotted: https://github.com/heronarts/LX/commit/f587a9faaa86442dea40f06c9e7e29a2e0a0f431

jvyduna commented 1 year ago

The one change I'd make here is have saturation match the saturation of the alternate color rather than 0

Great add.

it is doing that "graying out" when fading to black, vs. fading out properly when brightness is set to 1% instead of pure black

Isn't this the same thing? I've encountered that, but then I realized I was setting the saturation on the black to zero (a low point on the left 2D hue/sat map). I had lost awareness of that since the left hue/sat map is attenuated by the brightness slider on the right.

The grey was from HSV BlendMode lerping saturation from 1 to 0.

Monosnap Monosnap Titanic's End 2023-04-10 12-47-36 png 2023-04-10 12-48-52

mcslee commented 1 year ago

The graying is expected in the image you've posted because saturation is set to 0. If you had set saturation to 100, it wouldn't have grayed (since your brightness was still at 5%).

Note that in one of the images I sent, saturation was set to 100%, brightness to 0%, and the graying was occurring, but not occurring when brightness was at 1% (saturation is 100% in both cases). Basically, the HSV was losing its ability to interpolate saturation when brightness went to 0, since it started snapping saturation down to 0 at that point.

I think what's almost always desired here is to have the saturation stay with whatever color it came from. We'll lose the ability here to do this intentional gray-fade, but I doubt anyone really wants this. And I suppose they can keep brightness at 1% if they really really do.

raldi commented 1 year ago

Maybe instead of a special case for black, a better approach would be to consider three potential routes and pick whichever is shortest?

  1. LERP H, S, and V between the two colors in a straightforward manner
  2. Head toward the pole where S=0 and thus H can do a discontinuous and immedia jump
  3. Head toward the pole where V=0 and thus H can do a discontinuous and immedia jump

In other words, do 1 when the hue difference is smallest, do 2 when (S1 + S2) is smallest, and do 3 when (V1 + V2) is smallest.

That way, you could transition from RGB 81,80,80 to RGB 80,80,81 or from RGB 5,0,0 to 0,0,5 without having to take Hue on a tour of eight different time zones.

jvyduna commented 1 year ago

Ah, OK - I think you're noting the same problem that's in my original notes on this to Justin

Black is a hue and saturation oddity in that they're either undefined, or black is just the only color invariant to differing values (and is HSV really a cylinder or an inverted cone!?) The internal representation conversion of black to HSV gives 0 for the sat & value. Justin has a sensible less-surprise solution for hue and you're extending that to saturation.

I like the 1% brightness fallback.

@raldi That's an interesting suggestion, but it results in sudden jarring path jumps. For someone just picking two static colors, it might not be clear to them why small changes in endpoint color picking seem to result in sudden changes from fade-through-grey to fade-through-black. A similar issue happens today with dynamic oscillating colors today and HSV2 - see the animated gif in the doc linked.

In fact, won't most Case1 gradients now introduce a greyish midpoint for small changes in hue values? Shortest path between fully saturated nearby hues would always be a chord... Maybe unless the path length is computed from the slerp path length. Visually:

Monosnap Conceptual depiction of the Hue-Saturation-Value (HSV) space at the  | Download Scientific Diagram 2023-04-10 13-26-42

I believe the straight line is essentially what the RGB BlendMode already does.

jkbelcher commented 1 year ago

A similar issue happens today with dynamic oscillating colors today and HSV2 - see the animated gif in the doc linked.

Opened a separate issue for this at #85 @jvyduna

jkbelcher commented 1 year ago

It sounds like the conclusion is to start with this and leave a note that someone could pursue a smarter blend mode later.

mcslee commented 1 year ago

Sorry had been meaning to weigh in on this but hadn't found the time. I'm gonna clean up the HSV naming with better names as @jvyduna suggests, HSVCW HSVCCW and probably HSVN (nearest?)

Having some jumps with the HSVN mode is unavoidable. I'll look at whether it's not too horribly messy to fix the oscillating color case - but that becomes very context-dependent.

Another version of this, which is not sensibly fixable:

I don't think this state is sensibly fixable, even with individually stateful color stops. It would seem crazy to preserve a certain hue-blending direction because the previous swatch loaded was closer that way.

Dealing with the dynamically oscillating hue colors is a more locally scoped version of this same problem. I'm still wary about getting too clever there, e.g. should the ColorizeEffect for instance think that "HSVN means shortest hue-path, unless I know that these hue values are coming from a particular type of palette, in which case I'll change my behavior"

If the gradient generation were coming from the palette itself, I'd feel less wary of that. And maybe that's a reasonable change to consider. But for now, the palette is just providing a set of colors.

@raldi - yeah interesting idea on a more dynamic implementation, that works when the colors are static, but the issues here all pop up when the src/destination can be on the move.

I personally do mostly use RGB blending and find that it looks better - but I'm also sort of just a hater on the LED candy rainbow after seeing so much of it over the years.

Thanks for all the comments and feedback on this!

mcslee commented 1 year ago

Okay a big refactor/cleanup here: https://github.com/heronarts/LX/commit/30ada6045bf165c9c6412cacb81677c908958411

This does not fix the snapping issue, but it makes all the code consistent and gives explicit options for all the various HSV behaviors.

For dynamic colors in the oscillating mode, this will break old project files, HSV/RGB blend mode will be inverted because I coalesced to a single enum here, and the order of RGB/HSV changed. I figured that's the more tolerable breakage here, since those dynamic colors are probably far more rarely used than the gradient pattern and colorize effect.

jkbelcher commented 1 year ago

Another version of this, which is not sensibly fixable: Color palette is on setting A, where H2 - H1 < 180 HSV blend is using those colors Animate the palette to a swatch, where H2 - H1 > 180

That's a tricky one, anything attached to the middle of the gradient has a long way to travel and is going to get snapped.

If the gradient generation were coming from the palette itself, I'd feel less wary of that. And maybe that's a reasonable change to consider.

This might be cleaner. And expose some utilities for "blend from gradient A to gradient B" for use by gradient DIYers.