Add image as a draw type that can be filtered

Add image as a draw type that can be filtered

Add image as a draw type that can be filtered.

This is needed when SkImage is added as an object to be drawn so that
the draw is forwarded to SkBaseDevice. This would be used in making
filters use SkImages.

BUG=skia:3388
Committed: https://skia.googlesource.com/skia/+/fa77eb1e51b9317ff993d1be504ada173b561e5f

[Kimmo Kinnunen, 2015-02-26 12:25:42]

kkinnunen@nvidia.com changed reviewers:
+ bsalomon@google.com, reed@google.com

[reed1, 2015-02-26 22:04:18]

Could drawImage not use LOOPER_BEGIN insetad, to reuse that macro?

[Kimmo Kinnunen, 2015-02-27 12:21:39]

On 2015/02/26 22:04:18, reed1 wrote:
> Could drawImage not use LOOPER_BEGIN insetad, to reuse that macro?

Done.

[reed1, 2015-02-27 15:26:26]

lgtm

[reed1, 2015-02-27 15:26:29]

The CQ bit was checked by reed@google.com

[commit-bot: I haz the power, 2015-02-27 15:26:41]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/960783003/40001

[commit-bot: I haz the power, 2015-02-27 15:26:43]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-02-27 15:26:44]

Failed to apply patch for tests/SkImageTest.cpp:
While running git apply --index -3 -p1;
  error: patch failed: tests/SkImageTest.cpp:6
  error: repository lacks the necessary blob to fall back on 3-way merge.
  error: tests/SkImageTest.cpp: patch does not apply

Patch:       tests/SkImageTest.cpp
Index: tests/SkImageTest.cpp
diff --git a/tests/SkImageTest.cpp b/tests/SkImageTest.cpp
index
935b5ca6c6a77a2d7c8da8b63215ab497056b3ad..13bd3731077263d193bd542f8810b009bf19d29c
100644
--- a/tests/SkImageTest.cpp
+++ b/tests/SkImageTest.cpp
@@ -6,6 +6,8 @@
  */
 
 #include "SkBitmapDevice.h"
+#include "SkColor.h"
+#include "SkDrawFilter.h"
 #include "SkImagePriv.h"
 #include "SkImage_Base.h"
 #include "Test.h"
@@ -84,4 +86,145 @@ DEF_TEST(SkImageFromBitmap_sharing, reporter) {
         SkAssertResult(as_IB(image)->getROPixels(&imageBitmap));
         REPORTER_ASSERT(reporter, verifyPixelRef == imageBitmap.pixelRef());
     }
+}
+
+namespace {
+class TestDrawFilterImage : public SkDrawFilter {
+public:
+    TestDrawFilterImage()
+            : fFilteredImage(0)
+            , fFilteredOthers(0)
+            , fPreventImages(true)
+            , fPreventOthers(true) {
+    }
+
+    bool filter(SkPaint*, Type type) SK_OVERRIDE {
+        if (type == SkDrawFilter::kImage_Type) {
+            if (fPreventImages) {
+                fFilteredImage++;
+                return false;
+            }
+            return true;
+        }
+
+        if (fPreventOthers) {
+            fFilteredOthers++;
+            return false;
+        }
+        return true;
+    }
+    int fFilteredImage;
+    int fFilteredOthers;
+    bool fPreventImages;
+    bool fPreventOthers;
 };
+}
+
+DEF_TEST(SkCanvas_test_draw_filter_image, reporter) {
+    SkBitmap bitmap;
+    bitmap.allocN32Pixels(1, 1);
+    bitmap.eraseColor(SK_ColorTRANSPARENT);
+    TestDrawFilterImage drawFilter;
+    SkCanvas canvas(bitmap);
+
+    SkBitmap imageBitmap;
+    imageBitmap.allocN32Pixels(1, 1);
+    imageBitmap.eraseColor(SK_ColorGREEN);
+    SkAutoTUnref<SkImage> image(SkNewImageFromBitmap(imageBitmap, NULL));
+    canvas.drawImage(image, 0, 0);
+
+    uint32_t pixel;
+    SkImageInfo info = SkImageInfo::MakeN32Premul(1, 1);
+    canvas.readPixels(info, &pixel, 4, 0, 0);
+    REPORTER_ASSERT(reporter, pixel == SK_ColorGREEN);
+
+    canvas.setDrawFilter(&drawFilter);
+    imageBitmap.eraseColor(SK_ColorRED);
+    image.reset(SkNewImageFromBitmap(imageBitmap, NULL));
+    canvas.drawImage(image, 0, 0);
+    canvas.readPixels(info, &pixel, 4, 0, 0);
+    REPORTER_ASSERT(reporter, pixel == SK_ColorGREEN);
+    REPORTER_ASSERT(reporter, drawFilter.fFilteredImage == 1);
+    REPORTER_ASSERT(reporter, drawFilter.fFilteredOthers == 0);
+
+    // Document a strange case: filtering everything but the images does not
work as
+    // expected. Images go through, but no pixels appear. (This due to
SkCanvas::drawImage() using
+    // SkCanvas::drawBitmap() instead of non-existing
SkBaseDevice::drawImage()).
+    drawFilter.fFilteredImage = 0;
+    drawFilter.fPreventImages = false;
+
+    canvas.drawImage(image, 0, 0);
+    canvas.readPixels(info, &pixel, 4, 0, 0);
+    REPORTER_ASSERT(reporter, pixel == SK_ColorGREEN);
+    REPORTER_ASSERT(reporter, drawFilter.fFilteredImage == 0);
+    REPORTER_ASSERT(reporter, drawFilter.fFilteredOthers == 1);
+}
+
+namespace {
+/*
+ *  Subclass of looper that just draws once with one pixel offset.
+ */
+class OnceTestLooper : public SkDrawLooper {
+public:
+    OnceTestLooper() { }
+
+    SkDrawLooper::Context* createContext(SkCanvas*, void* storage) const
SK_OVERRIDE {
+        return SkNEW_PLACEMENT(storage, OnceTestLooperContext());
+    }
+
+    size_t contextSize() const SK_OVERRIDE { return
sizeof(OnceTestLooperContext); }
+
+#ifndef SK_IGNORE_TO_STRING
+    void toString(SkString* str) const SK_OVERRIDE {
+        str->append("OnceTestLooper:");
+    }
+#endif
+
+    SK_DECLARE_PUBLIC_FLATTENABLE_DESERIALIZATION_PROCS(OnceTestLooper);
+
+private:
+    class OnceTestLooperContext : public SkDrawLooper::Context {
+    public:
+        OnceTestLooperContext() : fCount(0) {}
+        virtual ~OnceTestLooperContext() {}
+
+        bool next(SkCanvas* canvas, SkPaint*) SK_OVERRIDE {
+            SkASSERT(fCount <= 1);
+            canvas->translate(0, 1);
+            return fCount++ < 1;
+        }
+    private:
+        unsigned fCount;
+    };
+};
+
+SkFlattenable* OnceTestLooper::CreateProc(SkReadBuffer&) { return
SkNEW(OnceTestLooper); }
+}
+
+DEF_TEST(SkCanvas_test_draw_looper_image, reporter) {
+    // Test that drawImage loops with the looper the correct way. At the time
of writing, this was
+    // tricky because drawImage was implemented with drawBitmap. The drawBitmap
uses applies the
+    // looper.
+    SkBitmap bitmap;
+    bitmap.allocN32Pixels(10, 10);
+    bitmap.eraseColor(SK_ColorRED);
+    OnceTestLooper drawLooper;
+    SkCanvas canvas(bitmap);
+
+    SkBitmap imageBitmap;
+    imageBitmap.allocN32Pixels(1, 1);
+    imageBitmap.eraseColor(SK_ColorGREEN);
+    SkAutoTUnref<SkImage> image(SkNewImageFromBitmap(imageBitmap, NULL));
+    SkPaint p;
+    p.setLooper(&drawLooper);
+    canvas.drawImage(image, 0, 0, &p);
+
+    uint32_t pixel;
+    SkImageInfo info = SkImageInfo::MakeN32Premul(1, 1);
+    canvas.readPixels(info, &pixel, 4, 0, 1);
+    REPORTER_ASSERT(reporter, pixel == SK_ColorGREEN);
+    canvas.readPixels(info, &pixel, 4, 0, 0);
+    REPORTER_ASSERT(reporter, pixel == SK_ColorRED);
+    canvas.readPixels(info, &pixel, 4, 0, 2);
+    REPORTER_ASSERT(reporter, pixel == SK_ColorRED);
+}

