Modifications to DisplayList for better Slimming Paint

Source/platform/graphics/GraphicsContext.cpp

Index: Source/platform/graphics/GraphicsContext.cpp
diff --git a/Source/platform/graphics/GraphicsContext.cpp b/Source/platform/graphics/GraphicsContext.cpp
index a4aca79b256e33ae0af1ce6411d82da15a655122..9b967b7880f315c45faf4fc4cfa7262fddb98bad 100644
--- a/Source/platform/graphics/GraphicsContext.cpp
+++ b/Source/platform/graphics/GraphicsContext.cpp
@@ -44,6 +44,7 @@
 #include "third_party/skia/include/core/SkData.h"
 #include "third_party/skia/include/core/SkDevice.h"
 #include "third_party/skia/include/core/SkPicture.h"
+#include "third_party/skia/include/core/SkPictureRecorder.h"
 #include "third_party/skia/include/core/SkRRect.h"
 #include "third_party/skia/include/core/SkRefCnt.h"
 #include "third_party/skia/include/core/SkSurface.h"
@@ -99,15 +100,17 @@ struct GraphicsContext::CanvasSaveState {
 };
 
 struct GraphicsContext::RecordingState {
-    RecordingState(SkCanvas* currentCanvas, const SkMatrix& currentMatrix, PassRefPtr<DisplayList> displayList)
-        : m_savedCanvas(currentCanvas)
-        , m_displayList(displayList)
-        , m_savedMatrix(currentMatrix)
-    {
-    }
+    RecordingState(SkPictureRecorder* recorder, SkCanvas* currentCanvas, const SkMatrix& currentMatrix, PassRefPtr<DisplayList> displayList)
+        : m_displayList(displayList)
+        , m_recorder(recorder)
+        , m_savedCanvas(currentCanvas)
+        , m_savedMatrix(currentMatrix) { }
+
+    ~RecordingState() { }
 
-    SkCanvas* m_savedCanvas;
     RefPtr<DisplayList> m_displayList;
+    SkPictureRecorder* m_recorder;
+    SkCanvas* m_savedCanvas;
     const SkMatrix m_savedMatrix;
 };
 
@@ -492,16 +495,21 @@ void GraphicsContext::endLayer()
 #endif
 }
 
-void GraphicsContext::beginRecording(const FloatRect& bounds)
+void GraphicsContext::beginRecording(PassRefPtr<DisplayList> displayList, uint32_t recordFlags)
 {
-    RefPtr<DisplayList> displayList = adoptRef(new DisplayList(bounds));
+    RefPtr<DisplayList> pDisplayList = displayList;
+
+    pDisplayList->clearPicture();

[chrishtr, 2014/09/02 22:05:58]

What's the use case of beginning a recording of a display list that is not
created from scratch just before?
Are you just changing this around to make clear the ownership of the pointer?

If it's always from scratch, we can change beginRecording() to return a fresh
PasRefPtr<DisplayList> that the caller can keep.
This simplifies the DisplayList API.

[Stephen Chennney, 2014/09/03 14:19:37]

If the display list is invalidated we should be able to just re-record it
without creating another. Now that I think about it I don't think we take
advantage of this. I also want to clear the picture to maintain the invariant
stated in DisplayList: that the picture will be null when recording.
And that too.

[chrishtr, 2014/09/03 16:56:11]

What's the performance advantage of this though? There is a code correctness
downside risk to reusing.
If we don't take advantage of it, then beginRecording could return a new display
list rather than
using an existing one.

+    SkPictureRecorder* recorder = 0;
 
     SkCanvas* savedCanvas = m_canvas;
     SkMatrix savedMatrix = getTotalMatrix();
 
     if (!contextDisabled()) {
-        IntRect recordingRect = enclosingIntRect(bounds);
-        m_canvas = displayList->beginRecording(recordingRect.size());
+        FloatRect bounds = pDisplayList->bounds();
+        IntSize recordingSize = enclosingIntRect(bounds).size();
+        recorder = new SkPictureRecorder;
+        m_canvas = recorder->beginRecording(recordingSize.width(), recordingSize.height(), 0, recordFlags);
 
         // We want the bounds offset mapped to (0, 0), such that the display list content
         // is fully contained within the SkPictureRecord's bounds.
@@ -512,23 +520,20 @@ void GraphicsContext::beginRecording(const FloatRect& bounds)
         }
     }
 
-    m_recordingStateStack.append(RecordingState(savedCanvas, savedMatrix, displayList));
+    m_recordingStateStack.append(RecordingState(recorder, savedCanvas, savedMatrix, pDisplayList));
 }
 
-PassRefPtr<DisplayList> GraphicsContext::endRecording()
+void GraphicsContext::endRecording()
 {
     ASSERT(!m_recordingStateStack.isEmpty());
 
     RecordingState recording = m_recordingStateStack.last();
-    if (!contextDisabled()) {
-        ASSERT(recording.m_displayList->isRecording());
-        recording.m_displayList->endRecording();
-    }
+    if (!contextDisabled())
+        recording.m_displayList->setPicture(recording.m_recorder->endRecording());
 
-    m_recordingStateStack.removeLast();
     m_canvas = recording.m_savedCanvas;
-
-    return recording.m_displayList.release();
+    delete recording.m_recorder;
+    m_recordingStateStack.removeLast();
 }
 
 bool GraphicsContext::isRecording() const
@@ -539,21 +544,33 @@ bool GraphicsContext::isRecording() const
 void GraphicsContext::drawDisplayList(DisplayList* displayList)
 {
     ASSERT(displayList);
-    ASSERT(!displayList->isRecording());
 
     if (contextDisabled() || displayList->bounds().isEmpty())
         return;
 
+    bool performClip = !displayList->clip().isEmpty();
+    bool performTransform = !displayList->transform().isIdentity();
+    if (performClip || performTransform) {
+        save();
+        if (performTransform)
+            concat(displayList->transform());
+        if (performClip)
+            clipRect(displayList->clip());
+    }
+
     realizeCanvasSave();
 
-    const FloatRect& bounds = displayList->bounds();
-    if (bounds.x() || bounds.y()) {
+    const FloatPoint& location = displayList->bounds().location();
+    if (location.x() || location.y()) {
         SkMatrix m;
-        m.setTranslate(bounds.x(), bounds.y());
+        m.setTranslate(location.x(), location.y());
         m_canvas->drawPicture(displayList->picture(), &m, 0);
     } else {
         m_canvas->drawPicture(displayList->picture());
     }
+
+    if (performClip || performTransform)
+        restore();
 }
 
 void GraphicsContext::drawConvexPolygon(size_t numPoints, const FloatPoint* points, bool shouldAntialias)