From 5326c36a5ff8a3a465d386cc31172aa90847a9e9 Mon Sep 17 00:00:00 2001
From: Seth Hillbrand <seth@kipro-pcb.com>
Date: Fri, 3 Jan 2025 17:24:24 -0800
Subject: [PATCH] Move GL Context into Singleton class
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 | 18 ++++++++--------
.../3d_model_viewer/eda_3d_model_viewer.cpp | 12 +++++------
.../3d_rendering/opengl/render_3d_opengl.cpp | 15 +++++++------
common/gal/opengl/gl_context_mgr.cpp | 8 -------
common/gal/opengl/opengl_gal.cpp | 21 ++++++++++---------
eeschema/CMakeLists.txt | 3 +++
gerbview/CMakeLists.txt | 1 +
include/gal/opengl/gl_context_mgr.h | 11 ++--------
include/pgm_base.h | 2 ++
include/singleton.h | 10 ++++++++-
pagelayout_editor/CMakeLists.txt | 1 +
pcb_calculator/CMakeLists.txt | 1 +
pcbnew/CMakeLists.txt | 1 +
13 files changed, 55 insertions(+), 49 deletions(-)
diff --git a/3d-viewer/3d_canvas/eda_3d_canvas.cpp b/3d-viewer/3d_canvas/eda_3d_canvas.cpp
index 0214ac0999..5cdb68bec4 100644
--- a/3d-viewer/3d_canvas/eda_3d_canvas.cpp
+++ b/3d-viewer/3d_canvas/eda_3d_canvas.cpp
@@ -187,7 +187,7 @@ void EDA_3D_CANVAS::releaseOpenGL()
{
if( m_glRC )
{
- GL_CONTEXT_MANAGER::Get().LockCtx( m_glRC, this );
+ Pgm().GetGLContextManager()->LockCtx( m_glRC, this );
delete m_3d_render_raytracing;
m_3d_render_raytracing = nullptr;
@@ -198,8 +198,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 );
+ Pgm().GetGLContextManager()->UnlockCtx( m_glRC );
+ Pgm().GetGLContextManager()->DestroyCtx( m_glRC );
m_glRC = nullptr;
}
}
@@ -390,7 +390,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 = Pgm().GetGLContextManager()->CreateCtx( this );
// CreateCtx could and does fail per sentry crash events, lets be graceful
if( m_glRC == nullptr )
@@ -401,7 +401,7 @@ void EDA_3D_CANVAS::DoRePaint()
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
@@ -418,7 +418,7 @@ void EDA_3D_CANVAS::DoRePaint()
{
if( !initializeOpenGL() )
{
- GL_CONTEXT_MANAGER::Get().UnlockCtx( m_glRC );
+ Pgm().GetGLContextManager()->UnlockCtx( m_glRC );
m_is_currently_painting.clear();
return;
@@ -440,7 +440,7 @@ void EDA_3D_CANVAS::DoRePaint()
SwapBuffers();
- GL_CONTEXT_MANAGER::Get().UnlockCtx( m_glRC );
+ Pgm().GetGLContextManager()->UnlockCtx( m_glRC );
m_is_currently_painting.clear();
return;
@@ -528,7 +528,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 );
+ Pgm().GetGLContextManager()->UnlockCtx( m_glRC );
m_is_currently_painting.clear();
return;
}
@@ -553,7 +553,7 @@ void EDA_3D_CANVAS::DoRePaint()
// commands is displayed on the window."
SwapBuffers();
- GL_CONTEXT_MANAGER::Get().UnlockCtx( m_glRC );
+ Pgm().GetGLContextManager()->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..584ee6a3fd 100644
--- a/3d-viewer/3d_model_viewer/eda_3d_model_viewer.cpp
+++ b/3d-viewer/3d_model_viewer/eda_3d_model_viewer.cpp
@@ -112,13 +112,13 @@ EDA_3D_MODEL_VIEWER::~EDA_3D_MODEL_VIEWER()
if( m_glRC )
{
- GL_CONTEXT_MANAGER::Get().LockCtx( m_glRC, this );
+ Pgm().GetGLContextManager()->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 );
+ Pgm().GetGLContextManager()->UnlockCtx( m_glRC );
+ Pgm().GetGLContextManager()->DestroyCtx( m_glRC );
}
}
@@ -256,7 +256,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 +265,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 +383,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..c7656b08aa 100644
--- a/3d-viewer/3d_rendering/opengl/render_3d_opengl.cpp
+++ b/3d-viewer/3d_rendering/opengl/render_3d_opengl.cpp
@@ -26,22 +26,25 @@
#include <cstdint>
#include <gal/opengl/kiglew.h> // Must be included first
-#include "plugins/3dapi/xv3d_types.h"
-#include "render_3d_opengl.h"
#include "opengl_utils.h"
-#include "common_ogl/ogl_utils.h"
+#include "render_3d_opengl.h"
+
+#include <3d_math.h>
+#include <common_ogl/ogl_utils.h>
+#include <plugins/3dapi/xv3d_types.h>
+
+#include <base_units.h>
#include <board.h>
#include <footprint.h>
#include <gal/opengl/gl_context_mgr.h>
-#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>
#include <wx/log.h>
-#include <base_units.h>
/**
* Scale conversion from 3d model units to pcb units
@@ -490,7 +493,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..c4fb3d9a00 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 );
diff --git a/common/gal/opengl/opengl_gal.cpp b/common/gal/opengl/opengl_gal.cpp
index ab6bfbea03..4fdcae364f 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>
@@ -331,7 +332,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" );
@@ -340,7 +341,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" );
@@ -420,7 +421,7 @@ OPENGL_GAL::OPENGL_GAL( const KIGFX::VC_SETTINGS& aVcSettings, GAL_DISPLAY_OPTIO
OPENGL_GAL::~OPENGL_GAL()
{
- GL_CONTEXT_MANAGER::Get().LockCtx( m_glPrivContext, this );
+ Pgm().GetGLContextManager()->LockCtx( m_glPrivContext, this );
--m_instanceCounter;
glFlush();
@@ -437,19 +438,19 @@ OPENGL_GAL::~OPENGL_GAL()
delete m_tempManager;
}
- GL_CONTEXT_MANAGER::Get().UnlockCtx( m_glPrivContext );
+ Pgm().GetGLContextManager()->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 );
+ Pgm().GetGLContextManager()->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 );
+ Pgm().GetGLContextManager()->LockCtx( m_glMainContext, this );
if( m_isBitmapFontLoaded )
{
@@ -457,8 +458,8 @@ OPENGL_GAL::~OPENGL_GAL()
m_isBitmapFontLoaded = false;
}
- GL_CONTEXT_MANAGER::Get().UnlockCtx( m_glMainContext );
- GL_CONTEXT_MANAGER::Get().DestroyCtx( m_glMainContext );
+ Pgm().GetGLContextManager()->UnlockCtx( m_glMainContext );
+ Pgm().GetGLContextManager()->DestroyCtx( m_glMainContext );
m_glMainContext = nullptr;
}
}
@@ -776,7 +777,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 );
}
@@ -790,7 +791,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/eeschema/CMakeLists.txt b/eeschema/CMakeLists.txt
index d58fe5ba5e..d291ce6d56 100644
--- a/eeschema/CMakeLists.txt
+++ b/eeschema/CMakeLists.txt
@@ -33,6 +33,7 @@ include_directories(
${CMAKE_SOURCE_DIR}/common
${CMAKE_SOURCE_DIR}/common/dialogs
${CMAKE_SOURCE_DIR}/libs/sexpr/include
+
${INC_AFTER}
./dialogs
./libview
@@ -565,6 +566,7 @@ set_source_files_properties( ${CMAKE_SOURCE_DIR}/common/single_top.cpp PROPERTIE
target_link_libraries( eeschema
kicommon
+ gal
${wxWidgets_LIBRARIES}
)
@@ -603,6 +605,7 @@ add_library( eeschema_kiface MODULE
target_link_libraries( eeschema_kiface
PRIVATE
common
+ gal
eeschema_kiface_objects
markdown_lib
scripting
diff --git a/gerbview/CMakeLists.txt b/gerbview/CMakeLists.txt
index 0f24c8ceb2..7c28da3808 100644
--- a/gerbview/CMakeLists.txt
+++ b/gerbview/CMakeLists.txt
@@ -112,6 +112,7 @@ set_source_files_properties( ${CMAKE_SOURCE_DIR}/common/single_top.cpp PROPERTIE
target_link_libraries( gerbview
core
kicommon
+ gal
nlohmann_json
${wxWidgets_LIBRARIES}
)
diff --git a/include/gal/opengl/gl_context_mgr.h b/include/gal/opengl/gl_context_mgr.h
index ebf3d358c6..5440c7df13 100644
--- a/include/gal/opengl/gl_context_mgr.h
+++ b/include/gal/opengl/gl_context_mgr.h
@@ -35,10 +35,8 @@
class GAL_API GL_CONTEXT_MANAGER
{
public:
- /**
- * Return the GL_CONTEXT_MANAGER instance (singleton).
- */
- static GL_CONTEXT_MANAGER& Get();
+
+ GL_CONTEXT_MANAGER();
/**
* Create a managed OpenGL context.
@@ -142,11 +140,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 27b9dd74ca..f24c1454e3 100644
--- a/include/singleton.h
+++ b/include/singleton.h
@@ -20,8 +20,9 @@
#ifndef KICAD_SINGLETON_H
#define KICAD_SINGLETON_H
-#include <bs_thread_pool.hpp>
#include <advanced_config.h>
+#include <bs_thread_pool.hpp>
+#include <gal/opengl/gl_context_mgr.h>
class KICAD_SINGLETON
{
@@ -34,6 +35,10 @@ public:
delete m_ThreadPool;
m_ThreadPool = nullptr;
+
+ m_GLContextManager->DeleteAll();
+ delete m_GLContextManager;
+ m_GLContextManager = nullptr;
};
@@ -41,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;
+
};
diff --git a/pagelayout_editor/CMakeLists.txt b/pagelayout_editor/CMakeLists.txt
index f287e5368a..dcbe151d65 100644
--- a/pagelayout_editor/CMakeLists.txt
+++ b/pagelayout_editor/CMakeLists.txt
@@ -82,6 +82,7 @@ set_source_files_properties( ${CMAKE_SOURCE_DIR}/common/single_top.cpp PROPERTIE
)
target_link_libraries( pl_editor
kicommon
+ gal
${wxWidgets_LIBRARIES}
)
diff --git a/pcb_calculator/CMakeLists.txt b/pcb_calculator/CMakeLists.txt
index 8fcf58e46c..e2e1b75390 100644
--- a/pcb_calculator/CMakeLists.txt
+++ b/pcb_calculator/CMakeLists.txt
@@ -96,6 +96,7 @@ set_source_files_properties( ${CMAKE_SOURCE_DIR}/common/single_top.cpp PROPERTIE
)
target_link_libraries( pcb_calculator
kicommon
+ gal
${wxWidgets_LIBRARIES}
)
diff --git a/pcbnew/CMakeLists.txt b/pcbnew/CMakeLists.txt
index 7630594371..a104d3646b 100644
--- a/pcbnew/CMakeLists.txt
+++ b/pcbnew/CMakeLists.txt
@@ -690,6 +690,7 @@ set_source_files_properties( ${CMAKE_SOURCE_DIR}/common/single_top.cpp pcbnew.cp
)
target_link_libraries( pcbnew
kicommon
+ gal
${wxWidgets_LIBRARIES}
)