From 68250c8e7c2bf5d669397c849259c3bcad40237e Mon Sep 17 00:00:00 2001 From: "senorblanco@chromium.org" Date: Tue, 6 May 2014 22:52:55 +0000 Subject: 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 --- src/core/SkBBoxHierarchyRecord.cpp | 23 +++++++++++++++- src/core/SkPictureRecord.cpp | 14 ++++++---- src/core/SkPictureRecord.h | 3 +++ tests/ImageFilterTest.cpp | 55 ++++++++++++++++++++++++++++++++++++++ 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 green( + SkColorFilter::CreateModeFilter(SK_ColorGREEN, SkXfermode::kSrc_Mode)); + SkAutoTUnref 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 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 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 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); -- cgit v1.2.3