From 8dcf60ecbe9c159831ece3b6201882f1d0033472 Mon Sep 17 00:00:00 2001 From: Tomaž Vajngerl Date: Wed, 8 Jun 2016 19:07:46 +0900 Subject: tdf#100184 fix the lifecycle of a texture in an atlas Previously, when a texture atlas was destroyed we teared down the ImplOpenGLTexture even if there were OpenGLTexture instances around. This caused that we could try to access an already deallocated ImplOpenGLTexture which causes a seg. fault. Now we change this so that a FixedTexture is no different than a OpenGLTexture - we just release the reference, so any existing OpenGLTextures for our texture would still be valid. An additional problem is that FixedTexture registers a callback for slot deallocation so we know when a OpenGLTextures that holds a specific "slot" on the texture is deallocated. However if FixedTexture is not existent anymore, the callback still gets triggered and is trying to access invalid memory. To solve this we need to unregister callbacks before FixedTexture is destroyed. Additionally improve validity of a OpenGLTexture is valid. If ImplOpenGLTexture is not allocated (nullptr) is one case, but in addition to that if ImplOpenGLTexture has an id == 0 it also means that it is not valid (anymore). Change-Id: I87346198e8928e112619da62687d5856cb8aafb8 --- vcl/inc/opengl/texture.hxx | 10 ++++++++++ vcl/opengl/FixedTextureAtlas.cxx | 19 +++++++++---------- vcl/opengl/texture.cxx | 16 ++++++++-------- 3 files changed, 27 insertions(+), 18 deletions(-) (limited to 'vcl') diff --git a/vcl/inc/opengl/texture.hxx b/vcl/inc/opengl/texture.hxx index e120c080ffa1..95126ff5b7bb 100644 --- a/vcl/inc/opengl/texture.hxx +++ b/vcl/inc/opengl/texture.hxx @@ -66,6 +66,11 @@ public: mFunctSlotDeallocateCallback = aCallback; } + void ResetSlotDeallocateCallback() + { + mFunctSlotDeallocateCallback = std::function(); + } + GLuint AddStencil(); }; @@ -80,6 +85,11 @@ private: inline bool GetTextureRect(const SalTwoRect& rPosAry, bool bInverted, GLfloat& x1, GLfloat& x2, GLfloat& y1, GLfloat& y2) const; + inline bool IsValid() const + { + return (mpImpl && mpImpl->mnTexture != 0); + } + public: OpenGLTexture(); OpenGLTexture(ImplOpenGLTexture* pImpl, Rectangle aRectangle, int nSlotNumber); diff --git a/vcl/opengl/FixedTextureAtlas.cxx b/vcl/opengl/FixedTextureAtlas.cxx index 420121d91475..7a980c9251e4 100644 --- a/vcl/opengl/FixedTextureAtlas.cxx +++ b/vcl/opengl/FixedTextureAtlas.cxx @@ -25,8 +25,8 @@ struct FixedTexture int mnFreeSlots; std::vector maAllocatedSlots; - FixedTexture(ImplOpenGLTexture* pTexture, int nNumberOfSlots) - : mpTexture(pTexture) + FixedTexture(int nTextureWidth, int nTextureHeight, int nNumberOfSlots) + : mpTexture(new ImplOpenGLTexture(nTextureWidth, nTextureHeight, true)) , mnFreeSlots(nNumberOfSlots) , maAllocatedSlots(nNumberOfSlots, false) { @@ -39,6 +39,12 @@ struct FixedTexture mpTexture->InitializeSlotMechanism(nNumberOfSlots); } + ~FixedTexture() + { + mpTexture->ResetSlotDeallocateCallback(); + mpTexture->DecreaseRefCount(-1); + } + void allocateSlot(int nSlot) { maAllocatedSlots[nSlot] = true; @@ -74,20 +80,13 @@ FixedTextureAtlasManager::FixedTextureAtlasManager(int nWidthFactor, int nHeight FixedTextureAtlasManager::~FixedTextureAtlasManager() { - for (std::unique_ptr& pFixedTexture : maFixedTextures) - { - // Free texture early in VCL shutdown while we have a context. - delete pFixedTexture->mpTexture; - } } void FixedTextureAtlasManager::CreateNewTexture() { int nTextureWidth = mWidthFactor * mSubTextureSize; int nTextureHeight = mHeightFactor * mSubTextureSize; - maFixedTextures.push_back(o3tl::make_unique(new ImplOpenGLTexture(nTextureWidth, nTextureHeight, true), - mWidthFactor * mHeightFactor)); - + maFixedTextures.push_back(o3tl::make_unique(nTextureWidth, nTextureHeight, mWidthFactor * mHeightFactor)); } OpenGLTexture FixedTextureAtlasManager::Reserve(int nWidth, int nHeight) diff --git a/vcl/opengl/texture.cxx b/vcl/opengl/texture.cxx index 3de7ac8d705e..03055c451bf6 100644 --- a/vcl/opengl/texture.cxx +++ b/vcl/opengl/texture.cxx @@ -364,7 +364,7 @@ void OpenGLTexture::GetCoord( GLfloat* pCoord, const SalTwoRect& rPosAry, bool b { VCL_GL_INFO( "Getting coord " << Id() << " [" << maRect.Left() << "," << maRect.Top() << "] " << GetWidth() << "x" << GetHeight() ); - if( mpImpl == nullptr ) + if (!IsValid()) { pCoord[0] = pCoord[1] = pCoord[2] = pCoord[3] = 0.0f; pCoord[4] = pCoord[5] = pCoord[6] = pCoord[7] = 0.0f; @@ -388,7 +388,7 @@ void OpenGLTexture::GetCoord( GLfloat* pCoord, const SalTwoRect& rPosAry, bool b bool OpenGLTexture::GetTextureRect(const SalTwoRect& rPosAry, bool bInverted, GLfloat& x1, GLfloat& x2, GLfloat& y1, GLfloat& y2) const { - if (mpImpl) + if (IsValid()) { double fTextureWidth(mpImpl->mnWidth); double fTextureHeight(mpImpl->mnHeight); @@ -463,7 +463,7 @@ void OpenGLTexture::GetWholeCoord( GLfloat* pCoord ) const OpenGLTexture OpenGLTexture::GetWholeTexture() { - if (mpImpl) + if (IsValid()) return OpenGLTexture(mpImpl, Rectangle(Point(0, 0), Size(mpImpl->mnWidth, mpImpl->mnHeight)), -1); return OpenGLTexture(); } @@ -477,7 +477,7 @@ GLenum OpenGLTexture::GetFilter() const bool OpenGLTexture::CopyData(int nWidth, int nHeight, int nFormat, int nType, sal_uInt8* pData) { - if (!pData || mpImpl == nullptr) + if (!pData || !IsValid()) return false; int nX = maRect.Left(); @@ -500,7 +500,7 @@ void OpenGLTexture::SetFilter( GLenum nFilter ) void OpenGLTexture::Bind() { - if (mpImpl) + if (IsValid()) { std::unique_ptr& rState = OpenGLContext::getVCLContext()->state(); rState->texture().bind(mpImpl->mnTexture); @@ -513,7 +513,7 @@ void OpenGLTexture::Bind() void OpenGLTexture::Unbind() { - if (mpImpl) + if (IsValid()) { std::unique_ptr& rState = OpenGLContext::getVCLContext()->state(); rState->texture().unbind(mpImpl->mnTexture); @@ -540,7 +540,7 @@ void OpenGLTexture::SaveToFile(const OUString& rFileName) void OpenGLTexture::Read( GLenum nFormat, GLenum nType, sal_uInt8* pData ) { - if( mpImpl == nullptr ) + if (!IsValid()) { SAL_WARN( "vcl.opengl", "Can't read invalid texture" ); return; @@ -581,7 +581,7 @@ void OpenGLTexture::Read( GLenum nFormat, GLenum nType, sal_uInt8* pData ) OpenGLTexture::operator bool() const { - return ( mpImpl != nullptr ); + return IsValid(); } OpenGLTexture& OpenGLTexture::operator=( const OpenGLTexture& rTexture ) -- cgit v1.2.3