RenderViewBuilder: fix leak with EntityRenderCommandData

EntityRenderCommandData would only be released in a separate job than the one
it was allocated in. However this would only happen if there was at least a
single renderable objects after filtering. In case there was no renderable
following filtering, the EntityRenderCommandData was leaked.

To fix the issue and make it less error prone, we now switch to using shared
pointers to avoid having to handle all possible leak cases with raw pointers.

Change-Id: I842d50d2b35ebba8303f6d6c4e72a2427ce31da3
Reviewed-by: Mike Krus <mike.krus@kdab.com>

2 files changed, 9 insertions, 21 deletions

diff --git a/src/render/renderers/opengl/renderer/rendercommand_p.h b/src/render/renderers/opengl/renderer/rendercommand_p.h
index 0180d6996..be00fb753 100644
--- a/src/render/renderers/opengl/renderer/rendercommand_p.h
+++ b/src/render/renderers/opengl/renderer/rendercommand_p.h
@@ -158,14 +158,6 @@ struct EntityRenderCommandData
         passesData.push_back(std::move(p));
     }
 
-    EntityRenderCommandData &operator+=(const EntityRenderCommandData &t)
-    {
-        entities += t.entities;
-        commands += t.commands;
-        passesData += t.passesData;
-        return *this;
-    }
-
     EntityRenderCommandData &operator+=(EntityRenderCommandData &&t)
     {
         entities += std::move(t.entities);
@@ -174,14 +166,10 @@ struct EntityRenderCommandData
         return *this;
     }
 
-    EntityRenderCommandData mid(int idx, int len) const
-    {
-        return { entities.mid(idx, len), commands.mid(idx, len), passesData.mid(idx, len) };
-    }
-
-
 };
 
+using EntityRenderCommandDataPtr = QSharedPointer<EntityRenderCommandData>;
+
 
 } // namespace Render
 
diff --git a/src/render/renderers/opengl/renderer/renderviewbuilder.cpp b/src/render/renderers/opengl/renderer/renderviewbuilder.cpp
index 9f7589ecc..8f1b17119 100644
--- a/src/render/renderers/opengl/renderer/renderviewbuilder.cpp
+++ b/src/render/renderers/opengl/renderer/renderviewbuilder.cpp
@@ -125,14 +125,10 @@ public:
         // Append all the commands and sort them
         RenderView *rv = m_renderViewJob->renderView();
 
-        const EntityRenderCommandData *commandData = m_renderViewCommandUpdaterJobs.first()->renderables();
+        const EntityRenderCommandDataPtr commandData = m_renderViewCommandUpdaterJobs.first()->renderables();
 
-        if (commandData != nullptr) {
+        if (commandData) {
             const QVector<RenderCommand> commands = std::move(commandData->commands);
-
-            // TO DO: Cache the EntityRenderCommandData so that it can be reused if nothing in the scene has changed
-            delete commandData;
-
             rv->setCommands(commands);
 
             // TO DO: Find way to store commands once or at least only when required
@@ -320,9 +316,13 @@ public:
                     renderableEntities = RenderViewBuilder::entitiesInSubset(renderableEntities, m_filterProximityJob->filteredEntities());
             }
 
+            // Early return in case we have nothing to filter
+            if (renderableEntities.size() == 0)
+                return;
+
             // Filter out Render commands for which the Entity wasn't selected because
             // of frustum, proximity or layer filtering
-            EntityRenderCommandData *filteredCommandData = new EntityRenderCommandData();
+            EntityRenderCommandDataPtr filteredCommandData = EntityRenderCommandDataPtr::create();
             filteredCommandData->reserve(renderableEntities.size());
             // Because dataCacheForLeaf.renderableEntities or computeEntities are sorted
             // What we get out of EntityRenderCommandData is also sorted by Entity