From e5b8d3cb1030b1cd4f45a9ad969a2953b5ec8041 Mon Sep 17 00:00:00 2001 From: Ken Martin Date: Fri, 23 Aug 2019 13:05:28 -0400 Subject: [PATCH] Fix some issues with shared textures Move texture units into the OpenGLState class as they are part of the state. This commit fixes a few issues when textures were being shared between RenderWindows as they could end up using one render window for recording the texture unit and another for looking it up resulting in incorrect texture usage. Also fix a couple minor issues encountered as part of testing. --- GUISupport/Qt/vtkQWidgetTexture.cxx | 4 + Rendering/Core/vtkRenderer.cxx | 4 + .../vtkEquirectangularToCubemapTexture.cxx | 11 +++ .../vtkEquirectangularToCubemapTexture.h | 8 ++ Rendering/OpenGL2/vtkOpenGLRenderWindow.cxx | 77 ++------------- Rendering/OpenGL2/vtkOpenGLRenderWindow.h | 10 -- Rendering/OpenGL2/vtkOpenGLSkybox.cxx | 1 + Rendering/OpenGL2/vtkOpenGLState.cxx | 98 +++++++++++++++++++ Rendering/OpenGL2/vtkOpenGLState.h | 43 ++++++++ Rendering/OpenGL2/vtkOpenGLTexture.cxx | 12 +-- Rendering/OpenGL2/vtkTextureUnitManager.cxx | 42 +++----- Rendering/OpenGL2/vtkTextureUnitManager.h | 12 +-- 12 files changed, 194 insertions(+), 128 deletions(-) diff --git a/GUISupport/Qt/vtkQWidgetTexture.cxx b/GUISupport/Qt/vtkQWidgetTexture.cxx index ba620cfe296..0dbeaa21917 100644 --- a/GUISupport/Qt/vtkQWidgetTexture.cxx +++ b/GUISupport/Qt/vtkQWidgetTexture.cxx @@ -66,9 +66,13 @@ vtkQWidgetTexture::vtkQWidgetTexture() #ifdef GL_MULTISAMPLE state->ResetEnumState(GL_MULTISAMPLE); #endif + state->ResetGLScissorState(); state->ResetGLViewportState(); state->ResetGLDepthFuncState(); state->ResetGLBlendFuncState(); + // reset the depth test to LEQUAL as all vtk classes + // expect this to be the case when called + state->vtkglDepthFunc(GL_LEQUAL); } }; } diff --git a/Rendering/Core/vtkRenderer.cxx b/Rendering/Core/vtkRenderer.cxx index d2b3287315f..f314f09c47e 100644 --- a/Rendering/Core/vtkRenderer.cxx +++ b/Rendering/Core/vtkRenderer.cxx @@ -220,6 +220,10 @@ vtkTexture* vtkRenderer::GetLeftBackgroundTexture() void vtkRenderer::ReleaseGraphicsResources(vtkWindow *renWin) { + if(this->EnvironmentCubeMap != nullptr) + { + this->EnvironmentCubeMap->ReleaseGraphicsResources(renWin); + } if(this->BackgroundTexture != nullptr) { this->BackgroundTexture->ReleaseGraphicsResources(renWin); diff --git a/Rendering/OpenGL2/vtkEquirectangularToCubemapTexture.cxx b/Rendering/OpenGL2/vtkEquirectangularToCubemapTexture.cxx index f40a59f850c..fcc0a8fff64 100644 --- a/Rendering/OpenGL2/vtkEquirectangularToCubemapTexture.cxx +++ b/Rendering/OpenGL2/vtkEquirectangularToCubemapTexture.cxx @@ -45,6 +45,17 @@ vtkEquirectangularToCubemapTexture::~vtkEquirectangularToCubemapTexture() } } +// --------------------------------------------------------------------------- +// Release the graphics resources used by this texture. +void vtkEquirectangularToCubemapTexture::ReleaseGraphicsResources(vtkWindow *win) +{ + if (this->InputTexture) + { + this->InputTexture->ReleaseGraphicsResources(win); + } + this->Superclass::ReleaseGraphicsResources(win); +} + //------------------------------------------------------------------------------ void vtkEquirectangularToCubemapTexture::PrintSelf(ostream& os, vtkIndent indent) { diff --git a/Rendering/OpenGL2/vtkEquirectangularToCubemapTexture.h b/Rendering/OpenGL2/vtkEquirectangularToCubemapTexture.h index 1b3c30c840b..0f5e079361b 100644 --- a/Rendering/OpenGL2/vtkEquirectangularToCubemapTexture.h +++ b/Rendering/OpenGL2/vtkEquirectangularToCubemapTexture.h @@ -65,6 +65,14 @@ public: vtkSetMacro(CubemapSize, unsigned int); //@} + /** + * Release any graphics resources that are being consumed by this texture. + * The parameter window could be used to determine which graphic + * resources to release. Using the same texture object in multiple + * render windows is NOT currently supported. + */ + void ReleaseGraphicsResources(vtkWindow*) override; + protected: vtkEquirectangularToCubemapTexture(); ~vtkEquirectangularToCubemapTexture() override; diff --git a/Rendering/OpenGL2/vtkOpenGLRenderWindow.cxx b/Rendering/OpenGL2/vtkOpenGLRenderWindow.cxx index 93c40cc65c4..9309e313f63 100644 --- a/Rendering/OpenGL2/vtkOpenGLRenderWindow.cxx +++ b/Rendering/OpenGL2/vtkOpenGLRenderWindow.cxx @@ -54,8 +54,6 @@ using std::ostringstream; #include -vtkCxxSetObjectMacro(vtkOpenGLRenderWindow, TextureUnitManager, vtkTextureUnitManager); - // Initialize static member that controls global maximum number of multisamples // (off by default on Apple because it causes problems on some Mac models). #if defined(__APPLE__) @@ -190,8 +188,6 @@ vtkOpenGLRenderWindow::vtkOpenGLRenderWindow() this->ShaderCache = vtkOpenGLShaderCache::New(); this->VBOCache = vtkOpenGLVertexBufferObjectCache::New(); - this->TextureUnitManager = nullptr; - this->MultiSamples = vtkOpenGLRenderWindowGlobalMaximumNumberOfMultiSamples; delete [] this->WindowName; this->WindowName = new char[strlen(defaultWindowName) + 1]; @@ -241,14 +237,6 @@ vtkOpenGLRenderWindow::~vtkOpenGLRenderWindow() this->DrawPixelsTextureObject->UnRegister(this); this->DrawPixelsTextureObject = nullptr; } - this->TextureResourceIds.clear(); - if(this->TextureUnitManager!=nullptr) - { - this->TextureUnitManager->SetContext(nullptr); - } - - this->SetTextureUnitManager(nullptr); - this->GLStateIntegers.clear(); this->ShaderCache->UnRegister(this); @@ -354,16 +342,7 @@ void vtkOpenGLRenderWindow::ReleaseGraphicsResources(vtkRenderWindow *renWin) this->ShaderCache->ReleaseGraphicsResources(renWin); //this->VBOCache->ReleaseGraphicsResources(renWin); - if (!this->TextureResourceIds.empty()) - { - vtkErrorMacro("There are still active textures when there should not be."); - typedef std::map::const_iterator TRIter; - TRIter found = this->TextureResourceIds.begin(); - for ( ; found != this->TextureResourceIds.end(); ++found) - { - vtkErrorMacro("Leaked for texture object: " << const_cast(found->first)); - } - } + this->GetState()->VerifyNoActiveTextures(); this->RenderTimer->ReleaseGraphicsResources(); @@ -2118,49 +2097,17 @@ int vtkOpenGLRenderWindow::SetZbufferData( int x1, int y1, void vtkOpenGLRenderWindow::ActivateTexture(vtkTextureObject *texture) { - // Only add if it isn't already there - typedef std::map::const_iterator TRIter; - TRIter found = this->TextureResourceIds.find(texture); - if (found == this->TextureResourceIds.end()) - { - int activeUnit = this->GetTextureUnitManager()->Allocate(); - if (activeUnit < 0) - { - vtkErrorMacro("Hardware does not support the number of textures defined."); - return; - } - this->TextureResourceIds.insert(std::make_pair(texture, activeUnit)); - glActiveTexture(GL_TEXTURE0 + activeUnit); - } - else - { - glActiveTexture(GL_TEXTURE0 + found->second); - } + this->GetState()->ActivateTexture(texture); } void vtkOpenGLRenderWindow::DeactivateTexture(vtkTextureObject *texture) { - // Only deactivate if it isn't already there - typedef std::map::iterator TRIter; - TRIter found = this->TextureResourceIds.find(texture); - if (found != this->TextureResourceIds.end()) - { - this->GetTextureUnitManager()->Free(found->second); - this->TextureResourceIds.erase(found); - } + this->GetState()->DeactivateTexture(texture); } int vtkOpenGLRenderWindow::GetTextureUnitForTexture(vtkTextureObject *texture) { - // Only deactivate if it isn't already there - typedef std::map::const_iterator TRIter; - TRIter found = this->TextureResourceIds.find(texture); - if (found != this->TextureResourceIds.end()) - { - return found->second; - } - - return -1; + return this->GetState()->GetTextureUnitForTexture(texture); } // ---------------------------------------------------------------------------- @@ -2210,17 +2157,7 @@ int vtkOpenGLRenderWindow::CreateOffScreenFramebuffer(int width, int height) // hasn't already been set up. vtkTextureUnitManager *vtkOpenGLRenderWindow::GetTextureUnitManager() { - if(this->TextureUnitManager==nullptr) - { - vtkTextureUnitManager *manager=vtkTextureUnitManager::New(); - - // This does not form a reference loop since vtkOpenGLHardwareSupport does - // not keep a reference to the render window. - manager->SetContext(this); - this->SetTextureUnitManager(manager); - manager->Delete(); - } - return this->TextureUnitManager; + return this->GetState()->GetTextureUnitManager(); } // ---------------------------------------------------------------------------- @@ -2241,11 +2178,9 @@ void vtkOpenGLRenderWindow::SaveGLState() this->MakeCurrent(); glGetIntegerv(GL_ACTIVE_TEXTURE, &this->GLStateIntegers["GL_ACTIVE_TEXTURE"]); - // GetTextureUnitManager() will create a new texture unit - // manager if one does not exist if (this->GLStateIntegers["GL_ACTIVE_TEXTURE"] < 0 || this->GLStateIntegers["GL_ACTIVE_TEXTURE"] > - this->GetTextureUnitManager()->GetNumberOfTextureUnits()) + this->GetState()->GetTextureUnitManager()->GetNumberOfTextureUnits()) { this->GLStateIntegers["GL_ACTIVE_TEXTURE"] = 0; } diff --git a/Rendering/OpenGL2/vtkOpenGLRenderWindow.h b/Rendering/OpenGL2/vtkOpenGLRenderWindow.h index 313704bb80e..147b5b751be 100644 --- a/Rendering/OpenGL2/vtkOpenGLRenderWindow.h +++ b/Rendering/OpenGL2/vtkOpenGLRenderWindow.h @@ -469,8 +469,6 @@ protected: int TextureInternalFormats[VTK_UNICODE_STRING][3][5]; void InitializeTextureInternalFormats(); - std::map TextureResourceIds; - virtual int ReadPixels(const vtkRecti& rect, int front, int glFormat, int glType, void* data, int right=0); /** @@ -501,12 +499,6 @@ protected: */ virtual void ReleaseGraphicsResources(vtkRenderWindow *); - /** - * Set the texture unit manager. - */ - void SetTextureUnitManager(vtkTextureUnitManager *textureUnitManager); - - /** * Query and save OpenGL state */ @@ -534,8 +526,6 @@ protected: vtkTimeStamp ContextCreationTime; - vtkTextureUnitManager *TextureUnitManager; - vtkTextureObject *DrawPixelsTextureObject; bool Initialized; // ensure glewinit has been called diff --git a/Rendering/OpenGL2/vtkOpenGLSkybox.cxx b/Rendering/OpenGL2/vtkOpenGLSkybox.cxx index 8d13564afde..e1af211b06e 100644 --- a/Rendering/OpenGL2/vtkOpenGLSkybox.cxx +++ b/Rendering/OpenGL2/vtkOpenGLSkybox.cxx @@ -227,6 +227,7 @@ void vtkOpenGLSkybox::Render(vtkRenderer *ren, vtkMapper *mapper) // get opacity static_cast(ren)->GetState()->vtkglDepthMask(GL_TRUE); + static_cast(ren)->GetState()->vtkglDepthFunc(GL_LEQUAL); // send a render to the mapper; update pipeline this->Texture->Render(ren); diff --git a/Rendering/OpenGL2/vtkOpenGLState.cxx b/Rendering/OpenGL2/vtkOpenGLState.cxx index cb070fac2b7..e23b4315e15 100644 --- a/Rendering/OpenGL2/vtkOpenGLState.cxx +++ b/Rendering/OpenGL2/vtkOpenGLState.cxx @@ -17,6 +17,7 @@ #include "vtkOpenGLRenderWindow.h" #include "vtkRenderer.h" +#include "vtkTextureUnitManager.h" // must be included after a vtkObject subclass #include "vtkOpenGLError.h" @@ -268,6 +269,7 @@ bool reportOpenGLErrors(std::string &result) // ////////////////////////////////////////////////////////////////////////////// + vtkOpenGLState::ScopedglDepthMask::ScopedglDepthMask(vtkOpenGLState *s) { this->State = s; @@ -816,6 +818,8 @@ void vtkOpenGLState::vtkglDisable(GLenum cap) // state ivars void vtkOpenGLState::Initialize(vtkOpenGLRenderWindow *) { + this->TextureUnitManager->Initialize(); + this->CurrentState.Blend ? ::glEnable(GL_BLEND) : ::glDisable(GL_BLEND); this->CurrentState.DepthTest @@ -983,6 +987,92 @@ void vtkOpenGLState::vtkglClear(GLbitfield val) ::glClear(val); } +// ---------------------------------------------------------------------------- +// Description: +// Returns its texture unit manager object. +vtkTextureUnitManager *vtkOpenGLState::GetTextureUnitManager() +{ + return this->TextureUnitManager; +} + +void vtkOpenGLState::SetTextureUnitManager(vtkTextureUnitManager *tum) +{ + if (this->TextureUnitManager == tum) + { + return; + } + if (tum) + { + tum->Register(nullptr); + } + if (this->TextureUnitManager) + { + this->TextureUnitManager->Delete(); + } + this->TextureUnitManager = tum; +} + +void vtkOpenGLState::ActivateTexture(vtkTextureObject *texture) +{ + // Only add if it isn't already there + typedef std::map::const_iterator TRIter; + TRIter found = this->TextureResourceIds.find(texture); + if (found == this->TextureResourceIds.end()) + { + int activeUnit = this->GetTextureUnitManager()->Allocate(); + if (activeUnit < 0) + { + vtkGenericWarningMacro("Hardware does not support the number of textures defined."); + return; + } + this->TextureResourceIds.insert(std::make_pair(texture, activeUnit)); + glActiveTexture(GL_TEXTURE0 + activeUnit); + } + else + { + glActiveTexture(GL_TEXTURE0 + found->second); + } +} + +void vtkOpenGLState::DeactivateTexture(vtkTextureObject *texture) +{ + // Only deactivate if it isn't already there + typedef std::map::iterator TRIter; + TRIter found = this->TextureResourceIds.find(texture); + if (found != this->TextureResourceIds.end()) + { + this->GetTextureUnitManager()->Free(found->second); + this->TextureResourceIds.erase(found); + } +} + +int vtkOpenGLState::GetTextureUnitForTexture(vtkTextureObject *texture) +{ + // Only deactivate if it isn't already there + typedef std::map::const_iterator TRIter; + TRIter found = this->TextureResourceIds.find(texture); + if (found != this->TextureResourceIds.end()) + { + return found->second; + } + + return -1; +} + +void vtkOpenGLState::VerifyNoActiveTextures() +{ + if (!this->TextureResourceIds.empty()) + { + vtkGenericWarningMacro("There are still active textures when there should not be."); + typedef std::map::const_iterator TRIter; + TRIter found = this->TextureResourceIds.begin(); + for ( ; found != this->TextureResourceIds.end(); ++found) + { + vtkGenericWarningMacro("Leaked for texture object: " << const_cast(found->first)); + } + } +} + // initialize all state values. This is important so that in // ::Initialize we can just set the state to the current // values (knowing that they are set). The reason we want @@ -1005,6 +1095,8 @@ void vtkOpenGLState::vtkglClear(GLbitfield val) // vtkOpenGLState::vtkOpenGLState() { + this->TextureUnitManager = vtkTextureUnitManager::New(); + this->CurrentState.Blend = true; this->CurrentState.DepthTest = true; this->CurrentState.StencilTest = false; @@ -1050,3 +1142,9 @@ vtkOpenGLState::vtkOpenGLState() this->CurrentState.BlendEquationValue1 = GL_FUNC_ADD; this->CurrentState.BlendEquationValue2 = GL_FUNC_ADD; } + +vtkOpenGLState::~vtkOpenGLState() +{ + this->TextureResourceIds.clear(); + this->SetTextureUnitManager(nullptr); +} diff --git a/Rendering/OpenGL2/vtkOpenGLState.h b/Rendering/OpenGL2/vtkOpenGLState.h index 4c345f5bdaa..41095bf3a5a 100644 --- a/Rendering/OpenGL2/vtkOpenGLState.h +++ b/Rendering/OpenGL2/vtkOpenGLState.h @@ -60,14 +60,19 @@ #define vtkOpenGLState_h #include "vtkRenderingOpenGL2Module.h" // For export macro +#include "vtkObject.h" #include // for ivar +#include // for ivar class vtkOpenGLRenderWindow; +class vtkTextureObject; +class vtkTextureUnitManager; class VTKRENDERINGOPENGL2_EXPORT vtkOpenGLState { public: vtkOpenGLState(); // set initial values + ~vtkOpenGLState(); //@{ // cached OpenGL methods. By calling these the context will check @@ -155,6 +160,26 @@ public: void (vtkOpenGLState::*Method)(T); }; + /** + * Activate a texture unit for this texture + */ + void ActivateTexture(vtkTextureObject *); + + /** + * Deactivate a previously activated texture + */ + void DeactivateTexture(vtkTextureObject *); + + /** + * Get the texture unit for a given texture object + */ + int GetTextureUnitForTexture(vtkTextureObject *); + + /** + * Check to make sure no textures have been left active + */ + void VerifyNoActiveTextures(); + // Scoped classes you can use to save state class VTKRENDERINGOPENGL2_EXPORT ScopedglDepthMask : public ScopedValue { @@ -204,6 +229,17 @@ public: */ void Initialize(vtkOpenGLRenderWindow*); + /** + * Set the texture unit manager. + */ + void SetTextureUnitManager(vtkTextureUnitManager *textureUnitManager); + + /** + * Returns its texture unit manager object. A new one will be created if one + * hasn't already been set up. + */ + vtkTextureUnitManager *GetTextureUnitManager(); + protected: void BlendFuncSeparate(std::array val); void ClearColor(std::array val); @@ -211,6 +247,9 @@ protected: void Scissor(std::array val); void Viewport(std::array val); + vtkTextureUnitManager *TextureUnitManager; + std::map TextureResourceIds; + /** * Check that this OpenGL state has consistent values * with the current OpenGL context @@ -245,6 +284,10 @@ protected: }; GLState CurrentState; + +private: + vtkOpenGLState(const vtkOpenGLState&) = delete; + void operator=(const vtkOpenGLState&) = delete; }; #endif diff --git a/Rendering/OpenGL2/vtkOpenGLTexture.cxx b/Rendering/OpenGL2/vtkOpenGLTexture.cxx index 15f7bf900b9..a0ba9f2bdae 100644 --- a/Rendering/OpenGL2/vtkOpenGLTexture.cxx +++ b/Rendering/OpenGL2/vtkOpenGLTexture.cxx @@ -174,12 +174,12 @@ void vtkOpenGLTexture::Load(vtkRenderer *ren) // like the graphics context. // has something changed so that we need to rebuild the texture? - if (this->GetMTime() > this->LoadTime.GetMTime() || - inputTime > this->LoadTime.GetMTime() || - (this->GetLookupTable() && this->GetLookupTable()->GetMTime () > - this->LoadTime.GetMTime()) || - renWin->GetGenericContext() != this->RenderWindow->GetGenericContext() || - renWin->GetContextCreationTime() > this->LoadTime) + if (this->GetMTime() > this->LoadTime.GetMTime() + || inputTime > this->LoadTime.GetMTime() + || (this->GetLookupTable() && this->GetLookupTable()->GetMTime () > + this->LoadTime.GetMTime()) + || renWin->GetGenericContext() != this->RenderWindow->GetGenericContext() + || renWin->GetContextCreationTime() > this->LoadTime) { int size[3]; int xsize, ysize; diff --git a/Rendering/OpenGL2/vtkTextureUnitManager.cxx b/Rendering/OpenGL2/vtkTextureUnitManager.cxx index 35f2bd21461..1efcd199920 100644 --- a/Rendering/OpenGL2/vtkTextureUnitManager.cxx +++ b/Rendering/OpenGL2/vtkTextureUnitManager.cxx @@ -25,7 +25,6 @@ vtkStandardNewMacro(vtkTextureUnitManager); // ---------------------------------------------------------------------------- vtkTextureUnitManager::vtkTextureUnitManager() { - this->Context=nullptr; this->NumberOfTextureUnits=0; this->TextureUnits=nullptr; } @@ -34,7 +33,6 @@ vtkTextureUnitManager::vtkTextureUnitManager() vtkTextureUnitManager::~vtkTextureUnitManager() { this->DeleteTable(); - this->Context=nullptr; } // ---------------------------------------------------------------------------- @@ -64,31 +62,23 @@ void vtkTextureUnitManager::DeleteTable() } // ---------------------------------------------------------------------------- -void vtkTextureUnitManager::SetContext(vtkOpenGLRenderWindow *context) +void vtkTextureUnitManager::Initialize() { - if(this->Context!=context) + // this->DeleteTable(); + if (!this->NumberOfTextureUnits) { - if(this->Context!=nullptr) + glGetIntegerv(GL_MAX_TEXTURE_IMAGE_UNITS, &this->NumberOfTextureUnits); + if(this->NumberOfTextureUnits > 0) { - this->DeleteTable(); - } - this->Context=context; - if(this->Context!=nullptr) - { - glGetIntegerv(GL_MAX_TEXTURE_IMAGE_UNITS, &this->NumberOfTextureUnits); - if(this->NumberOfTextureUnits > 0) + this->TextureUnits = new bool [this->NumberOfTextureUnits]; + size_t i=0; + size_t c=this->NumberOfTextureUnits; + while(iTextureUnits = new bool [this->NumberOfTextureUnits]; - size_t i=0; - size_t c=this->NumberOfTextureUnits; - while(iTextureUnits[i]=false; - ++i; - } + this->TextureUnits[i]=false; + ++i; } } - this->Modified(); } } @@ -173,14 +163,4 @@ void vtkTextureUnitManager::Free(int textureUnitId) void vtkTextureUnitManager::PrintSelf(ostream& os, vtkIndent indent) { this->Superclass::PrintSelf(os, indent); - - os << indent << "Context: "; - if(this->Context!=nullptr) - { - os << static_cast(this->Context) <