-
Notifications
You must be signed in to change notification settings - Fork 47
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
MSC()
is strange
#349
Comments
Using for hsv_h in 0:0.1:360
hsv = HSV(hsv_h,1.0,1.0) # most saturated
lch = convert(LCHuv, hsv)
msc = MSC(lch.h)
@test msc ≈ lch atol=1e-6
end So, the following patch may not be necessary. Lines 195 to 203 in e7f4723
|
It turns out it was a simple reason. Lines 259 to 260 in e7f4723
The sRGB gamut is not triangular in L-C section, especially in blue to red via purple, as shown in Figure 3 from "Generating Color Palettes using Intuitive Parameters".
see also: https://commons.wikimedia.org/wiki/File:SRGB_gamut_within_CIELUV_color_space_mesh.webm It may be a good approximation for generating colormaps. However, I doubt it is sufficient for any purpose. |
Really nice diagnosis and analysis, @kimikage. Your instincts are excellent, when you think you have a solution you like I look forward to your proposal. |
After hours of trial and error, I found that, somehow, the ugly function |
Julia doesn't offer intrinsic support for TCO, except in cases where the result can be computed at compile time. But here it doesn't matter because most of the time is taken up by the |
Fix erroneous `MSC(h)` and improve accuracy of `MSC(h, l)` (#349)
Probably However, it is not strictly "the first". It is related to the problem of gamut (cf. #372 (comment)). |
MSC()
have been introduced to realize the colormap function since 3e2dcf1 .I think
MSC()
is strange in some respects.1. Name
As
MSC()
is not a constructor but an ordinaryl function, it is to be desired that the name is lowercase to follow the Style Guide, even though the original reference paper usesMSC()
.In particular, I think the naming is important because
MSC
is exported.However, renaming
MSC
tomsc
,most_saturated_color
,maximally_saturated_color
or something else is not sufficient to solve the problems. The reasons are as follows.By the way, although "colorfulness", "chroma" and "saturation" are often used loosely, the term "chroma" is used in the CIELAB and CIELUV color spaces. Perhaps "saturation" may mean that it is saturated in sRGB HSV space, though.
2. Return value
MSC(h)
returnsLCHuv
color, butMSC(h, l)
returns saturation value.So, if we follow the behavior, they must have different names and especially the latter should be renamed.
Do we really need two different functions?
3. Color space
The current
MSC()
calculates inLuv
(LCHuv
) color space. I think the behavior is OK, but the nameMSC()
and its arguments are not informative about the color space.The function to get the maximally saturated color in
Lab
color space, might improvedistinguishable_colors()
.When we add such a variant function or method, we should modify the interface.
4. Validity
Edit: see added comments below
I wrote the following ugly function using binary search:
And then I got some strange results:
The disagreement is found in not only light colors but also purple colors:
I don't know whether it is a feature. I have not investigated the cause of it.Although it is not the main cause, I found a discrepancy in the gamma correction:
Colors.jl/src/algorithms.jl
Lines 233 to 234 in e7f4723
Colors.jl/src/conversions.jl
Lines 76 to 78 in e7f4723
Proposal
What about a new function
maximize_chroma(c::Union{Luv,LCHuv}; ltol=0, htol=0)
?Where
ltol
: lightness tolerance,htol
: hue tolerance.The current
MSC()
will be redefined as:Well, it is still in the planning stage and I am not ready for implementing it.
The text was updated successfully, but these errors were encountered: