test (and fix) clip_restriction in canvas

BUG=skia:

Change-Id: I86d25d0fd82be35d01471fba59f77b360be5373c
Reviewed-on: https://skia-review.googlesource.com/9995
Reviewed-by: Derek Sollenberger <djsollen@google.com>
Commit-Queue: Derek Sollenberger <djsollen@google.com>

diff --git a/bench/PDFBench.cpp b/bench/PDFBench.cpp
index e0d4934..3716ff3 100644 (file)
--- a/bench/PDFBench.cpp
+++ b/bench/PDFBench.cpp
@@ -17,16 +17,9 @@
 #include "SkStream.h"
 
 namespace {
-struct NullWStream : public SkWStream {
-    NullWStream() : fN(0) {}
-    bool write(const void*, size_t n) override { fN += n; return true; }
-    size_t bytesWritten() const override { return fN; }
-    size_t fN;
-};
-
 struct WStreamWriteTextBenchmark : public Benchmark {
     std::unique_ptr<SkWStream> fWStream;
-    WStreamWriteTextBenchmark() : fWStream(new NullWStream) {}
+    WStreamWriteTextBenchmark() : fWStream(new SkNullWStream) {}
     const char* onGetName() override { return "WStreamWriteText"; }
     bool isSuitableFor(Backend backend) override {
         return backend == kNonRendering_Backend;
@@ -53,7 +46,7 @@ DEF_BENCH(return new WStreamWriteTextBenchmark;)
 namespace {
 static void test_pdf_object_serialization(const sk_sp<SkPDFObject> object) {
     // SkDebugWStream wStream;
-    NullWStream wStream;
+    SkNullWStream wStream;
     SkPDFObjNumMap objNumMap;
     objNumMap.addObjectRecursively(object.get());
     for (int i = 0; i < objNumMap.objects().count(); ++i) {
@@ -220,7 +213,7 @@ struct PDFShaderBench : public Benchmark {
     void onDraw(int loops, SkCanvas*) final {
         SkASSERT(fShader);
         while (loops-- > 0) {
-            NullWStream nullStream;
+            SkNullWStream nullStream;
             SkPDFDocument doc(&nullStream, nullptr, 72,
                               SkDocument::PDFMetadata(), nullptr, false);
             sk_sp<SkPDFObject> shader(
@@ -233,7 +226,7 @@ struct PDFShaderBench : public Benchmark {
 
 struct WritePDFTextBenchmark : public Benchmark {
     std::unique_ptr<SkWStream> fWStream;
-    WritePDFTextBenchmark() : fWStream(new NullWStream) {}
+    WritePDFTextBenchmark() : fWStream(new SkNullWStream) {}
     const char* onGetName() override { return "WritePDFText"; }
     bool isSuitableFor(Backend backend) override {
         return backend == kNonRendering_Backend;

diff --git a/include/core/SkStream.h b/include/core/SkStream.h
index 0a53d47..e10aece 100644 (file)
--- a/include/core/SkStream.h
+++ b/include/core/SkStream.h
@@ -232,6 +232,18 @@ public:
     static int SizeOfPackedUInt(size_t value);
 };
 
+class SK_API SkNullWStream : public SkWStream {
+public:
+    SkNullWStream() : fBytesWritten(0) {}
+
+    bool write(const void*, size_t n) override { fBytesWritten += n; return true; }
+    void flush() override {}
+    size_t bytesWritten() const override { return fBytesWritten; }
+
+private:
+    size_t fBytesWritten;
+};
+
 ////////////////////////////////////////////////////////////////////////////////////////
 
 #include <stdio.h>

diff --git a/src/core/SkRasterClip.cpp b/src/core/SkRasterClip.cpp
index 6f9eb6b..090297d 100644 (file)
--- a/src/core/SkRasterClip.cpp
+++ b/src/core/SkRasterClip.cpp
@@ -62,43 +62,35 @@ static MutateResult mutate_conservative_op(SkRegion::Op* op, bool inverseFilled)
 
 void SkConservativeClip::op(const SkRect& localRect, const SkMatrix& ctm, const SkIRect& devBounds,
                             SkRegion::Op op, bool doAA) {
-    SkRect devRect;
-
-    SkIRect bounds(devBounds);
-    this->applyClipRestriction(op, &bounds);
     SkIRect ir;
     switch (mutate_conservative_op(&op, false)) {
         case kDoNothing_MutateResult:
             return;
         case kReplaceClippedAgainstGlobalBounds_MutateResult:
-            ir = bounds;
+            ir = devBounds;
             break;
-        case kContinue_MutateResult:
+        case kContinue_MutateResult: {
+            SkRect devRect;
             ctm.mapRect(&devRect, localRect);
             ir = doAA ? devRect.roundOut() : devRect.round();
-            break;
+        } break;
     }
     this->op(ir, op);
 }
 
 void SkConservativeClip::op(const SkRRect& rrect, const SkMatrix& ctm, const SkIRect& devBounds,
                             SkRegion::Op op, bool doAA) {
-    SkIRect bounds(devBounds);
-    this->applyClipRestriction(op, &bounds);
-    this->op(rrect.getBounds(), ctm, bounds, op, doAA);
+    this->op(rrect.getBounds(), ctm, devBounds, op, doAA);
 }
 
 void SkConservativeClip::op(const SkPath& path, const SkMatrix& ctm, const SkIRect& devBounds,
                             SkRegion::Op op, bool doAA) {
-    SkIRect bounds(devBounds);
-    this->applyClipRestriction(op, &bounds);
-
     SkIRect ir;
     switch (mutate_conservative_op(&op, path.isInverseFillType())) {
         case kDoNothing_MutateResult:
             return;
         case kReplaceClippedAgainstGlobalBounds_MutateResult:
-            ir = bounds;
+            ir = devBounds;
             break;
         case kContinue_MutateResult: {
             SkRect bounds = path.getBounds();
@@ -129,6 +121,7 @@ void SkConservativeClip::op(const SkIRect& devRect, SkRegion::Op op) {
     SkRegion result;
     result.op(SkRegion(fBounds), SkRegion(devRect), op);
     fBounds = result.getBounds();
+    this->applyClipRestriction(op, &fBounds);
 }
 
 ///////////////////////////////////////////////////////////////////////////////////////////////////

diff --git a/tests/CanvasTest.cpp b/tests/CanvasTest.cpp
index cfdce62..d18324a 100644 (file)
--- a/tests/CanvasTest.cpp
+++ b/tests/CanvasTest.cpp
@@ -44,6 +44,7 @@
  *      works the same way as SIMPLE_TEST_STEP, and additionally verifies
  *      that the invoked method returns a non-zero value.
  */
+
 #include "SkBitmap.h"
 #include "SkCanvas.h"
 #include "SkClipStack.h"
@@ -103,6 +104,59 @@ DEF_TEST(canvas_clipbounds, reporter) {
     }
 }
 
+const SkIRect gBaseRestrictedR = { 0, 0, 10, 10 };
+
+static void test_restriction(skiatest::Reporter* reporter, SkCanvas* canvas) {
+    REPORTER_ASSERT(reporter, canvas->getDeviceClipBounds() == gBaseRestrictedR);
+
+    const SkIRect restrictionR = { 2, 2, 8, 8 };
+    canvas->androidFramework_setDeviceClipRestriction(restrictionR);
+    REPORTER_ASSERT(reporter, canvas->getDeviceClipBounds() == restrictionR);
+
+    const SkIRect clipR = { 4, 4, 6, 6 };
+    canvas->clipRect(SkRect::Make(clipR), SkClipOp::kIntersect);
+    REPORTER_ASSERT(reporter, canvas->getDeviceClipBounds() == clipR);
+
+    // now test that expanding clipops can't exceed the restriction
+    const SkClipOp expanders[] = {
+        SkClipOp::kUnion_deprecated,
+        SkClipOp::kXOR_deprecated,
+        SkClipOp::kReverseDifference_deprecated,
+        SkClipOp::kReplace_deprecated,
+    };
+
+    const SkRect expandR = { 0, 0, 5, 9 };
+    SkASSERT(!SkRect::Make(restrictionR).contains(expandR));
+
+    for (SkClipOp op : expanders) {
+        canvas->save();
+        canvas->clipRect(expandR, op);
+        REPORTER_ASSERT(reporter, gBaseRestrictedR.contains(canvas->getDeviceClipBounds()));
+        canvas->restore();
+    }
+}
+
+/**
+ *  Clip restriction logic exists in the canvas itself, and in various kinds of devices.
+ *
+ *  This test explicitly tries to exercise that variety:
+ *  - picture : empty device but exercises canvas itself
+ *  - pdf : uses SkClipStack in its device (as does SVG and GPU)
+ *  - raster : uses SkRasterClip in its device
+ */
+DEF_TEST(canvas_clip_restriction, reporter) {
+    test_restriction(reporter, SkPictureRecorder().beginRecording(SkRect::Make(gBaseRestrictedR)));
+
+    SkNullWStream stream;
+    auto doc = SkDocument::MakePDF(&stream);
+    test_restriction(reporter, doc->beginPage(SkIntToScalar(gBaseRestrictedR.width()),
+                                              SkIntToScalar(gBaseRestrictedR.height())));
+
+    auto surf = SkSurface::MakeRasterN32Premul(gBaseRestrictedR.width(),
+                                               gBaseRestrictedR.height(), nullptr);
+    test_restriction(reporter, surf->getCanvas());
+}
+
 static const int kWidth = 2, kHeight = 2;
 
 static void createBitmap(SkBitmap* bm, SkColor color) {