New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 689188 link

Starred by 0 users

Issue metadata

Status: WontFix
Owner:
Last visit 29 days ago
Closed: Feb 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 3
Type: Bug



Sign in to add a comment

DevTools: generate icon spritesheet automatically

Project Member Reported by lushnikov@chromium.org, Feb 6 2017

Issue description

Today, we have to maintain three separate sprite sheets with icons manually:
- smallIcons.svg
- toolbarButtonGlyphs.svg
- resourceGlyphs.svg

The biggest downside of such an approach is that adding an icon is a hassle.
One have to find a free space for the icon in the spritesheet, calculate its coordinates,
and then manually declare the icon in UI.Icon.Descriptors map.

Other downsides are:
- it's hard to grow a spritesheet
- it's hard to cleanup the spritesheet of unused icons (there's a helper script for this, but still: https://gist.github.com/aslushnikov/4aa0e0865523db1b72d990c701ce39d4)
- when two different patches add a new icon to the same spritesheet, someone will have to resolve conflict with inkscape.

Ideally, all of our icons should be stored as a separate SVG files. We should have a script which assembles them into a spritesheet and which generates the UI.Icon.Descriptors map.

The upsides of this approach would be:
- no need to manage spritesheet manually
- ability to write a presubmit script which validates that every icon is in use
- ease of adding a new icon: just drop the icon svg in the icons/ folder
- no icon-related merge conflicts

Also, this approach should save us some more space since all icons will be densely packed.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Feb 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b4eda7f379a496b5167b6905cadea3b38be6bdf8

commit b4eda7f379a496b5167b6905cadea3b38be6bdf8
Author: lushnikov <lushnikov@chromium.org>
Date: Tue Feb 07 00:23:17 2017

DevTools: eliminate UI.Icon transforms

Today, a few icons are using transforms in their descriptors. This
saves some space in the spritesheet, but this is a hackish
solution which makes dealing with icons harder.

This patch explicitly defines these icons in spritesheets.
This is a prerequisite for the spritesheet assembler introduction.

As a drive-by and as a preporation for icon migration, this patch
cleans up spritesheets:
- groups all icons in smallIcons.svg, toolbarButtonGlyphs.svg and
   resourceGlyphs.svg
- kills hidden SVG layer which was not actually used in smallIcons.svg

R=dgozman@chromium.org, pfeldman

BUG= 689188 
R=dgozman@chromium.org, pfeldman@chromium.org

Review-Url: https://codereview.chromium.org/2678863004
Cr-Commit-Position: refs/heads/master@{#448461}

[modify] https://crrev.com/b4eda7f379a496b5167b6905cadea3b38be6bdf8/third_party/WebKit/Source/devtools/front_end/Images/smallIcons.png
[modify] https://crrev.com/b4eda7f379a496b5167b6905cadea3b38be6bdf8/third_party/WebKit/Source/devtools/front_end/Images/smallIcons_2x.png
[modify] https://crrev.com/b4eda7f379a496b5167b6905cadea3b38be6bdf8/third_party/WebKit/Source/devtools/front_end/Images/src/optimize_png.hashes
[modify] https://crrev.com/b4eda7f379a496b5167b6905cadea3b38be6bdf8/third_party/WebKit/Source/devtools/front_end/Images/src/smallIcons.svg
[modify] https://crrev.com/b4eda7f379a496b5167b6905cadea3b38be6bdf8/third_party/WebKit/Source/devtools/front_end/Images/src/svg2png.hashes
[modify] https://crrev.com/b4eda7f379a496b5167b6905cadea3b38be6bdf8/third_party/WebKit/Source/devtools/front_end/Images/src/toolbarButtonGlyphs.svg
[modify] https://crrev.com/b4eda7f379a496b5167b6905cadea3b38be6bdf8/third_party/WebKit/Source/devtools/front_end/Images/toolbarButtonGlyphs.png
[modify] https://crrev.com/b4eda7f379a496b5167b6905cadea3b38be6bdf8/third_party/WebKit/Source/devtools/front_end/Images/toolbarButtonGlyphs_2x.png
[modify] https://crrev.com/b4eda7f379a496b5167b6905cadea3b38be6bdf8/third_party/WebKit/Source/devtools/front_end/ui/Icon.js

Status: WontFix (was: Assigned)
For the reference: spritesheet assembler was introduced here: https://codereview.chromium.org/2671413004/

However, the team decided to keep spritesheets for now since they are more convenient to edit.

Sign in to add a comment