Force serialization of render commands

This avoids us to break ign-rendering ABI while also simplifying the
amount of work to be done

Serializing work is easier to maintain and debug
Only CPU-bound scenario would potentially benefit from parallel command
generation (in terms of UI responsiveness)
Parallel command generation can be added back later

Also fixed coding style

Refer to
gazebosim/gz-rendering#304 (comment)
for discussion

Affects ign-rendering#304

Signed-off-by: Matias N. Goldberg <[email protected]>

src/gui/plugins/scene3d/Scene3D.cc

@@ -449,23 +449,23 @@ using namespace gazebo;
 QList<QThread *> RenderWindowItemPrivate::threads;
 
 /////////////////////////////////////////////////
-void RenderSync::requestQtThreadToBlock(std::unique_lock<std::mutex> &lock)
+void RenderSync::RequestQtThreadToBlock(std::unique_lock<std::mutex> &lock)
 {
   this->renderStallState = RenderStallState::WorkerThreadRequested;
   this->cv.wait(lock, [this]
   { return this->renderStallState == RenderStallState::QtThreadBlocked; });
 }
 
 /////////////////////////////////////////////////
-void RenderSync::releaseQtThreadFromBlock(std::unique_lock<std::mutex> &lock)
+void RenderSync::ReleaseQtThreadFromBlock(std::unique_lock<std::mutex> &lock)
 {
   this->renderStallState = RenderStallState::Unblocked;
   lock.unlock();
   this->cv.notify_one();
 }
 
 /////////////////////////////////////////////////
-void RenderSync::waitForWorkerThread()
+void RenderSync::WaitForWorkerThread()
 {
   {
     std::unique_lock<std::mutex> lock(this->mutex);
@@ -526,10 +526,20 @@ void IgnRenderer::Render(RenderSync *renderSync)
 
   IGN_PROFILE_THREAD_NAME("RenderThread");
   IGN_PROFILE("IgnRenderer::Render");
+
+  std::unique_lock<std::mutex> lock(renderSync->mutex);
+  renderSync->RequestQtThreadToBlock(lock);
+
   if (this->textureDirty)
   {
-    std::unique_lock<std::mutex> lock(renderSync->mutex);
-    renderSync->requestQtThreadToBlock(lock);
+    // TODO(anyone) If SwapFromThread gets implemented,
+    // then we only need to lock when texture is dirty
+    // (but we still need to lock the whole routine if
+    // debugging from RenderDoc or if user is not willing
+    // to sacrifice VRAM)
+    //
+    // std::unique_lock<std::mutex> lock(renderSync->mutex);
+    // renderSync->RequestQtThreadToBlock(lock);
     this->dataPtr->camera->SetImageWidth(this->textureSize.width());
     this->dataPtr->camera->SetImageHeight(this->textureSize.height());
     this->dataPtr->camera->SetAspectRatio(this->textureSize.width() /
@@ -540,7 +550,9 @@ void IgnRenderer::Render(RenderSync *renderSync)
       this->dataPtr->camera->PreRender();
     }
     this->textureDirty = false;
-    renderSync->releaseQtThreadFromBlock(lock);
+
+    // TODO(anyone) See SwapFromThread comments
+    // renderSync->ReleaseQtThreadFromBlock(lock);
   }
 
   // texture id could change so get the value in every render update
@@ -899,7 +911,13 @@ void IgnRenderer::Render(RenderSync *renderSync)
   // this notifes ECM to continue updating the scene
   g_renderCv.notify_one();
 
-  this->dataPtr->camera->SwapFromThread();
+  // TODO(anyone) implement a SwapFromThread for parallel command generation
+  // See https://github.com/ignitionrobotics/ign-rendering/issues/304
+  // if( bForcedSerialization )
+  //   this->dataPtr->camera->SwapFromThread();
+  // else
+  //  renderSync->ReleaseQtThreadFromBlock(lock);
+  renderSync->ReleaseQtThreadFromBlock(lock);
 }
 
 /////////////////////////////////////////////////
@@ -2312,7 +2330,7 @@ void TextureNode::NewTexture(uint _id, const QSize &_size)
 /////////////////////////////////////////////////
 void TextureNode::PrepareNode()
 {
-  renderSync.waitForWorkerThread();
+  renderSync.WaitForWorkerThread();
   this->mutex.lock();
   uint newId = this->id;
   QSize sz = this->size;

src/gui/plugins/scene3d/Scene3D.hh

@@ -197,12 +197,12 @@ inline namespace IGNITION_GAZEBO_VERSION_NAMESPACE {
 
     /// \brief Must be called from worker thread when we want to block
     /// \param[in] lock Acquired lock. Must be based on this->mutex
-    public: void requestQtThreadToBlock(std::unique_lock<std::mutex> &lock);
+    public: void RequestQtThreadToBlock(std::unique_lock<std::mutex> &lock);
     /// \brief Must be called from worker thread when we are done
     /// \param[in] lock Acquired lock. Must be based on this->mutex
-    public: void releaseQtThreadFromBlock(std::unique_lock<std::mutex> &lock);
+    public: void ReleaseQtThreadFromBlock(std::unique_lock<std::mutex> &lock);
     /// \brief Must be called from Qt thread periodically
-    public: void waitForWorkerThread();
+    public: void WaitForWorkerThread();
   };
 
   /// \brief Ign-rendering renderer.