From 19cde1bbd4faf067a6690a15e48ae48f2ac0a28e Mon Sep 17 00:00:00 2001 From: James Seibel Date: Sat, 23 Sep 2023 16:05:56 -0500 Subject: [PATCH] Add a config for synchronous GPU uploading hopefully to help with Sodium/AMD issues --- .../distanthorizons/core/config/Config.java | 12 +++- .../bufferBuilding/ColumnRenderBuffer.java | 61 ++++++++++++++++--- .../ColumnRenderBufferBuilder.java | 12 +--- .../core/render/glObject/buffer/GLBuffer.java | 2 +- .../glObject/buffer/GLVertexBuffer.java | 14 +++-- .../assets/distanthorizons/lang/en_us.json | 4 ++ 6 files changed, 81 insertions(+), 24 deletions(-) diff --git a/core/src/main/java/com/seibel/distanthorizons/core/config/Config.java b/core/src/main/java/com/seibel/distanthorizons/core/config/Config.java index 57cd4b8a2..99de6a13f 100644 --- a/core/src/main/java/com/seibel/distanthorizons/core/config/Config.java +++ b/core/src/main/java/com/seibel/distanthorizons/core/config/Config.java @@ -965,7 +965,7 @@ public class Config + "How long should a buffer wait per Megabyte of data uploaded? \n" + "Helpful resource for frame times: https://fpstoms.com \n" + "\n" - + "Longer times may reduce stuttering but will make fake chunks \n" + + "Longer times may reduce stuttering but will make LODs \n" + "transition and load slower. Change this to [0] for no timeout. \n" + "\n" + "NOTE:\n" @@ -973,6 +973,16 @@ public class Config + "") .build(); + public static ConfigEntry gpuUploadAsync = new ConfigEntry.Builder() + .set(true) + .comment("" + + "If true geometry data will be uploaded on a DH controlled thread, reducing FPS stuttering. \n" + + "If false uploading will be done on Minecraft's main rendering thread. \n" + + "\n" + + "Setting this to false may reduce crashes or corrupted geometry on systems with an AMD GPU when Sodium is installed.\n" + + "") + .build(); + // deprecated and not implemented, can be made public if we ever re-implement it @Deprecated private static ConfigEntry rebuildTimes = new ConfigEntry.Builder() 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 873745cef..2d3404ca4 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 @@ -20,22 +20,22 @@ package com.seibel.distanthorizons.core.dataObjects.render.bufferBuilding; import com.seibel.distanthorizons.core.config.Config; +import com.seibel.distanthorizons.core.dependencyInjection.SingletonInjector; +import com.seibel.distanthorizons.core.enums.EGLProxyContext; import com.seibel.distanthorizons.core.logging.DhLoggerBuilder; import com.seibel.distanthorizons.core.pos.DhBlockPos; import com.seibel.distanthorizons.core.pos.DhSectionPos; import com.seibel.distanthorizons.core.render.AbstractRenderBuffer; import com.seibel.distanthorizons.core.render.glObject.GLProxy; import com.seibel.distanthorizons.core.render.glObject.buffer.GLVertexBuffer; -import com.seibel.distanthorizons.core.render.renderer.DebugRenderer; -import com.seibel.distanthorizons.core.render.renderer.IDebugRenderable; import com.seibel.distanthorizons.core.render.renderer.LodRenderer; import com.seibel.distanthorizons.core.util.LodUtil; import com.seibel.distanthorizons.core.util.objects.StatsMap; import com.seibel.distanthorizons.api.enums.config.EGpuUploadMethod; import com.seibel.distanthorizons.core.util.*; +import com.seibel.distanthorizons.core.wrapperInterfaces.minecraft.IMinecraftClientWrapper; import org.apache.logging.log4j.Logger; -import java.awt.*; import java.nio.ByteBuffer; import java.util.Iterator; import java.util.concurrent.*; @@ -48,6 +48,7 @@ import java.util.concurrent.*; public class ColumnRenderBuffer extends AbstractRenderBuffer { private static final Logger LOGGER = DhLoggerBuilder.getLogger(); + private static final IMinecraftClientWrapper minecraftClient = SingletonInjector.INSTANCE.get(IMinecraftClientWrapper.class); private static final long MAX_BUFFER_UPLOAD_TIMEOUT_NANOSECONDS = 1_000_000; @@ -83,20 +84,66 @@ public class ColumnRenderBuffer extends AbstractRenderBuffer // buffer uploading // //==================// - public void uploadBuffer(LodQuadBuilder builder, EGpuUploadMethod method) throws InterruptedException + /** Should be run on a DH thread. */ + public void uploadBuffer(LodQuadBuilder builder, EGpuUploadMethod gpuUploadMethod) throws InterruptedException { - if (method.useEarlyMapping) + LodUtil.assertTrue(Thread.currentThread().getName().startsWith(ThreadUtil.THREAD_NAME_PREFIX), "Buffer uploading needs to be done on a DH thread to prevent locking up any MC threads."); + + + // the async is relative to MC's render thread + boolean uploadAsync = Config.Client.Advanced.GpuBuffers.gpuUploadAsync.get(); + if (uploadAsync) { - this.uploadBuffersMapped(builder, method); + // upload here on a DH thread + GLProxy glProxy = GLProxy.getInstance(); + EGLProxyContext oldContext = glProxy.getGlContext(); + glProxy.setGlContext(EGLProxyContext.LOD_BUILDER); + try + { + this.uploadBuffersUsingUploadMethod(builder, gpuUploadMethod); + } + finally + { + glProxy.setGlContext(oldContext); + } } else { - this.uploadBuffersDirect(builder, method); + // upload on MC's render thread + CompletableFuture uploadFuture = new CompletableFuture<>(); + minecraftClient.executeOnRenderThread(() -> + { + try + { + this.uploadBuffersUsingUploadMethod(builder, gpuUploadMethod); + uploadFuture.complete(null); + } + catch (InterruptedException e) + { + throw new CompletionException(e); + } + }); + + // freeze this DH thread while we wait for MC to upload the buffer + uploadFuture.join(); + } + } + private void uploadBuffersUsingUploadMethod(LodQuadBuilder builder, EGpuUploadMethod gpuUploadMethod) throws InterruptedException + { + if (gpuUploadMethod.useEarlyMapping) + { + this.uploadBuffersMapped(builder, gpuUploadMethod); + } + else + { + this.uploadBuffersDirect(builder, gpuUploadMethod); } this.buffersUploaded = true; } + + private void uploadBuffersMapped(LodQuadBuilder builder, EGpuUploadMethod method) { // opaque vbos // diff --git a/core/src/main/java/com/seibel/distanthorizons/core/dataObjects/render/bufferBuilding/ColumnRenderBufferBuilder.java b/core/src/main/java/com/seibel/distanthorizons/core/dataObjects/render/bufferBuilding/ColumnRenderBufferBuilder.java index a5b25e158..ed8481c3b 100644 --- a/core/src/main/java/com/seibel/distanthorizons/core/dataObjects/render/bufferBuilding/ColumnRenderBufferBuilder.java +++ b/core/src/main/java/com/seibel/distanthorizons/core/dataObjects/render/bufferBuilding/ColumnRenderBufferBuilder.java @@ -117,12 +117,8 @@ public class ColumnRenderBufferBuilder try { EVENT_LOGGER.trace("RenderRegion start Upload @ " + renderSource.sectionPos); - GLProxy glProxy = GLProxy.getInstance(); - EGpuUploadMethod method = GLProxy.getInstance().getGpuUploadMethod(); - EGLProxyContext oldContext = glProxy.getGlContext(); - glProxy.setGlContext(EGLProxyContext.LOD_BUILDER); - ColumnRenderBuffer buffer = renderBufferRef.swap(null); + ColumnRenderBuffer buffer = renderBufferRef.swap(null); if (buffer == null) { buffer = new ColumnRenderBuffer(new DhBlockPos(renderSource.sectionPos.getMinCornerLodPos().getCornerBlockPos(), clientLevel.getMinY()), renderSource.sectionPos); @@ -130,7 +126,7 @@ public class ColumnRenderBufferBuilder try { - buffer.uploadBuffer(quadBuilder, method); + buffer.uploadBuffer(quadBuilder, GLProxy.getInstance().getGpuUploadMethod()); LodUtil.assertTrue(buffer.buffersUploaded); EVENT_LOGGER.trace("RenderRegion end Upload @ " + renderSource.sectionPos); return buffer; @@ -140,10 +136,6 @@ public class ColumnRenderBufferBuilder buffer.close(); throw e; } - finally - { - glProxy.setGlContext(oldContext); - } } catch (InterruptedException e) { 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 8ba6fd245..9b4291f4b 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 @@ -145,7 +145,7 @@ public class GLBuffer implements AutoCloseable GL32.glBufferSubData(getBufferBindingTarget(), 0, bb); } - // Requires already binded + /** Assumes the GL Context is already bound */ public void uploadBuffer(ByteBuffer bb, EGpuUploadMethod uploadMethod, int maxExpansionSize, int bufferHint) { LodUtil.assertTrue(!uploadMethod.useEarlyMapping, "UploadMethod signal that this should use Mapping instead of uploadBuffer!"); diff --git a/core/src/main/java/com/seibel/distanthorizons/core/render/glObject/buffer/GLVertexBuffer.java b/core/src/main/java/com/seibel/distanthorizons/core/render/glObject/buffer/GLVertexBuffer.java index adf087798..614c8cbba 100644 --- a/core/src/main/java/com/seibel/distanthorizons/core/render/glObject/buffer/GLVertexBuffer.java +++ b/core/src/main/java/com/seibel/distanthorizons/core/render/glObject/buffer/GLVertexBuffer.java @@ -61,16 +61,20 @@ public class GLVertexBuffer extends GLBuffer return GL32.GL_ARRAY_BUFFER; } - public void uploadBuffer(ByteBuffer bb, int vertCount, EGpuUploadMethod uploadMethod, int maxExpensionSize) + public void uploadBuffer(ByteBuffer byteBuffer, int vertCount, EGpuUploadMethod uploadMethod, int maxExpensionSize) { - if (vertCount < 0) throw new IllegalArgumentException("VertCount is negative!"); + if (vertCount < 0) + { + throw new IllegalArgumentException("VertCount is negative!"); + } + // If size is zero, just ignore it. - if (bb.limit() - bb.position() != 0) + if (byteBuffer.limit() - byteBuffer.position() != 0) { boolean useBuffStorage = uploadMethod.useBufferStorage; - super.uploadBuffer(bb, uploadMethod, maxExpensionSize, useBuffStorage ? 0 : GL32.GL_STATIC_DRAW); + super.uploadBuffer(byteBuffer, uploadMethod, maxExpensionSize, useBuffStorage ? 0 : GL32.GL_STATIC_DRAW); } - vertexCount = vertCount; + this.vertexCount = vertCount; } public ByteBuffer mapBuffer(int targetSize, EGpuUploadMethod uploadMethod, int maxExpensionSize) diff --git a/core/src/main/resources/assets/distanthorizons/lang/en_us.json b/core/src/main/resources/assets/distanthorizons/lang/en_us.json index 341cd807c..0acd693d0 100644 --- a/core/src/main/resources/assets/distanthorizons/lang/en_us.json +++ b/core/src/main/resources/assets/distanthorizons/lang/en_us.json @@ -441,6 +441,10 @@ "GPU upload speed (milliseconds)", "distanthorizons.config.client.advanced.buffers.gpuUploadPerMegabyteInMilliseconds.@tooltip": "How long should a buffer wait per Megabyte of data uploaded?\nMay be increased if there is frame stuttering.", + "distanthorizons.config.client.advanced.buffers.gpuUploadAsync": + "GPU upload Async", + "distanthorizons.config.client.advanced.buffers.gpuUploadAsync.@tooltip": + "If true geometry data will be uploaded on a DH controlled thread, reducing FPS stuttering. \nIf false uploading will be done on Minecraft's main rendering thread. \n\nSetting this to false may reduce crashes or corrupted geometry on systems with an AMD GPU when Sodium is installed.",