From 29be3014af65761110329e2c1e76a9ab8f64037c Mon Sep 17 00:00:00 2001 From: "Matias N. Goldberg" Date: Sat, 24 Apr 2021 16:44:23 -0300 Subject: [PATCH] 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 https://github.com/ignitionrobotics/ign-rendering/issues/304#issuecomment-824130450 for discussion Affects ign-rendering#304 Signed-off-by: Matias N. Goldberg --- src/gui/plugins/scene3d/Scene3D.cc | 34 +++++++++++++++++++++++------- src/gui/plugins/scene3d/Scene3D.hh | 6 +++--- 2 files changed, 29 insertions(+), 11 deletions(-) diff --git a/src/gui/plugins/scene3d/Scene3D.cc b/src/gui/plugins/scene3d/Scene3D.cc index 32515a8280..5639a4baf9 100644 --- a/src/gui/plugins/scene3d/Scene3D.cc +++ b/src/gui/plugins/scene3d/Scene3D.cc @@ -449,7 +449,7 @@ using namespace gazebo; QList RenderWindowItemPrivate::threads; ///////////////////////////////////////////////// -void RenderSync::requestQtThreadToBlock(std::unique_lock &lock) +void RenderSync::RequestQtThreadToBlock(std::unique_lock &lock) { this->renderStallState = RenderStallState::WorkerThreadRequested; this->cv.wait(lock, [this] @@ -457,7 +457,7 @@ void RenderSync::requestQtThreadToBlock(std::unique_lock &lock) } ///////////////////////////////////////////////// -void RenderSync::releaseQtThreadFromBlock(std::unique_lock &lock) +void RenderSync::ReleaseQtThreadFromBlock(std::unique_lock &lock) { this->renderStallState = RenderStallState::Unblocked; lock.unlock(); @@ -465,7 +465,7 @@ void RenderSync::releaseQtThreadFromBlock(std::unique_lock &lock) } ///////////////////////////////////////////////// -void RenderSync::waitForWorkerThread() +void RenderSync::WaitForWorkerThread() { { std::unique_lock lock(this->mutex); @@ -526,10 +526,20 @@ void IgnRenderer::Render(RenderSync *renderSync) IGN_PROFILE_THREAD_NAME("RenderThread"); IGN_PROFILE("IgnRenderer::Render"); + + std::unique_lock lock(renderSync->mutex); + renderSync->RequestQtThreadToBlock(lock); + if (this->textureDirty) { - std::unique_lock 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 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; diff --git a/src/gui/plugins/scene3d/Scene3D.hh b/src/gui/plugins/scene3d/Scene3D.hh index 9a6025c8c7..575504052f 100644 --- a/src/gui/plugins/scene3d/Scene3D.hh +++ b/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 &lock); + public: void RequestQtThreadToBlock(std::unique_lock &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 &lock); + public: void ReleaseQtThreadFromBlock(std::unique_lock &lock); /// \brief Must be called from Qt thread periodically - public: void waitForWorkerThread(); + public: void WaitForWorkerThread(); }; /// \brief Ign-rendering renderer.