ok: refactor Src/Dst interactions

This makes everything a lot more like DM, for the same reason:
it's the best way to make Vias work.

Instead of exposing a canvas, Dsts take a Src to draw.  Vias still are
Dsts that wrap Dsts.  They do their internal work in draw() then pass a
proxy Src encapsulating that work to the next Dst's draw().

A little refactoring in ok.cpp allows arbitrary chains of Vias.

I removed the guarantee that Src methods are called in strict order.
It's easy enough to make each Src initialize itself as needed.

I moved the .png encoding back to ok.cpp.  It seemed weird for Dsts to
have to think about files and paths.  One day Dst will want a data()
method for non-image output (.pdf, .skp), and then we'll want ok.cpp to
be the one to coordinate what to write where.

Change-Id: Id4a3674b2d05aef2b5f10e0077df0a8407c07b61
Reviewed-on: https://skia-review.googlesource.com/10175
Reviewed-by: Mike Klein <mtklein@chromium.org>
Commit-Queue: Mike Klein <mtklein@chromium.org>

diff --git a/tools/ok.cpp b/tools/ok.cpp
index a8cf6f4..e4bf739 100644 (file)
--- a/tools/ok.cpp
+++ b/tools/ok.cpp
@@ -163,13 +163,13 @@ static std::vector<StreamType> stream_types;
 
 struct DstType {
     const char* name;
-    std::unique_ptr<Dst> (*factory)(Options, SkISize);
+    std::unique_ptr<Dst> (*factory)(Options);
 };
 static std::vector<DstType> dst_types;
 
 struct ViaType {
     const char* name;
-    std::unique_ptr<Dst> (*factory)(Options, SkISize, std::unique_ptr<Dst>);
+    std::unique_ptr<Dst> (*factory)(Options, std::unique_ptr<Dst>);
 };
 static std::vector<ViaType> via_types;
 
