From 3c4e57cff51f18c90dbfa51dcd0fa4108b6d5236 Mon Sep 17 00:00:00 2001 From: James Seibel Date: Thu, 18 Jan 2024 19:11:33 -0600 Subject: [PATCH] Move async GL calls to the render thread Hopefully to fix potential crashes on non-Nvidia or less stable GPU drivers --- .../bufferBuilding/ColumnRenderBuffer.java | 2 +- .../core/render/glObject/GLProxy.java | 68 +++++++++++++++---- .../core/render/glObject/buffer/GLBuffer.java | 11 +-- .../core/render/renderer/LodRenderer.java | 5 +- 4 files changed, 67 insertions(+), 19 deletions(-) diff --git a/core/src/main/java/com/seibel/distanthorizons/core/dataObjects/render/bufferBuilding/ColumnRenderBuffer.java b/core/src/main/java/com/seibel/distanthorizons/core/dataObjects/render/bufferBuilding/ColumnRenderBuffer.java index 192890072..86ce44c6e 100644 --- a/core/src/main/java/com/seibel/distanthorizons/core/dataObjects/render/bufferBuilding/ColumnRenderBuffer.java +++ b/core/src/main/java/com/seibel/distanthorizons/core/dataObjects/render/bufferBuilding/ColumnRenderBuffer.java @@ -382,7 +382,7 @@ public class ColumnRenderBuffer extends AbstractRenderBuffer { this.buffersUploaded = false; - GLProxy.getInstance().recordOpenGlCall(() -> + GLProxy.getInstance().queueRunningOnRenderThread(() -> { for (GLVertexBuffer buffer : this.vbos) { diff --git a/core/src/main/java/com/seibel/distanthorizons/core/render/glObject/GLProxy.java b/core/src/main/java/com/seibel/distanthorizons/core/render/glObject/GLProxy.java index 4373120e8..de76d719f 100644 --- a/core/src/main/java/com/seibel/distanthorizons/core/render/glObject/GLProxy.java +++ b/core/src/main/java/com/seibel/distanthorizons/core/render/glObject/GLProxy.java @@ -50,7 +50,6 @@ import java.util.Arrays; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; -import java.util.stream.Stream; /** * A singleton that holds references to different openGL contexts @@ -65,15 +64,11 @@ import java.util.stream.Stream; * * https://gamedev.stackexchange.com/questions/91995/edit-vbo-data-or-create-a-new-one
* https://stackoverflow.com/questions/63509735/massive-performance-loss-with-glmapbuffer

- * - * @author James Seibel */ public class GLProxy { private static final IMinecraftClientWrapper MC = SingletonInjector.INSTANCE.get(IMinecraftClientWrapper.class); - private ExecutorService workerThread = Executors.newSingleThreadExecutor(new ThreadFactoryBuilder().setNameFormat(GLProxy.class.getSimpleName() + "-Worker-Thread").build()); - private static final Logger LOGGER = DhLoggerBuilder.getLogger(MethodHandles.lookup().lookupClass().getSimpleName()); public static final ConfigBasedLogger GL_LOGGER = new ConfigBasedLogger(LogManager.getLogger(GLProxy.class), () -> Config.Client.Advanced.Logging.logRendererGLEvent.get()); @@ -88,6 +83,17 @@ public class GLProxy private static GLProxy instance = null; + private ExecutorService workerThread = Executors.newSingleThreadExecutor(new ThreadFactoryBuilder().setNameFormat(GLProxy.class.getSimpleName() + "-Worker-Thread").build()); + + /** + * the list that items should be added to.
+ * Two lists exist to prevent potential concurrency issues where we are reading and writing + * at the same time. + */ + private final ArrayList addRenderThreadRunnableList = new ArrayList<>(); + /** the list that items should be read from */ + private final ArrayList readRenderThreadRunnableList = new ArrayList<>(); + /** Minecraft's GLFW window */ public final long minecraftGlContext; /** Minecraft's GL capabilities */ @@ -459,9 +465,9 @@ public class GLProxy - //============================// - // MC render thread runnables // - //============================// + //=========================// + // Worker Thread Runnables // + //=========================// /** * Asynchronously calls the given runnable on proxy's OpenGL context. @@ -472,14 +478,18 @@ public class GLProxy public void recordOpenGlCall(Runnable renderCall) { StackTraceElement[] stackTrace = Thread.currentThread().getStackTrace(); - this.workerThread.execute(() -> this.runnableContainer(renderCall, stackTrace)); + this.workerThread.execute(() -> this.runOpenGlCall(renderCall, stackTrace, true)); } - private void runnableContainer(Runnable renderCall, StackTraceElement[] stackTrace) + private void runOpenGlCall(Runnable renderCall, StackTraceElement[] stackTrace, boolean useProxyWorkerContext) { try { - // set up the context... - this.setGlContext(EGLProxyContext.PROXY_WORKER); + // set up the context if requested... + if (useProxyWorkerContext) + { + this.setGlContext(EGLProxyContext.PROXY_WORKER); + } + // ...run the actual code... renderCall.run(); } @@ -492,7 +502,10 @@ public class GLProxy finally { // ...and make sure the context is released when the thread finishes - this.setGlContext(EGLProxyContext.NONE); + if (useProxyWorkerContext) + { + this.setGlContext(EGLProxyContext.NONE); + } } } @@ -528,6 +541,35 @@ public class GLProxy + //=========================// + // Render Thread Runnables // + //=========================// + + public void queueRunningOnRenderThread(Runnable renderCall) + { + StackTraceElement[] stackTrace = Thread.currentThread().getStackTrace(); + this.addRenderThreadRunnableList.add(() -> this.runOpenGlCall(renderCall, stackTrace, false)); + } + + /** + * Doesn't do any thread/GL Context validation. + * Running this outside of the render thread may cause crashes or other issues. + */ + public void runRenderThreadTasks() + { + ArrayList tempList = this.addRenderThreadRunnableList; + this.addRenderThreadRunnableList = this.readRenderThreadRunnableList; + this.readRenderThreadRunnableList = tempList; + + for (Runnable runnable : this.readRenderThreadRunnableList) + { + runnable.run(); + } + this.readRenderThreadRunnableList.clear(); + } + + + //=========// // logging // //=========// diff --git a/core/src/main/java/com/seibel/distanthorizons/core/render/glObject/buffer/GLBuffer.java b/core/src/main/java/com/seibel/distanthorizons/core/render/glObject/buffer/GLBuffer.java index 0d857b84b..648b5da5e 100644 --- a/core/src/main/java/com/seibel/distanthorizons/core/render/glObject/buffer/GLBuffer.java +++ b/core/src/main/java/com/seibel/distanthorizons/core/render/glObject/buffer/GLBuffer.java @@ -122,9 +122,12 @@ public class GLBuffer implements AutoCloseable } private static void destroyBufferId(boolean async, int id) { - if (async && GLProxy.getInstance().getGlContext() != EGLProxyContext.PROXY_WORKER) + EGLProxyContext glContext = GLProxy.getInstance().getGlContext(); + if (async + && glContext != EGLProxyContext.PROXY_WORKER + && glContext != EGLProxyContext.MINECRAFT) { - GLProxy.getInstance().recordOpenGlCall(() -> destroyBufferId(false, id)); + GLProxy.getInstance().queueRunningOnRenderThread(() -> destroyBufferId(false, id)); } else { @@ -200,7 +203,7 @@ public class GLBuffer implements AutoCloseable LodUtil.assertTrue(this.bufferStorage, "Buffer is not bufferStorage but its trying to use bufferStorage upload method!"); int bbSize = bb.limit() - bb.position(); - this.destroy(false); + this.destroy(true); this.create(true); this.bind(); GL44.glBufferStorage(this.getBufferBindingTarget(), bb, bufferStorageHint); @@ -313,7 +316,7 @@ public class GLBuffer implements AutoCloseable { // recreate if the buffer storage type changed this.bind(); - this.destroy(false); + this.destroy(true); this.create(uploadMethod.useBufferStorage); this.bind(); } diff --git a/core/src/main/java/com/seibel/distanthorizons/core/render/renderer/LodRenderer.java b/core/src/main/java/com/seibel/distanthorizons/core/render/renderer/LodRenderer.java index 003b4d1d4..e35e66b67 100644 --- a/core/src/main/java/com/seibel/distanthorizons/core/render/renderer/LodRenderer.java +++ b/core/src/main/java/com/seibel/distanthorizons/core/render/renderer/LodRenderer.java @@ -486,6 +486,9 @@ public class LodRenderer profiler.popPush("LOD cleanup"); LagSpikeCatcher drawCleanup = new LagSpikeCatcher(); + + GLProxy.getInstance().runRenderThreadTasks(); + lightmap.unbind(); if (ENABLE_IBO) { @@ -666,7 +669,7 @@ public class LodRenderer this.setupLock.lock(); EVENT_LOGGER.info("Queuing Renderer Cleanup for main render thread"); - GLProxy.getInstance().recordOpenGlCall(() -> + GLProxy.getInstance().queueRunningOnRenderThread(() -> { EVENT_LOGGER.info("Renderer Cleanup Started");