From a3b8c67ea6e542aecf40d46ce9be45d50ef89660 Mon Sep 17 00:00:00 2001 From: joshualitt Date: Wed, 14 Oct 2015 14:45:07 -0700 Subject: Revert of small tidy of benchmarkstream (patchset #2 id:20001 of https://codereview.chromium.org/1395703002/ ) Reason for revert: Breaks visualbench Original issue's description: > small tidy of benchmarkstream > > BUG=skia: > > Committed: https://skia.googlesource.com/skia/+/691b6af907e55250a29a7a2a346b63c2026011c3 TBR=robertphillips@google.com,joshualitt@chromium.org NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=skia: Review URL: https://codereview.chromium.org/1392833006 --- tools/VisualBench/TimingStateMachine.cpp | 4 ++- tools/VisualBench/TimingStateMachine.h | 5 +-- tools/VisualBench/VisualBenchmarkStream.cpp | 30 ++++++++--------- tools/VisualBench/VisualBenchmarkStream.h | 2 -- tools/VisualBench/VisualStreamTimingModule.cpp | 45 +++++++++++++++----------- tools/VisualBench/VisualStreamTimingModule.h | 3 ++ 6 files changed, 49 insertions(+), 40 deletions(-) diff --git a/tools/VisualBench/TimingStateMachine.cpp b/tools/VisualBench/TimingStateMachine.cpp index 85aab42ba9..ae03f5d188 100644 --- a/tools/VisualBench/TimingStateMachine.cpp +++ b/tools/VisualBench/TimingStateMachine.cpp @@ -91,7 +91,9 @@ void TimingStateMachine::recordMeasurement() { fLastMeasurement = this->elapsed() / (FLAGS_frames * fLoops); } -void TimingStateMachine::nextBenchmark() { +void TimingStateMachine::nextBenchmark(SkCanvas* canvas, Benchmark* benchmark) { + benchmark->postDraw(canvas); + benchmark->perCanvasPostDraw(canvas); fLoops = 1; fInnerState = kTuning_InnerState; fState = kPreWarm_State; diff --git a/tools/VisualBench/TimingStateMachine.h b/tools/VisualBench/TimingStateMachine.h index 3f40b6d31f..22150599ab 100644 --- a/tools/VisualBench/TimingStateMachine.h +++ b/tools/VisualBench/TimingStateMachine.h @@ -38,9 +38,10 @@ public: ParentEvents nextFrame(bool preWarmBetweenSamples); /* - * The caller should call this when they are ready to move to the next benchmark. + * 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 */ - void nextBenchmark(); + void nextBenchmark(SkCanvas*, Benchmark*); /* * When TimingStateMachine returns kTimingFinished_ParentEvents, then the owner can call diff --git a/tools/VisualBench/VisualBenchmarkStream.cpp b/tools/VisualBench/VisualBenchmarkStream.cpp index 49bafbfcee..c520eeed05 100644 --- a/tools/VisualBench/VisualBenchmarkStream.cpp +++ b/tools/VisualBench/VisualBenchmarkStream.cpp @@ -74,9 +74,6 @@ VisualBenchmarkStream::VisualBenchmarkStream() } } } - - // seed with an initial benchmark - this->next(); } bool VisualBenchmarkStream::ReadPicture(const char* path, SkAutoTUnref* pic) { @@ -101,24 +98,23 @@ bool VisualBenchmarkStream::ReadPicture(const char* path, SkAutoTUnrefinnerNext()) && - (SkCommandLineFlags::ShouldSkip(FLAGS_match, bench->getUniqueName()) || - !bench->isSuitableFor(Benchmark::kGPU_Backend))) { - bench->unref(); - } - } - if (bench && FLAGS_cpu) { - bench = new CpuWrappedBenchmark(bench); + return new WarmupBench; } - fBenchmark.reset(bench); - return fBenchmark; + Benchmark* bench; + + // 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); + } + return bench; } Benchmark* VisualBenchmarkStream::innerNext() { diff --git a/tools/VisualBench/VisualBenchmarkStream.h b/tools/VisualBench/VisualBenchmarkStream.h index 89ca632821..9e5d4590cb 100644 --- a/tools/VisualBench/VisualBenchmarkStream.h +++ b/tools/VisualBench/VisualBenchmarkStream.h @@ -23,7 +23,6 @@ public: static bool ReadPicture(const char* path, SkAutoTUnref* pic); Benchmark* next(); - Benchmark* current() { return fBenchmark.get(); } private: Benchmark* innerNext(); @@ -31,7 +30,6 @@ private: const BenchRegistry* fBenches; const skiagm::GMRegistry* fGMs; SkTArray fSKPs; - SkAutoTUnref 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 4148282ce6..db75890d11 100644 --- a/tools/VisualBench/VisualStreamTimingModule.cpp +++ b/tools/VisualBench/VisualStreamTimingModule.cpp @@ -16,43 +16,52 @@ VisualStreamTimingModule::VisualStreamTimingModule(VisualBench* owner, bool preW fBenchmarkStream.reset(new VisualBenchmarkStream); } +bool VisualStreamTimingModule::nextBenchmarkIfNecessary(SkCanvas* canvas) { + if (fBenchmark) { + return true; + } + + fBenchmark.reset(fBenchmarkStream->next()); + if (!fBenchmark) { + return false; + } + + fOwner->clear(canvas, SK_ColorWHITE, 2); + + fBenchmark->delayedSetup(); + fBenchmark->preTimingHooks(canvas); + return true; +} + void VisualStreamTimingModule::draw(SkCanvas* canvas) { - if (!fBenchmarkStream->current()) { - // this should never happen but just to be safe + if (!this->nextBenchmarkIfNecessary(canvas)) { + SkDebugf("Exiting VisualBench successfully\n"); + fOwner->closeWindow(); return; } if (fReinitializeBenchmark) { fReinitializeBenchmark = false; - fBenchmarkStream->current()->preTimingHooks(canvas); + fBenchmark->preTimingHooks(canvas); } - this->renderFrame(canvas, fBenchmarkStream->current(), fTSM.loops()); + this->renderFrame(canvas, fBenchmark, fTSM.loops()); fOwner->present(); TimingStateMachine::ParentEvents event = fTSM.nextFrame(fPreWarmBeforeSample); switch (event) { case TimingStateMachine::kReset_ParentEvents: - fBenchmarkStream->current()->postTimingHooks(canvas); + fBenchmark->postTimingHooks(canvas); fOwner->reset(); fReinitializeBenchmark = true; break; case TimingStateMachine::kTiming_ParentEvents: break; case TimingStateMachine::kTimingFinished_ParentEvents: - fBenchmarkStream->current()->postTimingHooks(canvas); + fBenchmark->postTimingHooks(canvas); fOwner->reset(); - if (this->timingFinished(fBenchmarkStream->current(), fTSM.loops(), - fTSM.lastMeasurement())) { - fTSM.nextBenchmark(); - if (!fBenchmarkStream->next()) { - SkDebugf("Exiting VisualBench successfully\n"); - fOwner->closeWindow(); - } else { - fOwner->clear(canvas, SK_ColorWHITE, 2); - - fBenchmarkStream->current()->delayedSetup(); - fBenchmarkStream->current()->preTimingHooks(canvas); - } + if (this->timingFinished(fBenchmark, fTSM.loops(), fTSM.lastMeasurement())) { + fTSM.nextBenchmark(canvas, fBenchmark); + fBenchmark.reset(nullptr); } else { fReinitializeBenchmark = true; } diff --git a/tools/VisualBench/VisualStreamTimingModule.h b/tools/VisualBench/VisualStreamTimingModule.h index b84f04f4bb..ac06ed4307 100644 --- a/tools/VisualBench/VisualStreamTimingModule.h +++ b/tools/VisualBench/VisualStreamTimingModule.h @@ -31,8 +31,11 @@ private: // subclasses should return true to advance the stream virtual bool timingFinished(Benchmark*, int loops, double measurement)=0; + bool nextBenchmarkIfNecessary(SkCanvas*); + TimingStateMachine fTSM; SkAutoTDelete fBenchmarkStream; + SkAutoTUnref fBenchmark; bool fReinitializeBenchmark; bool fPreWarmBeforeSample; -- cgit v1.2.3