Simplify life for people trying to pick a "readable" foreground color by not ...

Simplify life for people trying to pick a "readable" foreground color by not requiring them to provide two choices.  This also speeds the call up a tiny bit by not calculating the background luminance twice.

BUG=none
TEST=none

[Peter Kasting, 2009-09-24 17:59:39]

Brett, could you take a look at this since Ben is out?

[Peter Kasting, 2009-09-24 21:33:21]

Musical reviewers.  Brett seems to be under the weather.  Scott, you want to
rubber-stamp this?

[sky, 2009-09-24 21:46:32]

Rubber stamp away. I think you should add a DCHECK as mentioned, but otherwise
LGTM.

http://codereview.chromium.org/220029/diff/2002/2003
File app/gfx/color_utils.cc (right):

http://codereview.chromium.org/220029/diff/2002/2003#newcode73
Line 73: return (foreground_luminance > background_luminance) ?
Do you need to deal with getting 0,0 passed in, which results in divide by 0? At
least a DCHECK.

[Peter Kasting, 2009-09-24 21:50:05]

http://codereview.chromium.org/220029/diff/2002/2003
File app/gfx/color_utils.cc (right):

http://codereview.chromium.org/220029/diff/2002/2003#newcode73
Line 73: return (foreground_luminance > background_luminance) ?
On 2009/09/24 21:46:32, sky wrote:
> Do you need to deal with getting 0,0 passed in, which results in divide by 0?
At
> least a DCHECK.

In theory yes... in practice the results from RelativeLuminance() are guaranteed
to be > 0, so this can't happen.  If it ever does, a DCHECK is just going to
make us crash one line earlier.  I will add some comments though.

[sky, 2009-09-24 22:16:35]

On Thu, Sep 24, 2009 at 2:50 PM,  <pkasting@chromium.org> wrote:
>
> http://codereview.chromium.org/220029/diff/2002/2003
> File app/gfx/color_utils.cc (right):
>
> http://codereview.chromium.org/220029/diff/2002/2003#newcode73
> Line 73: return (foreground_luminance > background_luminance) ?
> On 2009/09/24 21:46:32, sky wrote:
>>
>> Do you need to deal with getting 0,0 passed in, which results in
>
> divide by 0? At
>>
>> least a DCHECK.
>
> In theory yes... in practice the results from RelativeLuminance() are
> guaranteed to be > 0, so this can't happen. =A0If it ever does, a DCHECK
> is just going to make us crash one line earlier. =A0I will add some
> comments though.

Ya, I saw that your code always guarantees at least .05, but someone
else that decided to call it might not realize they need a value > 0.

  -Scott

[Peter Kasting, 2009-09-24 22:20:16]

On 2009/09/24 22:16:35, sky wrote:
> Ya, I saw that your code always guarantees at least .05, but someone
> else that decided to call it might not realize they need a value > 0.

The checked-in version has some explicit comments here.

[sky, 2009-09-24 22:22:37]

Cool.

Thanks,

  -Scott

On Thu, Sep 24, 2009 at 3:20 PM,  <pkasting@chromium.org> wrote:
> On 2009/09/24 22:16:35, sky wrote:
>>
>> Ya, I saw that your code always guarantees at least .05, but someone
>> else that decided to call it might not realize they need a value > 0.
>
> The checked-in version has some explicit comments here.
>
> http://codereview.chromium.org/220029
>