mwoehlke added inline comments. INLINE COMMENTS
> kcolorutils.cpp:60 > *c = khcy.c; > *h = khcy.h; > *y = khcy.y; I guess it will be less confusing if this is also normalized? *h = khcy.h + (khcy.h < 0.0 ? 1.0 : 0.0); > kcolorutils.h:36 > +/** > + * Calculate the hue of a color. The range is 0.0 (red) to 0.999999 > (slightly blue red). > + * This is based on linear RGB. "0.999..." is a little odd, and I wonder if it is accurate for e.g. `QColor::fromRgba64(0xffff, 0, 1}`? Maybe something like "just under 1.0 (also red)" would be more useful? (If you want to get technical, "to (asymptotically) 1.0" would also work. Maybe "0.0 //approaching// 1.0"?) The range is of course [0, 1), but I don't know how to say that in straight-forward English 🙂. OTOH, I wonder if it's worth bothering, vs. just saying "0.0 to 1.0" and ignoring that it will never be //exactly// 1.0. > kcolorutils.h:37 > + * Calculate the hue of a color. The range is 0.0 (red) to 0.999999 > (slightly blue red). > + * This is based on linear RGB. > + * Suggestion: "The result is computed in linear (not sRGB) color space and may differ slightly from QColor::hue()." For the others, drop "and may differ...". > kcolorutils.h:69 > + * > + * The range of hue is cyclical. Examples: 0.0 and 1.0 are both red, > -0.166667 and 0.833333 are both magenta. > + * The range of chroma is from 0.0 (none) to 1.0 (full). I guess here should use the same text as `hue`? > kcolorutils.h:82 > + * > + * The range of hue is cyclical. Examples: 0.0 and 1.0 are both red, > -0.166667 and 0.833333 are both magenta. > + * The range of chroma is from 0.0 (none) to 1.0 (full). This reads a little odd. I think "For example, 0.0 and ..." would be better. > kcolorutils.h:83 > + * The range of hue is cyclical. Examples: 0.0 and 1.0 are both red, > -0.166667 and 0.833333 are both magenta. > + * The range of chroma is from 0.0 (none) to 1.0 (full). > + * The range of luma is from 0.0 (black) to 1.0 (white). It might be useful to add (to luma also) "Out of range values will be clamped.". > kcolorutils.h:90 > + */ > +KGUIADDONS_EXPORT QColor getHcyColor(qreal hue, qreal chroma, qreal luma, > qreal alpha = 1.0); > + This name feels a little odd, but I'd appreciate some broader input. Maybe `hcyColor`? REPOSITORY R273 KGuiAddons REVISION DETAIL https://phabricator.kde.org/D27017 To: ndavis, #frameworks, dfaure Cc: mwoehlke, broulik, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
