App icon size unification on web apps and bookmarks |
|||||||
Issue descriptionVersion 73.0.3644.0 Canary -- See the image attached. @Vlad, I cannot recall if the size unification is now active on web apps and bookmarks icons in shelf/launcher etc... If you look at the showtime bookmarks, you can see that it doesn't appear to be unified.
,
Jan 14
Ben, is there anything special about the source of the Showtime app icon? I was not able to track it down. The Showtime icon bitmap has an alphaType == kOpaque_SkAlphaType which is unusual for an extension icon. The resizing logic in md_icon_normalizer.cc currently works only for icons that support transparency. For this particular icon the code bails out here: https://cs.chromium.org/chromium/src/chrome/browser/ui/app_list/md_icon_normalizer.cc?rcl=0207eefb445f9855c2ed46280cb835b6f08bdb30&l=88, and no resizing is attempted.
,
Jan 15
Web app installs are sourced from the web sites manifest, so we get the app directly from showtime. Possibly the bookmark app system is processing these differently but it would be the same as the spotify icon for example. What is the actual problem? I don't understand what unified means in this context. Adding Dom who thinks / cares about all apps being treated equally.
,
Jan 15
I'm guessing that Showtime has a square icon that's exported without an alpha channel, and we don't add alpha at any point in the installation process. I would not be surprised if there were plenty of sites which did this. What exactly is the MD icon normalizer trying to do?
,
Jan 16
(6 days ago)
The normalizer adjusts the icon scale to enforce these requirements: https://material.io/design/iconography/product-icons.html#grid-keyline-shapes This results in app grid icons (and shelf icons as well) having roughly similar "visual weight". This was implemented in http://crrev.com/c/1183811 (see http://crbug.com/848007). As of Q4'18 most 1st party apps have their icons already MD compliant at the source. The normalizer have been doing a good job adjusting the scale for most 3rd party icons, but bailed out when encountering an icon with no alpha channel. This CL fixes this: http://crrev.com/c/1415511
,
Jan 16
(6 days ago)
Attaching the screenshot of the resized shelf icon for Showtime.
,
Jan 16
(6 days ago)
,
Jan 17
(5 days ago)
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a9b7bf941b7d08afb0db34c59b37659f56d606fb commit a9b7bf941b7d08afb0db34c59b37659f56d606fb Author: Vladislav Kaznacheev <kaznacheev@google.com> Date: Thu Jan 17 18:26:27 2019 Apply icon normalization to icons with no alpha channel Previously Material Design icon normalization was not attempted for icons with no alpha channel. However, it makes perfect sense to treat such icons as fully opaque and scale them accordingly. Bug: 920885 Test: MdIconNormalizerTest*, ChromeAppIcon* Change-Id: I4063eff7df5fddfc8a34cc6455cc9dc805b2b71a Reviewed-on: https://chromium-review.googlesource.com/c/1415511 Reviewed-by: Ben Wells <benwells@chromium.org> Reviewed-by: Yury Khmel <khmel@chromium.org> Commit-Queue: Vladislav Kaznacheev <kaznacheev@chromium.org> Cr-Commit-Position: refs/heads/master@{#623757} [modify] https://crrev.com/a9b7bf941b7d08afb0db34c59b37659f56d606fb/chrome/browser/extensions/chrome_app_icon_unittest.cc [modify] https://crrev.com/a9b7bf941b7d08afb0db34c59b37659f56d606fb/chrome/browser/ui/app_list/md_icon_normalizer.cc [modify] https://crrev.com/a9b7bf941b7d08afb0db34c59b37659f56d606fb/chrome/browser/ui/app_list/md_icon_normalizer_unittest.cc
,
Jan 17
(5 days ago)
,
Jan 17
(5 days ago)
Thanks for the quick fix! |
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by kaznacheev@chromium.org
, Jan 14