Fix for empty saveLayer() with a filter which affects transparent black.

If an saveLayer()/restore() is recorded, tilegrid/rtree will cull them
out and not draw anything. This is correct for most cases, but if the
paint in the saveLayer() is one that affects transparent black (e.g.,
it contains a color filter or image filter which affects transparent
black), this is incorrect: the filter should be applied.

Fixed by adding a no-op between the saveLayer() and restore(), and
adding a bbox node pointing at that node with the saveLayer()'s bounds.

This exposed a bug in SkPictureRecord.cpp's match(), where it would
assert if the NOOP was the last op seen. Fixed with an early-out before
calling peek_op_and_size().

BUG=skia:2254

Review URL: https://codereview.chromium.org/262363007

git-svn-id: http://skia.googlecode.com/svn/trunk@14604 2bbb7eff-a529-9590-31e7-b0007b416f81

4 files changed, 89 insertions, 6 deletions

diff --git a/src/core/SkBBoxHierarchyRecord.cpp b/src/core/SkBBoxHierarchyRecord.cpp
index f0382e6470..d6fbd21239 100644
--- a/src/core/SkBBoxHierarchyRecord.cpp
+++ b/src/core/SkBBoxHierarchyRecord.cpp
@@ -35,8 +35,29 @@ void SkBBoxHierarchyRecord::willSave(SaveFlags flags) {
 SkCanvas::SaveLayerStrategy SkBBoxHierarchyRecord::willSaveLayer(const SkRect* bounds,
                                                                  const SkPaint* paint,
                                                                  SaveFlags flags) {
+    // For now, assume all filters affect transparent black.
+    // FIXME: This could be made less conservative as an optimization.
+    bool paintAffectsTransparentBlack = NULL != paint &&
+        ((NULL != paint->getImageFilter()) ||
+         (NULL != paint->getColorFilter()));
+    SkRect drawBounds;
+    if (paintAffectsTransparentBlack) {
+        if (bounds) {
+            drawBounds = *bounds;
+            this->getTotalMatrix().mapRect(&drawBounds);
+        } else {
+            SkIRect deviceBounds;
+            this->getClipDeviceBounds(&deviceBounds);
+            drawBounds.set(deviceBounds);
+        }
+    }
     fStateTree->appendSaveLayer(this->writeStream().bytesWritten());
-    return this->INHERITED::willSaveLayer(bounds, paint, flags);
+    SkCanvas::SaveLayerStrategy strategy = this->INHERITED::willSaveLayer(bounds, paint, flags);
+    if (paintAffectsTransparentBlack) {
+        this->handleBBox(drawBounds);
+        this->addNoOp();
+    }
+    return strategy;
 }
 
 void SkBBoxHierarchyRecord::willRestore() {
diff --git a/src/core/SkPictureRecord.cpp b/src/core/SkPictureRecord.cpp
index f6da2f2793..4a5aff2d72 100644
--- a/src/core/SkPictureRecord.cpp
+++ b/src/core/SkPictureRecord.cpp
@@ -293,15 +293,14 @@ static bool match(SkWriter32* writer, uint32_t offset,
     int numMatched;
     for (numMatched = 0; numMatched < numCommands && curOffset < writer->bytesWritten(); ++numMatched) {
         DrawType op = peek_op_and_size(writer, curOffset, &curSize);
-        while (NOOP == op && curOffset < writer->bytesWritten()) {
+        while (NOOP == op) {
             curOffset += curSize;
+            if (curOffset >= writer->bytesWritten()) {
+                return false;
+            }
             op = peek_op_and_size(writer, curOffset, &curSize);
         }
 
-        if (curOffset >= writer->bytesWritten()) {
-            return false; // ran out of byte stream
-        }
-
         if (kDRAW_BITMAP_FLAVOR == pattern[numMatched]) {
             if (DRAW_BITMAP != op && DRAW_BITMAP_MATRIX != op &&
                 DRAW_BITMAP_NINE != op && DRAW_BITMAP_RECT_TO_RECT != op) {
@@ -1649,6 +1648,11 @@ void SkPictureRecord::addPoints(const SkPoint pts[], int count) {
 #endif
 }
 
+void SkPictureRecord::addNoOp() {
+    size_t size = kUInt32Size; // op
+    this->addDraw(NOOP, &size);
+}
+
 void SkPictureRecord::addRect(const SkRect& rect) {
 #ifdef SK_DEBUG_SIZE
     size_t start = fWriter.bytesWritten();
diff --git a/src/core/SkPictureRecord.h b/src/core/SkPictureRecord.h
index 0f0986fba7..22d2546072 100644
--- a/src/core/SkPictureRecord.h
+++ b/src/core/SkPictureRecord.h
@@ -89,6 +89,9 @@ public:
         fOptsEnabled = optsEnabled;
     }
 
+protected:
+    void addNoOp();
+
 private:
     void handleOptimization(int opt);
     size_t recordRestoreOffsetPlaceholder(SkRegion::Op);
diff --git a/tests/ImageFilterTest.cpp b/tests/ImageFilterTest.cpp
index 08277a1ec6..f8492a7b11 100644
--- a/tests/ImageFilterTest.cpp
+++ b/tests/ImageFilterTest.cpp
@@ -467,6 +467,61 @@ DEF_TEST(ImageFilterMatrixTest, reporter) {
     canvas.drawPicture(*picture);
 }
 
+DEF_TEST(ImageFilterEmptySaveLayerTest, reporter) {
+
+    // Even when there's an empty saveLayer()/restore(), ensure that an image
+    // filter or color filter which affects transparent black still draws.
+
+    SkBitmap bitmap;
+    bitmap.allocN32Pixels(10, 10);
+    SkBitmapDevice device(bitmap);
+    SkCanvas canvas(&device);
+
+    SkRTreeFactory factory;
+    SkPictureRecorder recorder;
+
+    SkAutoTUnref<SkColorFilter> green(
+        SkColorFilter::CreateModeFilter(SK_ColorGREEN, SkXfermode::kSrc_Mode));
+    SkAutoTUnref<SkColorFilterImageFilter> imageFilter(
+        SkColorFilterImageFilter::Create(green.get()));
+    SkPaint imageFilterPaint;
+    imageFilterPaint.setImageFilter(imageFilter.get());
+    SkPaint colorFilterPaint;
+    colorFilterPaint.setColorFilter(green.get());
+
+    SkRect bounds = SkRect::MakeWH(10, 10);
+
+    SkCanvas* recordingCanvas = recorder.beginRecording(10, 10, &factory, 0);
+    recordingCanvas->saveLayer(&bounds, &imageFilterPaint);
+    recordingCanvas->restore();
+    SkAutoTUnref<SkPicture> picture(recorder.endRecording());
+
+    canvas.clear(0);
+    canvas.drawPicture(*picture);
+    uint32_t pixel = *bitmap.getAddr32(0, 0);
+    REPORTER_ASSERT(reporter, pixel == SK_ColorGREEN);
+
+    recordingCanvas = recorder.beginRecording(10, 10, &factory, 0);
+    recordingCanvas->saveLayer(NULL, &imageFilterPaint);
+    recordingCanvas->restore();
+    SkAutoTUnref<SkPicture> picture2(recorder.endRecording());
+
+    canvas.clear(0);
+    canvas.drawPicture(*picture2);
+    pixel = *bitmap.getAddr32(0, 0);
+    REPORTER_ASSERT(reporter, pixel == SK_ColorGREEN);
+
+    recordingCanvas = recorder.beginRecording(10, 10, &factory, 0);
+    recordingCanvas->saveLayer(&bounds, &colorFilterPaint);
+    recordingCanvas->restore();
+    SkAutoTUnref<SkPicture> picture3(recorder.endRecording());
+
+    canvas.clear(0);
+    canvas.drawPicture(*picture3);
+    pixel = *bitmap.getAddr32(0, 0);
+    REPORTER_ASSERT(reporter, pixel == SK_ColorGREEN);
+}
+
 static void test_huge_blur(SkBaseDevice* device, skiatest::Reporter* reporter) {
     SkCanvas canvas(device);