-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Viridis color LUT #2420
Add Viridis color LUT #2420
Conversation
common/src/colors.cpp
Outdated
static const unsigned int VIRIDIS_LUT_SIZE = sizeof (VIRIDIS_LUT) / (sizeof (VIRIDIS_LUT[0]) * 3); | ||
|
||
static const unsigned char* LUTS[] = { GLASBEY_LUT, VIRIDIS_LUT }; | ||
static const unsigned int LUT_SIZES[] = { GLASBEY_LUT_SIZE, VIRIDIS_LUT_SIZE }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a minor inconsistency with the size types between VIRIDIS_LUT_SIZE
and GLASBEY_LUT_SIZE
. I can understand not fully requiring the entire precision of . Edit: I just noticed I failed to see the unsigned before the type. Sorry.size_t
but I don't really understand why go for a signed type
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch. This commit was fermenting in my cellar since April. In the meantime #2297 landed changing the type of GLASBEY_LUT_SIZE
to size_t
and I was not attentive enough when resolving conflicts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me although you didn't add any unit tests to this. Some simplistic ones would be nice.
There were no unit tests before. Do you have anything particular in mind? Just verify that the size and RGB value returned by |
Exactly what you proposed. I'm just covering our future asses, although this is just a color lut. |
Done |
This PR generalizes
GlasbeyLUT
class intoColorLUT
with a template parameter specifying particular palette to use. In addition to already existing categorical Glasbey palette it adds Viridis colormap. This is a perceptually uniform colormap which has been the default in Matplotlib since version 2. An option is added toPCLVisualizer
to enable it's usage as a LUT there.Since it's a minor change I'd like to squeeze this in before 1.9.0.