Do proper HSL > HSB conversion....

Do proper HSL > HSB conversion.

BUG=http://crbug.com/15760
TEST=slight visual change


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=22377

[Erik does not do reviews, 2009-08-03 22:09:42]

LGTM

http://codereview.chromium.org/159810/diff/1/2
File chrome/browser/browser_theme_provider_mac.mm (right):

http://codereview.chromium.org/159810/diff/1/2#newcode15
Line 15: void HSLToHSB(const skia::HSL& hsl, CGFloat* h, CGFloat* s, CGFloat* b)
{
One nit perhaps worth mentioning in a comment is that this is really Cocoa HSB,
since HSB in general is supposed to use the same 0-360 range for Hue as HSL.

[Avi (use Gerrit), 2009-08-04 13:58:21]

http://codereview.chromium.org/159810/diff/1/2
File chrome/browser/browser_theme_provider_mac.mm (right):

http://codereview.chromium.org/159810/diff/1/2#newcode15
Line 15: void HSLToHSB(const skia::HSL& hsl, CGFloat* h, CGFloat* s, CGFloat* b)
{
On 2009/08/03 22:09:42, Erik Kay wrote:
> since HSB in general is supposed to use the same 0-360 range for Hue as HSL.

But Skia::HSL also uses [0.0, 1.0). I think that a reasonable reader would grok
it from context.

[Erik does not do reviews, 2009-08-04 14:23:59]

On Tue, Aug 4, 2009 at 6:58 AM, <avi@chromium.org> wrote:
>
> http://codereview.chromium.org/159810/diff/1/2
> File chrome/browser/browser_theme_provider_mac.mm (right):
>
> http://codereview.chromium.org/159810/diff/1/2#newcode15
> Line 15: void HSLToHSB(const skia::HSL& hsl, CGFloat* h, CGFloat* s,
> CGFloat* b) {
> On 2009/08/03 22:09:42, Erik Kay wrote:
>>
>> since HSB in general is supposed to use the same 0-360 range for Hue
>
> as HSL.
>
> But Skia::HSL also uses [0.0, 1.0). I think that a reasonable reader
> would grok it from context.

Weird.  So it uses 0-1 for H in HSL, but when you convert it to HSV,
it converts H to 0-360?  And Apple expects it to be 0-1 at this point?

Gotta love it.


>
> http://codereview.chromium.org/159810
>

[Avi (use Gerrit), 2009-08-04 15:51:29]

Apple uses [0, 1) always, since a color for them is just a tuple of [0, 1)
values and the associated namespace to interpret. Like it or not, at least it
makes sense. Why Skia is of two minds about this I can't say.