commit 7466986d2055eb8711f36a85ac539b1572ffe805
author Chris Craik <ccraik@google.com> Thu Aug 07 17:27:30 2014 -0700
committer Chris Craik <ccraik@google.com> Fri Aug 08 00:52:54 2014 +0000
tree 6c560d37db0e9f709e8b3114c126423ce99488fc
parent f06009542390472872da986486d385001e91a2a7

Fix leak of SkPathRefs

bug:15939479

SkPath objects owned by DisplayListOps weren't being torn down, and
thus weren't releasing their SkPathRef innards.

Change-Id: I2581e124600a93a399ef3251f456c02ab52839a8

libs/hwui/DisplayList.h

diff --git a/libs/hwui/DisplayList.h b/libs/hwui/DisplayList.h
index 79a2f61..acfa98e 100644
--- a/libs/hwui/DisplayList.h
+++ b/libs/hwui/DisplayList.h
@@ -69,7 +69,9 @@
 class PlaybackStateStruct {
 protected:
     PlaybackStateStruct(OpenGLRenderer& renderer, int replayFlags, LinearAllocator* allocator)
-            : mRenderer(renderer), mReplayFlags(replayFlags), mAllocator(allocator){}
+            : mRenderer(renderer)
+            , mReplayFlags(replayFlags)
+            , mAllocator(allocator) {}
 
 public:
     OpenGLRenderer& mRenderer;
@@ -78,6 +80,15 @@
     // Allocator with the lifetime of a single frame.
     // replay uses an Allocator owned by the struct, while defer shares the DeferredDisplayList's Allocator
     LinearAllocator * const mAllocator;
+
+    SkPath* allocPathForFrame() {
+        mTempPaths.push_back();
+        return &mTempPaths.back();
+    }
+
+private:
+    // Paths kept alive for the duration of the frame
+    std::vector<SkPath> mTempPaths;
 };
 
 class DeferStateStruct : public PlaybackStateStruct {

libs/hwui/DisplayListOp.h

diff --git a/libs/hwui/DisplayListOp.h b/libs/hwui/DisplayListOp.h
index 6883cc5..c6d3db7 100644
--- a/libs/hwui/DisplayListOp.h
+++ b/libs/hwui/DisplayListOp.h
@@ -1505,20 +1505,18 @@
 class DrawShadowOp : public DrawOp {
 public:
     DrawShadowOp(const mat4& transformXY, const mat4& transformZ,
-            float casterAlpha, const SkPath* casterOutline, const SkPath* revealClip)
-            : DrawOp(NULL), mTransformXY(transformXY), mTransformZ(transformZ),
-            mCasterAlpha(casterAlpha) {
-        mOutline = *casterOutline;
-        if (revealClip) {
-            // intersect the outline with the convex reveal clip
-            Op(mOutline, *revealClip, kIntersect_PathOp, &mOutline);
-        }
+            float casterAlpha, const SkPath* casterOutline)
+        : DrawOp(NULL)
+        , mTransformXY(transformXY)
+        , mTransformZ(transformZ)
+        , mCasterAlpha(casterAlpha)
+        , mCasterOutline(casterOutline) {
     }
 
     virtual void onDefer(OpenGLRenderer& renderer, DeferInfo& deferInfo,
             const DeferredDisplayState& state) {
         renderer.getCaches().tessellationCache.precacheShadows(&state.mMatrix,
-                renderer.getLocalClipBounds(), isCasterOpaque(), &mOutline,
+                renderer.getLocalClipBounds(), isCasterOpaque(), mCasterOutline,
                 &mTransformXY, &mTransformZ, renderer.getLightCenter(), renderer.getLightRadius());
     }
 
@@ -1527,7 +1525,7 @@
         Matrix4 drawTransform;
         renderer.getMatrix(&drawTransform);
         renderer.getCaches().tessellationCache.getShadowBuffers(&drawTransform,
-                renderer.getLocalClipBounds(), isCasterOpaque(), &mOutline,
+                renderer.getLocalClipBounds(), isCasterOpaque(), mCasterOutline,
                 &mTransformXY, &mTransformZ, renderer.getLightCenter(), renderer.getLightRadius(),
                 buffers);
 
@@ -1546,7 +1544,7 @@
     const mat4 mTransformXY;
     const mat4 mTransformZ;
     const float mCasterAlpha;
-    SkPath mOutline;
+    const SkPath* mCasterOutline;
 };
 
 class DrawLayerOp : public DrawOp {

libs/hwui/RenderNode.cpp

diff --git a/libs/hwui/RenderNode.cpp b/libs/hwui/RenderNode.cpp
index f48b774..23940ee 100644
--- a/libs/hwui/RenderNode.cpp
+++ b/libs/hwui/RenderNode.cpp
@@ -534,6 +534,7 @@
     inline void endMark() {}
     inline int level() { return mLevel; }
     inline int replayFlags() { return mDeferStruct.mReplayFlags; }
+    inline SkPath* allocPathForFrame() { return mDeferStruct.allocPathForFrame(); }
 
 private:
     DeferStateStruct& mDeferStruct;
@@ -564,6 +565,7 @@
     }
     inline int level() { return mLevel; }
     inline int replayFlags() { return mReplayStruct.mReplayFlags; }
+    inline SkPath* allocPathForFrame() { return mReplayStruct.allocPathForFrame(); }
 
 private:
     ReplayStateStruct& mReplayStruct;
@@ -612,14 +614,24 @@
     mat4 shadowMatrixZ(transformFromParent);
     applyViewPropertyTransforms(shadowMatrixZ, true);
 
-    const SkPath* outlinePath = properties().getOutline().getPath();
+    const SkPath* casterOutlinePath = properties().getOutline().getPath();
     const SkPath* revealClipPath = properties().getRevealClip().getPath();
     if (revealClipPath && revealClipPath->isEmpty()) return;
 
     float casterAlpha = properties().getAlpha() * properties().getOutline().getAlpha();
+
+    const SkPath* outlinePath = casterOutlinePath;
+    if (revealClipPath) {
+        // if we can't simply use the caster's path directly, create a temporary one
+        SkPath* frameAllocatedPath = handler.allocPathForFrame();
+
+        // intersect the outline with the convex reveal clip
+        Op(*casterOutlinePath, *revealClipPath, kIntersect_PathOp, frameAllocatedPath);
+        outlinePath = frameAllocatedPath;
+    }
+
     DisplayListOp* shadowOp  = new (handler.allocator()) DrawShadowOp(
-            shadowMatrixXY, shadowMatrixZ, casterAlpha,
-            outlinePath, revealClipPath);
+            shadowMatrixXY, shadowMatrixZ, casterAlpha, outlinePath);
     handler(shadowOp, PROPERTY_SAVECOUNT, properties().getClipToBounds());
 }