@@ -182,9 +182,8 @@ int main(int argc, char** argv) {
     std::regex  search      {".*"};
     std::string write_dir   {""};
 
-    std::unique_ptr<Stream>                                             stream;
-    std::function<std::unique_ptr<Dst> (SkISize)>                       dst_factory;
-    std::function<std::unique_ptr<Dst> (SkISize, std::unique_ptr<Dst>)> via_factory;
+    std::unique_ptr<Stream>                   stream;
+    std::function<std::unique_ptr<Dst>(void)> dst_factory;
 
     auto help = [&] {
         std::string stream_names, dst_names, via_names;
@@ -208,7 +207,7 @@ int main(int argc, char** argv) {
         }
 
         printf("%s [-j N] [-m regex] [-s regex] [-w dir] [-h]                        \n"
-                "  src[:k=v,...] dst[:k=v,...] [via[:k=v,...]]                       \n"
+                "  src[:k=v,...] dst[:k=v,...] [via[:k=v,...] ...]                   \n"
                 "  -j: Run at most N processes at any time.                          \n"
                 "      If <0, use -N threads instead.                                \n"
                 "      If 0, use one thread in one process.                          \n"
@@ -219,7 +218,7 @@ int main(int argc, char** argv) {
                 "  -h: Print this message and exit.                                  \n"
                 " src: content to draw: %s                                           \n"
                 " dst: how to draw that content: %s                                  \n"
-                " via: front-patch the dst: %s                                       \n"
+                " via: front-patches to the dst: %s                                  \n"
                 " Some srcs, dsts and vias have options, e.g. skp:dir=skps sw:ct=565 \n",
                 argv[0], stream_names.c_str(), dst_names.c_str(), via_names.c_str());
         return 1;
@@ -246,8 +245,8 @@ int main(int argc, char** argv) {
             if (0 == strncmp(d.name, argv[i], len)) {
                 switch (argv[i][len]) {
                     case  ':': len++;
-                    case '\0': dst_factory = [=](SkISize size){
-                                   return d.factory(Options{argv[i]+len}, size);
+                    case '\0': dst_factory = [=]{
+                                   return d.factory(Options{argv[i]+len});
                                };
                 }
             }
@@ -255,10 +254,11 @@ int main(int argc, char** argv) {
         for (auto v : via_types) {
             size_t len = strlen(v.name);
             if (0 == strncmp(v.name, argv[i], len)) {
+                if (!dst_factory) { return help(); }
                 switch (argv[i][len]) {
                     case  ':': len++;
-                    case '\0': via_factory = [=](SkISize size, std::unique_ptr<Dst> dst) {
-                                   return v.factory(Options{argv[i]+len}, size, std::move(dst));
+                    case '\0': dst_factory = [=]{
+                                   return v.factory(Options{argv[i]+len}, dst_factory());
                                };
                 }
             }
@@ -322,18 +322,16 @@ int main(int argc, char** argv) {
                 return Status::Skipped;
             }
 
-            auto size = src->size();
-            auto dst = dst_factory(size);
-            if (via_factory) {
-                dst = via_factory(size, std::move(dst));
+            auto dst = dst_factory();
+            if (!dst->draw(src.get())) {
+                return Status::Failed;
             }
 
-            auto canvas = dst->canvas();
-            src->draw(canvas);
-            canvas->restoreToCount(0);
-
             if (!write_dir.empty()) {
-                dst->write(write_dir + "/" + name);
+                auto image = dst->image();
+                sk_sp<SkData> png{image->encode()};
+                SkFILEWStream{(write_dir + "/" + name + ".png").c_str()}
+                    .write(png->data(), png->size());
             }
             return Status::OK;
         });
@@ -348,16 +346,14 @@ int main(int argc, char** argv) {
 }
 
 
-Register::Register(const char* name,
-                   std::unique_ptr<Stream> (*factory)(Options)) {
+Register::Register(const char* name, std::unique_ptr<Stream> (*factory)(Options)) {
     stream_types.push_back(StreamType{name, factory});
 }
-Register::Register(const char* name,
-                   std::unique_ptr<Dst> (*factory)(Options, SkISize)) {
+Register::Register(const char* name, std::unique_ptr<Dst> (*factory)(Options)) {
     dst_types.push_back(DstType{name, factory});
 }
 Register::Register(const char* name,
-                   std::unique_ptr<Dst> (*factory)(Options, SkISize, std::unique_ptr<Dst>)) {
+                   std::unique_ptr<Dst> (*factory)(Options, std::unique_ptr<Dst>)) {
     via_types.push_back(ViaType{name, factory});
 }
 

diff --git a/tools/ok.h b/tools/ok.h
index 99a5ca0..e2a6a34 100644 (file)
--- a/tools/ok.h
+++ b/tools/ok.h
@@ -22,9 +22,9 @@ static std::unique_ptr<T> move_unique(T& v) {
 
 struct Src {
     virtual ~Src() {}
-    virtual std::string name()   = 0;  // ok always calls Src methods in order:
-    virtual SkISize     size()   = 0;  // name() -> size() -> draw(), possibly
-    virtual void draw(SkCanvas*) = 0;  // stopping after calling name().
+    virtual std::string name()   = 0;
+    virtual SkISize     size()   = 0;
+    virtual bool draw(SkCanvas*) = 0;
 };
 
 struct Stream {
@@ -34,8 +34,8 @@ struct Stream {
 
 struct Dst {
     virtual ~Dst() {}
-    virtual SkCanvas* canvas()                  = 0;
-    virtual void write(std::string path_prefix) = 0;  // All but the file extension.
+    virtual bool draw(Src*)        = 0;
+    virtual sk_sp<SkImage> image() = 0;
 };
 
 class Options {
@@ -47,12 +47,9 @@ public:
 
 // Create globals to register your new type of Stream or Dst.
 struct Register {
-    Register(const char* name,
-             std::unique_ptr<Stream> (*factory)(Options));
-    Register(const char* name,
-             std::unique_ptr<Dst> (*factory)(Options, SkISize));
-    Register(const char* name,
-             std::unique_ptr<Dst> (*factory)(Options, SkISize, std::unique_ptr<Dst>));
+    Register(const char* name, std::unique_ptr<Stream> (*factory)(Options));
+    Register(const char* name, std::unique_ptr<Dst>    (*factory)(Options));
+    Register(const char* name, std::unique_ptr<Dst>    (*factory)(Options, std::unique_ptr<Dst>));
 };
 
 #endif//ok_DEFINED

diff --git a/tools/ok_dsts.cpp b/tools/ok_dsts.cpp
index 41dfdcf..cc47ac4 100644 (file)
--- a/tools/ok_dsts.cpp
+++ b/tools/ok_dsts.cpp
@@ -9,25 +9,27 @@
 #include "SkSurface.h"
 
 struct SWDst : Dst {
+    SkImageInfo      info;
     sk_sp<SkSurface> surface;
 
-    static std::unique_ptr<Dst> Create(Options options, SkISize size) {
-        SkImageInfo info = SkImageInfo::MakeN32Premul(size.width(), size.height());
+    static std::unique_ptr<Dst> Create(Options options) {
+        SkImageInfo info = SkImageInfo::MakeN32Premul(0,0);
         if (options("ct") == "565") { info = info.makeColorType(kRGB_565_SkColorType); }
         if (options("ct") == "f16") { info = info.makeColorType(kRGBA_F16_SkColorType); }
+
         SWDst dst;
-        dst.surface = SkSurface::MakeRaster(info);
+        dst.info = info;
         return move_unique(dst);
     }
 
-    SkCanvas* canvas() override {
-        return surface->getCanvas();
+    bool draw(Src* src) override {
+        auto size = src->size();
+        surface = SkSurface::MakeRaster(info.makeWH(size.width(), size.height()));
+        return src->draw(surface->getCanvas());
     }
 
-    void write(std::string path_prefix) override {
-        auto image = surface->makeImageSnapshot();
-        sk_sp<SkData> png{image->encode()};
-        SkFILEWStream{(path_prefix + ".png").c_str()}.write(png->data(), png->size());
+    sk_sp<SkImage> image() override {
+        return surface->makeImageSnapshot();
     }
 };
 static Register sw{"sw", SWDst::Create};

diff --git a/tools/ok_srcs.cpp b/tools/ok_srcs.cpp
index ce3e653..a7d262c 100644 (file)
--- a/tools/ok_srcs.cpp
+++ b/tools/ok_srcs.cpp
@@ -23,19 +23,27 @@ struct GMStream : Stream {
         skiagm::GM* (*factory)(void*);
         std::unique_ptr<skiagm::GM> gm;
 
-        std::string name() override {
+        void init() {
+            if (gm) { return; }
             gm.reset(factory(nullptr));
+        }
+
+        std::string name() override {
+            this->init();
             return gm->getName();
         }
 
         SkISize size() override {
+            this->init();
             return gm->getISize();
         }
 
-        void draw(SkCanvas* canvas) override {
+        bool draw(SkCanvas* canvas) override {
+            this->init();
             canvas->clear(0xffffffff);
             canvas->concat(gm->getInitialTransform());
             gm->draw(canvas);
+            return true;
         }
     };
 
@@ -69,19 +77,26 @@ struct SKPStream : Stream {
         std::string dir, path;
         sk_sp<SkPicture> pic;
 
+        void init() {
+            if (pic) { return; }
+            auto skp = SkData::MakeFromFileName((dir+"/"+path).c_str());
+            pic = SkPicture::MakeFromData(skp.get());
+        }
+
         std::string name() override {
             return path;
         }
 
         SkISize size() override {
-            auto skp = SkData::MakeFromFileName((dir+"/"+path).c_str());
-            pic = SkPicture::MakeFromData(skp.get());
+            this->init();
             return pic->cullRect().roundOut().size();
         }
 
-        void draw(SkCanvas* canvas) override {
+        bool draw(SkCanvas* canvas) override {
+            this->init();
             canvas->clear(0xffffffff);
             pic->playback(canvas);
+            return true;
         }
     };
 

diff --git a/tools/ok_vias.cpp b/tools/ok_vias.cpp
index f13bcc1..b273d3c 100644 (file)
--- a/tools/ok_vias.cpp
+++ b/tools/ok_vias.cpp
@@ -8,30 +8,50 @@
 #include "ok.h"
 #include "SkPictureRecorder.h"
 
+static std::unique_ptr<Src> proxy(Src* original, std::function<bool(SkCanvas*)> fn) {
+    struct : Src {
+        Src*                           original;
+        std::function<bool(SkCanvas*)> fn;
+
+        std::string name() override { return original->name(); }
+        SkISize     size() override { return original->size(); }
+        bool draw(SkCanvas* canvas) override { return fn(canvas); }
+    } src;
+    src.original = original;
+    src.fn       = fn;
+    return move_unique(src);
+}
+
 struct ViaPic : Dst {
     std::unique_ptr<Dst> target;
-    SkPictureRecorder    rec;
+    bool                 rtree = false;
 
-    static std::unique_ptr<Dst> Create(Options options, SkISize size, std::unique_ptr<Dst> dst) {
-        SkBBHFactory* bbh = nullptr;
-        SkRTreeFactory rtree;
+    static std::unique_ptr<Dst> Create(Options options, std::unique_ptr<Dst> dst) {
+        ViaPic via;
+        via.target = std::move(dst);
+        if (options("bbh") == "rtree") { via.rtree = true; }
+        return move_unique(via);
+    }
 
-        if (options("bbh") == "rtree") { bbh = &rtree; }
+    bool draw(Src* src) override {
+        SkRTreeFactory factory;
+        SkPictureRecorder rec;
+        rec.beginRecording(SkRect::MakeSize(SkSize::Make(src->size())),
+                           rtree ? &factory : nullptr);
 
-        auto via = std::unique_ptr<ViaPic>(new ViaPic);
-        via->target = std::move(dst);
-        via->rec.beginRecording(SkRect::MakeSize(SkSize::Make(size)), bbh);
-        return std::move(via);
-    }
+        if (!src->draw(rec.getRecordingCanvas())) {
+            return false;
+        }
+        auto pic = rec.finishRecordingAsPicture();
 
-    SkCanvas* canvas() override {
-        return rec.getRecordingCanvas();
+        return target->draw(proxy(src, [=](SkCanvas* canvas) {
+            pic->playback(canvas);
+            return true;
+        }).get());
     }
 
-    void write(std::string path_prefix) override {
-        auto pic = rec.finishRecordingAsPicture();
-        pic->playback(target->canvas());
-        target->write(path_prefix);
+    sk_sp<SkImage> image() override {
+        return target->image();
     }
 };
 static Register via_pic{"via_pic", ViaPic::Create};