Dynamically compute tab/frame separator color.
For reference, this color is overlaid atop the frame/toolbar when outlining tabs
and for outlines/shadows when drawing the new tab button. It's either white or
black with a varying (but usually 0x40) alpha value.
Computing this is surprisingly complicated, beause we're trying to contrast with
two different colors simultaneously (tab and frame), and we also want to set our
magic numbers so as to achieve the colors from the design specs in the default
theme.
After quite a bit of thought, I elected to use a mechanism that defaults to
moving the frame away from the tab luminance; that is, if the tab is "brighter"
than the frame, we make the separator darken the frame, and if the tab is
darker, we try to lighten the frame. If the frame is already so dark or light
that the result has too low of contrast with the frame color, we reverse
direction. ("Too low" here is a lot lower than I'd like, but any higher and
we'd end up using light separators for the default theme in incognito mode,
which I think looks good but Sebastien dislikes.) In the case where we reversed
direction, we have to worry that the result of all this won't contrast enough
with the tab; in that case, we push up the alpha value so the result contrasts
enough with the tab as well. This last computation is the most expensive
because of how I chose to make it behave, but I think the behavior I selected
(too complicated to explain here, see code/comments for details) will feel more
consistent than any of the simpler methods I considered.
Because computing the separator color can be expensive (every call to
GetRelativeLuminance() can involve floating-point exponentiation among other
things, and in the worst case the color computation computes the desired color
via a 7-iteration loop), I elected to cache the computed value in a map. This
might be a case of premature optimization, but in debugging it looked like this
color could be requested frequently, and I didn't want to risk performance
problems.
Even this choice presented options. I used a simple map that I never prune
entries from; at worst, we'll add up to 2 entries per distinct theme the user
switches to while running, which didn't seem too bad. I considered instead
using base::MRUCache, which would let me cap the size. I also considered just
keeping a couple member structs containing the relevant information for the
normal and incognito color schemes, but this generally seemed like it ended up
as more code than the other routes. None of these options seems wildly better
or worse than the others; I'm willing to change course in the face of violent
opinion :)
Finally, because the alpha value of the separator can now vary, I converted the
code in tab_strip.cc that set it to fixed values to instead use scaling
multipliers. These will compute the same values as before when the separator
has its default (0x40) alpha, but in the case where we've computed some larger
alpha, scaling proportionally seemed like the best thing to do. I used a
saturated_cast in one place where I wasn't sure the result was guaranteed to
stay <= 255.
BUG=585470
TEST=See bug comment 0
Review URL: https://codereview.chromium.org/1785613004
Cr-Commit-Position: refs/heads/master@{#381617}
diff --git a/chrome/browser/themes/theme_service.cc b/chrome/browser/themes/theme_service.cc
index 208f11fd..8ce03c1 100644
--- a/chrome/browser/themes/theme_service.cc
+++ b/chrome/browser/themes/theme_service.cc
@@ -425,12 +425,28 @@
return SkColorSetA(
GetColor(ThemeProperties::COLOR_TOOLBAR_BUTTON_ICON, incognito),
0x33);
+ case ThemeProperties::COLOR_TOOLBAR_TOP_SEPARATOR: {
+ const SkColor tab_color =
+ GetColor(ThemeProperties::COLOR_TOOLBAR, incognito);
+ const SkColor frame_color =
+ GetColor(ThemeProperties::COLOR_FRAME, incognito);
+ const SeparatorColorKey key(tab_color, frame_color);
+ auto i = separator_color_cache_.find(key);
+ if (i != separator_color_cache_.end())
+ return i->second;
+ const SkColor separator_color = GetSeparatorColor(tab_color, frame_color);
+ separator_color_cache_[key] = separator_color;
+ return separator_color;
+ }
case ThemeProperties::COLOR_BACKGROUND_TAB: {
// The tints here serve a different purpose than TINT_BACKGROUND_TAB.
// That tint is used to create background tab images for custom themes by
// lightening the frame images. The tints here create solid colors for
- // background tabs by darkening the foreground tab (toolbar) color.
- const color_utils::HSL kTint = {-1, -1, 0.4296875};
+ // background tabs by darkening the foreground tab (toolbar) color. These
+ // values are chosen to turn the default normal and incognito MD frame
+ // colors (0xf2f2f2 and 0x505050) into 0xd0d0d0 and 0x373737,
+ // respectively.
+ const color_utils::HSL kTint = {-1, -1, 0.42975};
const color_utils::HSL kTintIncognito = {-1, -1, 0.34375};
return color_utils::HSLShift(
GetColor(ThemeProperties::COLOR_TOOLBAR, incognito),
@@ -591,6 +607,62 @@
theme_supplier_->HasCustomImage(id);
}
+// static
+SkColor ThemeService::GetSeparatorColor(SkColor tab_color,
+ SkColor frame_color) {
+ // We use this alpha value for the separator if possible.
+ const SkAlpha kAlpha = 0x40;
+
+ // In most cases, if the tab is lighter than the frame, we darken the
+ // frame; if the tab is darker than the frame, we lighten the frame.
+ // However, if the frame is already very dark or very light, respectively,
+ // this won't contrast sufficiently with the frame color, so we'll need to
+ // reverse when we're lightening and darkening.
+ const double tab_luminance = color_utils::GetRelativeLuminance(tab_color);
+ const double frame_luminance = color_utils::GetRelativeLuminance(frame_color);
+ const bool lighten = tab_luminance < frame_luminance;
+ SkColor separator_color = lighten ? SK_ColorWHITE : SK_ColorBLACK;
+ double separator_luminance = color_utils::GetRelativeLuminance(
+ color_utils::AlphaBlend(separator_color, frame_color, kAlpha));
+ // The minimum contrast ratio here is just under the ~1.1469 in the default MD
+ // incognito theme. We want the separator to still darken the frame in that
+ // theme, but that's about as low of contrast as we're willing to accept.
+ const double kMinContrastRatio = 1.1465;
+ if (color_utils::GetContrastRatio(separator_luminance, frame_luminance) >=
+ kMinContrastRatio)
+ return SkColorSetA(separator_color, kAlpha);
+
+ // We need to reverse whether we're darkening or lightening. We know the new
+ // separator color will contrast with the frame; check whether it also
+ // contrasts at least as well with the tab.
+ separator_color = color_utils::InvertColor(separator_color);
+ separator_luminance = color_utils::GetRelativeLuminance(
+ color_utils::AlphaBlend(separator_color, frame_color, kAlpha));
+ if (color_utils::GetContrastRatio(separator_luminance, tab_luminance) >=
+ color_utils::GetContrastRatio(separator_luminance, frame_luminance))
+ return SkColorSetA(separator_color, kAlpha);
+
+ // The reversed separator doesn't contrast enough with the tab. Compute the
+ // resulting luminance from adjusting the tab color, instead of the frame
+ // color, by the separator color.
+ const double target_luminance = color_utils::GetRelativeLuminance(
+ color_utils::AlphaBlend(separator_color, tab_color, kAlpha));
+
+ // Now try to compute an alpha for the separator such that, when blended with
+ // the frame, it results in the above luminance. Because the luminance
+ // computation is not easily invertible, we use a binary search over the
+ // possible range of alpha values.
+ SkAlpha alpha = 128;
+ for (int delta = lighten ? 64 : -64; delta != 0; delta /= 2) {
+ const double luminance = color_utils::GetRelativeLuminance(
+ color_utils::AlphaBlend(separator_color, frame_color, alpha));
+ if (luminance == target_luminance)
+ break;
+ alpha += (luminance < target_luminance) ? -delta : delta;
+ }
+ return SkColorSetA(separator_color, alpha);
+}
+
gfx::ImageSkia* ThemeService::GetImageSkiaNamed(int id, bool incognito) const {
gfx::Image image = GetImageNamed(id, incognito);
if (image.IsEmpty())
