Fix the leak of resources imported more than once

A compositor owns the resources it produces through its ResourceProvider.
A refcount for each resource is kept according to how many times it
is exported to parent compositors. If the same resource is sent more
than once to the parent, the resource needs to be returned as often
in CompositorFrameAck messages to make sure that it gets cleaned up.

Since we would overwrite any previous MailboxTexture with a new one
when receiving a TransferableResource, this would break the refcount
of the child compositor and keep the resource alive.

This would cause http://ie.microsoft.com/testdrive/performance/penguinmark/
to continuously allocate new textures in the GPU thread.

Fix the issue by counting how many times the resource was imported, as
ResourceProvider does itself, and populate ReturnedResource::count
according to that value.

Change-Id: I3a1f8da41338b5d431592f92fca8ef865ee2415c
Reviewed-by: Arvid Nilsson <anilsson@blackberry.com>
Reviewed-by: Michael Bruning <michael.bruning@digia.com>

1 files changed, 26 insertions, 13 deletions

diff --git a/src/core/delegated_frame_node.cpp b/src/core/delegated_frame_node.cpp
index 9353f4402..6573c99a5 100644
--- a/src/core/delegated_frame_node.cpp
+++ b/src/core/delegated_frame_node.cpp
@@ -126,8 +126,10 @@ public:
 
     bool needsToFetch() const { return !m_textureId; }
     cc::TransferableResource &resource() { return m_resource; }
+    cc::ReturnedResource returnResource();
     void fetchTexture(gpu::gles2::MailboxManager *mailboxManager);
     void setTarget(GLenum target);
+    void incImportCount() { ++m_importCount; }
 
 private:
     cc::TransferableResource m_resource;
@@ -135,6 +137,7 @@ private:
     QSize m_textureSize;
     bool m_hasAlpha;
     GLenum m_target;
+    int m_importCount;
 };
 
 static inline QSharedPointer<RenderPassTexture> findRenderPassTexture(const cc::RenderPass::Id &id, const QList<QSharedPointer<RenderPassTexture> > &list)
@@ -309,6 +312,7 @@ MailboxTexture::MailboxTexture(const cc::TransferableResource &resource)
     , m_textureSize(toQt(resource.size))
     , m_hasAlpha(false)
     , m_target(GL_TEXTURE_2D)
+    , m_importCount(1)
 {
 }
 
@@ -322,6 +326,22 @@ void MailboxTexture::setTarget(GLenum target)
     m_target = target;
 }
 
+cc::ReturnedResource MailboxTexture::returnResource()
+{
+    cc::ReturnedResource returned;
+    // The ResourceProvider ensures that the resource isn't used by the parent compositor's GL
+    // context in the GPU process by inserting a sync point to be waited for by the child
+    // compositor's GL context. We don't need this since we are triggering the delegated frame
+    // ack directly from our rendering thread. At this point (in updatePaintNode) we know that
+    // a frame that was compositing any of those resources has already been swapped and we thus
+    // don't need to use this mechanism.
+    returned.sync_point = 0;
+    returned.id = m_resource.id;
+    returned.count = m_importCount;
+    m_importCount = 0;
+    return returned;
+}
+
 void MailboxTexture::fetchTexture(gpu::gles2::MailboxManager *mailboxManager)
 {
     gpu::gles2::Texture *tex = ConsumeTexture(mailboxManager, m_target, *reinterpret_cast<const gpu::gles2::MailboxName*>(m_resource.mailbox.name));
@@ -400,7 +420,10 @@ void DelegatedFrameNode::commit(DelegatedFrameNodeData* data, cc::ReturnedResour
     // to the producing child compositor.
     for (unsigned i = 0; i < frameData->resource_list.size(); ++i) {
         const cc::TransferableResource &res = frameData->resource_list.at(i);
-        mailboxTextureCandidates[res.id] = QSharedPointer<MailboxTexture>(new MailboxTexture(res));
+        if (QSharedPointer<MailboxTexture> texture = mailboxTextureCandidates.value(res.id))
+            texture->incImportCount();
+        else
+            mailboxTextureCandidates[res.id] = QSharedPointer<MailboxTexture>(new MailboxTexture(res));
     }
 
     frameData->resource_list.clear();
@@ -562,18 +585,8 @@ void DelegatedFrameNode::commit(DelegatedFrameNodeData* data, cc::ReturnedResour
     }
 
     // Send resources of remaining candidates back to the child compositors so that they can be freed or reused.
-    Q_FOREACH (const QSharedPointer<MailboxTexture> &mailboxTexture, mailboxTextureCandidates.values()) {
-        // The ResourceProvider ensures that the resource isn't used by the parent compositor's GL
-        // context in the GPU process by inserting a sync point to be waited for by the child
-        // compositor's GL context. We don't need this since we are triggering the delegated frame
-        // ack directly from our rendering thread. At this point (in updatePaintNode) we know that
-        // a frame that was compositing any of those resources has already been swapped.
-        // Save a bit of overhead by resetting the sync point that has initially been put there
-        // for us (mainly to clean the output of --enable-gpu-service-logging).
-        mailboxTexture->resource().sync_point = 0;
-
-        resourcesToRelease->push_back(mailboxTexture->resource().ToReturnedResource());
-    }
+    Q_FOREACH (const QSharedPointer<MailboxTexture> &mailboxTexture, mailboxTextureCandidates.values())
+        resourcesToRelease->push_back(mailboxTexture->returnResource());
 }
 
 void DelegatedFrameNode::fetchTexturesAndUnlockQt(DelegatedFrameNode *frameNode, QList<MailboxTexture *> *mailboxesToFetch)