Fix texture deletion without a GL context (issue #229)

Texture objects are usually destroyed (e.g. when a source is removed) while
no OpenGL context is current, so calling glDeleteTextures() in ~Texture() was
unsafe: depending on the driver it leaked the texture or could delete an
unrelated one in whatever context happened to be current.

Defer the deletion instead. ~Texture() now queues the id, and the renderer
frees all queued ids via Texture::deleteOrphanedTextures() at the start of
painting, when the shared GL context is guaranteed to be current. Affects all
texture sources (Video, Image, Syphon).

Author: aalex

src/core/Source.cpp

@@ -32,6 +32,24 @@ namespace mmp {
 
 UidAllocator Source::allocator;
 
+QVector<GLuint> Texture::_orphanedTextures;
+QMutex          Texture::_orphanedTexturesMutex;
+
+void Texture::orphanTexture(GLuint id)
+{
+  QMutexLocker locker(&_orphanedTexturesMutex);
+  _orphanedTextures.append(id);
+}
+
+void Texture::deleteOrphanedTextures()
+{
+  QMutexLocker locker(&_orphanedTexturesMutex);
+  if (_orphanedTextures.isEmpty())
+    return;
+  glDeleteTextures((GLsizei) _orphanedTextures.size(), _orphanedTextures.constData());
+  _orphanedTextures.clear();
+}
+
 void Texture::update()
 {
   if (textureId == 0)

src/core/Source.h

@@ -28,6 +28,7 @@
 #include <QColor>
 #include <QElapsedTimer>
 #include <QMutex>
+#include <QVector>
 
 #if __APPLE__
 #include <OpenGL/gl.h>
@@ -169,12 +170,22 @@ class Texture : public Source
 
 public:
   virtual ~Texture() {
-    // TODO: this needs to be fixed: it will not work unless it is executed from within a GL context
-    // see issue #229
+    // A Texture is usually destroyed (e.g. when its source is removed) when no
+    // GL context is current, so we cannot call glDeleteTextures here (issue
+    // #229). Instead the id is queued and freed by deleteOrphanedTextures(),
+    // which the renderer calls while a context is current.
     if (textureId != 0)
-      glDeleteTextures(1, &textureId);
+      orphanTexture(textureId);
   }
 
+  /// Frees textures queued by destroyed Texture objects. MUST be called with a
+  /// current GL context (the renderer calls it at the start of painting).
+  static void deleteOrphanedTextures();
+
+protected:
+  /// Queues a texture id for deferred deletion (see issue #229).
+  static void orphanTexture(GLuint id);
+
 public:
   virtual void update();
 
@@ -220,6 +231,12 @@ class Texture : public Source
 protected:
   // Lists QProperties that should NOT be parsed automatically.
   virtual QList<QString> _propertiesSpecial() const { return Source::_propertiesSpecial() << "x" << "y"; }
+
+private:
+  // Texture ids whose Texture objects were destroyed without a current GL
+  // context; freed later by deleteOrphanedTextures(). See issue #229.
+  static QVector<GLuint> _orphanedTextures;
+  static QMutex          _orphanedTexturesMutex;
 };
 
 /**

src/gui/ShapeGraphicsItem.cpp

@@ -264,6 +264,10 @@ void TextureGraphicsItem::_prePaint(QPainter* painter,
   Q_UNUSED(option);
   painter->beginNativePainting();
 
+  // Free any textures orphaned by destroyed sources now that a GL context is
+  // current (see issue #229).
+  Texture::deleteOrphanedTextures();
+
   // Project source texture and sent it to destination.
   texture->update();