cc: Move SkShader construction to a single spot in PaintShader

cc/paint/paint_shader.h

 // Copyright 2017 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.
 
 #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;
-}
+class PaintOpBuffer;
+using PaintRecord = PaintOpBuffer;
 
-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);
-}
+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();
 
-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);
-}
+  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
 
 #endif  // CC_PAINT_PAINT_SHADER_H_