From 72ce5228130a83e26f8be6c47a82e200db52de0b Mon Sep 17 00:00:00 2001
From: Jonas Herzig <jonas@spark-squared.com>
Date: Thu, 7 May 2020 13:42:06 +0200
Subject: [PATCH] Ensure task queue is not locked during rendering (fixes #157)

Similar to 3fedc22 in effect and cause. However tasks are executed, they just
cannot be submitted pre-1.14 because MC would continuously hold the lock on the
scheduledTasks queue for the entire renderVideo call.
---
 .../java/com/replaymod/core/ReplayMod.java    | 31 ++++++++++++++-----
 .../replaymod/render/gui/GuiRenderQueue.java  |  4 +--
 .../render/gui/GuiRenderSettings.java         |  2 +-
 3 files changed, 27 insertions(+), 10 deletions(-)

diff --git a/src/main/java/com/replaymod/core/ReplayMod.java b/src/main/java/com/replaymod/core/ReplayMod.java
index 9e6710cc..3b0bff95 100644
--- a/src/main/java/com/replaymod/core/ReplayMod.java
+++ b/src/main/java/com/replaymod/core/ReplayMod.java
@@ -475,13 +475,30 @@ public class ReplayMod implements
     public final ReplayModExecutor executor = new ReplayModExecutor("Client/ReplayMod");
     //#endif
 
+    // Pre-1.14 MC would hold the lock on the scheduledTasks queue while executing its tasks
+    // such that no new tasks could be submitted while one of them was running.
+    // This would cause issues with long-running tasks (e.g. video rendering) as it would
+    // block all async tasks (e.g. skin loading).
+    public void runLaterWithoutLock(Runnable runnable) {
+        //#if MC>=11400
+        // MC 1.14+ no longer synchronizes on the queue while running its tasks
+        runLater(runnable);
+        //#else
+        //$$ runLater(() -> runLaterWithoutLock(runnable), runnable);
+        //#endif
+    }
+
     public void runLater(Runnable runnable) {
+        runLater(runnable, () -> runLater(runnable));
+    }
+
+    private void runLater(Runnable runnable, Runnable defer) {
         //#if MC>=11400
         if (mc.isOnThread() && inRunLater && !inRenderTaskQueue) {
             ((MinecraftAccessor) mc).getRenderTaskQueue().offer(() -> {
                 inRenderTaskQueue = true;
                 try {
-                    runLater(runnable);
+                    defer.run();
                 } finally {
                     inRenderTaskQueue = false;
                 }
@@ -503,13 +520,13 @@ public class ReplayMod implements
             //$$     @SubscribeEvent
             //$$     public void onRenderTick(TickEvent.RenderTickEvent event) {
             //$$         if (event.phase == TickEvent.Phase.START) {
-            //$$             runLater(runnable);
             //$$             FORGE_BUS.unregister(this);
+            //$$             defer.run();
             //$$         }
             //$$     }
             //$$ });
             //#else
-            //$$ FML_BUS.register(new RunLaterHelper(runnable));
+            //$$ FML_BUS.register(new RunLaterHelper(defer));
             //#endif
         //$$     return;
         //$$ }
@@ -539,17 +556,17 @@ public class ReplayMod implements
     //$$
     //$$ // in 1.7.10 apparently events can't be delivered to anonymous classes
     //$$ public class RunLaterHelper {
-    //$$     private final Runnable runnable;
+    //$$     private final Runnable defer;
     //$$
-    //$$     private RunLaterHelper(Runnable runnable) {
-    //$$         this.runnable = runnable;
+    //$$     private RunLaterHelper(Runnable defer) {
+    //$$         this.defer = defer;
     //$$     }
     //$$
     //$$     @SubscribeEvent
     //$$     public void onRenderTick(TickEvent.RenderTickEvent event) {
     //$$         if (event.phase == TickEvent.Phase.START) {
-    //$$             runLater(runnable);
     //$$             FML_BUS.unregister(this);
+    //$$             defer.run();
     //$$         }
     //$$     }
     //$$ }
diff --git a/src/main/java/com/replaymod/render/gui/GuiRenderQueue.java b/src/main/java/com/replaymod/render/gui/GuiRenderQueue.java
index 9fed269a..31877d3d 100644
--- a/src/main/java/com/replaymod/render/gui/GuiRenderQueue.java
+++ b/src/main/java/com/replaymod/render/gui/GuiRenderQueue.java
@@ -197,7 +197,7 @@ public class GuiRenderQueue extends AbstractGuiPopup<GuiRenderQueue> {
 
         renderButton.onClick(() -> {
             LOGGER.trace("Render button clicked");
-            ReplayMod.instance.runLater(() -> processQueue(queue));
+            ReplayMod.instance.runLaterWithoutLock(() -> processQueue(queue));
         });
 
         updateButtons();
@@ -233,7 +233,7 @@ public class GuiRenderQueue extends AbstractGuiPopup<GuiRenderQueue> {
                     // Update current job with fixed ffmpeg arguments
                     renderJob.setSettings(newSettings);
                     // Restart queue, skipping the already completed jobs
-                    processQueue(Iterables.skip(queue, jobsToSkip));
+                    ReplayMod.instance.runLaterWithoutLock(() -> processQueue(Iterables.skip(queue, jobsToSkip)));
                 });
                 return;
             } catch (Throwable t) {
diff --git a/src/main/java/com/replaymod/render/gui/GuiRenderSettings.java b/src/main/java/com/replaymod/render/gui/GuiRenderSettings.java
index 90a25f21..30c2f1fb 100644
--- a/src/main/java/com/replaymod/render/gui/GuiRenderSettings.java
+++ b/src/main/java/com/replaymod/render/gui/GuiRenderSettings.java
@@ -229,7 +229,7 @@ public class GuiRenderSettings extends GuiScreen implements Closeable {
             new GuiRenderQueue(GuiRenderSettings.this, GuiRenderSettings.this, replayHandler, timeline).open();
         }
     }).setSize(100, 20).setI18nLabel("replaymod.gui.renderqueue.open");
-    public final GuiButton renderButton = new GuiButton(buttonPanel).onClick(() -> ReplayMod.instance.runLater(new Runnable() {
+    public final GuiButton renderButton = new GuiButton(buttonPanel).onClick(() -> ReplayMod.instance.runLaterWithoutLock(new Runnable() {
         @Override
         public void run() {
             // Closing this GUI ensures that settings are saved