[Kimmo Kinnunen, 2015-03-05 08:33:19]

The CQ bit was checked by kkinnunen@nvidia.com

[Kimmo Kinnunen, 2015-03-05 08:33:20]

The patchset sent to the CQ was uploaded after l-g-t-m from reed@google.com
Link to the patchset: https://chromiumcodereview.appspot.com/960783003/#ps60001
(title: "rebase")

[commit-bot: I haz the power, 2015-03-05 08:34:08]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/960783003/60001

[commit-bot: I haz the power, 2015-03-05 08:39:52]

Committed patchset #4 (id:60001) as
https://skia.googlesource.com/skia/+/fa77eb1e51b9317ff993d1be504ada173b561e5f

[Kimmo Kinnunen, 2015-03-05 12:49:34]

A revert of this CL (patchset #4 id:60001) has been created in
https://codereview.chromium.org/980273002/ by kkinnunen@nvidia.com.

The reason for reverting is: Fails on mac for some reason.
Also is a bit wrong, but this should not be reason for the failure...

[Kimmo Kinnunen, 2015-03-05 16:06:21]

On 2015/03/05 12:49:34, Kimmo Kinnunen wrote:
> The reason for reverting is: Fails on mac for some reason.
> Also is a bit wrong, but this should not be reason for the failure...

Mike, could you look at the patch again? I fixed the problems.. Changes in
canvas and the test

[reed1, 2015-03-06 13:37:56]

Is the bug/fix that we didn't call fSaveCount += 1 in autodrawlooper? If so,
perhaps that should land as its own fix (it seems unrelated to kImage_Type).
Also, I wonder if we can restructure canvas so that this looks more natural
(e.g. some alternate to the public saveLayer() so that the bookkeeping of
(private) fSaveCount is still encapsulated).

If this is the crux, I can sketch the sort of restructure I have in mind...

[Kimmo Kinnunen, 2015-03-06 13:59:57]

On 2015/03/06 13:37:56, reed1 wrote:
> Is the bug/fix that we didn't call fSaveCount += 1 in autodrawlooper? If so,
> perhaps that should land as its own fix (it seems unrelated to kImage_Type).

Well, at the moment there's no re-entering, so it's matter of definition whether
or not this related to kImage.

The request to use the macro causes the re-entering and the bug.

> Also, I wonder if we can restructure canvas so that this looks more natural
> (e.g. some alternate to the public saveLayer() so that the bookkeeping of
> (private) fSaveCount is still encapsulated).
> 
> If this is the crux, I can sketch the sort of restructure I have in mind...

This is the crux. Feel free to restructure.