Decouple Canvas from DisplayList and map Picture and PictureRecorder more directly to their Skia co…

sky/engine/core/painting/PictureRecorder.cpp

Index: sky/engine/core/painting/PictureRecorder.cpp
diff --git a/sky/engine/core/painting/PictureRecorder.cpp b/sky/engine/core/painting/PictureRecorder.cpp
index 4896215210088f05beabeffd410676e47a4f8b2b..a0933c4e8ff40542c8e7f61b46fbbb8682d4b791 100644
--- a/sky/engine/core/painting/PictureRecorder.cpp
+++ b/sky/engine/core/painting/PictureRecorder.cpp
@@ -3,31 +3,42 @@
 // found in the LICENSE file.
 
 #include "sky/engine/config.h"
+#include "sky/engine/core/painting/Canvas.h"
 #include "sky/engine/core/painting/PictureRecorder.h"
 
 #include "sky/engine/core/painting/Picture.h"
 
 namespace blink {
 
-PassRefPtr<PictureRecorder> PictureRecorder::create(double width, double height)
+PictureRecorder::PictureRecorder()
+    : m_pictureRecorder(adoptPtr(new SkPictureRecorder()))
 {
-    return adoptRef(new PictureRecorder(FloatSize(width, height)));
 }
 
-PictureRecorder::PictureRecorder(const FloatSize& size)
-    : Canvas(size)
+PictureRecorder::~PictureRecorder()
 {
 }
 
-PictureRecorder::~PictureRecorder()
+bool PictureRecorder::isRecording() {
+    return m_canvas != nullptr && m_canvas->isRecording();

[eseidel, 2015/06/23 00:41:32]

Hmmm. you might be right, it's possible chromium style requires comparison to
nullptr/0.  Back in WebKit/Blink we would ahve just written return m_canvas &&
m_canvas->isRecording(), but maybe this is the correct chromium (and now sky)
style?

[abarth-chromium, 2015/06/23 02:28:44]

Chromium style doesn't have that rule.  I suspect Ian might just be noticing
that we use "!= null" a lot in Dart code because the Dart language requires it.

[iansf, 2015/06/23 22:46:09]

Oh -- originally this was just a check of m_canvas, and I felt like the code was
made more clear with the explicit nullptr comparison after the return, but if
that's not the style, I'm happy to remove the explicit check here and in the
other spot.

+}
+
+SkCanvas* PictureRecorder::beginRecording(double width, double height)
 {
+    return m_pictureRecorder->beginRecording(width, height);
 }
 
 PassRefPtr<Picture> PictureRecorder::endRecording()
 {
     if (!isRecording())
         return nullptr;
-    return Picture::create(finishRecording());
+    auto picture = Picture::create(

[eseidel, 2015/06/23 00:41:32]

As much as I like Auto, I don't think this is what you want.  PassRefPtrs
generally are bad news on the stack.  You want to get out of a PassRefPtr into a
RefPtr as soon as possible to avoid the auto-zeroing behavior.

RefPtr<Picture> picture = Picture::create();

return picture.release();

Would be better and would have fewer accidental clear gotchas.

[iansf, 2015/06/23 22:46:09]

Thanks for the explanation!  Done.

+        adoptRef(m_pictureRecorder->endRecording()));
+    m_canvas->clearSkCanvas();
+    m_canvas = nullptr;
+    return picture;
 }
 
+void PictureRecorder::set_canvas(PassRefPtr<Canvas> canvas) { m_canvas = canvas; }
+
 } // namespace blink