cc: Move SkShader construction to a single spot in PaintShader

cc/paint/paint_shader.h

Index: cc/paint/paint_shader.h
diff --git a/cc/paint/paint_shader.h b/cc/paint/paint_shader.h
index 019174443bb1470dbd448dd40f14ca7bbbcc3e70..3f7d64881674d7b62cf4f23d48a5be3a4cdff779 100644
--- a/cc/paint/paint_shader.h
+++ b/cc/paint/paint_shader.h
@@ -5,32 +5,83 @@
 #ifndef CC_PAINT_PAINT_SHADER_H_
 #define CC_PAINT_PAINT_SHADER_H_
 
-#include "cc/paint/paint_record.h"
+#include "cc/paint/paint_export.h"
 #include "third_party/skia/include/core/SkImage.h"
+#include "third_party/skia/include/core/SkScalar.h"
 #include "third_party/skia/include/core/SkShader.h"
 
 namespace cc {
-using PaintShader = SkShader;
-
-inline sk_sp<PaintShader> WrapSkShader(sk_sp<SkShader> shader) {
-  return shader;
-}
-
-inline sk_sp<PaintShader> MakePaintShaderImage(sk_sp<const SkImage> image,
-                                               SkShader::TileMode tx,
-                                               SkShader::TileMode ty,
-                                               const SkMatrix* local_matrix) {
-  return image->makeShader(tx, ty, local_matrix);
-}
-
-inline sk_sp<PaintShader> MakePaintShaderRecord(sk_sp<PaintRecord> record,
-                                                const SkRect& tile,
-                                                SkShader::TileMode tx,
-                                                SkShader::TileMode ty,
-                                                const SkMatrix* local_matrix) {
-  return SkShader::MakePictureShader(ToSkPicture(record, tile), tx, ty,
-                                     local_matrix, nullptr);
-}
+
+class PaintOpBuffer;
+using PaintRecord = PaintOpBuffer;
+
+class CC_PAINT_EXPORT PaintShader {
+ public:
+  // Creates a color shader. See SkShader::MakeColorShader.
+  explicit PaintShader(SkColor color);
+  // Creates a linear gradient shader. See SkGradientShader::MakeLinear.
+  PaintShader(const SkPoint points[],
+              const SkColor colors[],
+              const SkScalar pos[],
+              int count,
+              SkShader::TileMode mode,
+              uint32_t flags = 0,
+              const SkMatrix* local_matrix = nullptr,
+              SkColor fallback_color = SK_ColorTRANSPARENT);

[piman, 2017/06/02 20:43:36]

At the call site, determining what kind of shader is created is very
non-obvious, since it's based on the exact number and type of the parameters
(doubly hard with the default values), which don't even necessarily have names.

Would you consider instead having factory functions, e.g.:
static PaintShader MakeLinearGradient(const SkPoint points[], const SkColor
colors[], const SkScalar pos[], int count, SkShader::TileMode mode, uint32_t
flags = 0, const SkMatrix* local_matrix = nullptr, SkColor fallback_color =
SK_ColorTRANSPARENT); and so on?

[vmpstr, 2017/06/02 20:52:00]

Yes, I'll do that. Thanks!

[vmpstr, 2017/06/05 21:02:06]

Done.

+  // Creates a radial gradient shader. See SkGradientShader::MakeRadial
+  PaintShader(const SkPoint& center,
+              SkScalar radius,
+              const SkColor colors[],
+              const SkScalar pos[],
+              int color_count,
+              SkShader::TileMode mode,
+              uint32_t flags = 0,
+              const SkMatrix* local_matrix = nullptr,
+              SkColor fallback_color = SK_ColorTRANSPARENT);
+  // Creates a two point conical gradient shader. See
+  // SkGradientShader::MakeTwoPointConical.
+  PaintShader(const SkPoint& start,
+              SkScalar start_radius,
+              const SkPoint& end,
+              SkScalar end_radius,
+              const SkColor colors[],
+              const SkScalar pos[],
+              int color_count,
+              SkShader::TileMode mode,
+              uint32_t flags = 0,
+              const SkMatrix* local_matrix = nullptr,
+              SkColor fallback_color = SK_ColorTRANSPARENT);
+  // Creates a sweep gradient shader. See SkGradientShader::MakeSweep.
+  PaintShader(SkScalar cx,
+              SkScalar cy,
+              const SkColor colors[],
+              const SkScalar pos[],
+              int color_count,
+              uint32_t flags = 0,
+              const SkMatrix* local_matrix = nullptr,
+              SkColor fallback_color = SK_ColorTRANSPARENT);
+  // Creates an image shader. See SkImage::makeShader.
+  PaintShader(sk_sp<const SkImage> image,
+              SkShader::TileMode tx,
+              SkShader::TileMode ty,
+              const SkMatrix* local_matrix);
+  // Creates a picture shader. See SkShader::MakePictureShader.
+  PaintShader(sk_sp<PaintRecord> record,
+              const SkRect& tile,
+              SkShader::TileMode tx,
+              SkShader::TileMode ty,
+              const SkMatrix* local_matrix);
+  PaintShader(const PaintShader& other);
+  PaintShader(PaintShader&& other);

[piman, 2017/06/02 20:43:36]

nit: should we also have copy-assign and move-assign operators (rule-of-five)?

[vmpstr, 2017/06/05 21:02:06]

Done.

+  ~PaintShader();
+
+  const sk_sp<SkShader>& sk_shader() const { return sk_shader_; }
+
+ private:
+  explicit PaintShader(sk_sp<SkShader> shader, SkColor fallback_color);

[piman, 2017/06/02 20:43:36]

nit: no need for explicit

[vmpstr, 2017/06/05 21:02:07]

Done.

+
+  sk_sp<SkShader> sk_shader_;
+};
 
 }  // namespace cc