From 2130f1799bf30ba90bc91a35f9aa70d545916726 Mon Sep 17 00:00:00 2001
From: itsmattkc <34096995+itsmattkc@users.noreply.github.com>
Date: Thu, 11 Aug 2022 10:03:17 -0700
Subject: [PATCH] renderer: add mutexes around texture cache

---
 app/render/renderer.cpp | 25 ++++++++++++++++++++++++-
 app/render/renderer.h   |  2 ++
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/app/render/renderer.cpp b/app/render/renderer.cpp
index 85f3aed69..e6754e5c7 100644
--- a/app/render/renderer.cpp
+++ b/app/render/renderer.cpp
@@ -21,6 +21,7 @@
 #include "renderer.h"
 
 #include <QDateTime>
+#include <QThread>
 #include <QTimer>
 #include <QVector2D>
 
@@ -36,6 +37,7 @@ TexturePtr Renderer::CreateTexture(const VideoParams &params, const void *data,
   QVariant v;
 
   if (USE_TEXTURE_CACHE) {
+    QMutexLocker locker(&texture_cache_lock_);
     for (auto it=texture_cache_.begin(); it!=texture_cache_.end(); it++) {
       if (it->width == params.effective_width()
           && it->height == params.effective_height()
@@ -64,6 +66,22 @@ TexturePtr Renderer::CreateTexture(const VideoParams &params, const void *data,
 void Renderer::DestroyTexture(Texture *texture)
 {
   if (USE_TEXTURE_CACHE) {
+    // HACK: Dirty, dirty hack. OpenGL uses "contexts" to store all of its data, and each context
+    //       can only be used by the thread that created it. However there are also "shared contexts"
+    //       where assets from one context can be used in another. We use shared contexts so that
+    //       textures rendered in the background can be displayed on the screen, travelling from
+    //       a background thread to the main UI thread. However, when that texture is destroyed, it
+    //       comes back here to be placed in the texture cache. But that leads to a race condition
+    //       because it will call the background thread's renderer in the main thread. Since all
+    //       assets are shared, we could technically just get the texture to call "destroy" in the
+    //       viewer's renderer instance, but that would mean all textures would end up stranded
+    //       there unusable by the background renderer, negating the very advantage of the texture
+    //       cache in the first place. Therefore, we simply allow the thread calling to happen, and
+    //       use mutexes to prevent race conditions.
+    //
+    //       Presumably Vulkan would not have this issue because it allows for application-wide
+    //       instances and multithreading.
+    texture_cache_lock_.lock();
     texture_cache_.push_back({texture->params().effective_width(),
                               texture->params().effective_height(),
                               texture->params().effective_depth(),
@@ -71,8 +89,11 @@ void Renderer::DestroyTexture(Texture *texture)
                               texture->params().channel_count(),
                               texture->id(),
                               QDateTime::currentMSecsSinceEpoch()});
+    texture_cache_lock_.unlock();
 
-    ClearOldTextures();
+    if (QThread::currentThread() == this->thread()) {
+      ClearOldTextures();
+    }
   } else {
     DestroyNativeTexture(texture->id());
   }
@@ -249,6 +270,8 @@ bool Renderer::GetColorContext(const ColorTransformJob &color_job, Renderer::Col
 
 void Renderer::ClearOldTextures()
 {
+  QMutexLocker locker(&texture_cache_lock_);
+
   for (auto it=texture_cache_.begin(); it!=texture_cache_.end(); ) {
     if (it->accessed < QDateTime::currentMSecsSinceEpoch() - MAX_TEXTURE_LIFE) {
       DestroyNativeTexture(it->handle);
diff --git a/app/render/renderer.h b/app/render/renderer.h
index 96141df4e..76651c05c 100644
--- a/app/render/renderer.h
+++ b/app/render/renderer.h
@@ -155,6 +155,8 @@ private:
 
   QVariant interlace_texture_;
 
+  QMutex texture_cache_lock_;
+
 };
 
 }