Fix for saveLayer() with filters vs. the BBox Hierarchy.
When building acceleration structures for SkPicture, we must transform
the primitive's bounds not only by its own SkPaint, but by the paints of
any saveLayer()s currently active above it.
We do this by pushing the SkPaint onto a stack on
saveLayer(), and popping them on restore(). We also push
a NULL paint in save(), so that the pushes and pops are
balanced.
BUG=skia:2734
R=mtklein@google.com, reed@google.com, robertphillips@google.com
Committed: https://skia.googlesource.com/skia/+/6ca0b6a46cbe9bef3e2b9b9db813ec864efd62de
Author: senorblanco@chromium.org
Review URL: https://codereview.chromium.org/380373003
diff --git a/gm/resizeimagefilter.cpp b/gm/resizeimagefilter.cpp
index 96a6118..e87dff0 100644
--- a/gm/resizeimagefilter.cpp
+++ b/gm/resizeimagefilter.cpp
@@ -21,8 +21,6 @@
}
protected:
- virtual uint32_t onGetFlags() const SK_OVERRIDE { return kNoBBH_Flag; }
-
virtual SkString onShortName() {
return SkString("resizeimagefilter");
}
diff --git a/src/core/SkBBoxRecord.cpp b/src/core/SkBBoxRecord.cpp
index a40ea8b..448e8f6 100644
--- a/src/core/SkBBoxRecord.cpp
+++ b/src/core/SkBBoxRecord.cpp
@@ -8,6 +8,10 @@
#include "SkBBoxRecord.h"
+SkBBoxRecord::~SkBBoxRecord() {
+ fSaveStack.deleteAll();
+}
+
void SkBBoxRecord::drawOval(const SkRect& rect, const SkPaint& paint) {
if (this->transformBounds(rect, &paint)) {
INHERITED::drawOval(rect, paint);
@@ -287,6 +291,26 @@
}
}
+void SkBBoxRecord::willSave() {
+ fSaveStack.push(NULL);
+ this->INHERITED::willSave();
+}
+
+SkCanvas::SaveLayerStrategy SkBBoxRecord::willSaveLayer(const SkRect* bounds,
+ const SkPaint* paint,
+ SaveFlags flags) {
+ // Image filters can affect the effective bounds of primitives drawn inside saveLayer().
+ // Copy the paint so we can compute the modified bounds in transformBounds().
+ fSaveStack.push(paint && paint->getImageFilter() ? new SkPaint(*paint) : NULL);
+ return this->INHERITED::willSaveLayer(bounds, paint, flags);
+}
+
+void SkBBoxRecord::willRestore() {
+ delete fSaveStack.top();
+ fSaveStack.pop();
+ this->INHERITED::willRestore();
+}
+
bool SkBBoxRecord::transformBounds(const SkRect& bounds, const SkPaint* paint) {
SkRect outBounds = bounds;
outBounds.sort();
@@ -305,6 +329,14 @@
}
}
+ for (int i = fSaveStack.count() - 1; i >= 0; --i) {
+ const SkPaint* paint = fSaveStack.getAt(i);
+ if (paint && paint->canComputeFastBounds()) {
+ SkRect temp;
+ outBounds = paint->computeFastBounds(outBounds, &temp);
+ }
+ }
+
if (!outBounds.isEmpty() && !this->quickReject(outBounds)) {
this->getTotalMatrix().mapRect(&outBounds);
this->handleBBox(outBounds);
diff --git a/src/core/SkBBoxRecord.h b/src/core/SkBBoxRecord.h
index f3d72b0..a6edc98 100644
--- a/src/core/SkBBoxRecord.h
+++ b/src/core/SkBBoxRecord.h
@@ -10,6 +10,7 @@
#define SkBBoxRecord_DEFINED
#include "SkPictureRecord.h"
+#include "SkTDArray.h"
/**
* This is an abstract SkPictureRecord subclass that intercepts draw calls and computes an
@@ -22,7 +23,7 @@
SkBBoxRecord(const SkISize& size, uint32_t recordFlags)
: INHERITED(size, recordFlags) {
}
- virtual ~SkBBoxRecord() { }
+ virtual ~SkBBoxRecord();
/**
* This is called each time we get a bounding box, it will be axis-aligned,
@@ -66,6 +67,9 @@
virtual void onDrawTextOnPath(const void* text, size_t byteLength, const SkPath& path,
const SkMatrix* matrix, const SkPaint&) SK_OVERRIDE;
virtual void onDrawPicture(const SkPicture* picture) SK_OVERRIDE;
+ virtual void willSave() SK_OVERRIDE;
+ virtual SaveLayerStrategy willSaveLayer(const SkRect*, const SkPaint*, SaveFlags) SK_OVERRIDE;
+ virtual void willRestore() SK_OVERRIDE;
private:
/**
@@ -75,6 +79,12 @@
**/
bool transformBounds(const SkRect& bounds, const SkPaint* paint);
+ /**
+ * Paints from currently-active saveLayers that need to be applied to bounding boxes of all
+ * primitives drawn inside them. We own these pointers.
+ **/
+ SkTDArray<const SkPaint*> fSaveStack;
+
typedef SkPictureRecord INHERITED;
};
diff --git a/tests/ImageFilterTest.cpp b/tests/ImageFilterTest.cpp
index 3282208..7da4a91 100644
--- a/tests/ImageFilterTest.cpp
+++ b/tests/ImageFilterTest.cpp
@@ -409,6 +409,70 @@
}
}
+static void drawBlurredRect(SkCanvas* canvas) {
+ SkAutoTUnref<SkImageFilter> filter(SkBlurImageFilter::Create(SkIntToScalar(8), 0));
+ SkPaint filterPaint;
+ filterPaint.setColor(SK_ColorWHITE);
+ filterPaint.setImageFilter(filter);
+ canvas->saveLayer(NULL, &filterPaint);
+ SkPaint whitePaint;
+ whitePaint.setColor(SK_ColorWHITE);
+ canvas->drawRect(SkRect::Make(SkIRect::MakeWH(4, 4)), whitePaint);
+ canvas->restore();
+}
+
+static void drawPictureClipped(SkCanvas* canvas, const SkRect& clipRect, const SkPicture* picture) {
+ canvas->save();
+ canvas->clipRect(clipRect);
+ canvas->drawPicture(picture);
+ canvas->restore();
+}
+
+DEF_TEST(ImageFilterDrawTiledBlurRTree, reporter) {
+ // Check that the blur filter when recorded with RTree acceleration,
+ // and drawn tiled (with subsequent clip rects) exactly
+ // matches the same filter drawn with without RTree acceleration.
+ // This tests that the "bleed" from the blur into the otherwise-blank
+ // tiles is correctly rendered.
+ // Tests pass by not asserting.
+
+ int width = 16, height = 8;
+ SkBitmap result1, result2;
+ result1.allocN32Pixels(width, height);
+ result2.allocN32Pixels(width, height);
+ SkCanvas canvas1(result1);
+ SkCanvas canvas2(result2);
+ int tileSize = 8;
+
+ canvas1.clear(0);
+ canvas2.clear(0);
+
+ SkRTreeFactory factory;
+
+ SkPictureRecorder recorder1, recorder2;
+ // The only difference between these two pictures is that one has RTree aceleration.
+ SkCanvas* recordingCanvas1 = recorder1.beginRecording(width, height, NULL, 0);
+ SkCanvas* recordingCanvas2 = recorder2.beginRecording(width, height, &factory, 0);
+ drawBlurredRect(recordingCanvas1);
+ drawBlurredRect(recordingCanvas2);
+ SkAutoTUnref<SkPicture> picture1(recorder1.endRecording());
+ SkAutoTUnref<SkPicture> picture2(recorder2.endRecording());
+ for (int y = 0; y < height; y += tileSize) {
+ for (int x = 0; x < width; x += tileSize) {
+ SkRect tileRect = SkRect::Make(SkIRect::MakeXYWH(x, y, tileSize, tileSize));
+ drawPictureClipped(&canvas1, tileRect, picture1);
+ drawPictureClipped(&canvas2, tileRect, picture2);
+ }
+ }
+ for (int y = 0; y < height; y++) {
+ int diffs = memcmp(result1.getAddr32(0, y), result2.getAddr32(0, y), result1.rowBytes());
+ REPORTER_ASSERT(reporter, !diffs);
+ if (diffs) {
+ break;
+ }
+ }
+}
+
DEF_TEST(ImageFilterMatrixConvolution, reporter) {
// Check that a 1x3 filter does not cause a spurious assert.
SkScalar kernel[3] = {
