Collapse consecutive SkTableColorFilters

gm/tablecolorfilter.cpp

Index: gm/tablecolorfilter.cpp
diff --git a/gm/tablecolorfilter.cpp b/gm/tablecolorfilter.cpp
index f806caad6ae850e68f2a521c7b2dca0bc4730ffb..a17c80152b2f690ddb529953ee2dccb7778fa2b6 100644
--- a/gm/tablecolorfilter.cpp
+++ b/gm/tablecolorfilter.cpp
@@ -7,6 +7,7 @@
 
 #include "gm.h"
 #include "SkCanvas.h"
+#include "SkColorFilterImageFilter.h"
 #include "SkGradientShader.h"
 #include "SkTableColorFilter.h"
 
@@ -68,6 +69,10 @@ static void make_table2(uint8_t table[]) {
     }
 }
 
+static SkColorFilter* make_null_cf() {
+    return NULL;
+}
+
 static SkColorFilter* make_cf0() {
     uint8_t table[256]; make_table0(table);
     return SkTableColorFilter::Create(table);
@@ -97,7 +102,7 @@ protected:
     }
 
     virtual SkISize onISize() {
-        return SkISize::Make(700, 300);
+        return SkISize::Make(700, 1650);
     }
 
     virtual void onDraw(SkCanvas* canvas) {
@@ -106,25 +111,48 @@ protected:
 
         SkScalar x = 0, y = 0;
 
+        static SkColorFilter* (*cfMakers[])() = { make_null_cf, make_cf0, make_cf1,

[Justin Novosad, 2014/12/19 23:54:37]

Rename: gColorFilterMakers, and below gMakers should be disambiguated:
gBitmapMakers
FYI 'g' means global (because the variable is static)

[cwallez, 2015/01/19 19:56:13]

Done.

+                                                 make_cf2, make_cf3 };
         static void (*gMakers[])(SkBitmap*) = { make_bm0, make_bm1 };
-        for (size_t maker = 0; maker < SK_ARRAY_COUNT(gMakers); ++maker) {
+
+        for (size_t gMaker = 0; gMaker < SK_ARRAY_COUNT(gMakers); ++gMaker) {
             SkBitmap bm;
-            gMakers[maker](&bm);
+            gMakers[gMaker](&bm);
 
-            SkPaint paint;
+            SkScalar xOffset = bm.width() * 9 / 8;
+            SkScalar yOffset = bm.height() * 9 / 8;
             x = 0;
-            canvas->drawBitmap(bm, x, y, &paint);
-            paint.setColorFilter(make_cf0())->unref();  x += bm.width() * 9 / 8;
-            canvas->drawBitmap(bm, x, y, &paint);
-            paint.setColorFilter(make_cf1())->unref();  x += bm.width() * 9 / 8;
-            canvas->drawBitmap(bm, x, y, &paint);
-            paint.setColorFilter(make_cf2())->unref();  x += bm.width() * 9 / 8;
-            canvas->drawBitmap(bm, x, y, &paint);
-            paint.setColorFilter(make_cf3())->unref();  x += bm.width() * 9 / 8;
+
+            SkPaint paint;
             canvas->drawBitmap(bm, x, y, &paint);
 
-            y += bm.height() * 9 / 8;
+            for (size_t cfMaker = 1; cfMaker < SK_ARRAY_COUNT(cfMakers); ++cfMaker) {
+                paint.setColorFilter(cfMakers[cfMaker]())->unref();
+                x += xOffset;
+                canvas->drawBitmap(bm, x, y, &paint);
+            }
+
+            y += yOffset;
+            paint.setColorFilter(NULL);
+
+            for (int i = 0; i < (int)SK_ARRAY_COUNT(cfMakers); ++i) {
+                x = 0;
+                SkAutoTUnref<SkColorFilter> colorFilter1(cfMakers[i]());
+                SkAutoTUnref<SkImageFilter> imageFilter1(SkColorFilterImageFilter::Create(
+                            colorFilter1, NULL, NULL, 0));
+
+                for (unsigned j = 1; j < SK_ARRAY_COUNT(cfMakers); ++j) {
+                    SkAutoTUnref<SkColorFilter> colorFilter2(cfMakers[j]());
+                    SkAutoTUnref<SkImageFilter> imageFilter2(SkColorFilterImageFilter::Create(
+                                colorFilter2, imageFilter1, NULL, 0));
+                    paint.setImageFilter(imageFilter2);
+                    canvas->drawBitmap(bm, x, y, &paint);
+                    x += xOffset;

[Justin Novosad, 2014/12/19 23:54:37]

This is very confusing, the x and y being offsetted in multiple places with th
three nested loops. I think you should consider breaking this into more than one
test

[cwallez, 2015/01/19 19:56:13]

Yes, it is somewhat confusing, the two solutions I see to this are
1) add a comment describing what the test result looks like (and what goes
where)
2) split the tests by bitmap

1) is fine, if not really a fix but 2) is kind of an arbitrary split, which I
don't like 100%. What do you think is best?

[Justin Novosad, 2015/01/19 20:03:51]

Yeah, I suppose a good comment would be fine

+                }
+                y += yOffset;
+            }
         }
+
     }
 
 private: