small tidy of benchmarkstream

BUG=skia:

Committed: https://skia.googlesource.com/skia/+/691b6af907e55250a29a7a2a346b63c2026011c3

Review URL: https://codereview.chromium.org/1395703002

7 files changed, 84 insertions, 59 deletions

diff --git a/tools/VisualBench/CpuWrappedBenchmark.h b/tools/VisualBench/CpuWrappedBenchmark.h
index e63187cca4..214ed00e81 100644
--- a/tools/VisualBench/CpuWrappedBenchmark.h
+++ b/tools/VisualBench/CpuWrappedBenchmark.h
@@ -20,6 +20,12 @@ public:
     const char* onGetName()       override { return fBench->getName(); }
     const char* onGetUniqueName() override { return fBench->getUniqueName(); }
 
+    void onDelayedSetup() override { fBench->delayedSetup(); }
+    void onPerCanvasPreDraw(SkCanvas* canvas) override { fBench->perCanvasPreDraw(canvas); }
+    void onPreDraw(SkCanvas* canvas) override { fBench->preDraw(canvas); }
+    void onPostDraw(SkCanvas* canvas) override { fBench->postDraw(canvas); }
+    void onPerCanvasPostDraw(SkCanvas* canvas) override { fBench->perCanvasPostDraw(canvas); }
+
     void onDraw(int loops, SkCanvas* canvas) override {
         // TODO: use onPreDraw() to move offscreen allocation/deallocation out of timing.
         SkAutoTUnref<SkSurface> offscreen(SkSurface::NewRaster(canvas->imageInfo()));
@@ -29,6 +35,8 @@ public:
         canvas->drawImage(image, 0,0);
     }
 
+    virtual SkIPoint onGetSize() override { return fBench->getSize(); }
+
 private:
     SkAutoTUnref<Benchmark> fBench;
 };
diff --git a/tools/VisualBench/TimingStateMachine.cpp b/tools/VisualBench/TimingStateMachine.cpp
index ae03f5d188..85aab42ba9 100644
--- a/tools/VisualBench/TimingStateMachine.cpp
+++ b/tools/VisualBench/TimingStateMachine.cpp
@@ -91,9 +91,7 @@ void TimingStateMachine::recordMeasurement() {
     fLastMeasurement = this->elapsed() / (FLAGS_frames * fLoops);
 }
 
-void TimingStateMachine::nextBenchmark(SkCanvas* canvas, Benchmark* benchmark) {
-    benchmark->postDraw(canvas);
-    benchmark->perCanvasPostDraw(canvas);
+void TimingStateMachine::nextBenchmark() {
     fLoops = 1;
     fInnerState = kTuning_InnerState;
     fState = kPreWarm_State;
diff --git a/tools/VisualBench/TimingStateMachine.h b/tools/VisualBench/TimingStateMachine.h
index 22150599ab..3f40b6d31f 100644
--- a/tools/VisualBench/TimingStateMachine.h
+++ b/tools/VisualBench/TimingStateMachine.h
@@ -38,10 +38,9 @@ public:
     ParentEvents nextFrame(bool preWarmBetweenSamples);
 
     /*
-     * The caller should call this when they are ready to move to the next benchmark.  The caller
-     * must call this with the *last* benchmark so post draw hooks can be invoked
+     * The caller should call this when they are ready to move to the next benchmark.
      */
-    void nextBenchmark(SkCanvas*, Benchmark*);
+    void nextBenchmark();
 
     /*
      * When TimingStateMachine returns kTimingFinished_ParentEvents, then the owner can call
diff --git a/tools/VisualBench/VisualBenchmarkStream.cpp b/tools/VisualBench/VisualBenchmarkStream.cpp
index c520eeed05..235200a761 100644
--- a/tools/VisualBench/VisualBenchmarkStream.cpp
+++ b/tools/VisualBench/VisualBenchmarkStream.cpp
@@ -74,6 +74,11 @@ VisualBenchmarkStream::VisualBenchmarkStream()
             }
         }
     }
+
+    // seed with an initial benchmark
+    // NOTE the initial benchmark will not have preTimingHooks called, but that is okay because
+    // it is the warmupbench
+    this->next();
 }
 
 bool VisualBenchmarkStream::ReadPicture(const char* path, SkAutoTUnref<SkPicture>* pic) {
@@ -98,23 +103,24 @@ bool VisualBenchmarkStream::ReadPicture(const char* path, SkAutoTUnref<SkPicture
 }
 
 Benchmark* VisualBenchmarkStream::next() {
+    Benchmark* bench;
     if (!fIsWarmedUp) {
         fIsWarmedUp = true;
-        return new WarmupBench;
-    }
-
-    Benchmark* bench;
-
-    // skips non matching benches
-    while ((bench = this->innerNext()) &&
-           (SkCommandLineFlags::ShouldSkip(FLAGS_match, bench->getUniqueName()) ||
-            !bench->isSuitableFor(Benchmark::kGPU_Backend))) {
-        bench->unref();
+        bench = new WarmupBench;
+    } else {
+        // skips non matching benches
+        while ((bench = this->innerNext()) &&
+               (SkCommandLineFlags::ShouldSkip(FLAGS_match, bench->getUniqueName()) ||
+                !bench->isSuitableFor(Benchmark::kGPU_Backend))) {
+            bench->unref();
+        }
     }
-    if (FLAGS_cpu) {
-        return new CpuWrappedBenchmark(bench);
+    if (bench && FLAGS_cpu) {
+        bench = new CpuWrappedBenchmark(bench);
     }
-    return bench;
+
+    fBenchmark.reset(bench);
+    return fBenchmark;
 }
 
 Benchmark* VisualBenchmarkStream::innerNext() {
diff --git a/tools/VisualBench/VisualBenchmarkStream.h b/tools/VisualBench/VisualBenchmarkStream.h
index 9e5d4590cb..89ca632821 100644
--- a/tools/VisualBench/VisualBenchmarkStream.h
+++ b/tools/VisualBench/VisualBenchmarkStream.h
@@ -23,6 +23,7 @@ public:
     static bool ReadPicture(const char* path, SkAutoTUnref<SkPicture>* pic);
 
     Benchmark* next();
+    Benchmark* current() { return fBenchmark.get(); }
 
 private:
     Benchmark* innerNext();
@@ -30,6 +31,7 @@ private:
     const BenchRegistry* fBenches;
     const skiagm::GMRegistry* fGMs;
     SkTArray<SkString> fSKPs;
+    SkAutoTUnref<Benchmark> fBenchmark;
 
     const char* fSourceType;  // What we're benching: bench, GM, SKP, ...
     const char* fBenchType;   // How we bench it: micro, playback, ...
diff --git a/tools/VisualBench/VisualStreamTimingModule.cpp b/tools/VisualBench/VisualStreamTimingModule.cpp
index db75890d11..0a57e67221 100644
--- a/tools/VisualBench/VisualStreamTimingModule.cpp
+++ b/tools/VisualBench/VisualStreamTimingModule.cpp
@@ -10,61 +10,66 @@
 #include "SkCanvas.h"
 
 VisualStreamTimingModule::VisualStreamTimingModule(VisualBench* owner, bool preWarmBeforeSample)
-    : fReinitializeBenchmark(false)
+    : fInitState(kReset_InitState)
     , fPreWarmBeforeSample(preWarmBeforeSample)
     , fOwner(owner) {
     fBenchmarkStream.reset(new VisualBenchmarkStream);
 }
 
-bool VisualStreamTimingModule::nextBenchmarkIfNecessary(SkCanvas* canvas) {
-    if (fBenchmark) {
-        return true;
-    }
-
-    fBenchmark.reset(fBenchmarkStream->next());
-    if (!fBenchmark) {
-        return false;
+inline void VisualStreamTimingModule::handleInitState(SkCanvas* canvas) {
+    switch (fInitState) {
+        case kNewBenchmark_InitState:
+            fOwner->clear(canvas, SK_ColorWHITE, 2);
+            fBenchmarkStream->current()->delayedSetup();
+            // fallthrough
+        case kReset_InitState:
+            fBenchmarkStream->current()->preTimingHooks(canvas);
+            break;
+        case kNone_InitState:
+            break;
     }
-
-    fOwner->clear(canvas, SK_ColorWHITE, 2);
-
-    fBenchmark->delayedSetup();
-    fBenchmark->preTimingHooks(canvas);
-    return true;
+    fInitState = kNone_InitState;
 }
 
-void VisualStreamTimingModule::draw(SkCanvas* canvas) {
-    if (!this->nextBenchmarkIfNecessary(canvas)) {
-        SkDebugf("Exiting VisualBench successfully\n");
-        fOwner->closeWindow();
-        return;
-    }
-
-    if (fReinitializeBenchmark) {
-        fReinitializeBenchmark = false;
-        fBenchmark->preTimingHooks(canvas);
-    }
-
-    this->renderFrame(canvas, fBenchmark, fTSM.loops());
-    fOwner->present();
-    TimingStateMachine::ParentEvents event = fTSM.nextFrame(fPreWarmBeforeSample);
+inline void VisualStreamTimingModule::handleTimingEvent(SkCanvas* canvas,
+                                                        TimingStateMachine::ParentEvents event) {
     switch (event) {
         case TimingStateMachine::kReset_ParentEvents:
-            fBenchmark->postTimingHooks(canvas);
+            fBenchmarkStream->current()->postTimingHooks(canvas);
             fOwner->reset();
-            fReinitializeBenchmark = true;
+            fInitState = kReset_InitState;
             break;
         case TimingStateMachine::kTiming_ParentEvents:
             break;
         case TimingStateMachine::kTimingFinished_ParentEvents:
-            fBenchmark->postTimingHooks(canvas);
+            fBenchmarkStream->current()->postTimingHooks(canvas);
             fOwner->reset();
-            if (this->timingFinished(fBenchmark, fTSM.loops(), fTSM.lastMeasurement())) {
-                fTSM.nextBenchmark(canvas, fBenchmark);
-                fBenchmark.reset(nullptr);
+            if (this->timingFinished(fBenchmarkStream->current(), fTSM.loops(),
+                                     fTSM.lastMeasurement())) {
+                fTSM.nextBenchmark();
+                if (!fBenchmarkStream->next()) {
+                    SkDebugf("Exiting VisualBench successfully\n");
+                    fOwner->closeWindow();
+                } else {
+                    fInitState = kNewBenchmark_InitState;
+                }
             } else {
-                fReinitializeBenchmark = true;
+                fInitState = kReset_InitState;
             }
             break;
     }
 }
+
+void VisualStreamTimingModule::draw(SkCanvas* canvas) {
+    if (!fBenchmarkStream->current()) {
+        // this should never happen but just to be safe
+        // TODO research why this does happen on mac
+        return;
+    }
+
+    this->handleInitState(canvas);
+    this->renderFrame(canvas, fBenchmarkStream->current(), fTSM.loops());
+    fOwner->present();
+    TimingStateMachine::ParentEvents event = fTSM.nextFrame(fPreWarmBeforeSample);
+    this->handleTimingEvent(canvas, event);
+}
diff --git a/tools/VisualBench/VisualStreamTimingModule.h b/tools/VisualBench/VisualStreamTimingModule.h
index ac06ed4307..5dbec693fa 100644
--- a/tools/VisualBench/VisualStreamTimingModule.h
+++ b/tools/VisualBench/VisualStreamTimingModule.h
@@ -14,6 +14,8 @@
 #include "VisualBench.h"
 #include "VisualBenchmarkStream.h"
 
+class SkCanvas;
+
 /*
  * VisualStreamTimingModule is the base class for modules which want to time a stream of Benchmarks.
  *
@@ -31,12 +33,17 @@ private:
     // subclasses should return true to advance the stream
     virtual bool timingFinished(Benchmark*, int loops, double measurement)=0;
 
-    bool nextBenchmarkIfNecessary(SkCanvas*);
+    inline void handleInitState(SkCanvas*);
+    inline void handleTimingEvent(SkCanvas*, TimingStateMachine::ParentEvents);
 
     TimingStateMachine fTSM;
     SkAutoTDelete<VisualBenchmarkStream> fBenchmarkStream;
-    SkAutoTUnref<Benchmark> fBenchmark;
-    bool fReinitializeBenchmark;
+    enum InitState {
+        kNone_InitState,
+        kReset_InitState,
+        kNewBenchmark_InitState,
+    };
+    InitState fInitState;
     bool fPreWarmBeforeSample;
 
     // support framework