First pass at splitting out SkPictureRecord from SkPicture

src/core/SkPictureRecorder.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.
  */
 
-// Need to include SkTypes first, so that SK_BUILD_FOR_ANDROID is defined.
-#include "SkTypes.h"
-#ifdef SK_BUILD_FOR_ANDROID
+#include "SkBBoxHierarchyRecord.h"
 #include "SkPicturePlayback.h"
-#endif
+#include "SkPictureRecord.h"
 #include "SkPictureRecorder.h"
+#include "SkTypes.h" 
 
 SkCanvas* SkPictureRecorder::beginRecording(int width, int height,
                                             SkBBHFactory* bbhFactory /* = NULL */,
                                             uint32_t recordFlags /* = 0 */) {
+    fCanvas.reset(NULL);

[mtklein, 2014/06/04 22:22:17]

You can probably drop this?  Either way, you're going to call fCanvas.reset()
below before the method's done.

[robertphillips, 2014/06/05 20:29:44]

This will eventually go away when SkPictureRecord doesn't need an SkPicture.
Right now a prior SkPictureRecord will have an (unreffed) pointer to the prior
picture.

     fPicture.reset(SkNEW(SkPicture));
-    return fPicture->beginRecording(width, height, bbhFactory, recordFlags);
+
+    fPicture->fWidth = width;
+    fPicture->fHeight = height;
+
+    const SkISize size = SkISize::Make(width, height);
+
+    SkPictureRecord* record = NULL;

[mtklein, 2014/06/04 22:22:17]

Might skip this and just call fCanvas.reset inside each branch of the if?

[robertphillips, 2014/06/05 20:29:44]

All this goofiness was b.c. SkPictureRecord.h is private while
SkPictureRecorder.h is public. Additionally, using SkAutoTUnref wants the full
type. I've switched to using a raw pointer (we we're getting much mileage out of
the AutoTUnref anyway) which makes the code simpler but incurs some extra care.

+
+    if (NULL != bbhFactory) {
+        SkAutoTUnref<SkBBoxHierarchy> tree((*bbhFactory)(width, height));
+        SkASSERT(NULL != tree);
+        record = SkNEW_ARGS(SkBBoxHierarchyRecord, (fPicture, size, recordFlags, tree.get()));
+    } else {
+        record = SkNEW_ARGS(SkPictureRecord, (fPicture, size, recordFlags));
+    }
+    fCanvas.reset(record);
+    record->beginRecording();
+
+    return fCanvas;
+}
+
+SkPicture* SkPictureRecorder::endRecording() {
+    if (NULL != fPicture.get()) {

[mtklein, 2014/06/04 22:22:17]

I'd probably invert this so the normal case is outside?

if (NULL == fPicture.get()) {
   return NULL;
}
SkASSERT(NULL == fPicture->fPlayback);
...
return fPicture.detach();

[robertphillips, 2014/06/05 20:29:44]

Done.

+        SkASSERT(NULL == fPicture->fPlayback);
+        SkASSERT(NULL != fCanvas);
+
+        SkPictureRecord* record = static_cast<SkPictureRecord*>(fCanvas.get());

[mtklein, 2014/06/04 22:22:17]

Store fCanvas as SkAutoTUnref<SkPictureRecord>?

[robertphillips, 2014/06/05 20:29:44]

SkPictureRecord.h is private.

+        record->endRecording();
+
+        SkPictInfo info;
+        fPicture->createHeader(&info);
+        fPicture->fPlayback = SkNEW_ARGS(SkPicturePlayback, (fPicture, *record, info));

[mtklein, 2014/06/04 22:22:17]

In the future this goes away too right?  Sort of spooky for SkPictureRecorder to
fiddle around inside fPicture.

[robertphillips, 2014/06/05 20:29:44]

Right. At this point SkPictureRecorder should just call one of SkPicture's ctors
to create and return a new one.

+
+        fCanvas.reset(NULL);
+
+        return fPicture.detach();
+    }
+    return NULL;
+}
+
+void SkPictureRecorder::internalOnly_EnableOpts(bool enableOpts) {
+    if (NULL == fCanvas) {
+        return;
+    }
+
+    SkPictureRecord* record = static_cast<SkPictureRecord*>(fCanvas.get());
+    record->internalOnly_EnableOpts(enableOpts);
 }
 
 #ifdef SK_BUILD_FOR_ANDROID
 void SkPictureRecorder::partialReplay(SkCanvas* canvas) const {
     if (NULL == fPicture.get() || NULL == canvas) {
         // Not recording or nothing to replay into
         return;
     }
 
-    SkASSERT(NULL != fPicture->fRecord);
+    SkASSERT(NULL != fCanvas.get());
+    SkPictureRecord* record = static_cast<SkPictureRecord*>(fCanvas.get());
 
     SkAutoTDelete<SkPicturePlayback> playback(SkPicture::FakeEndRecording(fPicture.get(),
-                                                                          *fPicture->fRecord,
+                                                                          *record,
                                                                           false));
     playback->draw(*canvas, NULL);
 }
 #endif