From 09e30adbb1f461f8e95e34b9f92218df7b3966c7 Mon Sep 17 00:00:00 2001
From: Seth Hillbrand <seth@kipro-pcb.com>
Date: Sun, 5 Jan 2025 13:16:17 -0800
Subject: [PATCH] Move GL_CONTEXT_MGR into PGM_BASE singleton
This is the second try at 5326c36a5f. The difference here is that we
have moved GL_CONTEXT_MGR into kicommon first which will hopefully
address some of the Windows linkage issues
The GL context lock needs to be shared across kifaces. Otherwise, we
can end up blocking the lock from one kiface. Unfortunately, I can't
find the issue in GitLab right now for where the footprint viewer shows
a blank screen after opening too many contexts. But that's what this
fixes.
---
3d-viewer/3d_canvas/eda_3d_canvas.cpp | 28 +++++++++--------
.../3d_model_viewer/eda_3d_model_viewer.cpp | 13 ++++----
.../3d_rendering/opengl/render_3d_opengl.cpp | 3 +-
common/gal/opengl/gl_context_mgr.cpp | 14 ---------
common/gal/opengl/opengl_gal.cpp | 23 ++++++++------
include/gal/opengl/gl_context_mgr.h | 31 +++++++------------
include/pgm_base.h | 2 ++
include/singleton.h | 7 +++++
8 files changed, 58 insertions(+), 63 deletions(-)
diff --git a/3d-viewer/3d_canvas/eda_3d_canvas.cpp b/3d-viewer/3d_canvas/eda_3d_canvas.cpp
index 0214ac0999..791d888a4b 100644
--- a/3d-viewer/3d_canvas/eda_3d_canvas.cpp
+++ b/3d-viewer/3d_canvas/eda_3d_canvas.cpp
@@ -187,7 +187,8 @@ void EDA_3D_CANVAS::releaseOpenGL()
{
if( m_glRC )
{
- GL_CONTEXT_MANAGER::Get().LockCtx( m_glRC, this );
+ GL_CONTEXT_MANAGER* gl_mgr = Pgm().GetGLContextManager();
+ gl_mgr->LockCtx( m_glRC, this );
delete m_3d_render_raytracing;
m_3d_render_raytracing = nullptr;
@@ -198,8 +199,8 @@ void EDA_3D_CANVAS::releaseOpenGL()
// This is just a copy of a pointer, can safely be set to NULL.
m_3d_render = nullptr;
- GL_CONTEXT_MANAGER::Get().UnlockCtx( m_glRC );
- GL_CONTEXT_MANAGER::Get().DestroyCtx( m_glRC );
+ gl_mgr->UnlockCtx( m_glRC );
+ gl_mgr->DestroyCtx( m_glRC );
m_glRC = nullptr;
}
}
@@ -379,10 +380,11 @@ void EDA_3D_CANVAS::DoRePaint()
if( !GetParent()->GetParent()->IsShownOnScreen() )
return; // The parent board editor frame is no more alive
- wxString err_messages;
- INFOBAR_REPORTER warningReporter( m_parentInfoBar );
- STATUSBAR_REPORTER activityReporter( m_parentStatusBar, EDA_3D_VIEWER_STATUSBAR::ACTIVITY );
- int64_t start_time = GetRunningMicroSecs();
+ wxString err_messages;
+ INFOBAR_REPORTER warningReporter( m_parentInfoBar );
+ STATUSBAR_REPORTER activityReporter( m_parentStatusBar, EDA_3D_VIEWER_STATUSBAR::ACTIVITY );
+ int64_t start_time = GetRunningMicroSecs();
+ GL_CONTEXT_MANAGER* gl_mgr = Pgm().GetGLContextManager();
// "Makes the OpenGL state that is represented by the OpenGL rendering
// context context current, i.e. it will be used by all subsequent OpenGL calls.
@@ -390,7 +392,7 @@ void EDA_3D_CANVAS::DoRePaint()
// Explicitly create a new rendering context instance for this canvas.
if( m_glRC == nullptr )
- m_glRC = GL_CONTEXT_MANAGER::Get().CreateCtx( this );
+ m_glRC = gl_mgr->CreateCtx( this );
// CreateCtx could and does fail per sentry crash events, lets be graceful
if( m_glRC == nullptr )
@@ -401,7 +403,7 @@ void EDA_3D_CANVAS::DoRePaint()
return;
}
- GL_CONTEXT_MANAGER::Get().LockCtx( m_glRC, this );
+ gl_mgr->LockCtx( m_glRC, this );
// Set the OpenGL viewport according to the client size of this canvas.
// This is done here rather than in a wxSizeEvent handler because our
@@ -418,7 +420,7 @@ void EDA_3D_CANVAS::DoRePaint()
{
if( !initializeOpenGL() )
{
- GL_CONTEXT_MANAGER::Get().UnlockCtx( m_glRC );
+ gl_mgr->UnlockCtx( m_glRC );
m_is_currently_painting.clear();
return;
@@ -440,7 +442,7 @@ void EDA_3D_CANVAS::DoRePaint()
SwapBuffers();
- GL_CONTEXT_MANAGER::Get().UnlockCtx( m_glRC );
+ gl_mgr->UnlockCtx( m_glRC );
m_is_currently_painting.clear();
return;
@@ -528,7 +530,7 @@ void EDA_3D_CANVAS::DoRePaint()
m_is_opengl_version_supported = false;
m_opengl_supports_raytracing = false;
m_is_opengl_initialized = false;
- GL_CONTEXT_MANAGER::Get().UnlockCtx( m_glRC );
+ gl_mgr->UnlockCtx( m_glRC );
m_is_currently_painting.clear();
return;
}
@@ -553,7 +555,7 @@ void EDA_3D_CANVAS::DoRePaint()
// commands is displayed on the window."
SwapBuffers();
- GL_CONTEXT_MANAGER::Get().UnlockCtx( m_glRC );
+ gl_mgr->UnlockCtx( m_glRC );
if( m_mouse_was_moved || m_camera_is_moving )
{
diff --git a/3d-viewer/3d_model_viewer/eda_3d_model_viewer.cpp b/3d-viewer/3d_model_viewer/eda_3d_model_viewer.cpp
index 632449ad5c..be34740111 100644
--- a/3d-viewer/3d_model_viewer/eda_3d_model_viewer.cpp
+++ b/3d-viewer/3d_model_viewer/eda_3d_model_viewer.cpp
@@ -109,16 +109,17 @@ EDA_3D_MODEL_VIEWER::EDA_3D_MODEL_VIEWER( wxWindow* aParent, const wxGLAttribute
EDA_3D_MODEL_VIEWER::~EDA_3D_MODEL_VIEWER()
{
wxLogTrace( m_logTrace, wxT( "EDA_3D_MODEL_VIEWER::~EDA_3D_MODEL_VIEWER" ) );
+ GL_CONTEXT_MANAGER* gl_mgr = Pgm().GetGLContextManager();
if( m_glRC )
{
- GL_CONTEXT_MANAGER::Get().LockCtx( m_glRC, this );
+ gl_mgr->LockCtx( m_glRC, this );
delete m_ogl_3dmodel;
m_ogl_3dmodel = nullptr;
- GL_CONTEXT_MANAGER::Get().UnlockCtx( m_glRC );
- GL_CONTEXT_MANAGER::Get().DestroyCtx( m_glRC );
+ gl_mgr->UnlockCtx( m_glRC );
+ gl_mgr->DestroyCtx( m_glRC );
}
}
@@ -256,7 +257,7 @@ void EDA_3D_MODEL_VIEWER::OnPaint( wxPaintEvent& event )
// context context current, i.e. it will be used by all subsequent OpenGL calls.
// This function may only be called when the window is shown on screen"
if( m_glRC == nullptr )
- m_glRC = GL_CONTEXT_MANAGER::Get().CreateCtx( this );
+ m_glRC = Pgm().GetGLContextManager()->CreateCtx( this );
// CreateCtx could and does fail per sentry crash events, lets be graceful
if( m_glRC == nullptr )
@@ -265,7 +266,7 @@ void EDA_3D_MODEL_VIEWER::OnPaint( wxPaintEvent& event )
return;
}
- GL_CONTEXT_MANAGER::Get().LockCtx( m_glRC, this );
+ Pgm().GetGLContextManager()->LockCtx( m_glRC, this );
// Set the OpenGL viewport according to the client size of this canvas.
// This is done here rather than in a wxSizeEvent handler because our
@@ -383,7 +384,7 @@ void EDA_3D_MODEL_VIEWER::OnPaint( wxPaintEvent& event )
// commands is displayed on the window."
SwapBuffers();
- GL_CONTEXT_MANAGER::Get().UnlockCtx( m_glRC );
+ Pgm().GetGLContextManager()->UnlockCtx( m_glRC );
}
diff --git a/3d-viewer/3d_rendering/opengl/render_3d_opengl.cpp b/3d-viewer/3d_rendering/opengl/render_3d_opengl.cpp
index d776a6e93d..dc6ea15fd5 100644
--- a/3d-viewer/3d_rendering/opengl/render_3d_opengl.cpp
+++ b/3d-viewer/3d_rendering/opengl/render_3d_opengl.cpp
@@ -36,6 +36,7 @@
#include <3d_math.h>
#include <glm/geometric.hpp>
#include <lset.h>
+#include <pgm_base.h>
#include <math/util.h> // for KiROUND
#include <utility>
#include <vector>
@@ -490,7 +491,7 @@ bool RENDER_3D_OPENGL::Redraw( bool aIsMoving, REPORTER* aStatusReporter,
// Careful here!
// We are in the middle of rendering and the reload method may show
// a dialog box that requires the opengl context for a redraw
- GL_CONTEXT_MANAGER::Get().RunWithoutCtxLock( [this, aStatusReporter, aWarningReporter]()
+ Pgm().GetGLContextManager()->RunWithoutCtxLock( [this, aStatusReporter, aWarningReporter]()
{
reload( aStatusReporter, aWarningReporter );
} );
diff --git a/common/gal/opengl/gl_context_mgr.cpp b/common/gal/opengl/gl_context_mgr.cpp
index 2a4d22d964..83ca6f60f3 100644
--- a/common/gal/opengl/gl_context_mgr.cpp
+++ b/common/gal/opengl/gl_context_mgr.cpp
@@ -27,14 +27,6 @@
#include <wx/debug.h>
-GL_CONTEXT_MANAGER& GL_CONTEXT_MANAGER::Get()
-{
- static GL_CONTEXT_MANAGER instance;
-
- return instance;
-}
-
-
wxGLContext* GL_CONTEXT_MANAGER::CreateCtx( wxGLCanvas* aCanvas, const wxGLContext* aOther )
{
wxGLContext* context = new wxGLContext( aCanvas, aOther );
@@ -124,9 +116,3 @@ void GL_CONTEXT_MANAGER::UnlockCtx( wxGLContext* aContext )
}
}
-
-GL_CONTEXT_MANAGER::GL_CONTEXT_MANAGER()
- : m_glCtx( nullptr )
-{
-}
-
diff --git a/common/gal/opengl/opengl_gal.cpp b/common/gal/opengl/opengl_gal.cpp
index ef063e1a9c..62b8904d7b 100644
--- a/common/gal/opengl/opengl_gal.cpp
+++ b/common/gal/opengl/opengl_gal.cpp
@@ -43,6 +43,7 @@
#include <bitmap_base.h>
#include <bezier_curves.h>
#include <math/util.h> // for KiROUND
+#include <pgm_base.h>
#include <trace_helpers.h>
#include <wx/frame.h>
@@ -332,7 +333,7 @@ OPENGL_GAL::OPENGL_GAL( const KIGFX::VC_SETTINGS& aVcSettings, GAL_DISPLAY_OPTIO
{
if( m_glMainContext == nullptr )
{
- m_glMainContext = GL_CONTEXT_MANAGER::Get().CreateCtx( this );
+ m_glMainContext = Pgm().GetGLContextManager()->CreateCtx( this );
if( !m_glMainContext )
throw std::runtime_error( "Could not create the main OpenGL context" );
@@ -341,7 +342,7 @@ OPENGL_GAL::OPENGL_GAL( const KIGFX::VC_SETTINGS& aVcSettings, GAL_DISPLAY_OPTIO
}
else
{
- m_glPrivContext = GL_CONTEXT_MANAGER::Get().CreateCtx( this, m_glMainContext );
+ m_glPrivContext = Pgm().GetGLContextManager()->CreateCtx( this, m_glMainContext );
if( !m_glPrivContext )
throw std::runtime_error( "Could not create a private OpenGL context" );
@@ -421,7 +422,9 @@ OPENGL_GAL::OPENGL_GAL( const KIGFX::VC_SETTINGS& aVcSettings, GAL_DISPLAY_OPTIO
OPENGL_GAL::~OPENGL_GAL()
{
- GL_CONTEXT_MANAGER::Get().LockCtx( m_glPrivContext, this );
+
+ GL_CONTEXT_MANAGER* gl_mgr = Pgm().GetGLContextManager();
+ gl_mgr->LockCtx( m_glPrivContext, this );
--m_instanceCounter;
glFlush();
@@ -438,19 +441,19 @@ OPENGL_GAL::~OPENGL_GAL()
delete m_tempManager;
}
- GL_CONTEXT_MANAGER::Get().UnlockCtx( m_glPrivContext );
+ gl_mgr->UnlockCtx( m_glPrivContext );
// If it was the main context, then it will be deleted
// when the last OpenGL GAL instance is destroyed (a few lines below)
if( m_glPrivContext != m_glMainContext )
- GL_CONTEXT_MANAGER::Get().DestroyCtx( m_glPrivContext );
+ gl_mgr->DestroyCtx( m_glPrivContext );
delete m_shader;
// Are we destroying the last GAL instance?
if( m_instanceCounter == 0 )
{
- GL_CONTEXT_MANAGER::Get().LockCtx( m_glMainContext, this );
+ gl_mgr->LockCtx( m_glMainContext, this );
if( m_isBitmapFontLoaded )
{
@@ -458,8 +461,8 @@ OPENGL_GAL::~OPENGL_GAL()
m_isBitmapFontLoaded = false;
}
- GL_CONTEXT_MANAGER::Get().UnlockCtx( m_glMainContext );
- GL_CONTEXT_MANAGER::Get().DestroyCtx( m_glMainContext );
+ gl_mgr->UnlockCtx( m_glMainContext );
+ gl_mgr->DestroyCtx( m_glMainContext );
m_glMainContext = nullptr;
}
}
@@ -777,7 +780,7 @@ void OPENGL_GAL::LockContext( int aClientCookie )
m_isContextLocked = true;
m_lockClientCookie = aClientCookie;
- GL_CONTEXT_MANAGER::Get().LockCtx( m_glPrivContext, this );
+ Pgm().GetGLContextManager()->LockCtx( m_glPrivContext, this );
}
@@ -791,7 +794,7 @@ void OPENGL_GAL::UnlockContext( int aClientCookie )
m_isContextLocked = false;
- GL_CONTEXT_MANAGER::Get().UnlockCtx( m_glPrivContext );
+ Pgm().GetGLContextManager()->UnlockCtx( m_glPrivContext );
}
diff --git a/include/gal/opengl/gl_context_mgr.h b/include/gal/opengl/gl_context_mgr.h
index 88398f1764..96a8d2f543 100644
--- a/include/gal/opengl/gl_context_mgr.h
+++ b/include/gal/opengl/gl_context_mgr.h
@@ -27,19 +27,17 @@
#ifndef GL_CONTEXT_MANAGER_H
#define GL_CONTEXT_MANAGER_H
-#include <import_export.h>
+#include <kicommon.h>
#include <gal/gal.h>
#include <wx/glcanvas.h>
#include <mutex>
#include <map>
-class APIEXPORT GL_CONTEXT_MANAGER
+class KICOMMON_API GL_CONTEXT_MANAGER
{
public:
- /**
- * Return the GL_CONTEXT_MANAGER instance (singleton).
- */
- static GL_CONTEXT_MANAGER& Get();
+
+ GL_CONTEXT_MANAGER() : m_glCtx( nullptr ) {}
/**
* Create a managed OpenGL context.
@@ -49,7 +47,7 @@ public:
*
* @return Created OpenGL context.
*/
- APIEXPORT wxGLContext* CreateCtx( wxGLCanvas* aCanvas, const wxGLContext* aOther = nullptr );
+ wxGLContext* CreateCtx( wxGLCanvas* aCanvas, const wxGLContext* aOther = nullptr );
/**
* Destroy a managed OpenGL context.
@@ -58,14 +56,14 @@ public:
*
* @param aContext is the OpenGL context to be destroyed. It will not be managed anymore.
*/
- APIEXPORT void DestroyCtx( wxGLContext* aContext );
+ void DestroyCtx( wxGLContext* aContext );
/**
* Destroy all managed OpenGL contexts.
*
* This method should be called in the final deinitialization routine.
*/
- APIEXPORT void DeleteAll();
+ void DeleteAll();
/**
* Set a context as current and prevents other canvases from switching it.
@@ -77,7 +75,7 @@ public:
* @param aCanvas (optional) allows caller to bind the context to a non-parent canvas
* (e.g. when a few canvases share a single GL context).
*/
- APIEXPORT void LockCtx( wxGLContext* aContext, wxGLCanvas* aCanvas );
+ void LockCtx( wxGLContext* aContext, wxGLCanvas* aCanvas );
/**
* Allow other canvases to bind an OpenGL context.
@@ -85,14 +83,14 @@ public:
* @param aContext is the currently bound context. It is only a check to assure the right
* canvas wants to unlock GL context.
*/
- APIEXPORT void UnlockCtx( wxGLContext* aContext );
+ void UnlockCtx( wxGLContext* aContext );
/**
* Get the currently bound GL context.
*
* @return the currently bound GL context.
*/
- APIEXPORT wxGLContext* GetCurrentCtx() const
+ wxGLContext* GetCurrentCtx() const
{
return m_glCtx;
}
@@ -102,7 +100,7 @@ public:
*
* @return the currently bound GL canvas.
*/
- APIEXPORT wxGLCanvas* GetCurrentCanvas() const
+ wxGLCanvas* GetCurrentCanvas() const
{
auto it = m_glContexts.find( m_glCtx );
return it != m_glContexts.end() ? it->second : nullptr;
@@ -114,7 +112,7 @@ public:
* @param aFunction is the function to be executed.
*/
template<typename Func, typename... Args>
- APIEXPORT auto RunWithoutCtxLock( Func&& aFunction, Args&&... args )
+ auto RunWithoutCtxLock( Func&& aFunction, Args&&... args )
{
wxGLContext* currentCtx = GetCurrentCtx();
wxGLCanvas* currentCanvas = GetCurrentCanvas();
@@ -143,11 +141,6 @@ private:
///< Lock to prevent unexpected GL context switching.
std::mutex m_glCtxMutex;
-
- // Singleton
- GL_CONTEXT_MANAGER();
- GL_CONTEXT_MANAGER( const GL_CONTEXT_MANAGER& );
- void operator=( const GL_CONTEXT_MANAGER& );
};
#endif /* GL_CONTEXT_MANAGER_H */
diff --git a/include/pgm_base.h b/include/pgm_base.h
index e43906fca9..2952ce5f90 100644
--- a/include/pgm_base.h
+++ b/include/pgm_base.h
@@ -112,6 +112,8 @@ public:
BS::thread_pool& GetThreadPool() { return *m_singleton.m_ThreadPool; }
+ GL_CONTEXT_MANAGER* GetGLContextManager() { return m_singleton.m_GLContextManager; }
+
/**
* Specific to MacOSX (not used under Linux or Windows).
*
diff --git a/include/singleton.h b/include/singleton.h
index 04e934540b..f24c1454e3 100644
--- a/include/singleton.h
+++ b/include/singleton.h
@@ -35,6 +35,10 @@ public:
delete m_ThreadPool;
m_ThreadPool = nullptr;
+
+ m_GLContextManager->DeleteAll();
+ delete m_GLContextManager;
+ m_GLContextManager = nullptr;
};
@@ -42,9 +46,12 @@ public:
{
int num_threads = std::max( 0, ADVANCED_CFG::GetCfg().m_MaximumThreads );
m_ThreadPool = new BS::thread_pool( num_threads );
+ m_GLContextManager = new GL_CONTEXT_MANAGER();
}
BS::thread_pool* m_ThreadPool;
+ GL_CONTEXT_MANAGER* m_GLContextManager;
+
};