Introduce some util code for drawing vector assets.

ui/gfx/vector_icons.cc

Index: ui/gfx/vector_icons.cc
diff --git a/ui/gfx/vector_icons.cc b/ui/gfx/vector_icons.cc
new file mode 100644
index 0000000000000000000000000000000000000000..a939cb471e60f1858b551af829cec1a9428a738c
--- /dev/null
+++ b/ui/gfx/vector_icons.cc
@@ -0,0 +1,195 @@
+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "ui/gfx/vector_icons.h"
+
+#include "ui/gfx/canvas.h"
+
+namespace gfx {
+
+namespace {
+
+const SkScalar reference_size = 48.0;

[tfarina, 2015/07/01 22:57:52]

kReferenceSize?

[Evan Stade, 2015/07/02 00:08:05]

Done.

+
+enum CommandType {
+  MOVE_TO,
+  R_MOVE_TO,
+  LINE_TO,
+  R_LINE_TO,
+  H_LINE_TO,
+  R_H_LINE_TO,
+  V_LINE_TO,
+  R_V_LINE_TO,
+  R_CUBIC_TO,
+  CIRCLE,
+  CLOSE,
+  END
+};
+
+struct PathElement {
+  PathElement(CommandType type) { this->type = type; }
+  PathElement(SkScalar arg) { this->arg = arg; }

[Peter Kasting, 2015/07/01 22:26:57]

Nit: explicit?  Or does that break the code below?

[Evan Stade, 2015/07/02 00:08:06]

breaks it

+
+  union {
+    CommandType type;
+    SkScalar arg;
+  };
+};
+
+const PathElement* GetPathForVectorIcon(VectorIconId id) {
+  switch (id) {
+    case VectorIconId::CHECK_CIRCLE: {
+      static PathElement path[] = {
+        CIRCLE, 24, 24, 20,

[Peter Kasting, 2015/07/01 22:26:57]

I don't love inlining these instructions here.

It seems like ideally, we'd have text files for each icon containing these sorts
of instructions, and during the build process, a script would find them all,
automatically declare the icon enum in the header, and then automatically
include the actual paths into the compiled source.

I suppose that could wait until after this change, but I'd find a way to do it
before expanding this functionality much more.

[Evan Stade, 2015/07/02 00:08:05]

Can you elaborate on why that is better? Do you just not like code and data in
the same file? If that's the case all we need to do is move this function to a
different file.

[Peter Kasting, 2015/07/02 00:23:53]

I want updating icons to not have to touch code, since that will make it easier
for designers to do this directly.  (I'd like adding and removing icons to be as
easy as possible as well.)  Also, as this list grows to dozens of items, I think
having these in separate, named files will be easier to find and read/change
what's happening for an icon than will a hundreds-of-lines file with enormous
switch statements.

[Evan Stade, 2015/07/02 00:58:24]

Currently image assets are separate from code, but I've still never seen a
designer check in a change to one. In my recent experience, designers don't even
have a working build and lean heavily on canary for testing new functionality.
Even if there were 200 icons in this switch statement, it would be easy to find
CHECK_CIRCLE with most text editors.

[Peter Kasting, 2015/07/02 08:06:11]

Designers frequently have me point them to the appropriate image files in the
tree and then send me new versions to plug in directly atop them.  That's easy
to reproduce with separate files here.
In theory, yes.  In practice, such files are a huge pain.  I went through this
same process with the search engine prepopulate data.  Initially everything was
in one giant file.  It became annoying to scan it quickly and find what you
wanted, know how many engines there were, etc.  Updating things required
touching several places in the file and making sure not to miss any.  We then
split things into more files with a bit of scripting for the build system and it
improved stuff.

The win here should be even larger.

[Evan Stade, 2015/07/06 18:35:59]

You bring up a good point, which is how to visualize these resources outside of
where they're actually used. However, I don't think drawing commands in a text
file is a good way to do it. A designer is not going to pick out an icon by
reading {R_CUBIC_TO, 0, 2.21, ...}

It would probably be useful to build an icon gallery (in views_examples_exe or a
similar small program) or a small program that took these drawing commands as
inputs and drew them on the screen.
Since you said "I suppose that could wait until after this change" is it ok to
put this off to another CL?

+        MOVE_TO, 20, 34,
+        LINE_TO, 10, 24,
+        R_LINE_TO, 2.83, -2.83,
+        LINE_TO, 20, 28.34,
+        R_LINE_TO, 15.17, -15.17,
+        LINE_TO, 38, 16,
+        LINE_TO, 20, 34,
+        END
+      };
+      return path;
+    }
+
+    case VectorIconId::PHOTO_CAMERA: {
+      static PathElement path[] = {
+        CIRCLE, 24, 24, 6.4,
+        MOVE_TO, 18, 4,
+        R_LINE_TO, -3.66, 4,
+        H_LINE_TO, 8,
+        R_CUBIC_TO, -2.21, 0, -4, 1.79, -4, 4,
+        R_V_LINE_TO, 24,
+        R_CUBIC_TO, 0, 2.21, 1.79, 4, 4, 4,
+        R_H_LINE_TO, 32,
+        R_CUBIC_TO, 2.21, 0, 4, -1.79, 4, -4,
+        V_LINE_TO, 12,
+        R_CUBIC_TO, 0, -2.21, -1.79, -4, -4, -4,
+        R_H_LINE_TO, -6.34,
+        LINE_TO, 30, 4,
+        H_LINE_TO, 18,
+        CLOSE,
+        CIRCLE, 24, 24, 10,
+        END
+      };
+      return path;
+    }
+
+    case VectorIconId::VECTOR_ICON_NONE:

[Peter Kasting, 2015/07/01 22:26:58]

Nit: Put in same order that the enum values were declared (i.e. put this first)

[Evan Stade, 2015/07/02 00:08:06]

Done.

+      NOTREACHED();
+      return nullptr;
+  }
+}
+
+}  // namespace
+
+void PaintVectorIcon(Canvas* canvas, VectorIconId id, size_t dp_size,
+                     SkColor color) {

[Peter Kasting, 2015/07/01 22:26:58]

Nit: I think it's easier to read function definitions with all args on
independent lines, if they don't all fit on one line.

[Evan Stade, 2015/07/02 00:08:06]

Done.

+  if (id == VectorIconId::VECTOR_ICON_NONE) {
+    NOTREACHED();

[Peter Kasting, 2015/07/01 22:26:58]

Don't handle DCHECK failure.  This block should just be:

  DCHECK_NE(VectorIconId::VECTOR_ICON_NONE, id);

[Evan Stade, 2015/07/02 00:08:05]

Done.

+    return;
+  }
+
+  const PathElement* path_elements = GetPathForVectorIcon(id);
+  SkPath path;
+  path.setFillType(SkPath::kEvenOdd_FillType);
+  SkScalar scale = dp_size / reference_size;

[Peter Kasting, 2015/07/01 22:26:58]

You compute this and then multiply every coordinate by it below.  It seems like
it would be clearer/simpler/faster/less error-prone to instead simply set a
scaling transform?

[Evan Stade, 2015/07/02 00:08:06]

good idea, done

+
+  for (size_t i = 0; path_elements[i].type != END;) {
+    switch (path_elements[i++].type) {
+      case CIRCLE: {

[Peter Kasting, 2015/07/01 22:26:57]

Nit: Put in same order that the enum values were declared

[Evan Stade, 2015/07/02 00:08:05]

Done.

+        SkScalar x = scale * path_elements[i++].arg;
+        SkScalar y = scale * path_elements[i++].arg;
+        SkScalar r = scale * path_elements[i++].arg;
+        path.addCircle(x, y, r);
+        break;
+      }
+
+      case MOVE_TO: {
+        SkScalar x = scale * path_elements[i++].arg;
+        SkScalar y = scale * path_elements[i++].arg;
+        path.moveTo(x, y);
+        break;
+      }
+
+      case R_MOVE_TO: {
+        SkScalar x = scale * path_elements[i++].arg;
+        SkScalar y = scale * path_elements[i++].arg;
+        path.rMoveTo(x, y);
+        break;
+      }
+
+      case LINE_TO: {
+        SkScalar x = scale * path_elements[i++].arg;
+        SkScalar y = scale * path_elements[i++].arg;
+        path.lineTo(x, y);
+        break;
+      }
+
+      case R_LINE_TO: {
+        SkScalar x = scale * path_elements[i++].arg;
+        SkScalar y = scale * path_elements[i++].arg;
+        path.rLineTo(x, y);
+        break;
+      }
+
+      case H_LINE_TO: {
+        SkPoint last_point;
+        path.getLastPt(&last_point);
+        SkScalar x = scale * path_elements[i++].arg;
+        path.lineTo(x, last_point.fY);
+        break;
+      }
+
+      case V_LINE_TO: {
+        SkPoint last_point;
+        path.getLastPt(&last_point);
+        SkScalar y = scale * path_elements[i++].arg;
+        path.lineTo(last_point.fX, y);
+        break;
+      }
+
+      case R_H_LINE_TO: {
+        SkScalar x = scale * path_elements[i++].arg;
+        path.rLineTo(x, 0);
+        break;
+      }
+
+      case R_V_LINE_TO: {
+        SkScalar y = scale * path_elements[i++].arg;
+        path.rLineTo(0, y);
+        break;
+      }
+
+      case R_CUBIC_TO: {
+        SkScalar x1 = scale * path_elements[i++].arg;
+        SkScalar y1 = scale * path_elements[i++].arg;
+        SkScalar x2 = scale * path_elements[i++].arg;
+        SkScalar y2 = scale * path_elements[i++].arg;
+        SkScalar x3 = scale * path_elements[i++].arg;
+        SkScalar y3 = scale * path_elements[i++].arg;
+        path.rCubicTo(x1, y1, x2, y2, x3, y3);
+        break;
+      }
+
+      case CLOSE: {
+        path.close();
+        break;
+      }
+
+      case END:
+        NOTREACHED();
+        break;
+    }
+  }
+
+  SkPaint paint;
+  paint.setStyle(SkPaint::kFill_Style);
+  paint.setAntiAlias(true);
+  paint.setColor(color);
+  canvas->DrawPath(path, paint);
+}
+
+}  // namespace gfx