From 565a08b95a56db2abdae315300b71f9facd3b612 Mon Sep 17 00:00:00 2001 From: "florent.teppe" Date: Mon, 12 Sep 2022 19:13:02 +0200 Subject: [PATCH 1/3] crashfix on game exit --- components/debug/debugdraw.cpp | 18 +++++++++++++----- components/debug/debugdraw.hpp | 12 ++++++++---- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/components/debug/debugdraw.cpp b/components/debug/debugdraw.cpp index 1180ceb513..8a2274d9a1 100644 --- a/components/debug/debugdraw.cpp +++ b/components/debug/debugdraw.cpp @@ -258,7 +258,7 @@ namespace Debug ext->glUniform1i(normalAsColorLocation, false); - for (const auto& shapeToDraw : mShapesToDraw) + for (const auto& shapeToDraw : *mShapesToDraw) { osg::Vec3f translation = shapeToDraw.mPosition; osg::Vec3f color = shapeToDraw.mColor; @@ -281,7 +281,7 @@ namespace Debug break; } } - mShapesToDraw.clear(); + mShapesToDraw->clear(); static_cast(mLinesToDraw->getVertexArray())->clear(); static_cast(mLinesToDraw->getNormalArray())->clear(); } @@ -377,6 +377,8 @@ Debug::DebugDrawer::DebugDrawer(Shader::ShaderManager& shaderManager, osg::ref_p for (std::size_t i = 0; i < mShapesToDraw.size(); i++) { + mShapesToDraw[i] = std::make_shared>(); + mCustomDebugDrawer[i] = new DebugCustomDraw(mShapesToDraw[i], mDebugLines->mLinesGeom[i]); mCustomDebugDrawer[i]->setStateSet(stateset); mCustomDebugDrawer[i]->mWireCubeGeometry = wireCube; @@ -390,11 +392,17 @@ Debug::DebugDrawer::DebugDrawer(Shader::ShaderManager& shaderManager, osg::ref_p Debug::DebugDrawer::~DebugDrawer() { -} + auto parentsList = mDebugDrawSceneObjects->getParents(); + + for (auto parent : parentsList) + { + parent->removeChild(mDebugDrawSceneObjects); + } +} void Debug::DebugDrawer::drawCube(osg::Vec3f mPosition, osg::Vec3f mDims, osg::Vec3f mColor) { - mShapesToDraw[getIdexBufferWriteFromFrame(this->mCurrentFrame)].push_back({ mPosition, mDims, mColor, DrawShape::Cube }); + mShapesToDraw[getIdexBufferWriteFromFrame(this->mCurrentFrame)]->push_back({ mPosition, mDims, mColor, DrawShape::Cube }); } void Debug::DebugDrawer::drawCubeMinMax(osg::Vec3f min, osg::Vec3f max, osg::Vec3f color) @@ -406,7 +414,7 @@ void Debug::DebugDrawer::drawCubeMinMax(osg::Vec3f min, osg::Vec3f max, osg::Vec void Debug::DebugDrawer::addDrawCall(const DrawCall& draw) { - mShapesToDraw[getIdexBufferWriteFromFrame(this->mCurrentFrame)].push_back(draw); + mShapesToDraw[getIdexBufferWriteFromFrame(this->mCurrentFrame)]->push_back(draw); } void Debug::DebugDrawer::addLine(const osg::Vec3& start, const osg::Vec3& end, const osg::Vec3 color) diff --git a/components/debug/debugdraw.hpp b/components/debug/debugdraw.hpp index dcf9132712..dde84c65af 100644 --- a/components/debug/debugdraw.hpp +++ b/components/debug/debugdraw.hpp @@ -58,10 +58,14 @@ namespace Debug class DebugCustomDraw : public osg::Drawable { public: - DebugCustomDraw(std::vector& cubesToDraw, osg::ref_ptr& linesToDraw) : mShapesToDraw(cubesToDraw), mLinesToDraw(linesToDraw) {} + DebugCustomDraw(std::shared_ptr> cubesToDraw, osg::ref_ptr& linesToDraw) + : mShapesToDraw(cubesToDraw) + , mLinesToDraw(linesToDraw) + { + } - std::vector& mShapesToDraw; - osg::ref_ptr& mLinesToDraw; + std::shared_ptr> mShapesToDraw; + osg::ref_ptr mLinesToDraw; osg::ref_ptr mCubeGeometry; osg::ref_ptr mCylinderGeometry; @@ -87,7 +91,7 @@ namespace Debug private: std::unique_ptr mDebugLines; - std::array, 2> mShapesToDraw; + std::array>, 2> mShapesToDraw; long long int mCurrentFrame; std::array, 2> mCustomDebugDrawer; From e811f7ed00b5dedbaf2d2a6f46c6554e866018ef Mon Sep 17 00:00:00 2001 From: "florent.teppe" Date: Tue, 13 Sep 2022 18:20:32 +0200 Subject: [PATCH 2/3] Simplified the data structures. DebugCustomDraw owns the vector of drawcalls and the line geometry. There are two DebugCustomDraw, so anything they own is double buffered. Because DebugDrawer has a ref_ptr on the DebugCustomDraw, they live at least as long as DebugDrawer, making memory access from it safe. removed redundent this --- components/debug/debugdraw.cpp | 88 +++++++++++++++------------------- components/debug/debugdraw.hpp | 13 +---- 2 files changed, 41 insertions(+), 60 deletions(-) diff --git a/components/debug/debugdraw.cpp b/components/debug/debugdraw.cpp index 8a2274d9a1..ed16fbec08 100644 --- a/components/debug/debugdraw.cpp +++ b/components/debug/debugdraw.cpp @@ -225,12 +225,34 @@ static int getIdexBufferWriteFromFrame(const long long int& nFrame) namespace Debug { + static void makeLineInstance(osg::Geometry& lines) + { + auto vertices = new osg::Vec3Array; + auto color = new osg::Vec3Array; + lines.setDataVariance(osg::Object::STATIC); + lines.setUseVertexArrayObject(true); + lines.setUseDisplayList(false); + lines.setCullingActive(false); + + lines.setVertexArray(vertices); + lines.setNormalArray(color, osg::Array::BIND_PER_VERTEX); + + lines.addPrimitiveSet(new osg::DrawArrays(osg::PrimitiveSet::LINES, 0, vertices->size())); + } + + + DebugCustomDraw::DebugCustomDraw() + { + mLinesToDraw = new osg::Geometry(); + makeLineInstance(*mLinesToDraw); + } + void DebugCustomDraw::drawImplementation(osg::RenderInfo& renderInfo) const { auto state = renderInfo.getState(); osg::GLExtensions* ext = osg::GLExtensions::Get(state->getContextID(), true); - const osg::StateSet* stateSet = this->getStateSet(); + const osg::StateSet* stateSet = getStateSet(); auto program = static_cast(stateSet->getAttribute(osg::StateAttribute::PROGRAM)); const osg::Program::PerContextProgram* pcp = program->getPCP(*state); @@ -258,7 +280,7 @@ namespace Debug ext->glUniform1i(normalAsColorLocation, false); - for (const auto& shapeToDraw : *mShapesToDraw) + for (const auto& shapeToDraw : mShapesToDraw) { osg::Vec3f translation = shapeToDraw.mPosition; osg::Vec3f color = shapeToDraw.mColor; @@ -271,51 +293,21 @@ namespace Debug switch (shapeToDraw.mDrawShape) { case DrawShape::Cube: - this->mCubeGeometry->drawImplementation(renderInfo); + mCubeGeometry->drawImplementation(renderInfo); break; case DrawShape::Cylinder: - this->mCylinderGeometry->drawImplementation(renderInfo); + mCylinderGeometry->drawImplementation(renderInfo); break; case DrawShape::WireCube: - this->mWireCubeGeometry->drawImplementation(renderInfo); + mWireCubeGeometry->drawImplementation(renderInfo); break; } } - mShapesToDraw->clear(); + mShapesToDraw.clear(); static_cast(mLinesToDraw->getVertexArray())->clear(); static_cast(mLinesToDraw->getNormalArray())->clear(); } - struct DebugLines - { - - static void makeLineInstance(osg::Geometry& lines) - { - auto vertices = new osg::Vec3Array; - auto color = new osg::Vec3Array; - - lines.setUseVertexArrayObject(true); - lines.setUseDisplayList(false); - lines.setCullingActive(false); - - lines.setVertexArray(vertices); - lines.setNormalArray(color, osg::Array::BIND_PER_VERTEX); - - lines.addPrimitiveSet(new osg::DrawArrays(osg::PrimitiveSet::LINES, 0, vertices->size())); - } - - DebugLines() - { - mLinesGeom[0] = new osg::Geometry(); - mLinesGeom[1] = new osg::Geometry(); - - makeLineInstance(*mLinesGeom[0]); - makeLineInstance(*mLinesGeom[1]); - } - - std::array, 2> mLinesGeom; - }; - class DebugDrawCallback : public SceneUtil::NodeCallback { public: @@ -325,9 +317,9 @@ namespace Debug { mDebugDrawer.mCurrentFrame = nv->getTraversalNumber(); int indexRead = getIdexBufferReadFromFrame(mDebugDrawer.mCurrentFrame); - auto& lines = mDebugDrawer.mDebugLines; - lines->mLinesGeom[indexRead]->removePrimitiveSet(0, 1); - lines->mLinesGeom[indexRead]->addPrimitiveSet(new osg::DrawArrays(osg::PrimitiveSet::LINES, 0, static_cast(lines->mLinesGeom[indexRead]->getVertexArray())->size())); + auto& lines = mDebugDrawer.mCustomDebugDrawer[indexRead]->mLinesToDraw; + lines->removePrimitiveSet(0, 1); + lines->addPrimitiveSet(new osg::DrawArrays(osg::PrimitiveSet::LINES, 0, static_cast(lines->getVertexArray())->size())); nv->pushOntoNodePath(mDebugDrawer.mCustomDebugDrawer[indexRead]); nv->apply(*mDebugDrawer.mCustomDebugDrawer[indexRead]); @@ -345,7 +337,6 @@ Debug::DebugDrawer::DebugDrawer(Shader::ShaderManager& shaderManager, osg::ref_p auto fragmentShader = shaderManager.getShader("debug_fragment.glsl", Shader::ShaderManager::DefineMap(), osg::Shader::Type::FRAGMENT); auto program = shaderManager.getProgram(vertexShader, fragmentShader); - mDebugLines = std::make_unique(); mDebugDrawSceneObjects = new osg::Group; mDebugDrawSceneObjects->setCullingActive(false); @@ -375,11 +366,9 @@ Debug::DebugDrawer::DebugDrawer(Shader::ShaderManager& shaderManager, osg::ref_p wireCube->setUseVertexBufferObjects(true); generateWireCube(*wireCube, 1.); - for (std::size_t i = 0; i < mShapesToDraw.size(); i++) + for (std::size_t i = 0; i < mCustomDebugDrawer.size(); i++) { - mShapesToDraw[i] = std::make_shared>(); - - mCustomDebugDrawer[i] = new DebugCustomDraw(mShapesToDraw[i], mDebugLines->mLinesGeom[i]); + mCustomDebugDrawer[i] = new DebugCustomDraw(); mCustomDebugDrawer[i]->setStateSet(stateset); mCustomDebugDrawer[i]->mWireCubeGeometry = wireCube; mCustomDebugDrawer[i]->mCubeGeometry = cubeGeometry; @@ -402,7 +391,7 @@ Debug::DebugDrawer::~DebugDrawer() void Debug::DebugDrawer::drawCube(osg::Vec3f mPosition, osg::Vec3f mDims, osg::Vec3f mColor) { - mShapesToDraw[getIdexBufferWriteFromFrame(this->mCurrentFrame)]->push_back({ mPosition, mDims, mColor, DrawShape::Cube }); + mCustomDebugDrawer[getIdexBufferWriteFromFrame(mCurrentFrame)]->mShapesToDraw.push_back({ mPosition, mDims, mColor, DrawShape::Cube }); } void Debug::DebugDrawer::drawCubeMinMax(osg::Vec3f min, osg::Vec3f max, osg::Vec3f color) @@ -414,14 +403,15 @@ void Debug::DebugDrawer::drawCubeMinMax(osg::Vec3f min, osg::Vec3f max, osg::Vec void Debug::DebugDrawer::addDrawCall(const DrawCall& draw) { - mShapesToDraw[getIdexBufferWriteFromFrame(this->mCurrentFrame)]->push_back(draw); + mCustomDebugDrawer[getIdexBufferWriteFromFrame(mCurrentFrame)]->mShapesToDraw.push_back(draw); } void Debug::DebugDrawer::addLine(const osg::Vec3& start, const osg::Vec3& end, const osg::Vec3 color) { - const int indexWrite = getIdexBufferWriteFromFrame(this->mCurrentFrame); - auto vertices = static_cast(mDebugLines->mLinesGeom[indexWrite]->getVertexArray()); - auto colors = static_cast(mDebugLines->mLinesGeom[indexWrite]->getNormalArray()); + const int indexWrite = getIdexBufferWriteFromFrame(mCurrentFrame); + auto lines = mCustomDebugDrawer[indexWrite]->mLinesToDraw; + auto vertices = static_cast(lines->getVertexArray()); + auto colors = static_cast(lines->getNormalArray()); vertices->push_back(start); vertices->push_back(end); diff --git a/components/debug/debugdraw.hpp b/components/debug/debugdraw.hpp index dde84c65af..7efbb1a373 100644 --- a/components/debug/debugdraw.hpp +++ b/components/debug/debugdraw.hpp @@ -58,13 +58,9 @@ namespace Debug class DebugCustomDraw : public osg::Drawable { public: - DebugCustomDraw(std::shared_ptr> cubesToDraw, osg::ref_ptr& linesToDraw) - : mShapesToDraw(cubesToDraw) - , mLinesToDraw(linesToDraw) - { - } + DebugCustomDraw(); - std::shared_ptr> mShapesToDraw; + mutable std::vector mShapesToDraw; osg::ref_ptr mLinesToDraw; osg::ref_ptr mCubeGeometry; @@ -74,8 +70,6 @@ namespace Debug virtual void drawImplementation(osg::RenderInfo&) const; }; - struct DebugLines; - struct DebugDrawer { friend DebugDrawCallback; @@ -89,9 +83,6 @@ namespace Debug void addLine(const osg::Vec3& start, const osg::Vec3& end, const osg::Vec3 color = colorWhite); private: - std::unique_ptr mDebugLines; - - std::array>, 2> mShapesToDraw; long long int mCurrentFrame; std::array, 2> mCustomDebugDrawer; From 943198e3251e6dffa3c007570e352d946c607f49 Mon Sep 17 00:00:00 2001 From: "florent.teppe" Date: Wed, 14 Sep 2022 13:20:35 +0200 Subject: [PATCH 3/3] Small changes --- components/debug/debugdraw.cpp | 13 ++++--------- components/debug/debugdraw.hpp | 3 ++- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/components/debug/debugdraw.cpp b/components/debug/debugdraw.cpp index ed16fbec08..6ee165c7ef 100644 --- a/components/debug/debugdraw.cpp +++ b/components/debug/debugdraw.cpp @@ -330,7 +330,7 @@ namespace Debug }; } -Debug::DebugDrawer::DebugDrawer(Shader::ShaderManager& shaderManager, osg::ref_ptr parentNode) +Debug::DebugDrawer::DebugDrawer(Shader::ShaderManager& shaderManager, osg::ref_ptr parentNode) : mParentNode(parentNode) { mCurrentFrame = 0; auto vertexShader = shaderManager.getShader("debug_vertex.glsl", Shader::ShaderManager::DefineMap(), osg::Shader::Type::VERTEX); @@ -376,17 +376,12 @@ Debug::DebugDrawer::DebugDrawer(Shader::ShaderManager& shaderManager, osg::ref_p } mDebugDrawSceneObjects->addCullCallback(new DebugDrawCallback(*this)); - parentNode->addChild(mDebugDrawSceneObjects); + mParentNode->addChild(mDebugDrawSceneObjects); } Debug::DebugDrawer::~DebugDrawer() { - auto parentsList = mDebugDrawSceneObjects->getParents(); - - for (auto parent : parentsList) - { - parent->removeChild(mDebugDrawSceneObjects); - } + mParentNode->removeChild(mDebugDrawSceneObjects); } void Debug::DebugDrawer::drawCube(osg::Vec3f mPosition, osg::Vec3f mDims, osg::Vec3f mColor) @@ -409,7 +404,7 @@ void Debug::DebugDrawer::addDrawCall(const DrawCall& draw) void Debug::DebugDrawer::addLine(const osg::Vec3& start, const osg::Vec3& end, const osg::Vec3 color) { const int indexWrite = getIdexBufferWriteFromFrame(mCurrentFrame); - auto lines = mCustomDebugDrawer[indexWrite]->mLinesToDraw; + const auto& lines = mCustomDebugDrawer[indexWrite]->mLinesToDraw; auto vertices = static_cast(lines->getVertexArray()); auto colors = static_cast(lines->getNormalArray()); diff --git a/components/debug/debugdraw.hpp b/components/debug/debugdraw.hpp index 7efbb1a373..842a1ab753 100644 --- a/components/debug/debugdraw.hpp +++ b/components/debug/debugdraw.hpp @@ -67,7 +67,7 @@ namespace Debug osg::ref_ptr mCylinderGeometry; osg::ref_ptr mWireCubeGeometry; - virtual void drawImplementation(osg::RenderInfo&) const; + virtual void drawImplementation(osg::RenderInfo&) const override; }; struct DebugDrawer @@ -87,6 +87,7 @@ namespace Debug std::array, 2> mCustomDebugDrawer; osg::ref_ptr mDebugDrawSceneObjects; + osg::ref_ptr mParentNode; }; } #endif // !