Skip to content

Commit

Permalink
Fix crash in StelTexture destructor on exit
Browse files Browse the repository at this point in the history
On deletion of StelApp StelTextureMgr is deleted, after which some
instances of StelTexture are deleted. But StelTexture destructor logs
VRAM usage via StelTextureMgr, accessing a no longer existing object.

This commit turns the nonstatic raw pointer to texture manager into a
static weak pointer (QPointer), so that it was possible to check whether
texture manager still exists before trying to access it. Making it
static is supposed to prevent useless copying of this smart pointer
while the texture manager is effectively a singleton.

Fixes #3427
  • Loading branch information
10110111 authored and alex-w committed Dec 10, 2023
1 parent 3e8f78a commit fc10e8e
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 10 deletions.
16 changes: 14 additions & 2 deletions src/core/StelTexture.cpp
Expand Up @@ -39,13 +39,19 @@
# define GL_TEXTURE_MAX_ANISOTROPY 0x84FE
#endif

StelTexture::StelTexture(StelTextureMgr *mgr)
: textureMgr(mgr)
QPointer<StelTextureMgr> StelTexture::textureMgr;

StelTexture::StelTexture()
{
}

StelTexture::~StelTexture()
{
// Don't try to access texture manager or OpenGL context if texture
// manager has been deleted, because at this point we are shutting
// down, and the OpenGL context may have also been deleted.
if (!textureMgr) return;

if (id != 0)
{
/// FS: make sure the correct GL context is bound!
Expand Down Expand Up @@ -228,6 +234,7 @@ template <typename T, typename...Params, typename...Args>
void StelTexture::startAsyncLoader(T (*functionPointer)(Params...), Args&&...args)
{
Q_ASSERT(loader==Q_NULLPTR);
if (!textureMgr) return;
//own thread pool supported with Qt 5.4+
loader = new QFuture<GLData>(QtConcurrent::run(textureMgr->loaderThreadPool,
functionPointer, std::forward<Args>(args)...));
Expand Down Expand Up @@ -379,6 +386,11 @@ QByteArray StelTexture::convertToGLFormat(QImage image, GLint& format, GLint& ty

bool StelTexture::glLoad(const GLData& data)
{
// Don't try to access texture manager or OpenGL context if texture
// manager has been deleted, because at this point we are shutting
// down, and the OpenGL context may have also been deleted.
if (!textureMgr) return false;

if (data.data.isEmpty())
{
reportError(data.loaderError.isEmpty() ? "Unknown error" : data.loaderError);
Expand Down
5 changes: 3 additions & 2 deletions src/core/StelTexture.hpp
Expand Up @@ -22,6 +22,7 @@

#include <QOpenGLFunctions>
#include <QSharedPointer>
#include <QPointer>
#include <QObject>
#include <QImage>

Expand Down Expand Up @@ -143,7 +144,7 @@ private slots:
static GLData loadFromData( const QByteArray &data, const int decimateBy);

//! Private constructor
StelTexture(StelTextureMgr* mgr);
StelTexture();

//! Wrap an existing GL texture with this object
void wrapGLTexture(GLuint texId);
Expand Down Expand Up @@ -173,7 +174,7 @@ private slots:
void startAsyncLoader(T (*functionPointer)(Params...), Args&&...args);

//! The parent texture manager
StelTextureMgr* textureMgr = nullptr;
static QPointer<StelTextureMgr> textureMgr;

QOpenGLFunctions* gl = nullptr;
StelTextureParams loadParams;
Expand Down
11 changes: 6 additions & 5 deletions src/core/StelTextureMgr.cpp
Expand Up @@ -46,6 +46,7 @@ StelTextureMgr::StelTextureMgr(QObject *parent)
ctx->functions()->glGetIntegerv(GL_MAX_TEXTURE_SIZE, &maxTexSize);
if (maxTexSize<8192)
qDebug() << "Max texture size:" << maxTexSize;
StelTexture::textureMgr = this;
}

StelTextureSP StelTextureMgr::createTexture(const QString& afilename, const StelTexture::StelTextureParams& params)
Expand All @@ -67,7 +68,7 @@ StelTextureSP StelTextureMgr::createTexture(const QString& afilename, const Stel
StelTextureSP cache = lookupCache(canPath);
if(!cache.isNull()) return cache;

StelTextureSP tex = StelTextureSP(new StelTexture(this));
StelTextureSP tex = StelTextureSP(new StelTexture);
tex->fullPath = canPath;

QImage image(tex->fullPath);
Expand Down Expand Up @@ -159,7 +160,7 @@ StelTextureSP StelTextureMgr::createTextureThread(const QString& url, const Stel
StelTextureSP cache = lookupCache(canPath);
if(!cache.isNull()) return cache;

StelTextureSP tex = StelTextureSP(new StelTexture(this));
StelTextureSP tex = StelTextureSP(new StelTexture);
tex->loadParams = params;
tex->fullPath = canPath;
if (!lazyLoading)
Expand All @@ -175,7 +176,7 @@ StelTextureSP StelTextureMgr::createTextureThread(const QString& url, const Stel
//! Create a texture from a QImage.
StelTextureSP StelTextureMgr::createTexture(const QImage &image, const StelTexture::StelTextureParams& params)
{
StelTextureSP tex = StelTextureSP(new StelTexture(this));
StelTextureSP tex = StelTextureSP(new StelTexture);
tex->loadParams = params;
bool r = tex->glLoad(image);
Q_ASSERT(r);
Expand All @@ -202,7 +203,7 @@ StelTextureSP StelTextureMgr::wrapperForGLTexture(GLuint texId)


//no existing tex with this ID found, create a new wrapper
StelTextureSP newTex(new StelTexture(this));
StelTextureSP newTex(new StelTexture);
newTex->wrapGLTexture(texId);
if(!newTex->errorOccured)
{
Expand Down Expand Up @@ -230,7 +231,7 @@ StelTextureSP StelTextureMgr::getDitheringTexture(const int samplerToBindTo)

const auto texId = ForTextureMgr::makeDitherPatternTexture(gl);

ditheringTexture.reset(new StelTexture(this));
ditheringTexture.reset(new StelTexture);
ditheringTexture->wrapGLTexture(texId);
if(!ditheringTexture->errorOccured)
{
Expand Down
2 changes: 1 addition & 1 deletion src/core/StelTextureMgr.hpp
Expand Up @@ -34,7 +34,7 @@ class QThreadPool;
//! @class StelTextureMgr
//! Manage textures loading.
//! It provides method for loading images in a separate thread.
class StelTextureMgr : QObject
class StelTextureMgr : public QObject
{
Q_OBJECT
public:
Expand Down

0 comments on commit fc10e8e

Please sign in to comment.