Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1977693004:
[MD] Adjust algorithm for colorizing icons in incognito contexts. (Closed)
|
Created:
4 years, 7 months ago by Evan Stade Modified:
4 years, 7 months ago CC:
chromium-reviews Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Description[MD] Adjust algorithm for colorizing icons in incognito contexts.
BUG=611617
Committed: https://crrev.com/b216164c4d208a54f3469fc4b6fb5c3f845d38c6
Cr-Commit-Position: refs/heads/master@{#393892}
Patch Set 1 #
Total comments: 2
Patch Set 2 : multiply #
Total comments: 2
Patch Set 3 : static cast #Messages
Total messages: 17 (6 generated)
estade@chromium.org changed reviewers: + pkasting@chromium.org, sky@chromium.org
+pkasting for review +sky for OWNERS review
The CQ bit was checked by estade@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1977693004/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1977693004/1
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/1977693004/diff/1/ui/gfx/color_utils.cc File ui/gfx/color_utils.cc (right): https://codereview.chromium.org/1977693004/diff/1/ui/gfx/color_utils.cc#newco... ui/gfx/color_utils.cc:338: return SkColorSetA(text_color, 0xCC); Might test_color have an alpha < 0xCC?
https://codereview.chromium.org/1977693004/diff/1/ui/gfx/color_utils.cc File ui/gfx/color_utils.cc (right): https://codereview.chromium.org/1977693004/diff/1/ui/gfx/color_utils.cc#newco... ui/gfx/color_utils.cc:338: return SkColorSetA(text_color, 0xCC); On 2016/05/13 19:29:34, sky wrote: > Might test_color have an alpha < 0xCC? yea, I guess text color can be non-opaque (theme-controlled text as on the bookmark bar being the only example I can find), so this should actually be a multiplication.
LGTM
LGTM https://codereview.chromium.org/1977693004/diff/20001/ui/gfx/color_utils.cc File ui/gfx/color_utils.cc (right): https://codereview.chromium.org/1977693004/diff/20001/ui/gfx/color_utils.cc#n... ui/gfx/color_utils.cc:338: return SkColorSetA(text_color, 0.8 * SkColorGetA(text_color)); Nit: I would static_cast the second arg to avoid the implicit double truncation. (Might want to use 0.8f too.)
https://codereview.chromium.org/1977693004/diff/20001/ui/gfx/color_utils.cc File ui/gfx/color_utils.cc (right): https://codereview.chromium.org/1977693004/diff/20001/ui/gfx/color_utils.cc#n... ui/gfx/color_utils.cc:338: return SkColorSetA(text_color, 0.8 * SkColorGetA(text_color)); On 2016/05/14 02:07:47, Peter Kasting wrote: > Nit: I would static_cast the second arg to avoid the implicit double truncation. > > (Might want to use 0.8f too.) Done.
The CQ bit was checked by estade@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from pkasting@chromium.org, sky@chromium.org Link to the patchset: https://codereview.chromium.org/1977693004/#ps40001 (title: "static cast")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1977693004/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1977693004/40001
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== [MD] Adjust algorithm for colorizing icons in incognito contexts. BUG=611617 ========== to ========== [MD] Adjust algorithm for colorizing icons in incognito contexts. BUG=611617 Committed: https://crrev.com/b216164c4d208a54f3469fc4b6fb5c3f845d38c6 Cr-Commit-Position: refs/heads/master@{#393892} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/b216164c4d208a54f3469fc4b6fb5c3f845d38c6 Cr-Commit-Position: refs/heads/master@{#393892} |
