Issue metadata
Sign in to add a comment
|
Change IsDark() to use relative luminance, eliminate most calls to GetLuma() |
||||||||||||||||||||||
Issue description
color_utils::IsDark() checks whether the luma is < 128. Callers use this to pick a contrasting (or matching) light or dark color.
We should be using relative luminance instead, which represents the actual visible contrast. Luma uses the gamma-compressed color values and thus doesn't match the human visual system as well.
The implementation of this would be something like:
bool IsDark(SkColor color) {
// The midpoint luminance where white and black contrast equally. This is
// equal to sqrt(1.05 * 0.05).
static constexpr kMidpointLuminance = 0.229128784747792;
return GetRelativeLuminance(color) <= kMidpointLuminance;
}
Note that the brightest grey that's considered "dark" by the old function was (127, 127, 127), whereas by the new one it's (131, 131, 131). This difference is probably not very perceptible to most people, but picking white to contrast with (128, 128, 128) (as the new function would) instead of black is the difference between a contrast ratio of 4.32 and 4.86; WCAG 1.4.3 specifies a minimum of 4.5, so this can fix real violations of this spec.
In the process of making this change, we should audit callers of this function and update comments/names/usage as need be. For example, BlendTowardOppositeLuma() calls this, but should probably use the new implementation and be BlendTowardsOppositeLuminance() (or BlendTowardsContrastingWhiteOrBlack() or something).
We should also audit and remove most other callers of GetLuma(). It looks like the common usage is to compare against a threshold to pick a light or dark color. Probably these thresholds should be removed and the callers changed to use the above IsDark() implementation; otherwise they risk having contrast well below accessibility standards.
,
Mar 27 2017
,
Apr 21 2017
,
Aug 4 2017
,
Aug 7 2017
,
Dec 14 2017
,
Mar 5 2018
Also note that even if this change is made, specific light or dark colors may still not be sufficiently readable. For example, the contrast ratio of kGoogleGreen700 (#188038) against the darkest "light" grey #848484 is 1.3.
,
Jun 8 2018
,
Jun 26 2018
,
Jul 11
,
Aug 2
,
Jan 8
Note that the value in comment 0 was wrong because I failed to subtract 0.05 at the end. Also note that since Material Refresh sets a new darkest color of Google Grey 900 (#202124), the midpoint has changed. Turns out these two effects roughly cancel each other out and the new midpoint is 0.211692036 :)
,
Jan 8
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4aad48c08d9d3a86af8e4a2f1a79af3a0cea7ab5 commit 4aad48c08d9d3a86af8e4a2f1a79af3a0cea7ab5 Author: Peter Kasting <pkasting@chromium.org> Date: Tue Jan 08 23:41:49 2019 Change IsDark() to use relative luminance. This also hardcodes grey 900 as the darkest color now that it's not conditional on any flags, and limits overriding that to a test-only setter. This means BlendTowardOppositeLuma() technically blends toward the opposite luminance. I will be fixing up the name, the callers, and uses of luma in general in separate CLs. Bug: 659451 Change-Id: I5943cc8018ea87c8272ee53ac6270736d96d5324 Reviewed-on: https://chromium-review.googlesource.com/c/1400057 Commit-Queue: Peter Kasting <pkasting@chromium.org> Reviewed-by: Michael Wasserman <msw@chromium.org> Reviewed-by: Allen Bauer <kylixrd@chromium.org> Cr-Commit-Position: refs/heads/master@{#620937} [modify] https://crrev.com/4aad48c08d9d3a86af8e4a2f1a79af3a0cea7ab5/chrome/browser/themes/theme_service.cc [modify] https://crrev.com/4aad48c08d9d3a86af8e4a2f1a79af3a0cea7ab5/ui/base/material_design/material_design_controller.cc [modify] https://crrev.com/4aad48c08d9d3a86af8e4a2f1a79af3a0cea7ab5/ui/gfx/color_utils.cc [modify] https://crrev.com/4aad48c08d9d3a86af8e4a2f1a79af3a0cea7ab5/ui/gfx/color_utils.h [modify] https://crrev.com/4aad48c08d9d3a86af8e4a2f1a79af3a0cea7ab5/ui/gfx/color_utils_unittest.cc
,
Jan 9
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4ba3d240849dad673869122004a3923db48335a2 commit 4ba3d240849dad673869122004a3923db48335a2 Author: Peter Kasting <pkasting@chromium.org> Date: Wed Jan 09 01:04:10 2019 Rename BlendTowardOppositeLuma() to BlendTowardContrastingEndpoint(). This is clearer, and now more accurate as well. Also add GetContrastingEndpoint() as a much more readable substitute for BlendTowardOppositeLuma(color, SK_AlphaOPAQUE). Bug: 659451 Change-Id: I62d818422aae1b1e96dc8578f2ac01e9eb6adef9 Reviewed-on: https://chromium-review.googlesource.com/c/1400060 Commit-Queue: Peter Kasting <pkasting@chromium.org> Reviewed-by: Michael Wasserman <msw@chromium.org> Cr-Commit-Position: refs/heads/master@{#620974} [modify] https://crrev.com/4ba3d240849dad673869122004a3923db48335a2/chrome/browser/themes/theme_service.cc [modify] https://crrev.com/4ba3d240849dad673869122004a3923db48335a2/chrome/browser/ui/omnibox/omnibox_theme.cc [modify] https://crrev.com/4ba3d240849dad673869122004a3923db48335a2/chrome/browser/ui/sync/profile_signin_confirmation_helper.cc [modify] https://crrev.com/4ba3d240849dad673869122004a3923db48335a2/chrome/browser/ui/views/download/download_shelf_view.cc [modify] https://crrev.com/4ba3d240849dad673869122004a3923db48335a2/chrome/browser/ui/views/frame/glass_browser_frame_view.cc [modify] https://crrev.com/4ba3d240849dad673869122004a3923db48335a2/chrome/browser/ui/views/tabs/new_tab_button.cc [modify] https://crrev.com/4ba3d240849dad673869122004a3923db48335a2/chrome/browser/ui/views/toolbar/browser_actions_container.cc [modify] https://crrev.com/4ba3d240849dad673869122004a3923db48335a2/chrome/browser/ui/views/toolbar/toolbar_ink_drop_util.cc [modify] https://crrev.com/4ba3d240849dad673869122004a3923db48335a2/ui/gfx/color_utils.cc [modify] https://crrev.com/4ba3d240849dad673869122004a3923db48335a2/ui/gfx/color_utils.h [modify] https://crrev.com/4ba3d240849dad673869122004a3923db48335a2/ui/views/controls/button/checkbox.cc [modify] https://crrev.com/4ba3d240849dad673869122004a3923db48335a2/ui/views/controls/button/md_text_button.cc [modify] https://crrev.com/4ba3d240849dad673869122004a3923db48335a2/ui/views/controls/focusable_border.cc [modify] https://crrev.com/4ba3d240849dad673869122004a3923db48335a2/ui/views/controls/progress_bar.cc
,
Jan 9
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b029e67ee93dd000b7d082e660351f0cdbf39b11 commit b029e67ee93dd000b7d082e660351f0cdbf39b11 Author: Peter Kasting <pkasting@chromium.org> Date: Wed Jan 09 16:04:06 2019 Remove the majority of uses of GetLuma(). Typically this is to determine a contrasting color or to distinguish light/dark. There are better fucntions for these purposes. Bug: 659451 Change-Id: Iad770610ef6b93c75c880badcc2ac9784c74ac12 Reviewed-on: https://chromium-review.googlesource.com/c/1401428 Reviewed-by: Ben Wells <benwells@chromium.org> Reviewed-by: Michael Wasserman <msw@chromium.org> Commit-Queue: Peter Kasting <pkasting@chromium.org> Cr-Commit-Position: refs/heads/master@{#621161} [modify] https://crrev.com/b029e67ee93dd000b7d082e660351f0cdbf39b11/apps/ui/views/app_window_frame_view.cc [modify] https://crrev.com/b029e67ee93dd000b7d082e660351f0cdbf39b11/chrome/browser/web_applications/components/web_app_icon_generator.cc [modify] https://crrev.com/b029e67ee93dd000b7d082e660351f0cdbf39b11/ui/gfx/sys_color_change_listener.cc
,
Jan 10
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/54f86638d4471f07a0cc73cda30640a971ee1531 commit 54f86638d4471f07a0cc73cda30640a971ee1531 Author: Peter Kasting <pkasting@chromium.org> Date: Thu Jan 10 06:45:22 2019 Add a test for GetColorWithMaxContrast(). Bug: 659451 Change-Id: Ie3afcbf6a05ab1591453cfecc7199e5019e257bf Reviewed-on: https://chromium-review.googlesource.com/c/1404479 Reviewed-by: Michael Wasserman <msw@chromium.org> Commit-Queue: Peter Kasting <pkasting@chromium.org> Cr-Commit-Position: refs/heads/master@{#621487} [modify] https://crrev.com/54f86638d4471f07a0cc73cda30640a971ee1531/ui/gfx/color_utils_unittest.cc
,
Jan 10
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b20fd3ca8b8c5b8f04845e204473eba155aedb2f commit b20fd3ca8b8c5b8f04845e204473eba155aedb2f Author: Peter Kasting <pkasting@chromium.org> Date: Thu Jan 10 19:18:44 2019 Replace calls to IsDark() where possible. For example, PickContrastingColor() is a better way of picking between a light and dark color to place against a background, because it guarantees using the one that actually contrasts more. Bug: 659451 Change-Id: I27febca506971ad7ce6db265fc92d680c3f81575 Reviewed-on: https://chromium-review.googlesource.com/c/1404406 Reviewed-by: Scott Violet <sky@chromium.org> Commit-Queue: Peter Kasting <pkasting@chromium.org> Cr-Commit-Position: refs/heads/master@{#621691} [modify] https://crrev.com/b20fd3ca8b8c5b8f04845e204473eba155aedb2f/ash/public/cpp/default_frame_header.cc [modify] https://crrev.com/b20fd3ca8b8c5b8f04845e204473eba155aedb2f/chrome/browser/ui/views/tabs/tab_strip.cc [modify] https://crrev.com/b20fd3ca8b8c5b8f04845e204473eba155aedb2f/ui/views/window/frame_caption_button.cc
,
Jan 11
On https://chromium-review.googlesource.com/c/chromium/src/+/1404395 Bret asked for screenshots of that particular patch's effects with (light, dark) frame x (light, dark) background. Attached are comparisons. The effects are not dramatic. My take is: dark frame dark bg: Neither trunk nor patch look native, but patch looks more seamless (trunk's seems way too bright and colorful) dark frame light bg: Trunk and patch are very similar (neither looks native, but neither looks bad) light frame dark bg: Same reaction as dark on light light frame light bg: Patch looks near-native, trunk is too dark So in these particular cases, the patch looks like a win.
,
Jan 11
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d4658038522bbb583275fe77237f4ffb5d441007 commit d4658038522bbb583275fe77237f4ffb5d441007 Author: Peter Kasting <pkasting@chromium.org> Date: Fri Jan 11 22:34:47 2019 Make glass frame inactive border color better approximate native behavior. The existing blend color/alpha were not accurate. Update them. Also, it seems unnecessary (and gives a behavioral cliff) to do very different things for light and dark toolbar colors. Instead, just always compute the inactive border as if it's being blended atop the inactive frame color, which won't look bad since that color is in the adjacent pixel. This has the added bonus of being simpler as well. Bug: 659451 Change-Id: I4fd2aa9d24af266b590c6dfcc2cbc7c0c96e2e86 Reviewed-on: https://chromium-review.googlesource.com/c/1404395 Reviewed-by: Bret Sepulveda <bsep@chromium.org> Commit-Queue: Peter Kasting <pkasting@chromium.org> Cr-Commit-Position: refs/heads/master@{#622192} [modify] https://crrev.com/d4658038522bbb583275fe77237f4ffb5d441007/chrome/browser/ui/views/frame/glass_browser_frame_view.cc
,
Jan 11
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/773dad5c29dda13caf05e7cba5f3a712df68e8b6 commit 773dad5c29dda13caf05e7cba5f3a712df68e8b6 Author: Peter Kasting <pkasting@chromium.org> Date: Fri Jan 11 22:46:52 2019 Don't use InvertColor() to try and guarantee contrast. In particular, inverting a light color may give you an even lighter color, e.g. #8070ff (contrast ratio against white 3.69) inverts to #7f8f00 (contrast ratio against white 3.6). There's a function in color_utils called GetReadableColor() that tries to lightness-invert the color, but that turns out not to work well either, e.g. HSL 60,100%,36% (contrast ratio against white 2.12) lightness-inverts to HSL 60,100%,64% (contrast ratio against white 1.06). Instead use GetColorWithMinimumContrast(). This will reduce the ontrast of some cases compared to current behavior, but it will also guarantee no cases are below the minimum. Bug: 659451 Change-Id: I09b5850e0ba3b76ca6e004c9320131ac175cbe43 Reviewed-on: https://chromium-review.googlesource.com/c/1406241 Reviewed-by: Kristi Park <kristipark@chromium.org> Commit-Queue: Peter Kasting <pkasting@chromium.org> Cr-Commit-Position: refs/heads/master@{#622194} [modify] https://crrev.com/773dad5c29dda13caf05e7cba5f3a712df68e8b6/chrome/browser/search/ntp_icon_source.cc
,
Jan 15
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d3adca4c9f6eec0cd94bbee54db119483772d682 commit d3adca4c9f6eec0cd94bbee54db119483772d682 Author: Peter Kasting <pkasting@chromium.org> Date: Tue Jan 15 02:04:32 2019 Better contrast for payment handler sheet headers. These were intentionally implemented to match Clank, but the Clank algorithm fails to meet WCAG AA standards for contrast, which we're trying to reach in Chrome. In particular, the chosen algorithm uses "white if it's contrast >= 3, or the default label color (which is currently GG900)". Contrast 3 is well below the accessibility minimum of 4.5. Instead use the built-in function to pick the color with max contrast. In our current color space, that guarantees a minimum contrast of just over 4. If in the future we return to allowing black (not GG900) as the darkest color, it will guarantee contrast of >= ~4.6. Bug: 659451 Change-Id: I41b7c66df5eca16380548fd703cf21211dc3f56d Reviewed-on: https://chromium-review.googlesource.com/c/1407691 Reviewed-by: anthonyvd <anthonyvd@chromium.org> Commit-Queue: Peter Kasting <pkasting@chromium.org> Cr-Commit-Position: refs/heads/master@{#622678} [modify] https://crrev.com/d3adca4c9f6eec0cd94bbee54db119483772d682/chrome/browser/ui/views/payments/payment_handler_web_flow_view_controller.cc [modify] https://crrev.com/d3adca4c9f6eec0cd94bbee54db119483772d682/chrome/browser/ui/views/payments/payment_request_views_util.cc [modify] https://crrev.com/d3adca4c9f6eec0cd94bbee54db119483772d682/chrome/browser/ui/views/payments/payment_request_views_util.h
,
Jan 16
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/40f742b90554fb75744a896eabf627fcdc657de5 commit 40f742b90554fb75744a896eabf627fcdc657de5 Author: Peter Kasting <pkasting@chromium.org> Date: Wed Jan 16 02:07:38 2019 Update contrast ratio on the IconLabelBubbleView separator. This was originally written in https://codereview.chromium.org/1998493002/ to implement the spec at https://drive.google.com/corp/drive/folders/0B6Wxmj9LZL6XZjJURlVaN1hUTzQ . The spec produced contrast ratios for the separator of 2.44 in normal and 3.69 in incognito. Over time the behavior diverged, primarily since the incognito omnibox background color changed. The current omnibox ratio is 7.39. The new code produces ratios of 2.42 in normal and 3.11 in incognito, similar to the original behavior, but bringing incognito closer to normal (presuming that the old normal behavior was the "gold standard" design-wise). The primary motivation here, though, was to unify handling of light and dark so the code didn't "toggle", but simply behaved consistently for all inputs. Bug: 659451 Change-Id: Ic33c506e46864389bed8285f5cfbf439915b5222 Reviewed-on: https://chromium-review.googlesource.com/c/1411926 Reviewed-by: Evan Stade <estade@chromium.org> Commit-Queue: Peter Kasting <pkasting@chromium.org> Cr-Commit-Position: refs/heads/master@{#623030} [modify] https://crrev.com/40f742b90554fb75744a896eabf627fcdc657de5/chrome/browser/ui/views/location_bar/icon_label_bubble_view.cc
,
Jan 16
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8aa7657aa863a45d7571f82a63352180ab41f36c commit 8aa7657aa863a45d7571f82a63352180ab41f36c Author: Peter Kasting <pkasting@chromium.org> Date: Wed Jan 16 02:11:50 2019 Eliminate GetReadableColor(); it's ineffective. It uses the mechanic of lightness-inverting the source color and picking the option that has better contrast. But lightness-inverting may fail to improve contrast, since contrast is based on luminance, which is not linearly related to lightness. For example, HSL 60,100%,36% (contrast ratio against white 2.12) lightness-inverts to HSL 60,100%,64% (contrast ratio against white 1.06). Instead, use functions like GetColorWithMinimumContrast() or GetColorWithMaxContrast() to ensure that the colors in question have sufficient contrast. I tried to make the changes here relatively behavior-preserving, so GetReadableColor(SK_ColorWHITE, ...) was converted to GetColorWithMaxContrast() since the old code would have chosen between white and black, and other cases were converted to GetColorWithMinimumContrast(). Bug: 659451 Change-Id: Id8508a9c8d65236f85b7fc5246b22ec5bb8afe0b Reviewed-on: https://chromium-review.googlesource.com/c/1407690 Commit-Queue: Peter Kasting <pkasting@chromium.org> Reviewed-by: Xiyuan Xia <xiyuan@chromium.org> Reviewed-by: Michael Wasserman <msw@chromium.org> Cr-Commit-Position: refs/heads/master@{#623037} [modify] https://crrev.com/8aa7657aa863a45d7571f82a63352180ab41f36c/ash/app_list/views/app_list_item_view.cc [modify] https://crrev.com/8aa7657aa863a45d7571f82a63352180ab41f36c/ash/shelf/shelf_tooltip_bubble.cc [modify] https://crrev.com/8aa7657aa863a45d7571f82a63352180ab41f36c/ash/system/power/power_button_menu_item_view.cc [modify] https://crrev.com/8aa7657aa863a45d7571f82a63352180ab41f36c/ash/touch/touch_observer_hud.cc [modify] https://crrev.com/8aa7657aa863a45d7571f82a63352180ab41f36c/chrome/browser/themes/theme_service.cc [modify] https://crrev.com/8aa7657aa863a45d7571f82a63352180ab41f36c/chrome/browser/ui/views/frame/opaque_browser_frame_view.cc [modify] https://crrev.com/8aa7657aa863a45d7571f82a63352180ab41f36c/chrome/browser/ui/views/frame/opaque_browser_frame_view_browsertest.cc [modify] https://crrev.com/8aa7657aa863a45d7571f82a63352180ab41f36c/chrome/browser/ui/views/location_bar/custom_tab_bar_view.cc [modify] https://crrev.com/8aa7657aa863a45d7571f82a63352180ab41f36c/chrome/browser/ui/views/status_bubble_views.cc [modify] https://crrev.com/8aa7657aa863a45d7571f82a63352180ab41f36c/chrome/browser/ui/views/subtle_notification_view.cc [modify] https://crrev.com/8aa7657aa863a45d7571f82a63352180ab41f36c/ui/gfx/color_utils.cc [modify] https://crrev.com/8aa7657aa863a45d7571f82a63352180ab41f36c/ui/gfx/color_utils.h [modify] https://crrev.com/8aa7657aa863a45d7571f82a63352180ab41f36c/ui/views/controls/label.cc [modify] https://crrev.com/8aa7657aa863a45d7571f82a63352180ab41f36c/ui/views/controls/label.h [modify] https://crrev.com/8aa7657aa863a45d7571f82a63352180ab41f36c/ui/views/controls/styled_label_unittest.cc
,
Jan 16
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d34a559d1de241b84b4c7221ecb64e59b442b22e commit d34a559d1de241b84b4c7221ecb64e59b442b22e Author: Peter Kasting <pkasting@chromium.org> Date: Wed Jan 16 06:30:18 2019 Several minor fixes noted in self-review. * Ensure frame title and caption buttons change between light and dark simultaneously. This was broken in https://crrev.com/621691 . Along the way this attempts to enforce a minimum contrast of 3 (for the caption buttons) or 4.5 (for the window title), for accessibility. * Fix comment. * Tiny tweak to glass frame caption alpha to be more native. * Use n-way max/min. Bug: 659451 Change-Id: I33e2154e5425500a0908e917cf11ba420b3cb8ac Reviewed-on: https://chromium-review.googlesource.com/c/1407949 Commit-Queue: Peter Kasting <pkasting@chromium.org> Reviewed-by: Scott Violet <sky@chromium.org> Cr-Commit-Position: refs/heads/master@{#623136} [modify] https://crrev.com/d34a559d1de241b84b4c7221ecb64e59b442b22e/ash/public/cpp/default_frame_header.cc [modify] https://crrev.com/d34a559d1de241b84b4c7221ecb64e59b442b22e/chrome/browser/ui/views/frame/glass_browser_frame_view.cc [modify] https://crrev.com/d34a559d1de241b84b4c7221ecb64e59b442b22e/chrome/browser/ui/views/frame/glass_browser_frame_view.h [modify] https://crrev.com/d34a559d1de241b84b4c7221ecb64e59b442b22e/ui/gfx/color_utils.cc [modify] https://crrev.com/d34a559d1de241b84b4c7221ecb64e59b442b22e/ui/views/window/frame_caption_button.cc
,
Jan 16
|
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by enne@chromium.org
, Jan 25 2017