Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2099983002:
Tweak Ash material design battery vector icons (Closed)
|
Created:
4 years, 5 months ago by tdanderson Modified:
4 years, 5 months ago Reviewers:
Evan Stade CC:
chromium-reviews Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionTweak Ash material design battery vector icons
Small changes to the 1x versions of the MD battery
icon and the alert badge to make the battery
badges appear centered.
BUG=623417
TEST=manual
Committed: https://crrev.com/41d9d852ac8fd621fbff7892bdd9f83332de1e14
Cr-Commit-Position: refs/heads/master@{#402214}
Patch Set 1 #
Total comments: 4
Messages
Total messages: 13 (3 generated)
tdanderson@chromium.org changed reviewers: + estade@chromium.org
Evan, can you please take a look?
rs lgtm https://codereview.chromium.org/2099983002/diff/1/ui/gfx/vector_icons/system_... File ui/gfx/vector_icons/system_tray_battery.1x.icon (right): https://codereview.chromium.org/2099983002/diff/1/ui/gfx/vector_icons/system_... ui/gfx/vector_icons/system_tray_battery.1x.icon:7: R_H_LINE_TO, 5.99f, aside: I wonder if this really should be 6. The skiafy script already rounds to the nearest hundredth. Maybe we should be rounding to the nearest 50th? 10th?
https://codereview.chromium.org/2099983002/diff/1/ui/gfx/vector_icons/system_... File ui/gfx/vector_icons/system_tray_battery.1x.icon (right): https://codereview.chromium.org/2099983002/diff/1/ui/gfx/vector_icons/system_... ui/gfx/vector_icons/system_tray_battery.1x.icon:7: R_H_LINE_TO, 5.99f, On 2016/06/27 15:10:36, Evan Stade wrote: > aside: I wonder if this really should be 6. The skiafy script already rounds to > the nearest hundredth. Maybe we should be rounding to the nearest 50th? 10th? From examining a random sample of icons on go/icons, two decimal places of precision (where the last digit can be even or odd) looks to be the norm. I don't really see much advantage of changing the rounding logic, and doing so will have the effect of changing the literals provided by these SVGs in ways that could (potentially) cause incorrect rendering of the icon.
The CQ bit was checked by tdanderson@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Committed patchset #1 (id:1)
Message was sent while issue was closed.
Description was changed from ========== Tweak Ash material design battery vector icons Small changes to the 1x versions of the MD battery icon and the alert badge to make the battery badges appear centered. BUG=623417 TEST=manual ========== to ========== Tweak Ash material design battery vector icons Small changes to the 1x versions of the MD battery icon and the alert badge to make the battery badges appear centered. BUG=623417 TEST=manual Committed: https://crrev.com/41d9d852ac8fd621fbff7892bdd9f83332de1e14 Cr-Commit-Position: refs/heads/master@{#402214} ==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/41d9d852ac8fd621fbff7892bdd9f83332de1e14 Cr-Commit-Position: refs/heads/master@{#402214}
Message was sent while issue was closed.
https://codereview.chromium.org/2099983002/diff/1/ui/gfx/vector_icons/system_... File ui/gfx/vector_icons/system_tray_battery.1x.icon (right): https://codereview.chromium.org/2099983002/diff/1/ui/gfx/vector_icons/system_... ui/gfx/vector_icons/system_tray_battery.1x.icon:7: R_H_LINE_TO, 5.99f, On 2016/06/27 17:23:59, tdanderson wrote: > On 2016/06/27 15:10:36, Evan Stade wrote: > > aside: I wonder if this really should be 6. The skiafy script already rounds > to > > the nearest hundredth. Maybe we should be rounding to the nearest 50th? 10th? > > From examining a random sample of icons on go/icons, two decimal places of > precision (where the last digit can be even or odd) looks to be the norm. Yes, that would be because the skiafy tool rounds to the nearest hundredth. > I > don't really see much advantage of changing the rounding logic, and doing so > will have the effect of changing the literals provided by these SVGs in ways > that could (potentially) cause incorrect rendering of the icon. Well if there were no rounding logic at all, we'd have random numbers like 5.9995838842. I took this to be an artifact of the authoring tool that Sebastien uses, and that this might as well be 6. I arbitrarily decided to round to the nearest hundredth. This 5.99 is making me wonder if that was the best decision. But, like I said, this is an "aside" and not something you need to worry about right now.
Message was sent while issue was closed.
https://codereview.chromium.org/2099983002/diff/1/ui/gfx/vector_icons/system_... File ui/gfx/vector_icons/system_tray_battery.1x.icon (right): https://codereview.chromium.org/2099983002/diff/1/ui/gfx/vector_icons/system_... ui/gfx/vector_icons/system_tray_battery.1x.icon:7: R_H_LINE_TO, 5.99f, On 2016/06/27 22:41:14, Evan Stade wrote: > On 2016/06/27 17:23:59, tdanderson wrote: > > On 2016/06/27 15:10:36, Evan Stade wrote: > > > aside: I wonder if this really should be 6. The skiafy script already rounds > > to > > > the nearest hundredth. Maybe we should be rounding to the nearest 50th? > 10th? > > > > From examining a random sample of icons on go/icons, two decimal places of > > precision (where the last digit can be even or odd) looks to be the norm. > > Yes, that would be because the skiafy tool rounds to the nearest hundredth. I was referring to the values inside the SVG source of the icons pulled directly from go/icons before the skiafy tool has been used. > > > I > > don't really see much advantage of changing the rounding logic, and doing so > > will have the effect of changing the literals provided by these SVGs in ways > > that could (potentially) cause incorrect rendering of the icon. > > Well if there were no rounding logic at all, we'd have random numbers like > 5.9995838842. I took this to be an artifact of the authoring tool that Sebastien > uses, and that this might as well be 6. I arbitrarily decided to round to the > nearest hundredth. This 5.99 is making me wonder if that was the best decision. > But, like I said, this is an "aside" and not something you need to worry about > right now.
Message was sent while issue was closed.
On 2016/06/28 15:33:36, tdanderson wrote: > https://codereview.chromium.org/2099983002/diff/1/ui/gfx/vector_icons/system_... > File ui/gfx/vector_icons/system_tray_battery.1x.icon (right): > > https://codereview.chromium.org/2099983002/diff/1/ui/gfx/vector_icons/system_... > ui/gfx/vector_icons/system_tray_battery.1x.icon:7: R_H_LINE_TO, 5.99f, > On 2016/06/27 22:41:14, Evan Stade wrote: > > On 2016/06/27 17:23:59, tdanderson wrote: > > > On 2016/06/27 15:10:36, Evan Stade wrote: > > > > aside: I wonder if this really should be 6. The skiafy script already > rounds > > > to > > > > the nearest hundredth. Maybe we should be rounding to the nearest 50th? > > 10th? > > > > > > From examining a random sample of icons on go/icons, two decimal places of > > > precision (where the last digit can be even or odd) looks to be the norm. > > > > Yes, that would be because the skiafy tool rounds to the nearest hundredth. > > I was referring to the values inside the SVG source of the icons pulled directly > from go/icons before the skiafy tool has been used. ah, my mistake. Do you see any .99 values in those go/icons SVGs?
Message was sent while issue was closed.
On 2016/06/28 23:22:40, Evan Stade wrote: > On 2016/06/28 15:33:36, tdanderson wrote: > > > https://codereview.chromium.org/2099983002/diff/1/ui/gfx/vector_icons/system_... > > File ui/gfx/vector_icons/system_tray_battery.1x.icon (right): > > > > > https://codereview.chromium.org/2099983002/diff/1/ui/gfx/vector_icons/system_... > > ui/gfx/vector_icons/system_tray_battery.1x.icon:7: R_H_LINE_TO, 5.99f, > > On 2016/06/27 22:41:14, Evan Stade wrote: > > > On 2016/06/27 17:23:59, tdanderson wrote: > > > > On 2016/06/27 15:10:36, Evan Stade wrote: > > > > > aside: I wonder if this really should be 6. The skiafy script already > > rounds > > > > to > > > > > the nearest hundredth. Maybe we should be rounding to the nearest 50th? > > > 10th? > > > > > > > > From examining a random sample of icons on go/icons, two decimal places of > > > > precision (where the last digit can be even or odd) looks to be the norm. > > > > > > Yes, that would be because the skiafy tool rounds to the nearest hundredth. > > > > I was referring to the values inside the SVG source of the icons pulled > directly > > from go/icons before the skiafy tool has been used. > > ah, my mistake. Do you see any .99 values in those go/icons SVGs? (sorry for the delay in replying) Yes, I do see x.99 and x.01 values in some of the go/icons SVGs. |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
