RenderView: unset thread local storage after we are done with it

Change-Id: I28d05464f22a0632fb12e1408063e7fa962e85c1
Reviewed-by: Sean Harmer <sean.harmer@kdab.com>

2 files changed, 34 insertions, 16 deletions

diff --git a/src/render/backend/renderview.cpp b/src/render/backend/renderview.cpp
index 0c45366c4..9226f8fac 100644
--- a/src/render/backend/renderview.cpp
+++ b/src/render/backend/renderview.cpp
@@ -398,6 +398,13 @@ void RenderView::addClearBuffers(const ClearBuffers *cb) {
 // If we are there, we know that entity had a GeometryRenderer + Material
 QVector<RenderCommand *> RenderView::buildDrawRenderCommands(const QVector<Entity *> &entities) const
 {
+    // Note: since many threads can be building render commands
+    // we need to ensure that the UniformBlockValueBuilder they are using
+    // is only accessed from the same thread
+    UniformBlockValueBuilder *builder = new UniformBlockValueBuilder();
+    builder->shaderDataManager = m_manager->shaderDataManager();
+    m_localData.setLocalData(builder);
+
     QVector<RenderCommand *> commands;
     commands.reserve(entities.size());
 
@@ -444,6 +451,8 @@ QVector<RenderCommand *> RenderView::buildDrawRenderCommands(const QVector<Entit
                     std::sort(lightSources.begin(), lightSources.end(), LightSourceCompare(node));
 
                 ParameterInfoList globalParameters = passData.parameterInfo;
+                // setShaderAndUniforms can initialize a localData
+                // make sure this is cleared before we leave this function
                 setShaderAndUniforms(command, pass, globalParameters, *(node->worldTransform()), lightSources.mid(0, std::max(lightSources.size(), MAX_LIGHTS)));
 
                 buildSortingKey(command);
@@ -451,11 +460,22 @@ QVector<RenderCommand *> RenderView::buildDrawRenderCommands(const QVector<Entit
             }
         }
     }
+
+    // We reset the local data once we are done with it
+    m_localData.setLocalData(nullptr);
+
     return commands;
 }
 
 QVector<RenderCommand *> RenderView::buildComputeRenderCommands(const QVector<Entity *> &entities) const
 {
+    // Note: since many threads can be building render commands
+    // we need to ensure that the UniformBlockValueBuilder they are using
+    // is only accessed from the same thread
+    UniformBlockValueBuilder *builder = new UniformBlockValueBuilder();
+    builder->shaderDataManager = m_manager->shaderDataManager();
+    m_localData.setLocalData(builder);
+
     // If the RenderView contains only a ComputeDispatch then it cares about
     // A ComputeDispatch is also implicitely a NoDraw operation
     // enabled flag
@@ -492,6 +512,10 @@ QVector<RenderCommand *> RenderView::buildComputeRenderCommands(const QVector<En
             }
         }
     }
+
+    // We reset the local data once we are done with it
+    m_localData.setLocalData(nullptr);
+
     return commands;
 }
 
@@ -627,28 +651,22 @@ void RenderView::setShaderStorageValue(ShaderParameterPack &uniformPack,
 
 void RenderView::setDefaultUniformBlockShaderDataValue(ShaderParameterPack &uniformPack, Shader *shader, ShaderData *shaderData, const QString &structName) const
 {
-    // Note: since many threads can be building render commands
-    // we need to ensure that the UniformBlockValueBuilder they are using
-    // is only accessed from the same thread
-    if (!m_localData.hasLocalData()) {
-        m_localData.setLocalData(UniformBlockValueBuilder());
-        m_localData.localData().shaderDataManager = m_manager->shaderDataManager();
-    }
-
-    UniformBlockValueBuilder &builder = m_localData.localData();
-    builder.activeUniformNamesToValue.clear();
+    UniformBlockValueBuilder *builder = m_localData.localData();
+    builder->activeUniformNamesToValue.clear();
 
     // updates transformed properties;
+    // Fix me: this will lead to races when having multiple cameras
     shaderData->updateViewTransform(m_data.m_viewMatrix);
+
     // Force to update the whole block
-    builder.updatedPropertiesOnly = false;
+    builder->updatedPropertiesOnly = false;
     // Retrieve names and description of each active uniforms in the uniform block
-    builder.uniforms = shader->activeUniformsForUniformBlock(-1);
+    builder->uniforms = shader->activeUniformsForUniformBlock(-1);
     // Build name-value map for the block
-    builder.buildActiveUniformNameValueMapStructHelper(shaderData, structName);
+    builder->buildActiveUniformNameValueMapStructHelper(shaderData, structName);
     // Set uniform values for each entrie of the block name-value map
-    QHash<int, QVariant>::const_iterator activeValuesIt = builder.activeUniformNamesToValue.constBegin();
-    const QHash<int, QVariant>::const_iterator activeValuesEnd = builder.activeUniformNamesToValue.constEnd();
+    QHash<int, QVariant>::const_iterator activeValuesIt = builder->activeUniformNamesToValue.constBegin();
+    const QHash<int, QVariant>::const_iterator activeValuesEnd = builder->activeUniformNamesToValue.constEnd();
 
     while (activeValuesIt != activeValuesEnd) {
         setUniformValue(uniformPack, activeValuesIt.key(), activeValuesIt.value());
diff --git a/src/render/backend/renderview_p.h b/src/render/backend/renderview_p.h
index 1e943a80b..7be9903f6 100644
--- a/src/render/backend/renderview_p.h
+++ b/src/render/backend/renderview_p.h
@@ -242,7 +242,7 @@ private:
     void setShaderAndUniforms(RenderCommand *command, RenderPass *pass, ParameterInfoList &parameters, const QMatrix4x4 &worldTransform,
                               const QVector<LightSource> &activeLightSources) const;
 
-    mutable QThreadStorage<UniformBlockValueBuilder> m_localData;
+    mutable QThreadStorage<UniformBlockValueBuilder*> m_localData;
 
     Renderer *m_renderer;
     NodeManagers *m_manager;