diff --git a/core/src/main/java/com/seibel/distanthorizons/core/dataObjects/render/bufferBuilding/LodBufferContainer.java b/core/src/main/java/com/seibel/distanthorizons/core/dataObjects/render/bufferBuilding/LodBufferContainer.java index 7d320dcf7..c25ef4158 100644 --- a/core/src/main/java/com/seibel/distanthorizons/core/dataObjects/render/bufferBuilding/LodBufferContainer.java +++ b/core/src/main/java/com/seibel/distanthorizons/core/dataObjects/render/bufferBuilding/LodBufferContainer.java @@ -21,6 +21,7 @@ package com.seibel.distanthorizons.core.dataObjects.render.bufferBuilding; import com.seibel.distanthorizons.core.logging.DhLogger; import com.seibel.distanthorizons.core.logging.DhLoggerBuilder; +import com.seibel.distanthorizons.core.pos.DhSectionPos; import com.seibel.distanthorizons.core.pos.blockPos.DhBlockPos; import com.seibel.distanthorizons.core.render.glObject.GLProxy; import com.seibel.distanthorizons.core.render.glObject.buffer.GLVertexBuffer; @@ -32,6 +33,7 @@ import org.lwjgl.system.MemoryUtil; import java.nio.ByteBuffer; import java.util.ArrayList; import java.util.concurrent.*; +import java.util.concurrent.atomic.AtomicReference; /** * Java representation of one or more OpenGL buffers for rendering. @@ -59,7 +61,7 @@ public class LodBufferContainer implements AutoCloseable public GLVertexBuffer[] vbos; public GLVertexBuffer[] vbosTransparent; - private CompletableFuture uploadFuture = null; + private final AtomicReference> uploadFutureRef = new AtomicReference<>(null); @@ -85,16 +87,31 @@ public class LodBufferContainer implements AutoCloseable public synchronized CompletableFuture makeAndUploadBuffersAsync(LodQuadBuilder builder) { // separate variable to prevent race condition when checking null - CompletableFuture future = this.uploadFuture; - if (future != null) + CompletableFuture oldFuture = this.uploadFutureRef.get(); + if (oldFuture != null) { // upload already in process - return future; + return oldFuture; } // new upload needed - future = new CompletableFuture<>(); - this.uploadFuture = future; + CompletableFuture future = new CompletableFuture<>(); + future.handle((lodBufferContainer, throwable) -> + { + if (!this.uploadFutureRef.compareAndSet(future, null)) + { + LOGGER.warn("upload future ref changed for pos ["+DhSectionPos.toString(this.pos)+"]."); + } + + return null; + }); + + if (!this.uploadFutureRef.compareAndSet(null, future)) + { + oldFuture = this.uploadFutureRef.get(); + LodUtil.assertTrue(oldFuture != null, "Concurrency error"); + return oldFuture; + } @@ -113,7 +130,7 @@ public class LodBufferContainer implements AutoCloseable { // skip this event if requested if (Thread.interrupted() - || this.uploadFuture.isCancelled()) + || future.isCancelled()) { throw new InterruptedException(); } @@ -126,20 +143,17 @@ public class LodBufferContainer implements AutoCloseable this.buffersUploaded = true; // success - this.uploadFuture.complete(this); - this.uploadFuture = null; + future.complete(this); } catch (InterruptedException ignore) { - this.uploadFuture.complete(this); - this.uploadFuture = null; + future.complete(this); } catch (Exception e) { LOGGER.error("Unexpected issue uploading buffer ["+this.minCornerBlockPos +"], error: ["+e.getMessage()+"].", e); - this.uploadFuture.completeExceptionally(e); - this.uploadFuture = null; + future.completeExceptionally(e); } finally { @@ -252,13 +266,13 @@ public class LodBufferContainer implements AutoCloseable return count; } - public boolean uploadInProgress() { return this.uploadFuture != null; } + public boolean uploadInProgress() { return this.uploadFutureRef.get() != null; } public void debugDumpStats(StatsMap statsMap) { statsMap.incStat("RenderBuffers"); statsMap.incStat("SimpleRenderBuffers"); - for (GLVertexBuffer vertexBuffer : vbos) + for (GLVertexBuffer vertexBuffer : this.vbos) { if (vertexBuffer != null) { diff --git a/core/src/main/java/com/seibel/distanthorizons/core/render/QuadTree/LodRenderSection.java b/core/src/main/java/com/seibel/distanthorizons/core/render/QuadTree/LodRenderSection.java index 03816fe6c..3367e1879 100644 --- a/core/src/main/java/com/seibel/distanthorizons/core/render/QuadTree/LodRenderSection.java +++ b/core/src/main/java/com/seibel/distanthorizons/core/render/QuadTree/LodRenderSection.java @@ -40,6 +40,7 @@ import com.seibel.distanthorizons.core.render.renderer.DebugRenderer; import com.seibel.distanthorizons.core.render.renderer.generic.BeaconRenderHandler; import com.seibel.distanthorizons.core.sql.dto.BeaconBeamDTO; import com.seibel.distanthorizons.core.sql.repo.BeaconBeamRepo; +import com.seibel.distanthorizons.core.util.LodUtil; import com.seibel.distanthorizons.core.util.threading.PriorityTaskPicker; import com.seibel.distanthorizons.core.util.threading.ThreadPoolUtil; import com.seibel.distanthorizons.core.wrapperInterfaces.minecraft.IMinecraftClientWrapper; @@ -51,6 +52,7 @@ import java.awt.*; import java.util.*; import java.util.List; import java.util.concurrent.*; +import java.util.concurrent.atomic.AtomicReference; /** * A render section represents an area that could be rendered. @@ -94,20 +96,20 @@ public class LodRenderSection implements IDebugRenderable, AutoCloseable * Encapsulates everything between pulling data from the database (including neighbors) * up to the point when geometry data is uploaded to the GPU. */ - private CompletableFuture getAndBuildRenderDataFuture = null; + private final AtomicReference> getAndBuildRenderDataFutureRef = new AtomicReference<>(null); /** - * used alongside {@link LodRenderSection#getAndBuildRenderDataFuture} so we can remove + * used alongside {@link LodRenderSection#getAndBuildRenderDataFutureRef} so we can remove * unnecessary tasks from the executor. */ private Runnable getAndBuildRenderDataRunnable = null; /** * Represents just uploading the {@link LodQuadBuilder} to the GPU.
- * Separate from {@link LodRenderSection#getAndBuildRenderDataFuture} because they run on + * Separate from {@link LodRenderSection#getAndBuildRenderDataFutureRef} because they run on * different threads (buffer uploading is on the MC render thread) and need to be canceled separately. */ - private CompletableFuture bufferUploadFuture = null; + private final AtomicReference> bufferUploadFutureRef = new AtomicReference<>(null); @@ -152,7 +154,7 @@ public class LodRenderSection implements IDebugRenderable, AutoCloseable return false; } - if (this.getAndBuildRenderDataFuture != null) + if (this.getAndBuildRenderDataFutureRef.get() != null) { // don't accidentally queue multiple uploads at the same time return false; @@ -168,7 +170,7 @@ public class LodRenderSection implements IDebugRenderable, AutoCloseable future.handle((voidObj, throwable) -> { // the task is done, we don't need to track these anymore - this.getAndBuildRenderDataFuture = null; + this.getAndBuildRenderDataFutureRef.compareAndSet(future, null); this.getAndBuildRenderDataRunnable = null; return null; @@ -176,7 +178,14 @@ public class LodRenderSection implements IDebugRenderable, AutoCloseable try { - this.getAndBuildRenderDataFuture = future; + if (!this.getAndBuildRenderDataFutureRef.compareAndSet(null, future)) + { + CompletableFuture oldFuture = this.getAndBuildRenderDataFutureRef.get(); + LodUtil.assertTrue(oldFuture != null, "Concurrency error"); + return true; + } + + this.getAndBuildRenderDataRunnable = () -> { try @@ -216,7 +225,7 @@ public class LodRenderSection implements IDebugRenderable, AutoCloseable } } @Nullable - private LodQuadBuilder getAndBuildRenderData() + private synchronized LodQuadBuilder getAndBuildRenderData() { try (ColumnRenderSource thisRenderSource = this.getRenderSourceForPos(this.pos, null)) { @@ -301,16 +310,27 @@ public class LodRenderSection implements IDebugRenderable, AutoCloseable detailLevel += DhSectionPos.SECTION_MINIMUM_DETAIL_LEVEL; return detailLevel == DhSectionPos.getDetailLevel(this.pos); } - private CompletableFuture uploadToGpuAsync(LodQuadBuilder lodQuadBuilder) + private synchronized CompletableFuture uploadToGpuAsync(LodQuadBuilder lodQuadBuilder) { - if (this.bufferUploadFuture != null) + CompletableFuture oldFuture = this.bufferUploadFutureRef.getAndSet(null); + if (oldFuture != null) { // shouldn't normally happen, but just in case canceling the previous future // prevents the CPU from working on something that won't be used - this.bufferUploadFuture.cancel(true); + oldFuture.cancel(true); } CompletableFuture future = ColumnRenderBufferBuilder.uploadBuffersAsync(this.level, this.pos, lodQuadBuilder); + future.handle((a, throwable) -> + { + if (!this.bufferUploadFutureRef.compareAndSet(future, null)) + { + LOGGER.error("Buffer upload future ref changed for pos: ["+DhSectionPos.toString(this.pos)+"]."); + } + + return null; + }); + future.thenAccept((LodBufferContainer buffer) -> { // needed to clean up the old data @@ -324,7 +344,13 @@ public class LodRenderSection implements IDebugRenderable, AutoCloseable previousContainer.close(); } }); - this.bufferUploadFuture = future; + + + if (!this.bufferUploadFutureRef.compareAndSet(null, future)) + { + LodUtil.assertNotReach("Buffer upload future ref couldn't be set due to concurrency error, pos: ["+DhSectionPos.toString(this.pos)+"]."); + } + return future; } @@ -342,7 +368,7 @@ public class LodRenderSection implements IDebugRenderable, AutoCloseable public boolean getRenderingEnabled() { return this.renderingEnabled; } public void setRenderingEnabled(boolean enabled) { this.renderingEnabled = enabled;} - public boolean gpuUploadInProgress() { return this.getAndBuildRenderDataFuture != null; } + public boolean gpuUploadInProgress() { return this.getAndBuildRenderDataFutureRef.get() != null; } //endregion @@ -447,7 +473,7 @@ public class LodRenderSection implements IDebugRenderable, AutoCloseable //color = Color.green; return; } - else if (this.getAndBuildRenderDataFuture != null) + else if (this.getAndBuildRenderDataFutureRef.get() != null) { color = Color.yellow; } @@ -503,7 +529,7 @@ public class LodRenderSection implements IDebugRenderable, AutoCloseable } // removes any in-progress futures since they aren't needed any more - CompletableFuture buildFuture = this.getAndBuildRenderDataFuture; + CompletableFuture buildFuture = this.getAndBuildRenderDataFutureRef.get(); if (buildFuture != null) { // remove the task from our executor if present @@ -520,7 +546,7 @@ public class LodRenderSection implements IDebugRenderable, AutoCloseable } } - CompletableFuture uploadFuture = this.bufferUploadFuture; + CompletableFuture uploadFuture = this.bufferUploadFutureRef.get(); if (uploadFuture != null) { uploadFuture.cancel(true);