Refactor SkLiteDL to no longer extend SkDrawable.

Change-Id: Ieaa849bceba0e98e4c99491c721fe945a0694e68
Reviewed-on: https://skia-review.googlesource.com/9111
Reviewed-by: Mike Klein <mtklein@chromium.org>
Reviewed-by: Herb Derby <herb@google.com>
Reviewed-by: Stan Iliev <stani@google.com>
Commit-Queue: Derek Sollenberger <djsollen@google.com>

diff --git a/bench/PictureOverheadBench.cpp b/bench/PictureOverheadBench.cpp
index 27424a2..1fea9e9 100644 (file)
--- a/bench/PictureOverheadBench.cpp
+++ b/bench/PictureOverheadBench.cpp
@@ -25,14 +25,15 @@ struct PictureOverheadBench : public Benchmark {
     void onDraw(int loops, SkCanvas*) override {
         SkLiteRecorder lite;
         SkPictureRecorder rec;
-        for (int i = 0; i < loops; i++) {
-            SkRect bounds{0,0, 2000,3000};
 
-            sk_sp<SkLiteDL> liteDL;
+        SkIRect iBounds = {0,0, 2000,3000};
+        SkRect bounds = SkRect::Make(iBounds);
+
+        for (int i = 0; i < loops; i++) {
+            SkLiteDL liteDL;
             SkCanvas* canvas;
             if (kLite) {
-                liteDL = SkLiteDL::New(bounds);
-                lite.reset(liteDL.get());
+                lite.reset(&liteDL, iBounds);
                 canvas = &lite;
             } else {
                 rec.beginRecording(bounds);

diff --git a/bench/RecordingBench.cpp b/bench/RecordingBench.cpp
index 69be911..aa31052 100644 (file)
--- a/bench/RecordingBench.cpp
+++ b/bench/RecordingBench.cpp
@@ -40,9 +40,9 @@ RecordingBench::RecordingBench(const char* name, const SkPicture* pic, bool useB
 {
     // If we're recording into an SkLiteDL, also record _from_ one.
     if (lite) {
-        fDL = SkLiteDL::New(fSrc->cullRect());
+        fDL.reset(new SkLiteDL());
         SkLiteRecorder r;
-        r.reset(fDL.get());
+        r.reset(fDL.get(), fSrc->cullRect().roundOut());
         fSrc->playback(&r);
     }
 }
@@ -51,10 +51,10 @@ void RecordingBench::onDraw(int loops, SkCanvas*) {
     if (fDL) {
         SkLiteRecorder rec;
         while (loops --> 0) {
-            sk_sp<SkLiteDL> dl = SkLiteDL::New(fSrc->cullRect());
-            rec.reset(dl.get());
+            SkLiteDL dl;
+            rec.reset(&dl, fSrc->cullRect().roundOut());
             fDL->draw(&rec);
-            dl->makeThreadsafe();
+            dl.makeThreadsafe();
         }
 
     } else {

diff --git a/bench/RecordingBench.h b/bench/RecordingBench.h
index 4e34276..beaea3e 100644 (file)
--- a/bench/RecordingBench.h
+++ b/bench/RecordingBench.h
@@ -36,7 +36,7 @@ protected:
     void onDraw(int loops, SkCanvas*) override;
 
 private:
-    sk_sp<SkLiteDL> fDL;
+    std::unique_ptr<SkLiteDL> fDL;
     bool fUseBBH;
 
     typedef PictureCentricBench INHERITED;

diff --git a/dm/DMSrcSink.cpp b/dm/DMSrcSink.cpp
index a499111..1a35381 100644 (file)
--- a/dm/DMSrcSink.cpp
+++ b/dm/DMSrcSink.cpp
@@ -1813,18 +1813,17 @@ Error ViaSingletonPictures::draw(
 
 Error ViaLite::draw(const Src& src, SkBitmap* bitmap, SkWStream* stream, SkString* log) const {
     auto size = src.size();
-    SkRect bounds = {0,0, (SkScalar)size.width(), (SkScalar)size.height()};
+    SkIRect bounds = {0,0, size.width(), size.height()};
     return draw_to_canvas(fSink.get(), bitmap, stream, log, size, [&](SkCanvas* canvas) -> Error {
-        sk_sp<SkLiteDL> dl = SkLiteDL::New(bounds);
-
+        SkLiteDL dl;
         SkLiteRecorder rec;
-        rec.reset(dl.get());
+        rec.reset(&dl, bounds);
 
         Error err = src.draw(&rec);
         if (!err.isEmpty()) {
             return err;
         }
-        dl->draw(canvas);
+        dl.draw(canvas);
         return check_against_reference(bitmap, src, fSink.get());
     });
 }

diff --git a/src/core/SkLiteDL.cpp b/src/core/SkLiteDL.cpp
index 95e801a..9c0bd51 100644 (file)
--- a/src/core/SkLiteDL.cpp
+++ b/src/core/SkLiteDL.cpp
@@ -773,45 +773,20 @@ static const void_fn make_threadsafe_fns[] = { TYPES(M) };
 static const void_fn dtor_fns[] = { TYPES(M) };
 #undef M
 
-void SkLiteDL::onDraw(SkCanvas* canvas) { this->map(draw_fns, canvas, canvas->getTotalMatrix()); }
-void SkLiteDL::makeThreadsafe()         { this->map(make_threadsafe_fns); }
+void SkLiteDL::makeThreadsafe() { this->map(make_threadsafe_fns); }
 
-SkRect SkLiteDL::onGetBounds() {
-    return fBounds;
+void SkLiteDL::draw(SkCanvas* canvas) {
+    SkAutoCanvasRestore acr(canvas, false);
+    this->map(draw_fns, canvas, canvas->getTotalMatrix());
 }
 
-SkLiteDL:: SkLiteDL(SkRect bounds) : fUsed(0), fReserved(0), fBounds(bounds) {}
-
 SkLiteDL::~SkLiteDL() {
-    this->reset(SkRect::MakeEmpty());
-}
-
-sk_sp<SkLiteDL> SkLiteDL::New(SkRect bounds) {
-    return sk_sp<SkLiteDL>(new SkLiteDL(bounds));
+    this->reset();
 }
 
-void SkLiteDL::reset(SkRect bounds) {
-    SkASSERT(this->unique());
+void SkLiteDL::reset() {
     this->map(dtor_fns);
 
     // Leave fBytes and fReserved alone.
     fUsed   = 0;
-    fBounds = bounds;
-}
-
-void SkLiteDL::drawAsLayer(SkCanvas* canvas, const SkMatrix* matrix, const SkPaint* paint) {
-    auto fallback_plan = [&] {
-        SkRect bounds = this->getBounds();
-        canvas->saveLayer(&bounds, paint);
-            this->draw(canvas, matrix);
-        canvas->restore();
-    };
-
-    // TODO: single-draw specializations
-
-    return fallback_plan();
-}
-
-void SkLiteDL::setBounds(const SkRect& bounds) {
-    fBounds = bounds;
 }

diff --git a/src/core/SkLiteDL.h b/src/core/SkLiteDL.h
index 330131f..7ffc22a 100644 (file)
--- a/src/core/SkLiteDL.h
+++ b/src/core/SkLiteDL.h
@@ -15,11 +15,13 @@
 #include "SkRect.h"
 #include "SkTDArray.h"
 
-class SkLiteDL final : public SkDrawable {
+class SkLiteDL final {
 public:
-    static sk_sp<SkLiteDL> New(SkRect);
-    void reset(SkRect);
+    ~SkLiteDL();
+
+    void draw(SkCanvas* canvas);
 
+    void reset();
     void makeThreadsafe();
     bool empty() const { return fUsed == 0; }
 
@@ -27,13 +29,6 @@ public:
     void setDrawFilter(SkDrawFilter*);
 #endif
 
-    // Draws as if...
-    //   SkRect bounds = this->getBounds();
-    //   canvas->saveLayer(&bounds, paint);
-    //       this->draw(canvas, matrix);
-    //   canvas->restore();
-    void drawAsLayer(SkCanvas*, const SkMatrix*, const SkPaint*);
-
     void save();
     void saveLayer(const SkRect*, const SkPaint*, const SkImageFilter*, SkCanvas::SaveLayerFlags);
     void restore();
@@ -85,15 +80,7 @@ public:
     void drawAtlas(const SkImage*, const SkRSXform[], const SkRect[], const SkColor[], int,
                    SkBlendMode, const SkRect*, const SkPaint*);
 
-    void setBounds(const SkRect& bounds);
-
 private:
-    SkLiteDL(SkRect);
-    ~SkLiteDL();
-
-    SkRect   onGetBounds() override;
-    void onDraw(SkCanvas*) override;
-
     template <typename T, typename... Args>
     void* push(size_t, Args&&...);
 
@@ -101,9 +88,8 @@ private:
     void map(const Fn[], Args...);
 
     SkAutoTMalloc<uint8_t> fBytes;
-    size_t                 fUsed;
-    size_t                 fReserved;
-    SkRect                 fBounds;
+    size_t                 fUsed = 0;
+    size_t                 fReserved = 0;
 };
 
 #endif//SkLiteDL_DEFINED

diff --git a/src/core/SkLiteRecorder.cpp b/src/core/SkLiteRecorder.cpp
index 9f7ae39..b3314b0 100644 (file)
--- a/src/core/SkLiteRecorder.cpp
+++ b/src/core/SkLiteRecorder.cpp
@@ -13,8 +13,8 @@ SkLiteRecorder::SkLiteRecorder()
     : INHERITED(1, 1)
     , fDL(nullptr) {}
 
-void SkLiteRecorder::reset(SkLiteDL* dl) {
-    this->resetForNextPicture(dl->getBounds().roundOut());
+void SkLiteRecorder::reset(SkLiteDL* dl, const SkIRect& bounds) {
+    this->resetForNextPicture(bounds);
     fDL = dl;
 }
 

diff --git a/src/core/SkLiteRecorder.h b/src/core/SkLiteRecorder.h
index ad8f264..4de9789 100644 (file)
--- a/src/core/SkLiteRecorder.h
+++ b/src/core/SkLiteRecorder.h
@@ -15,7 +15,7 @@ class SkLiteDL;
 class SkLiteRecorder final : public SkNoDrawCanvas {
 public:
     SkLiteRecorder();
-    void reset(SkLiteDL*);
+    void reset(SkLiteDL*, const SkIRect& bounds);
 
     sk_sp<SkSurface> onNewSurface(const SkImageInfo&, const SkSurfaceProps&) override;
 

diff --git a/tests/SkLiteDLTest.cpp b/tests/SkLiteDLTest.cpp
index 96046cc..514464a 100644 (file)
--- a/tests/SkLiteDLTest.cpp
+++ b/tests/SkLiteDLTest.cpp
@@ -10,21 +10,41 @@
 #include "SkLiteRecorder.h"
 
 DEF_TEST(SkLiteDL_basics, r) {
-    sk_sp<SkLiteDL> p { SkLiteDL::New({2,2,3,3}) };
+    SkLiteDL p;
+    p.save();
+        p.clipRect(SkRect{2,3,4,5}, kIntersect_SkClipOp, true);
+        p.drawRect(SkRect{0,0,9,9}, SkPaint{});
+    p.restore();
+}
 
-    p->save();
-        p->clipRect(SkRect{2,3,4,5}, kIntersect_SkClipOp, true);
-        p->drawRect(SkRect{0,0,9,9}, SkPaint{});
-    p->restore();
+DEF_TEST(SkLiteDL_unbalanced, r) {
+    SkLiteRecorder rec;
+    SkCanvas* c = &rec;
+
+    SkLiteDL p;
+    rec.reset(&p, {2,2,3,3});
+    c->save();
+        c->scale(2,2);
+        c->save();
+            c->translate(1,1);
+        // missing restore() but SkLiteDL::draw should balance it for us
+    c->restore();
+
+    // reinit the recorder so we can playback the original SkLiteDL
+    SkLiteDL p2;
+    rec.reset(&p2, {2,2,3,3});
+
+    REPORTER_ASSERT(r, 1 == rec.getSaveCount());
+    p.draw(c);
+    REPORTER_ASSERT(r, 1 == rec.getSaveCount());
 }
 
 DEF_TEST(SkLiteRecorder, r) {
-    sk_sp<SkLiteDL> p { SkLiteDL::New({2,2,3,3}) };
-
+    SkLiteDL p;
     SkLiteRecorder rec;
     SkCanvas* c = &rec;
 
-    rec.reset(p.get());
+    rec.reset(&p, {2,2,3,3});
 
     c->save();
         c->clipRect(SkRect{2,3,4,5}, kIntersect_SkClipOp, true);