From 6daebd34590a600196d4a1497a803a3f73b507e9 Mon Sep 17 00:00:00 2001 From: valid-ptr Date: Fri, 17 Jun 2022 19:04:24 +0300 Subject: [PATCH] Rare segfault in Text::drawImplementationSinglePass fixed: _textureGlyphQuadMap[glyphTexture] can exist but it's _primitive can be invalid. GlyphQuads's interface need to be changed (osg::Referenced now, setupPrimitives() method without definition deleted); TextureGlyphQuadMap changed. Intersection with Text improved (no dynamic_casts, less code) --- include/osgText/Text | 10 ++--- src/osgText/Text.cpp | 81 ++++++++++++++++++++++++----------------- src/osgWidget/Input.cpp | 2 +- 3 files changed, 52 insertions(+), 41 deletions(-) diff --git a/include/osgText/Text b/include/osgText/Text index 5be160086b0..3937b4078dd 100644 --- a/include/osgText/Text +++ b/include/osgText/Text @@ -212,7 +212,7 @@ public: BackdropImplementation getBackdropImplementation() const { return DELAYED_DEPTH_WRITES; } // internal structures, variable and methods used for rendering of characters. - struct OSGTEXT_EXPORT GlyphQuads + struct OSGTEXT_EXPORT GlyphQuads : public osg::Referenced { typedef std::vector Glyphs; @@ -222,8 +222,6 @@ public: GlyphQuads(); GlyphQuads(const GlyphQuads& gq); - void setupPrimitives(Text::BackdropType backdropType); - Glyphs& getGlyphs() { return _glyphs; } const Glyphs& getGlyphs() const { return _glyphs; } @@ -231,7 +229,7 @@ public: void resizeGLObjectBuffers(unsigned int maxSize); /** If State is non-zero, this function releases OpenGL objects for - * the specified graphics context. Otherwise, releases OpenGL objexts + * the specified graphics context. Otherwise, releases OpenGL objects * for all graphics contexts. */ void releaseGLObjects(osg::State* state=0) const; @@ -240,7 +238,7 @@ public: GlyphQuads& operator = (const GlyphQuads&) { return *this; } }; - typedef std::map,GlyphQuads> TextureGlyphQuadMap; + typedef std::map< osg::ref_ptr, osg::ref_ptr > TextureGlyphQuadMap; /** Direct Access to GlyphQuads */ const GlyphQuads* getGlyphQuads(GlyphTexture* texture) const @@ -248,7 +246,7 @@ public: TextureGlyphQuadMap::const_iterator itGlyphQuad = _textureGlyphQuadMap.find(texture); if (itGlyphQuad == _textureGlyphQuadMap.end()) return NULL; - return &itGlyphQuad->second; + return itGlyphQuad->second.get(); } const TextureGlyphQuadMap& getTextureGlyphQuadMap() const diff --git a/src/osgText/Text.cpp b/src/osgText/Text.cpp index 86b800e7781..934a5e8eb93 100644 --- a/src/osgText/Text.cpp +++ b/src/osgText/Text.cpp @@ -11,7 +11,7 @@ * OpenSceneGraph Public License for more details. */ - +#include #include #include @@ -393,18 +393,38 @@ void Text::addGlyphQuad(Glyph* glyph, const osg::Vec2& minc, const osg::Vec2& ma // set up the coords of the quad const Glyph::TextureInfo* info = glyph->getOrCreateTextureInfo(_shaderTechnique); GlyphTexture* glyphTexture = info ? info->texture : 0; - GlyphQuads& glyphquad = _textureGlyphQuadMap[glyphTexture]; - glyphquad._glyphs.push_back(glyph); + osg::ref_ptr primitives; + + TextureGlyphQuadMap::iterator gqIter = _textureGlyphQuadMap.find(glyphTexture); + if (gqIter != _textureGlyphQuadMap.end()) // Found in the map + { + osg::ref_ptr gq = gqIter->second; + gq->_glyphs.push_back(glyph); - osg::DrawElements* primitives = glyphquad._primitives.get(); - if (!primitives) + if (gq->_primitives->getType() != osg::PrimitiveSet::DrawElementsUIntPrimitiveType && _text.size()*4 >= USHRT_MAX) + { + primitives = new osg::DrawElementsUInt(GL_TRIANGLES); + primitives->setBufferObject(_ebo.get()); + gq->_primitives = primitives; // The same buffer object will be used, so we should not worry about releaseGLObjects() + } + else + primitives = gq->_primitives.get(); + } + else { - unsigned int maxIndices = _text.size()*4; - if (maxIndices>=16384) primitives = new osg::DrawElementsUInt(GL_TRIANGLES); - else primitives = new osg::DrawElementsUShort(GL_TRIANGLES); + osg::ref_ptr gq = new GlyphQuads; + gq->_glyphs.push_back(glyph); + + if (_text.size()*4 >= USHRT_MAX) + primitives = new osg::DrawElementsUInt(GL_TRIANGLES); + else + primitives = new osg::DrawElementsUShort(GL_TRIANGLES); + primitives->setBufferObject(_ebo.get()); - glyphquad._primitives = primitives; + gq->_primitives = primitives; + + _textureGlyphQuadMap[glyphTexture] = gq; // Insert new GlyphQuads with valid _primitives } @@ -448,12 +468,12 @@ void Text::computeGlyphRepresentation() itr != _textureGlyphQuadMap.end(); ++itr) { - GlyphQuads& glyphquads = itr->second; - glyphquads._glyphs.clear(); - if (glyphquads._primitives.valid()) + osg::ref_ptr gq = itr->second; + gq->_glyphs.clear(); + if (gq->_primitives.valid()) { - glyphquads._primitives->resizeElements(0); - glyphquads._primitives->dirty(); + gq->_primitives->resizeElements(0); + gq->_primitives->dirty(); } } @@ -1133,7 +1153,7 @@ void Text::drawImplementationSinglePass(osg::State& state, const osg::Vec4& colo // need to set the texture here... state.applyTextureAttribute(0,titr->first.get()); - const GlyphQuads& glyphquad = titr->second; + const GlyphQuads& glyphquad = *(titr->second); if(_colorGradientMode == SOLID) { @@ -1148,7 +1168,8 @@ void Text::drawImplementationSinglePass(osg::State& state, const osg::Vec4& colo } } - glyphquad._primitives->draw(state, usingVertexBufferObjects); + osg::ref_ptr primitives = glyphquad._primitives; + primitives->draw(state, usingVertexBufferObjects); } } } @@ -1269,22 +1290,14 @@ void Text::accept(osg::PrimitiveFunctor& pf) const titr!=_textureGlyphQuadMap.end(); ++titr) { - const GlyphQuads& glyphquad = titr->second; - if (glyphquad._primitives.valid()) + const GlyphQuads& glyphquad = *(titr->second); + osg::ref_ptr drawElements = glyphquad._primitives; + if (drawElements.valid() && drawElements->getNumIndices() > 0) { - const osg::DrawElementsUShort* drawElementsUShort = dynamic_cast(glyphquad._primitives.get()); - if (drawElementsUShort && drawElementsUShort->size() > 0) - { - pf.drawElements(GL_TRIANGLES, drawElementsUShort->size(), &(drawElementsUShort->front())); - } - else - { - const osg::DrawElementsUInt* drawElementsUInt = dynamic_cast(glyphquad._primitives.get()); - if (drawElementsUInt && drawElementsUInt->size() > 0) - { - pf.drawElements(GL_TRIANGLES, drawElementsUInt->size(), &(drawElementsUInt->front())); - } - } + if (drawElements->getType() == osg::PrimitiveSet::DrawElementsUShortPrimitiveType) + pf.drawElements(GL_TRIANGLES, drawElements->getNumIndices(), (const GLushort*)(drawElements->getDataPointer())); + else if (drawElements->getType() == osg::PrimitiveSet::DrawElementsUIntPrimitiveType) + pf.drawElements(GL_TRIANGLES, drawElements->getNumIndices(), (const GLuint*)(drawElements->getDataPointer())); } } } @@ -1312,7 +1325,7 @@ void Text::resizeGLObjectBuffers(unsigned int maxSize) itr != _textureGlyphQuadMap.end(); ++itr) { - itr->second.resizeGLObjectBuffers(maxSize); + itr->second->resizeGLObjectBuffers(maxSize); } } @@ -1324,7 +1337,7 @@ void Text::releaseGLObjects(osg::State* state) const itr != _textureGlyphQuadMap.end(); ++itr) { - itr->second.releaseGLObjects(state); + itr->second->releaseGLObjects(state); } } @@ -1405,7 +1418,7 @@ Text::GlyphQuads::GlyphQuads() { } -Text::GlyphQuads::GlyphQuads(const GlyphQuads&) +Text::GlyphQuads::GlyphQuads(const GlyphQuads& glyphQuads) { } diff --git a/src/osgWidget/Input.cpp b/src/osgWidget/Input.cpp index 899d7fd97af..8dd61b824d9 100644 --- a/src/osgWidget/Input.cpp +++ b/src/osgWidget/Input.cpp @@ -120,7 +120,7 @@ void Input::_calculateCursorOffsets() { std::vector glyphs; for ( ; tgqmi != tgqm.end(); tgqmi++ ) { - const osgText::Text::GlyphQuads& gq = tgqmi->second; + const osgText::Text::GlyphQuads& gq = *(tgqmi->second); //coords.insert(coords.end(),gq.getTransformedCoords(0).begin(),gq.getTransformedCoords(0).end()); for (unsigned int i=0; i