Preventing division by 0 in non-separable blend mode shaders.

gm/mixedxfermodes.cpp

Index: gm/mixedxfermodes.cpp
diff --git a/gm/mixedxfermodes.cpp b/gm/mixedxfermodes.cpp
index a5023e1b0b07218ad6b2acc171568ecae7d35de4..a22ead13aa27b3f9484979a0e212bd04fecfc7d6 100644
--- a/gm/mixedxfermodes.cpp
+++ b/gm/mixedxfermodes.cpp
@@ -22,6 +22,15 @@ public:
     }
 
 protected:
+    enum ShapeType {
+        kShapeTypeCircle,
+        kShapeTypeRoundRect,
+        kShapeTypeRect,
+        kShapeTypeConvexPath,
+        kShapeTypeConcavePath,
+        kNumShapeTypes
+    };
+
     virtual SkString onShortName() SK_OVERRIDE {
         return SkString("mixed_xfermodes");
     }
@@ -32,21 +41,20 @@ protected:
 
     void drawShape(SkCanvas* canvas,
                    const SkPaint& paint,
-                   SkRandom* random) {
+                   ShapeType type) {
         static const SkRect kRect = SkRect::MakeXYWH(SkIntToScalar(-50), SkIntToScalar(-50),
                                                      SkIntToScalar(75), SkIntToScalar(105));
-        int shape = random->nextULessThan(5);
-        switch (shape) {
-        case 0:
+        switch (type) {

[egdaniel, 2014/11/14 16:08:21]

You need to have a default case or else there will be errors for kNumShapeTypes
not being handled by the switch in certain compilers

[rosca, 2014/11/17 16:09:31]

Done.

+        case kShapeTypeCircle:
             canvas->drawCircle(0, 0, 50, paint);
             break;
-        case 1:
+        case kShapeTypeRoundRect:
             canvas->drawRoundRect(kRect, SkIntToScalar(10), SkIntToScalar(20), paint);
             break;
-        case 2:
+        case kShapeTypeRect:
             canvas->drawRect(kRect, paint);
             break;
-        case 3:
+        case kShapeTypeConvexPath:
             if (fConvexPath.isEmpty()) {
                 SkPoint points[4];
                 kRect.toQuad(points);
@@ -57,7 +65,7 @@ protected:
             }
             canvas->drawPath(fConvexPath, paint);
             break;
-        case 4:
+        case kShapeTypeConcavePath:
             if (fConcavePath.isEmpty()) {
                 SkPoint points[5] = {{0, SkIntToScalar(-50)} };
                 SkMatrix rot;
@@ -108,6 +116,13 @@ protected:
             SkColor color = random.nextU();
             SkXfermode::Mode mode =
                 static_cast<SkXfermode::Mode>(random.nextULessThan(SkXfermode::kLastMode + 1));
+            ShapeType shapeType = static_cast<ShapeType>(random.nextULessThan(kNumShapeTypes));
+
+            if (i == kNumShapes - 1) {

[egdaniel, 2014/11/14 16:08:21]

Since what you want to test ends up drawing nothing (or I believe that's what it
does from your previous patch), it would probably be best to not change any of
the current draws in this loop, and make a function like
draw_lum_zero_whatever_mode(canvas) that you call after the for loop (obviously
you need a better name than that). Give a good comment on the function as to why
it is there, its function, and that it should not actually do any draws.

The main advantage of this is that we can easily document the specific test case
we are looking at, and it shouldn't alter the gm (assuming nothing draws) so you
will not need to rebase afterwards.

[rosca, 2014/11/17 16:09:31]

Done.

+                color = SkColorSetARGBMacro(225, 100, 100, 100);
+                mode = SkXfermode::kSaturation_Mode;
+                shapeType = kShapeTypeConvexPath;
+            }
 
             SkPaint p;
             p.setAntiAlias(true);
@@ -117,7 +132,7 @@ protected:
             canvas->translate(dx, dy);
             canvas->scale(s, s);
             canvas->rotate(r);
-            this->drawShape(canvas, p, &random);
+            this->drawShape(canvas, p, shapeType);
             canvas->restore();
         }
     }