From bad2ac47f8a6a43df62208e80e114fdd554ed542 Mon Sep 17 00:00:00 2001
From: Eskil Abrahamsen Blomfeldt <eskil.abrahamsen-blomfeldt@qt.io>
Date: Fri, 17 Apr 2020 11:20:46 +0200
Subject: Resolve OpenGL version functions in thread-safe manner

In 73f3f501f331444b3f188b21db7265f723e4f383, the classes were
moved out of Qt Gui, and a mechanism to attach them to the
QOpenGLContext was implemented using a QMap and a connection
on destroyed to delete it.

This solution was not thread-safe, so the suggestion was to either
add a mutex or to make an opaque pointer for the storage in the
(thread-affine) QOpenGLContextPrivate.

I decided to go with the latter. A solution using hash lookups
and mutexes seems to complex when the only benefit is to avoid
forward declarations from Qt Gui to Qt OpenGL in private API.

Especially since this dependency already exists with
the "textureFunctions", which serve the same purpose,
although the destructor is being passed in as an explicit
function pointer there, probably because the ambition was
to use a forward declaration rather than a superclass.

Fixes: QTBUG-82742
Change-Id: I5c6b82c5b33d9cb73ad1ec05d3fc3e87a9eae4cf
Reviewed-by: Laszlo Agocs <laszlo.agocs@qt.io>
---
 src/gui/kernel/qopenglcontext_p.h      | 10 ++++++++++
 src/opengl/qopenglversionfunctions.cpp | 17 +++++------------
 src/opengl/qopenglversionfunctions_p.h | 10 +++++-----
 3 files changed, 20 insertions(+), 17 deletions(-)

diff --git a/src/gui/kernel/qopenglcontext_p.h b/src/gui/kernel/qopenglcontext_p.h
index c017bf9a34..ede41d5a93 100644
--- a/src/gui/kernel/qopenglcontext_p.h
+++ b/src/gui/kernel/qopenglcontext_p.h
@@ -192,6 +192,12 @@ class QPaintEngineEx;
 class QOpenGLFunctions;
 class QOpenGLTextureHelper;
 
+class Q_GUI_EXPORT QOpenGLContextVersionFunctionHelper
+{
+public:
+    virtual ~QOpenGLContextVersionFunctionHelper() {}
+};
+
 class Q_GUI_EXPORT QOpenGLContextPrivate : public QObjectPrivate
 {
     Q_DECLARE_PUBLIC(QOpenGLContext)
@@ -204,6 +210,7 @@ public:
         , surface(nullptr)
         , functions(nullptr)
         , textureFunctions(nullptr)
+        , versionFunctions(nullptr)
         , max_texture_size(-1)
         , workaround_brokenFBOReadBack(false)
         , workaround_brokenTexSubImage(false)
@@ -220,6 +227,8 @@ public:
     {
         //do not delete the QOpenGLContext handle here as it is deleted in
         //QWidgetPrivate::deleteTLSysExtra()
+
+        delete versionFunctions;
     }
 
     QSurfaceFormat requestedFormat;
@@ -232,6 +241,7 @@ public:
     mutable QSet<QByteArray> extensionNames;
     QOpenGLTextureHelper* textureFunctions;
     std::function<void()> textureFunctionsDestroyCallback;
+    QOpenGLContextVersionFunctionHelper *versionFunctions;
 
     GLint max_texture_size;
 
diff --git a/src/opengl/qopenglversionfunctions.cpp b/src/opengl/qopenglversionfunctions.cpp
index 61794fdec9..64d9d11218 100644
--- a/src/opengl/qopenglversionfunctions.cpp
+++ b/src/opengl/qopenglversionfunctions.cpp
@@ -68,19 +68,12 @@ QOpenGLContextVersionData::~QOpenGLContextVersionData()
 
 QOpenGLContextVersionData *QOpenGLContextVersionData::forContext(QOpenGLContext *context)
 {
-    auto *data = contextData.value(context);
-    if (!data) {
-        data = new QOpenGLContextVersionData;
-        // The data will live as long as the context. It could potentially be an opaque pointer
-        // member of QOpenGLContextPrivate, but this avoids polluting QOpenGLContext with version
-        // functions specifics
-        QObject::connect(context, &QObject::destroyed, context, [data](){ delete data; }, Qt::DirectConnection);
-        contextData[context] = data;
-    }
-    return data;
-}
+    QOpenGLContextPrivate *context_d = QOpenGLContextPrivate::get(context);
+    if (context_d->versionFunctions == nullptr)
+        context_d->versionFunctions = new QOpenGLContextVersionData;
 
-QMap<QOpenGLContext *, QOpenGLContextVersionData *> QOpenGLContextVersionData::contextData;
+    return static_cast<QOpenGLContextVersionData *>(context_d->versionFunctions);
+}
 
 #define QT_OPENGL_COUNT_FUNCTIONS(ret, name, args) +1
 #define QT_OPENGL_FUNCTION_NAMES(ret, name, args) \
diff --git a/src/opengl/qopenglversionfunctions_p.h b/src/opengl/qopenglversionfunctions_p.h
index cd1bb480ce..cad4374e27 100644
--- a/src/opengl/qopenglversionfunctions_p.h
+++ b/src/opengl/qopenglversionfunctions_p.h
@@ -62,6 +62,8 @@
 
 #include "qopenglversionfunctions.h"
 
+#include <QtGui/private/qopenglcontext_p.h>
+
 #include <QtOpenGL/qtopenglglobal.h>
 #include <QtOpenGL/QOpenGLVersionProfile>
 #include <QtCore/QSet>
@@ -70,16 +72,14 @@ QT_BEGIN_NAMESPACE
 
 class QAbstractOpenGLFunctions;
 
-class QOpenGLContextVersionData {
+class QOpenGLContextVersionData : public QOpenGLContextVersionFunctionHelper
+{
 public:
     QHash<QOpenGLVersionProfile, QAbstractOpenGLFunctions *> functions;
     QOpenGLVersionFunctionsStorage functionsStorage;
     QSet<QAbstractOpenGLFunctions *> externalFunctions;
-    // TODO: who calls delete?
-    ~QOpenGLContextVersionData();
+    ~QOpenGLContextVersionData() override;
     static QOpenGLContextVersionData *forContext(QOpenGLContext *context);
-private:
-    static QMap<QOpenGLContext *, QOpenGLContextVersionData *> contextData;
 };
 
 QT_END_NAMESPACE
-- 
cgit v1.2.3