From 8ea221e4f7024d81a9cd1405ea7fd3565d080627 Mon Sep 17 00:00:00 2001 From: Jozufozu Date: Sat, 18 Nov 2023 16:05:49 -0800 Subject: [PATCH] Full of red flags - Add concept of flags to TaskExecutor. - Can raise and lower flags from any thread. - Add TaskExecutor#syncTo - Behaves much like #syncPoint, but exits early as soon as it detects that the requested flag has been raised. - Document all methods in TaskExecutor. - Do not discard tasks when destroying a VisualizationManagerImpl. - Use flags in VisualizationManagerImpl to track frame plan and tick plan completion. - Use flags in BatchingEngine to track stage buffering completion and flush completion. - Synchronization is now needed in BatchedDrawTracker#draw. - Use flags in IndirectEngine and InstancingEngine to track flush completion. - Add unit tests to validate flag behavior. - Rename OnMainThreadPlan -> SyncedPlan. --- .gitignore | 1 + .../flywheel/api/event/RenderStage.java | 18 ++++ .../com/jozufozu/flywheel/api/task/Flag.java | 12 +++ .../flywheel/api/task/TaskExecutor.java | 59 +++++++++++- .../engine/batching/BatchedDrawTracker.java | 10 +- .../engine/batching/BatchedStagePlan.java | 18 +++- .../engine/batching/BatchingEngine.java | 22 ++++- .../engine/indirect/IndirectEngine.java | 34 ++++--- .../engine/instancing/InstancingEngine.java | 31 +++++-- .../impl/task/ParallelTaskExecutor.java | 83 ++++++++++++----- .../impl/task/SerialTaskExecutor.java | 29 +++++- .../VisualizationManagerImpl.java | 25 ++++- .../jozufozu/flywheel/lib/task/NamedFlag.java | 11 +++ .../jozufozu/flywheel/lib/task/RaisePlan.java | 16 ++++ .../jozufozu/flywheel/lib/task/StageFlag.java | 12 +++ ...{OnMainThreadPlan.java => SyncedPlan.java} | 8 +- .../flywheel/lib/task/PlanExecutionTest.java | 91 ++++++++++++++++++- .../lib/task/PlanSimplificationTest.java | 6 +- 18 files changed, 419 insertions(+), 67 deletions(-) create mode 100644 src/main/java/com/jozufozu/flywheel/api/task/Flag.java create mode 100644 src/main/java/com/jozufozu/flywheel/lib/task/NamedFlag.java create mode 100644 src/main/java/com/jozufozu/flywheel/lib/task/RaisePlan.java create mode 100644 src/main/java/com/jozufozu/flywheel/lib/task/StageFlag.java rename src/main/java/com/jozufozu/flywheel/lib/task/{OnMainThreadPlan.java => SyncedPlan.java} (65%) diff --git a/.gitignore b/.gitignore index fe6afb8ac..91d258c8f 100644 --- a/.gitignore +++ b/.gitignore @@ -5,6 +5,7 @@ run/ build/ gradle-app.setting out/ +logs/ ## IntelliJ IDEA diff --git a/src/main/java/com/jozufozu/flywheel/api/event/RenderStage.java b/src/main/java/com/jozufozu/flywheel/api/event/RenderStage.java index dc9883d09..dbefd1931 100644 --- a/src/main/java/com/jozufozu/flywheel/api/event/RenderStage.java +++ b/src/main/java/com/jozufozu/flywheel/api/event/RenderStage.java @@ -13,4 +13,22 @@ public enum RenderStage { AFTER_TRANSLUCENT_TERRAIN, AFTER_PARTICLES, AFTER_WEATHER; + + /** + * Is this stage the last one to be rendered in the frame? + * + * @return {@code true} if no other RenderStages will be dispatched this frame. + */ + public boolean isLast() { + return this == values()[values().length - 1]; + } + + /** + * Is this stage the first one to be rendered in the frame? + * + * @return {@code true} if this is the first RenderStage to be dispatched this frame. + */ + public boolean isFirst() { + return this == values()[0]; + } } diff --git a/src/main/java/com/jozufozu/flywheel/api/task/Flag.java b/src/main/java/com/jozufozu/flywheel/api/task/Flag.java new file mode 100644 index 000000000..9c6543653 --- /dev/null +++ b/src/main/java/com/jozufozu/flywheel/api/task/Flag.java @@ -0,0 +1,12 @@ +package com.jozufozu.flywheel.api.task; + +/** + * Marker interface for flags that can be raised and lowered in a {@link TaskExecutor}. + *
+ * Warning: flags will only be considered equal by reference. + * This is to allow multiple instances of the same high level structures to exist at once. + *
+ * Keep this in mind when using records as flags. + */ +public interface Flag { +} diff --git a/src/main/java/com/jozufozu/flywheel/api/task/TaskExecutor.java b/src/main/java/com/jozufozu/flywheel/api/task/TaskExecutor.java index abe9dc71c..5b5323c9b 100644 --- a/src/main/java/com/jozufozu/flywheel/api/task/TaskExecutor.java +++ b/src/main/java/com/jozufozu/flywheel/api/task/TaskExecutor.java @@ -4,11 +4,66 @@ import java.util.concurrent.Executor; public interface TaskExecutor extends Executor { /** - * Wait for all running tasks to finish. + * Wait for all running tasks to finish. + *
+ * This is useful as a nuclear option, but most of the time you should + * try to use {@link Flag flags} and {@link #syncTo(Flag) syncTo}. */ void syncPoint(); + /** + * Wait for running tasks, until the given Flag is {@link #raise raised}. + *
+ * The flag will remain raised until {@link #lower lowered} manually. + * + * @param flag The flag to wait for. + * @return {@code true} if the flag was encountered. May return false if + * this executor runs out of tasks before the flag was raised. + */ + boolean syncTo(Flag flag); + + /** + * Raise a flag indicating a key point in execution. + *
+ * If the flag was already raised, this method does nothing. + * + * @param flag The flag to raise. + */ + void raise(Flag flag); + + /** + * Lower a flag that may have been previously raised. + *
+ * If the flag was never raised, this method does nothing. + * + * @param flag The flag to lower. + */ + void lower(Flag flag); + + /** + * Check if a flag is raised without waiting for it. + * + * @param flag The flag to check. + * @return {@code true} if the flag is raised. + */ + boolean isRaised(Flag flag); + + /** + * Check for the number of threads this executor uses. + *
+ * May be helpful when determining how many chunks to divide a task into. + * + * @return The number of threads this executor uses. + */ int getThreadCount(); - void scheduleForMainThread(Runnable runnable); + /** + * Schedule a task to be run on the main thread. + *
+ * This method may be called from any thread, but the runnable will only + * be executed once somebody calls either {@link #syncPoint()} or + * {@link #syncTo(Flag)}. + * @param runnable The task to run. + */ + void scheduleForSync(Runnable runnable); } diff --git a/src/main/java/com/jozufozu/flywheel/backend/engine/batching/BatchedDrawTracker.java b/src/main/java/com/jozufozu/flywheel/backend/engine/batching/BatchedDrawTracker.java index 630635351..7e4e03517 100644 --- a/src/main/java/com/jozufozu/flywheel/backend/engine/batching/BatchedDrawTracker.java +++ b/src/main/java/com/jozufozu/flywheel/backend/engine/batching/BatchedDrawTracker.java @@ -52,7 +52,15 @@ public class BatchedDrawTracker { * @param stage The RenderStage to draw. */ public void draw(RenderStage stage) { - Set buffers = activeBuffers.get(stage); + // This may appear jank, but flag synchronization in BatchingEngine guarantees that + // the mapped-to Set will not be modified here. We don't have the same guarantee for + // activeBuffers itself, so we need to synchronize to fetch the Set. + + Set buffers; + synchronized (activeBuffers) { + buffers = activeBuffers.get(stage); + } + for (DrawBuffer buffer : buffers) { _draw(buffer); } diff --git a/src/main/java/com/jozufozu/flywheel/backend/engine/batching/BatchedStagePlan.java b/src/main/java/com/jozufozu/flywheel/backend/engine/batching/BatchedStagePlan.java index f7eb75ec3..16280fdc5 100644 --- a/src/main/java/com/jozufozu/flywheel/backend/engine/batching/BatchedStagePlan.java +++ b/src/main/java/com/jozufozu/flywheel/backend/engine/batching/BatchedStagePlan.java @@ -7,8 +7,10 @@ import java.util.Map; import java.util.concurrent.atomic.AtomicInteger; import com.jozufozu.flywheel.api.event.RenderStage; +import com.jozufozu.flywheel.api.task.Flag; import com.jozufozu.flywheel.api.task.TaskExecutor; import com.jozufozu.flywheel.lib.task.SimplyComposedPlan; +import com.jozufozu.flywheel.lib.task.StageFlag; import com.jozufozu.flywheel.lib.task.Synchronizer; import net.minecraft.client.renderer.RenderType; @@ -17,24 +19,34 @@ import net.minecraft.client.renderer.RenderType; * All the rendering that happens within a render stage. */ public class BatchedStagePlan implements SimplyComposedPlan { + /** + * This flag will be raised when this stage completes execution. + */ + public final Flag flag; + private final RenderStage stage; private final BatchedDrawTracker tracker; private final Map bufferPlans = new HashMap<>(); - public BatchedStagePlan(RenderStage renderStage, BatchedDrawTracker tracker) { - stage = renderStage; + public BatchedStagePlan(RenderStage stage, BatchedDrawTracker tracker) { + this.flag = new StageFlag(stage); + this.stage = stage; this.tracker = tracker; } @Override public void execute(TaskExecutor taskExecutor, BatchContext context, Runnable onCompletion) { if (isEmpty()) { + taskExecutor.raise(flag); onCompletion.run(); return; } taskExecutor.execute(() -> { - var sync = new Synchronizer(bufferPlans.size(), onCompletion); + var sync = new Synchronizer(bufferPlans.size(), () -> { + taskExecutor.raise(flag); + onCompletion.run(); + }); for (var plan : bufferPlans.values()) { plan.execute(taskExecutor, context, sync); diff --git a/src/main/java/com/jozufozu/flywheel/backend/engine/batching/BatchingEngine.java b/src/main/java/com/jozufozu/flywheel/backend/engine/batching/BatchingEngine.java index 671241c24..ffe9fe1dd 100644 --- a/src/main/java/com/jozufozu/flywheel/backend/engine/batching/BatchingEngine.java +++ b/src/main/java/com/jozufozu/flywheel/backend/engine/batching/BatchingEngine.java @@ -13,10 +13,12 @@ import com.jozufozu.flywheel.api.instance.InstanceType; import com.jozufozu.flywheel.api.instance.Instancer; import com.jozufozu.flywheel.api.model.Mesh; import com.jozufozu.flywheel.api.model.Model; +import com.jozufozu.flywheel.api.task.Flag; import com.jozufozu.flywheel.api.task.Plan; import com.jozufozu.flywheel.api.task.TaskExecutor; import com.jozufozu.flywheel.backend.engine.AbstractEngine; import com.jozufozu.flywheel.backend.engine.InstancerKey; +import com.jozufozu.flywheel.lib.task.NamedFlag; import com.jozufozu.flywheel.lib.task.SimplyComposedPlan; import com.jozufozu.flywheel.lib.task.Synchronizer; import com.mojang.blaze3d.vertex.VertexFormat; @@ -31,6 +33,8 @@ public class BatchingEngine extends AbstractEngine implements SimplyComposedPlan private final Map stagePlans = new EnumMap<>(RenderStage.class); private final Map meshPools = new HashMap<>(); + private final Flag flushFlag = new NamedFlag("flushed"); + public BatchingEngine(int maxOriginDistance) { super(maxOriginDistance); } @@ -52,6 +56,9 @@ public class BatchingEngine extends AbstractEngine implements SimplyComposedPlan public void execute(TaskExecutor taskExecutor, RenderContext context, Runnable onCompletion) { flush(); + // Now it's safe to read stage plans in #renderStage. + taskExecutor.raise(flushFlag); + BatchContext ctx = BatchContext.create(context, renderOrigin); var sync = new Synchronizer(stagePlans.values() @@ -69,7 +76,20 @@ public class BatchingEngine extends AbstractEngine implements SimplyComposedPlan @Override public void renderStage(TaskExecutor executor, RenderContext context, RenderStage stage) { - executor.syncPoint(); + executor.syncTo(flushFlag); + if (stage.isLast()) { + executor.lower(flushFlag); + } + + var stagePlan = stagePlans.get(stage); + + if (stagePlan == null) { + drawTracker.draw(stage); + return; + } + + executor.syncTo(stagePlan.flag); + executor.lower(stagePlan.flag); drawTracker.draw(stage); } diff --git a/src/main/java/com/jozufozu/flywheel/backend/engine/indirect/IndirectEngine.java b/src/main/java/com/jozufozu/flywheel/backend/engine/indirect/IndirectEngine.java index f84a71c06..d7f094b84 100644 --- a/src/main/java/com/jozufozu/flywheel/backend/engine/indirect/IndirectEngine.java +++ b/src/main/java/com/jozufozu/flywheel/backend/engine/indirect/IndirectEngine.java @@ -8,18 +8,22 @@ import com.jozufozu.flywheel.api.instance.Instance; import com.jozufozu.flywheel.api.instance.InstanceType; import com.jozufozu.flywheel.api.instance.Instancer; import com.jozufozu.flywheel.api.model.Model; +import com.jozufozu.flywheel.api.task.Flag; import com.jozufozu.flywheel.api.task.Plan; import com.jozufozu.flywheel.api.task.TaskExecutor; import com.jozufozu.flywheel.backend.engine.AbstractEngine; import com.jozufozu.flywheel.gl.GlStateTracker; import com.jozufozu.flywheel.gl.GlTextureUnit; -import com.jozufozu.flywheel.lib.task.OnMainThreadPlan; +import com.jozufozu.flywheel.lib.task.NamedFlag; +import com.jozufozu.flywheel.lib.task.RaisePlan; +import com.jozufozu.flywheel.lib.task.SyncedPlan; import com.mojang.blaze3d.systems.RenderSystem; import net.minecraft.client.Minecraft; public class IndirectEngine extends AbstractEngine { private final IndirectDrawManager drawManager = new IndirectDrawManager(); + private final Flag flushFlag = new NamedFlag("flushed"); public IndirectEngine(int maxOriginDistance) { super(maxOriginDistance); @@ -32,7 +36,8 @@ public class IndirectEngine extends AbstractEngine { @Override public Plan createFramePlan() { - return OnMainThreadPlan.of(this::flushDrawManager); + return SyncedPlan.of(this::flushDrawManager) + .then(RaisePlan.raise(flushFlag)); } private void flushDrawManager() { @@ -43,18 +48,23 @@ public class IndirectEngine extends AbstractEngine { @Override public void renderStage(TaskExecutor executor, RenderContext context, RenderStage stage) { - if (!drawManager.hasStage(stage)) { - return; + if (drawManager.hasStage(stage)) { + executor.syncTo(flushFlag); + + try (var restoreState = GlStateTracker.getRestoreState()) { + setup(); + + for (var list : drawManager.renderLists.values()) { + list.submit(stage); + } + } } - executor.syncPoint(); - - try (var restoreState = GlStateTracker.getRestoreState()) { - setup(); - - for (var list : drawManager.renderLists.values()) { - list.submit(stage); - } + if (stage.isLast()) { + // Need to sync here to ensure this frame has everything executed + // in case we didn't have any stages to draw this frame. + executor.syncTo(flushFlag); + executor.lower(flushFlag); } } diff --git a/src/main/java/com/jozufozu/flywheel/backend/engine/instancing/InstancingEngine.java b/src/main/java/com/jozufozu/flywheel/backend/engine/instancing/InstancingEngine.java index a0492528c..b473b1fda 100644 --- a/src/main/java/com/jozufozu/flywheel/backend/engine/instancing/InstancingEngine.java +++ b/src/main/java/com/jozufozu/flywheel/backend/engine/instancing/InstancingEngine.java @@ -9,6 +9,7 @@ import com.jozufozu.flywheel.api.instance.Instance; import com.jozufozu.flywheel.api.instance.InstanceType; import com.jozufozu.flywheel.api.instance.Instancer; import com.jozufozu.flywheel.api.model.Model; +import com.jozufozu.flywheel.api.task.Flag; import com.jozufozu.flywheel.api.task.Plan; import com.jozufozu.flywheel.api.task.TaskExecutor; import com.jozufozu.flywheel.backend.compile.InstancingPrograms; @@ -17,7 +18,9 @@ import com.jozufozu.flywheel.backend.engine.UniformBuffer; import com.jozufozu.flywheel.gl.GlStateTracker; import com.jozufozu.flywheel.gl.GlTextureUnit; import com.jozufozu.flywheel.lib.material.MaterialIndices; -import com.jozufozu.flywheel.lib.task.OnMainThreadPlan; +import com.jozufozu.flywheel.lib.task.NamedFlag; +import com.jozufozu.flywheel.lib.task.RaisePlan; +import com.jozufozu.flywheel.lib.task.SyncedPlan; import com.mojang.blaze3d.systems.RenderSystem; import net.minecraft.client.Minecraft; @@ -26,6 +29,8 @@ public class InstancingEngine extends AbstractEngine { private final Context context; private final InstancedDrawManager drawManager = new InstancedDrawManager(); + private final Flag flushFlag = new NamedFlag("flushed"); + public InstancingEngine(int maxOriginDistance, Context context) { super(maxOriginDistance); this.context = context; @@ -38,7 +43,8 @@ public class InstancingEngine extends AbstractEngine { @Override public Plan createFramePlan() { - return OnMainThreadPlan.of(this::flushDrawManager); + return SyncedPlan.of(this::flushDrawManager) + .then(RaisePlan.raise(flushFlag)); } private void flushDrawManager() { @@ -51,16 +57,21 @@ public class InstancingEngine extends AbstractEngine { public void renderStage(TaskExecutor executor, RenderContext context, RenderStage stage) { var drawSet = drawManager.get(stage); - if (drawSet.isEmpty()) { - return; + if (!drawSet.isEmpty()) { + executor.syncTo(flushFlag); + + try (var state = GlStateTracker.getRestoreState()) { + setup(); + + render(drawSet); + } } - executor.syncPoint(); - - try (var state = GlStateTracker.getRestoreState()) { - setup(); - - render(drawSet); + if (stage.isLast()) { + // Need to sync here to ensure this frame has everything executed + // in case we didn't have any stages to draw this frame. + executor.syncTo(flushFlag); + executor.lower(flushFlag); } } diff --git a/src/main/java/com/jozufozu/flywheel/impl/task/ParallelTaskExecutor.java b/src/main/java/com/jozufozu/flywheel/impl/task/ParallelTaskExecutor.java index 99e80d0ae..676b1fb84 100644 --- a/src/main/java/com/jozufozu/flywheel/impl/task/ParallelTaskExecutor.java +++ b/src/main/java/com/jozufozu/flywheel/impl/task/ParallelTaskExecutor.java @@ -1,9 +1,11 @@ package com.jozufozu.flywheel.impl.task; import java.util.ArrayList; +import java.util.Collections; import java.util.Deque; import java.util.List; import java.util.Queue; +import java.util.Set; import java.util.concurrent.ConcurrentLinkedDeque; import java.util.concurrent.ConcurrentLinkedQueue; import java.util.concurrent.atomic.AtomicBoolean; @@ -12,10 +14,12 @@ import org.jetbrains.annotations.NotNull; import org.slf4j.Logger; import com.jozufozu.flywheel.Flywheel; +import com.jozufozu.flywheel.api.task.Flag; import com.jozufozu.flywheel.api.task.TaskExecutor; import com.mojang.blaze3d.systems.RenderSystem; import com.mojang.logging.LogUtils; +import it.unimi.dsi.fastutil.objects.ReferenceOpenHashSet; import net.minecraft.util.Mth; // https://github.com/CaffeineMC/sodium-fabric/blob/5d364ed5ba63f9067fcf72a078ca310bff4db3e9/src/main/java/me/jellysquid/mods/sodium/client/render/chunk/compile/ChunkBuilder.java @@ -35,6 +39,8 @@ public class ParallelTaskExecutor implements TaskExecutor { private final Deque taskQueue = new ConcurrentLinkedDeque<>(); private final Queue mainThreadQueue = new ConcurrentLinkedQueue<>(); + private final Set flags = Collections.synchronizedSet(new ReferenceOpenHashSet<>()); + private final ThreadGroupNotifier taskNotifier = new ThreadGroupNotifier(); private final WaitGroup waitGroup = new WaitGroup(); @@ -116,7 +122,7 @@ public class ParallelTaskExecutor implements TaskExecutor { } @Override - public void scheduleForMainThread(Runnable runnable) { + public void scheduleForSync(Runnable runnable) { if (!running.get()) { throw new IllegalStateException("Executor is stopped"); } @@ -133,40 +139,67 @@ public class ParallelTaskExecutor implements TaskExecutor { */ @Override public void syncPoint() { - Runnable task; while (true) { - if ((task = mainThreadQueue.poll()) != null) { - // Prioritize main thread tasks. - processMainThreadTask(task); - } else if ((task = taskQueue.pollLast()) != null) { - // then work on tasks from the queue. - processTask(task); - } else { - // then wait for the other threads to finish. - boolean done = waitGroup.await(10_000); - // If we timed-out tasks may have been added to the queue, so check again. - if (done && mainThreadQueue.isEmpty()) { - // if they didn't, we're done. - break; - } + if (syncOneTask()) { + // Done! Nothing left to do. + break; } } } - public void discardAndAwait() { + @Override + public boolean syncTo(Flag flag) { while (true) { - // Discard everyone else's work... - while (taskQueue.pollLast() != null) { - waitGroup.done(); + if (isRaised(flag)) { + // The flag is already raised! + // Early return with true to indicate. + return true; } - // ...wait for any stragglers... - if (waitGroup.await(100_000)) { - break; + if (syncOneTask()) { + // Out of tasks entirely. + // The flag may have been raised though so return the result of isRaised. + return isRaised(flag); } } - // ...and clear the main thread queue. - mainThreadQueue.clear(); + } + + /** + * Attempt to process a single task. + * + * @return {@code true} if the executor has nothing left to do. + */ + private boolean syncOneTask() { + Runnable task; + if ((task = mainThreadQueue.poll()) != null) { + // Prioritize main thread tasks. + processMainThreadTask(task); + } else if ((task = taskQueue.pollLast()) != null) { + // then work on tasks from the queue. + processTask(task); + } else { + // then wait for the other threads to finish. + boolean done = waitGroup.await(10_000); + // If we timed-out tasks may have been added to the queue, so check again. + // if they didn't, we're done. + return done && mainThreadQueue.isEmpty(); + } + return false; + } + + @Override + public void raise(Flag flag) { + flags.add(flag); + } + + @Override + public void lower(Flag flag) { + flags.remove(flag); + } + + @Override + public boolean isRaised(Flag flag) { + return flags.contains(flag); } private void processTask(Runnable task) { diff --git a/src/main/java/com/jozufozu/flywheel/impl/task/SerialTaskExecutor.java b/src/main/java/com/jozufozu/flywheel/impl/task/SerialTaskExecutor.java index e5098a2b1..d2888c55c 100644 --- a/src/main/java/com/jozufozu/flywheel/impl/task/SerialTaskExecutor.java +++ b/src/main/java/com/jozufozu/flywheel/impl/task/SerialTaskExecutor.java @@ -1,10 +1,17 @@ package com.jozufozu.flywheel.impl.task; +import java.util.Set; + +import com.jozufozu.flywheel.api.task.Flag; import com.jozufozu.flywheel.api.task.TaskExecutor; +import it.unimi.dsi.fastutil.objects.ReferenceOpenHashSet; + public class SerialTaskExecutor implements TaskExecutor { public static final SerialTaskExecutor INSTANCE = new SerialTaskExecutor(); + private final Set flags = new ReferenceOpenHashSet<>(); + private SerialTaskExecutor() { } @@ -14,7 +21,7 @@ public class SerialTaskExecutor implements TaskExecutor { } @Override - public void scheduleForMainThread(Runnable runnable) { + public void scheduleForSync(Runnable runnable) { runnable.run(); } @@ -22,6 +29,26 @@ public class SerialTaskExecutor implements TaskExecutor { public void syncPoint() { } + @Override + public boolean syncTo(Flag flag) { + return isRaised(flag); + } + + @Override + public void raise(Flag flag) { + flags.add(flag); + } + + @Override + public void lower(Flag flag) { + flags.remove(flag); + } + + @Override + public boolean isRaised(Flag flag) { + return flags.contains(flag); + } + @Override public int getThreadCount() { return 1; diff --git a/src/main/java/com/jozufozu/flywheel/impl/visualization/VisualizationManagerImpl.java b/src/main/java/com/jozufozu/flywheel/impl/visualization/VisualizationManagerImpl.java index d54937110..abd6f48f0 100644 --- a/src/main/java/com/jozufozu/flywheel/impl/visualization/VisualizationManagerImpl.java +++ b/src/main/java/com/jozufozu/flywheel/impl/visualization/VisualizationManagerImpl.java @@ -7,6 +7,7 @@ import com.jozufozu.flywheel.api.backend.BackendManager; import com.jozufozu.flywheel.api.backend.Engine; import com.jozufozu.flywheel.api.event.RenderContext; import com.jozufozu.flywheel.api.event.RenderStage; +import com.jozufozu.flywheel.api.task.Flag; import com.jozufozu.flywheel.api.task.Plan; import com.jozufozu.flywheel.api.task.TaskExecutor; import com.jozufozu.flywheel.api.visual.DynamicVisual; @@ -22,7 +23,9 @@ import com.jozufozu.flywheel.impl.visualization.manager.BlockEntityVisualManager import com.jozufozu.flywheel.impl.visualization.manager.EffectVisualManager; import com.jozufozu.flywheel.impl.visualization.manager.EntityVisualManager; import com.jozufozu.flywheel.lib.math.MatrixUtil; +import com.jozufozu.flywheel.lib.task.NamedFlag; import com.jozufozu.flywheel.lib.task.NestedPlan; +import com.jozufozu.flywheel.lib.task.RaisePlan; import com.jozufozu.flywheel.lib.task.SimplyComposedPlan; import com.jozufozu.flywheel.lib.util.LevelAttached; @@ -49,6 +52,9 @@ public class VisualizationManagerImpl implements VisualizationManager { private final Plan tickPlan; private final Plan framePlan; + private final Flag tickFlag = new NamedFlag("tick"); + private final Flag frameFlag = new NamedFlag("frame"); + private VisualizationManagerImpl(LevelAccessor level) { engine = BackendManager.getBackend() .createEngine(level); @@ -62,8 +68,9 @@ public class VisualizationManagerImpl implements VisualizationManager { tickPlan = blockEntities.createTickPlan() .and(entities.createTickPlan()) .and(effects.createTickPlan()) + .then(RaisePlan.raise(tickFlag)) .simplify(); - framePlan = new FramePlan(); + framePlan = new FramePlan().then(RaisePlan.raise(frameFlag)); } public static boolean supportsVisualization(@Nullable LevelAccessor level) { @@ -145,7 +152,12 @@ public class VisualizationManagerImpl implements VisualizationManager { *

*/ public void tick(double cameraX, double cameraY, double cameraZ) { - taskExecutor.syncPoint(); + // Make sure we're done with any prior frame or tick to avoid racing. + taskExecutor.syncTo(frameFlag); + taskExecutor.lower(frameFlag); + + taskExecutor.syncTo(tickFlag); + taskExecutor.lower(tickFlag); tickPlan.execute(taskExecutor, new TickContext(cameraX, cameraY, cameraZ)); } @@ -159,7 +171,9 @@ public class VisualizationManagerImpl implements VisualizationManager { *

*/ public void beginFrame(RenderContext context) { - taskExecutor.syncPoint(); + // Make sure we're done with the last tick. + // Note we don't lower here because many frames may happen per tick. + taskExecutor.syncTo(tickFlag); framePlan.execute(taskExecutor, context); } @@ -175,7 +189,10 @@ public class VisualizationManagerImpl implements VisualizationManager { * Free all acquired resources and delete this manager. */ public void delete() { - taskExecutor.discardAndAwait(); + // Just finish everything. This may include the work of others but that's okay. + taskExecutor.syncPoint(); + + // Now clean up. blockEntities.invalidate(); entities.invalidate(); effects.invalidate(); diff --git a/src/main/java/com/jozufozu/flywheel/lib/task/NamedFlag.java b/src/main/java/com/jozufozu/flywheel/lib/task/NamedFlag.java new file mode 100644 index 000000000..dfa79629a --- /dev/null +++ b/src/main/java/com/jozufozu/flywheel/lib/task/NamedFlag.java @@ -0,0 +1,11 @@ +package com.jozufozu.flywheel.lib.task; + +import com.jozufozu.flywheel.api.task.Flag; + +/** + * A flag with an arbitrary name. + * + * @param name The name of the flag, mainly for debugging purposes. + */ +public record NamedFlag(String name) implements Flag { +} diff --git a/src/main/java/com/jozufozu/flywheel/lib/task/RaisePlan.java b/src/main/java/com/jozufozu/flywheel/lib/task/RaisePlan.java new file mode 100644 index 000000000..31d8c7b79 --- /dev/null +++ b/src/main/java/com/jozufozu/flywheel/lib/task/RaisePlan.java @@ -0,0 +1,16 @@ +package com.jozufozu.flywheel.lib.task; + +import com.jozufozu.flywheel.api.task.Flag; +import com.jozufozu.flywheel.api.task.TaskExecutor; + +public record RaisePlan(Flag flag) implements SimplyComposedPlan { + public static RaisePlan raise(Flag flag) { + return new RaisePlan<>(flag); + } + + @Override + public void execute(TaskExecutor taskExecutor, C context, Runnable onCompletion) { + taskExecutor.raise(flag); + onCompletion.run(); + } +} diff --git a/src/main/java/com/jozufozu/flywheel/lib/task/StageFlag.java b/src/main/java/com/jozufozu/flywheel/lib/task/StageFlag.java new file mode 100644 index 000000000..1a848128d --- /dev/null +++ b/src/main/java/com/jozufozu/flywheel/lib/task/StageFlag.java @@ -0,0 +1,12 @@ +package com.jozufozu.flywheel.lib.task; + +import com.jozufozu.flywheel.api.event.RenderStage; +import com.jozufozu.flywheel.api.task.Flag; + +/** + * A flag that is associated with a render stage. + *
+ * Useful for synchronizing tasks for a specific render stage. + */ +public record StageFlag(RenderStage stage) implements Flag { +} diff --git a/src/main/java/com/jozufozu/flywheel/lib/task/OnMainThreadPlan.java b/src/main/java/com/jozufozu/flywheel/lib/task/SyncedPlan.java similarity index 65% rename from src/main/java/com/jozufozu/flywheel/lib/task/OnMainThreadPlan.java rename to src/main/java/com/jozufozu/flywheel/lib/task/SyncedPlan.java index 7823877c8..d44d80a05 100644 --- a/src/main/java/com/jozufozu/flywheel/lib/task/OnMainThreadPlan.java +++ b/src/main/java/com/jozufozu/flywheel/lib/task/SyncedPlan.java @@ -3,18 +3,18 @@ package com.jozufozu.flywheel.lib.task; import com.jozufozu.flywheel.api.task.Plan; import com.jozufozu.flywheel.api.task.TaskExecutor; -public record OnMainThreadPlan(ContextConsumer task) implements SimplyComposedPlan { +public record SyncedPlan(ContextConsumer task) implements SimplyComposedPlan { public static Plan of(ContextConsumer task) { - return new OnMainThreadPlan<>(task); + return new SyncedPlan<>(task); } public static Plan of(ContextRunnable task) { - return new OnMainThreadPlan<>(task); + return new SyncedPlan<>(task); } @Override public void execute(TaskExecutor taskExecutor, C context, Runnable onCompletion) { - taskExecutor.scheduleForMainThread(() -> { + taskExecutor.scheduleForSync(() -> { task.accept(context); onCompletion.run(); }); diff --git a/src/test/java/com/jozufozu/flywheel/lib/task/PlanExecutionTest.java b/src/test/java/com/jozufozu/flywheel/lib/task/PlanExecutionTest.java index f06b19808..cf3c95b79 100644 --- a/src/test/java/com/jozufozu/flywheel/lib/task/PlanExecutionTest.java +++ b/src/test/java/com/jozufozu/flywheel/lib/task/PlanExecutionTest.java @@ -140,7 +140,7 @@ class PlanExecutionTest { @Test void mainThreadPlan() { var done = new AtomicBoolean(false); - var plan = OnMainThreadPlan.of(() -> done.set(true)); + var plan = SyncedPlan.of(() -> done.set(true)); plan.execute(EXECUTOR, Unit.INSTANCE); @@ -151,6 +151,95 @@ class PlanExecutionTest { Assertions.assertTrue(done.get()); } + @Test + void flagPlan() { + var first = new NamedFlag("ready right away"); + var second = new NamedFlag("ready after we sync"); + + var sync = new Synchronizer(2, () -> EXECUTOR.raise(second)); + + RaisePlan.raise(first) + .execute(EXECUTOR, Unit.INSTANCE, sync); + + Assertions.assertTrue(EXECUTOR.syncTo(first), "First flag should be raised since we submitted a plan that raises it"); + + Assertions.assertFalse(EXECUTOR.syncTo(second), "Second flag should not be raised yet."); + + sync.decrementAndEventuallyRun(); + + Assertions.assertTrue(EXECUTOR.syncTo(second), "Second flag should be raised since it was raised in sync."); + } + + @Test + void longWaitForFlag() { + var first = new NamedFlag("ready right away"); + var second = new NamedFlag("ready after 2s"); + + RaisePlan.raise(first) + .then(SimplePlan.of(() -> { + try { + Thread.sleep(2000); + } catch (InterruptedException e) { + throw new RuntimeException(e); + } + })) + .then(RaisePlan.raise(second)) + .execute(EXECUTOR, Unit.INSTANCE); + + Assertions.assertTrue(EXECUTOR.syncTo(first), "First flag should be raised since we submitted a plan that raises it."); + + Assertions.assertFalse(EXECUTOR.isRaised(second), "Second flag should not be raised immediately."); + + Assertions.assertTrue(EXECUTOR.syncTo(second), "Second flag should be raised since we were waiting for it."); + } + + @Test + void syncToReturnsExpected() { + var flag = new NamedFlag("ready right away"); + + Assertions.assertFalse(EXECUTOR.syncTo(flag), "Flag should not be raised yet."); + + EXECUTOR.raise(flag); + + Assertions.assertTrue(EXECUTOR.syncTo(flag), "Flag should be raised since we raised it manually."); + + EXECUTOR.lower(flag); + + Assertions.assertFalse(EXECUTOR.syncTo(flag), "Flag should not be raised since we lowered it."); + } + + @Test + void flagsAreReferenceEqual() { + var flagA = new NamedFlag("same"); + var flagB = new NamedFlag("same"); + + Assertions.assertNotSame(flagA, flagB, "Flags should not be the same object."); + Assertions.assertEquals(flagA, flagB, "Flags should be equal."); + + Assertions.assertFalse(EXECUTOR.isRaised(flagA), "Flag A should not be raised yet."); + Assertions.assertFalse(EXECUTOR.isRaised(flagB), "Flag B should not be raised yet."); + + EXECUTOR.raise(flagA); + + Assertions.assertTrue(EXECUTOR.isRaised(flagA), "Flag A should be raised since we raised it manually."); + Assertions.assertFalse(EXECUTOR.isRaised(flagB), "Flag B should not be raised yet."); + + EXECUTOR.raise(flagB); + + Assertions.assertTrue(EXECUTOR.isRaised(flagA), "Flag A should be raised since we raised it manually."); + Assertions.assertTrue(EXECUTOR.isRaised(flagB), "Flag B should be raised since we raised it manually."); + + EXECUTOR.lower(flagA); + + Assertions.assertFalse(EXECUTOR.isRaised(flagA), "Flag A should not be raised since we lowered it."); + Assertions.assertTrue(EXECUTOR.isRaised(flagB), "Flag B should be raised since we raised it manually."); + + EXECUTOR.lower(flagB); + + Assertions.assertFalse(EXECUTOR.isRaised(flagA), "Flag A should not be raised since we lowered it."); + Assertions.assertFalse(EXECUTOR.isRaised(flagB), "Flag B should not be raised since we lowered it."); + } + private static void assertExpectedSequence(IntArrayList sequence, int... expected) { Assertions.assertArrayEquals(expected, sequence.toIntArray()); } diff --git a/src/test/java/com/jozufozu/flywheel/lib/task/PlanSimplificationTest.java b/src/test/java/com/jozufozu/flywheel/lib/task/PlanSimplificationTest.java index e70b83c81..ea5501b14 100644 --- a/src/test/java/com/jozufozu/flywheel/lib/task/PlanSimplificationTest.java +++ b/src/test/java/com/jozufozu/flywheel/lib/task/PlanSimplificationTest.java @@ -36,7 +36,7 @@ public class PlanSimplificationTest { Assertions.assertEquals(oneSimple.simplify(), SIMPLE); - var mainThreadNoop = new OnMainThreadPlan<>(NOOP); + var mainThreadNoop = new SyncedPlan<>(NOOP); var oneMainThread = NestedPlan.of(mainThreadNoop); Assertions.assertEquals(oneMainThread.simplify(), mainThreadNoop); @@ -66,7 +66,7 @@ public class PlanSimplificationTest { @Test void complexNesting() { - var mainThreadNoop = OnMainThreadPlan.of(() -> { + var mainThreadNoop = SyncedPlan.of(() -> { }); var nested = NestedPlan.of(mainThreadNoop, SIMPLE); @@ -79,7 +79,7 @@ public class PlanSimplificationTest { @Test void nestedNoSimple() { - var mainThreadNoop = OnMainThreadPlan.of(() -> { + var mainThreadNoop = SyncedPlan.of(() -> { }); var barrier = new BarrierPlan<>(SIMPLE, SIMPLE); var oneMainThread = NestedPlan.of(mainThreadNoop, NestedPlan.of(mainThreadNoop, barrier, barrier));