Fixed a bug with linear gradient PDF matrices and added test cases

gm/gradient_matrix.cpp

+/*
+ * Copyright 2013 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */

[reed1, 2013/06/12 13:34:33]

nit: LF after */

[ducky, 2013/06/12 18:24:01]

Done.

+#include "SkRefCnt.h"
+

[vandebo (ex-Chrome), 2013/06/12 16:25:33]

Include what you use... SkColor, SkPoint, SkScalar, SkMatrix, SkRect, SkCanvas,
SkPaint, SkAutoTUnref

[reed1, 2013/06/12 17:11:26]

Possibly, but almost no where in skia to we really do this completely. Without
some form of lint, its hard to know if you're including too much, etc.

[ducky, 2013/06/12 18:24:01]

Done - added additional includes.

+#include "gm.h"
+#include "SkGradientShader.h"
+
+SkColor gColors[] = {

[reed1, 2013/06/12 13:34:33]

static const 

or move into scope of the caller

[ducky, 2013/06/12 18:24:01]

Done.

+    SK_ColorRED, SK_ColorYELLOW
+};
+
+// these arrays define the gradient stop points
+// as x1, y1, x2, y2 per gradient to draw
+SkPoint linearPts[][2] = {

[reed1, 2013/06/12 13:34:33]

static const

or move into scope of the caller

[ducky, 2013/06/12 18:24:01]

Done.

+    {{0, 0}, {1, 0}},
+    {{0, 0}, {0, 1}},
+    {{1, 0}, {0, 0}},
+    {{0, 1}, {0, 0}},
+
+    {{0, 0}, {1, 1}},
+    {{1, 1}, {0, 0}},
+    {{1, 0}, {0, 1}},
+    {{0, 1}, {1, 0}}
+};
+
+const SkScalar TESTGRID_X = 200;    // pixels allocated to each image in x dimension
+const SkScalar TESTGRID_Y = 200;    // pixels allocated to each image in y dimension
+
+const int IMAGES_X = 4;             // number of images per row
+
+static SkShader* make_linear_gradient(SkPoint pts[2]) {
+    return SkGradientShader::CreateLinear(pts, gColors, NULL, SK_ARRAY_COUNT(gColors),
+                                          SkShader::kClamp_TileMode, NULL);
+}
+
+void draw_gradients(SkCanvas* canvas, SkShader* (*makeShader)(SkPoint[2]),
+                    SkPoint ptsArray[][2], int numImages) {
+    // common test parameters
+    SkMatrix shaderMat = SkMatrix();

[reed1, 2013/06/12 13:34:33]

not sure why you need this assignment?

[ducky, 2013/06/12 18:24:01]

Done - removed.

+    shaderMat.setAll(138, 0,   43,

[reed1, 2013/06/12 13:34:33]

can we have a brief comment why you're using these values?

[ducky, 2013/06/12 18:24:01]

Done.

+                     0,   106, 61,
+                     0,   0,   1);
+    SkRect rectGrad = { 43, 61, 181, 167 }; // some nice prime numbers
+
+    canvas->save();
+    for (int i = 0; i < numImages; i++) {
+        // advance line if necessary
+        if (i % IMAGES_X == 0 && i != 0) {
+            canvas->restore();
+            canvas->translate(0, TESTGRID_Y);
+            canvas->save();
+        }
+
+        // unpack points, setup shader, draw

[vandebo (ex-Chrome), 2013/06/12 16:25:33]

Update comment.  Comments should be sentences, starting with a capital letter
and ending with a period.

[ducky, 2013/06/12 18:24:01]

Done.

+        SkAutoTUnref<SkShader> shader(makeShader(*ptsArray));
+        shader->setLocalMatrix(shaderMat);
+
+        SkPaint paint;
+        paint.setShader(shader);
+        canvas->drawRect(rectGrad, paint);
+
+        // advance to next position

[vandebo (ex-Chrome), 2013/06/12 16:25:33]

Comments should explain why you're doing something, not what the code is doing,
that should hopefully be obvious from the code.  With that in mind, you probably
don't need a comment here.

[reed1, 2013/06/12 17:11:26]

Not sure I agree. I find this comment helpful to know why we are calling
translate.

[ducky, 2013/06/12 18:24:01]

Done - leaving as is.

+        canvas->translate(TESTGRID_X, 0);
+        ptsArray++;
+    }
+    canvas->restore();
+}
+
+namespace skiagm {
+
+class GradientMatrixGM : public GM {
+public:
+    GradientMatrixGM() {
+        this->setBGColor(0xFFDDDDDD);
+    }
+
+protected:
+    SkString onShortName() {

[reed1, 2013/06/12 13:34:33]

SK_OVERRIDE

[ducky, 2013/06/12 18:24:01]

Done.

+        return SkString("gradient_matrix");
+    }
+
+    virtual SkISize onISize() {

[reed1, 2013/06/12 13:34:33]

SK_OVERRIDE

[ducky, 2013/06/12 18:24:01]

Done.

+        return SkISize::Make(800, 800);
+    }
+
+    virtual void onDraw(SkCanvas* canvas) {

[reed1, 2013/06/12 13:34:33]

SK_OVERRIDE

[ducky, 2013/06/12 18:24:01]

Done.

+        draw_gradients(canvas, &make_linear_gradient,
+                      linearPts, SK_ARRAY_COUNT(linearPts));
+    }
+
+private:
+    typedef GM INHERITED;
+};
+
+static GM* MyFactory(void*) { return new GradientMatrixGM; }

[reed1, 2013/06/12 13:34:33]

DEF_GM( return new GradientMatrixGM; )

[ducky, 2013/06/12 18:24:01]

Done.

+static GMRegistry reg(MyFactory);
+}