From 4ac36f80beb958c77a92a3e1a235f6ed9daaa510 Mon Sep 17 00:00:00 2001
From: Chris Craik <ccraik@google.com>
Date: Tue, 9 Dec 2014 16:54:03 -0800
Subject: [PATCH] Fix frame-allocated path lifecycles

bug:18667472

Previously, we were allocating per-frame temporary paths within the
PlaybackStateStruct, but these are not safe as layers allocate these
transiently. Instead, move these to the OpenGLRenderer, which has
better define lifecycle.

Additionally, don't store SkPath objects directly in vector, since
they are then subject to relocation.

Change-Id: I8187ef542fcd5b030502bb75eb123ee26c0daa96
---
 libs/hwui/AmbientShadow.cpp  |  1 +
 libs/hwui/DisplayList.h      | 12 ++++--------
 libs/hwui/OpenGLRenderer.cpp |  5 +++++
 libs/hwui/OpenGLRenderer.h   |  9 +++++++++
 4 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/libs/hwui/AmbientShadow.cpp b/libs/hwui/AmbientShadow.cpp
index 21c869b685723..b2dba002f3c8b 100644
--- a/libs/hwui/AmbientShadow.cpp
+++ b/libs/hwui/AmbientShadow.cpp
@@ -325,6 +325,7 @@ void AmbientShadow::createAmbientShadow(bool isCasterOpaque,
     // At the end, update the real index and vertex buffer size.
     shadowVertexBuffer.updateVertexCount(vertexBufferIndex);
     shadowVertexBuffer.updateIndexCount(indexBufferIndex);
+    shadowVertexBuffer.computeBounds<AlphaVertex>();
 
     ShadowTessellator::checkOverflow(vertexBufferIndex, totalVertexCount, "Ambient Vertex Buffer");
     ShadowTessellator::checkOverflow(indexBufferIndex, totalIndexCount, "Ambient Index Buffer");
diff --git a/libs/hwui/DisplayList.h b/libs/hwui/DisplayList.h
index cb8a8d1cbc880..7a43a2a989156 100644
--- a/libs/hwui/DisplayList.h
+++ b/libs/hwui/DisplayList.h
@@ -77,18 +77,14 @@ public:
     OpenGLRenderer& mRenderer;
     const int mReplayFlags;
 
-    // Allocator with the lifetime of a single frame.
-    // replay uses an Allocator owned by the struct, while defer shares the DeferredDisplayList's Allocator
+    // Allocator with the lifetime of a single frame. replay uses an Allocator owned by the struct,
+    // while defer shares the DeferredDisplayList's Allocator
+    // TODO: move this allocator to be owned by object with clear frame lifecycle
     LinearAllocator * const mAllocator;
 
     SkPath* allocPathForFrame() {
-        mTempPaths.push_back();
-        return &mTempPaths.back();
+        return mRenderer.allocPathForFrame();
     }
-
-private:
-    // Paths kept alive for the duration of the frame
-    std::vector<SkPath> mTempPaths;
 };
 
 class DeferStateStruct : public PlaybackStateStruct {
diff --git a/libs/hwui/OpenGLRenderer.cpp b/libs/hwui/OpenGLRenderer.cpp
index 075f2c509b24b..96257e4b474ab 100755
--- a/libs/hwui/OpenGLRenderer.cpp
+++ b/libs/hwui/OpenGLRenderer.cpp
@@ -311,6 +311,11 @@ void OpenGLRenderer::finish() {
     renderOverdraw();
     endTiling();
 
+    for (size_t i = 0; i < mTempPaths.size(); i++) {
+        delete mTempPaths[i];
+    }
+    mTempPaths.clear();
+
     // When finish() is invoked on FBO 0 we've reached the end
     // of the current frame
     if (getTargetFbo() == 0) {
diff --git a/libs/hwui/OpenGLRenderer.h b/libs/hwui/OpenGLRenderer.h
index e1c3d10fc9c9b..5eee2e2318afa 100755
--- a/libs/hwui/OpenGLRenderer.h
+++ b/libs/hwui/OpenGLRenderer.h
@@ -342,6 +342,12 @@ public:
     uint8_t getAmbientShadowAlpha() const { return mAmbientShadowAlpha; }
     uint8_t getSpotShadowAlpha() const { return mSpotShadowAlpha; }
 
+    SkPath* allocPathForFrame() {
+        SkPath* path = new SkPath();
+        mTempPaths.push_back(path);
+        return path;
+    }
+
 protected:
     /**
      * Perform the setup specific to a frame. This method does not
@@ -1014,6 +1020,9 @@ private:
     uint8_t mAmbientShadowAlpha;
     uint8_t mSpotShadowAlpha;
 
+    // Paths kept alive for the duration of the frame
+    std::vector<SkPath*> mTempPaths;
+
     friend class Layer;
     friend class TextSetupFunctor;
     friend class DrawBitmapOp;