Add the ability to select bbh type, and include quadtree support

tools/bench_record.cpp

 /*
  * Copyright 2014 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include "SkCommandLineFlags.h"
 #include "SkForceLinking.h"
 #include "SkGraphics.h"
 #include "SkOSFile.h"
 #include "SkPicture.h"
+#include "SkQuadTreePicture.h"
 #include "SkStream.h"
 #include "SkString.h"
 #include "SkTileGridPicture.h"
 #include "SkTime.h"
 
 __SK_FORCE_IMAGE_DECODER_LINKING;
 
 // Just reading all the SKPs takes about 2 seconds for me, which is the same as about 100 loops of
 // rerecording all the SKPs.  So we default to --loops=900, which makes ~90% of our time spent in
 // recording, and this should take ~20 seconds to run.
 
 DEFINE_string2(skps, r, "skps", "Directory containing SKPs to read and re-record.");
 DEFINE_int32(loops, 900, "Number of times to re-record each SKP.");
 DEFINE_int32(flags, SkPicture::kUsePathBoundsForClip_RecordingFlag, "RecordingFlags to use.");
 DEFINE_bool(endRecording, true, "If false, don't time SkPicture::endRecording()");
 DEFINE_int32(nullSize, 1000, "Pretend dimension of null source picture.");
-DEFINE_int32(tileGridSize, 0, "Set the tile grid size, if non zero switches to tile grid picture and enables kOptimizeForClippedPlayback_RecordingFlag.");
+DEFINE_int32(tileGridSize, 512, "Set the tile grid size. Has no effect if bbh is not set to tilegrid.");
+DEFINE_string(bbh, "bbh", "Turn on the bbh and select the type");

[tomhudson, 2014/03/03 15:00:03]

If I'm reading correctly, this sets the default value of bbh to "bbh"?

[mtklein, 2014/03/03 15:20:13]

Yeah, why default this to an invalid choice?

List the options in the help string so they show up in --help?

[iancottrell, 2014/03/03 16:03:47]

sorry, was meant to be the empty string so it defaults to none like before.

 
-static void bench_record(SkPicture* src, const char* name) {
+static bool bench_record(SkPicture* src, const char* name) {
     const SkMSec start = SkTime::GetMSecs();
     const int width  = src ? src->width()  : FLAGS_nullSize;
     const int height = src ? src->height() : FLAGS_nullSize;
 
     for (int i = 0; i < FLAGS_loops; i++) {
         SkAutoTUnref<SkPicture> dst;
         int recordingFlags = FLAGS_flags;
-        if (FLAGS_tileGridSize > 0) {
+        if (FLAGS_bbh.isEmpty()) {

[mtklein, 2014/03/03 15:20:13]

This is getting a little hard to digest.

Let's move the FLAGS_bbh validation logic to main just after
SkCommandLineFlags::Parse().  It seems weird to be doing it in here, and weirder
still to be doing it every time around the loop.

You might consider an approach like this:

typedef SkPicture* (*PictureFactory)();
static SkPicture* vanilla_factory() { return SkNEW(SkPicture); }
static SKPicture* rtree_factory() { ... }
...

static PictureFactory* parse_FLAGS_bbh() {
  if (FLAGS_bbh.isEmpty()) { return vanilla_factory; }
  else if (FLAGS_bbh.count != 1) { return NULL; }
  else if (...) { FLAGS_flags |= ...; return rtree_factory; }
  ....
  else { return NULL; }
}

...

int tool_main(int argc, char** argv) {
  SkCommandLineFlags::Parse(argc, argv);
  PictureFactory* factory = parse_FLAGS_bbh();
  if (!factory) { SkDebugf(...); exit(2); }

  bench_record(NULL, "NULL", factory);
  ...
}

[iancottrell, 2014/03/03 16:03:47]

Done, although it is a little more complicated as it has to be passed the
construction arguments.

BTW as an aside, I really don't like that we have to select a subclass of
SkPicture with a different constructor signature just to change the bbh
hierarchy implementation.
Seems like it should be possible to hide all this behind SkPicture itself.
I would like to see use take BbhType from gmmain and make it part of the
SkPicture, so you can just build an SkPicture and select what bbh it is going to
use.
It would also mean you could remove kOptimizeForClippedPlayback_RecordingFlag as
selecting kNone_BbhType gives you the same effect.
Would you be open to a cl that made such a change?

+            dst.reset(SkNEW(SkPicture));
+        } else if (FLAGS_bbh.count() !=1 ) {
+            SkDebugf("More than one bbh type specified.\n");
+            return false;
+        } else if (FLAGS_bbh.contains("rtree")) {
+            recordingFlags |= SkPicture::kOptimizeForClippedPlayback_RecordingFlag;
+            dst.reset(SkNEW(SkPicture));
+        } else if (FLAGS_bbh.contains("tilegrid")) {
+            recordingFlags |= SkPicture::kOptimizeForClippedPlayback_RecordingFlag;
             SkTileGridPicture::TileGridInfo info;
             info.fTileInterval.set(FLAGS_tileGridSize, FLAGS_tileGridSize);
             info.fMargin.setEmpty();
             info.fOffset.setZero();
             dst.reset(SkNEW_ARGS(SkTileGridPicture, (width, height, info)));
+        } else if (FLAGS_bbh.contains("quadtree")) {
             recordingFlags |= SkPicture::kOptimizeForClippedPlayback_RecordingFlag;
+            dst.reset(SkNEW_ARGS(SkQuadTreePicture, (SkIRect::MakeWH(width, height))));
         } else {
-            dst.reset(SkNEW(SkPicture));
+            SkDebugf("Invalid bbh type %s, must be one of rtree, tilegrid, quadtree.\n", FLAGS_bbh[0]);
+            return false;
         }
         SkCanvas* canvas = dst->beginRecording(width, height, recordingFlags);
         if (src) src->draw(canvas);
         if (FLAGS_endRecording) dst->endRecording();
     }
 
     const SkMSec elapsed = SkTime::GetMSecs() - start;
     const double msPerLoop = elapsed / (double)FLAGS_loops;
     printf("%.2g\t%s\n", msPerLoop, name);
+    return true;
 }
 
 int tool_main(int argc, char** argv);
 int tool_main(int argc, char** argv) {
     SkCommandLineFlags::Parse(argc, argv);
     SkAutoGraphics autoGraphics;
 
-    bench_record(NULL, "NULL");
+    if (!bench_record(NULL, "NULL")) {
+        return 1;
+    }
     if (FLAGS_skps.isEmpty()) return 0;
 
     SkOSFile::Iter it(FLAGS_skps[0], ".skp");
     SkString filename;
     bool failed = false;
     while (it.next(&filename)) {
         const SkString path = SkOSPath::SkPathJoin(FLAGS_skps[0], filename.c_str());
 
         SkAutoTUnref<SkStream> stream(SkStream::NewFromFile(path.c_str()));
         if (!stream) {
@@ skipping 10 matching lines @@
         bench_record(src, filename.c_str());
     }
     return failed ? 1 : 0;
 }
 
 #if !defined SK_BUILD_FOR_IOS
 int main(int argc, char * const argv[]) {
     return tool_main(argc, (char**) argv);
 }
 #endif