Page MenuHome

Fix T49979: HSV/HSL 'Far' interpolation in colorbands when both stops have same hue.
ClosedPublic

Authored by Bastien Montagne (mont29) on Mar 19 2019, 8:47 PM.

Details

Summary

When two stops had the exact same color, 'Far' interpolation would
behave like the three others and generate constant colors, instead of
generating expected 'full turn' in Hue space.

Note that this will break the 'constant' color between two stops with
same value in the 'Far' interpolation mode, but think that change is
OK/expected.

This fix also exposed that 'early out' case when we are after last stop
was not working properly, still triggering actual interpolation
computation in all cases, thus the refactor in BKE_colorband_evaluate()
itself. We also now avoid computing factors in constant case, and get
out early in all interpolation cases but the 'spline' ones (where color
ramping can start before first stop, and end after last one).

Work initiated by patch from Charlie Jolly (@Charlie Jolly (charlie)) in T49979, thanks.

Diff Detail

Repository
rB Blender

Event Timeline

Requesting review about that change because am unsure whether we really want that behavior for 'Far' hue interpolation…

On one hand, being able to do a full loop in Hue space is handy. On the other hand, it means that in Far mode, you cannot interpolate normally between two different colors that share a same hue, but different saturation/values…

Brecht Van Lommel (brecht) requested changes to this revision.Mar 20 2019, 8:00 PM

Logic seems fine.

source/blender/blenkernel/intern/colorband.c
417–418

Don't use < for enums, hard to see at a glance or search for what this actually affects then. Just use ELEM even if it's more verbose.

This revision now requires changes to proceed.Mar 20 2019, 8:00 PM
Bastien Montagne (mont29) marked an inline comment as done.Mar 20 2019, 8:48 PM
Bastien Montagne (mont29) added inline comments.
source/blender/blenkernel/intern/colorband.c
417–418

Will do (and also change existing code doing same kind of comparison then ;) ).

Bastien Montagne (mont29) marked an inline comment as done.

Updated from review.

This revision is now accepted and ready to land.Mar 22 2019, 4:32 PM
This revision was automatically updated to reflect the committed changes.