From a2fc11149962beb1025ab183feedd52e2064f26f Mon Sep 17 00:00:00 2001 From: Jozufozu Date: Sat, 15 Feb 2025 13:05:02 -0800 Subject: [PATCH] More hooks more problems - Remove Engine#setupRender - Combine DrawManager#flush and DrawManager#render to simplify engines - Document the hooks in RenderDispatcher --- .../flywheel/api/backend/Engine.java | 17 ++------ .../visualization/VisualizationManager.java | 21 +++++++++ .../flywheel/backend/engine/DrawManager.java | 6 +-- .../flywheel/backend/engine/EngineImpl.java | 14 +----- .../engine/indirect/IndirectDrawManager.java | 43 ++++++++----------- .../instancing/InstancedDrawManager.java | 13 ++---- .../VisualizationManagerImpl.java | 15 +------ 7 files changed, 50 insertions(+), 79 deletions(-) diff --git a/common/src/api/java/dev/engine_room/flywheel/api/backend/Engine.java b/common/src/api/java/dev/engine_room/flywheel/api/backend/Engine.java index 1df77bff5..9843d9094 100644 --- a/common/src/api/java/dev/engine_room/flywheel/api/backend/Engine.java +++ b/common/src/api/java/dev/engine_room/flywheel/api/backend/Engine.java @@ -58,22 +58,11 @@ public interface Engine { void onLightUpdate(SectionPos sectionPos, LightLayer layer); /** - * Set up rendering for the current level render. + * Render all instances necessary for the given visual type. * *

This method is guaranteed to be called after * {@linkplain #createFramePlan() the frame plan} has finished execution and before - * {@link #render} and {@link #renderCrumbling} are called. This method is guaranteed to - * be called on the render thread. - * - * @param context The context for the current level render. - */ - void setupRender(RenderContext context); - - /** - * Render all instances necessary for the given visual type. - * - *

This method is guaranteed to be called after {@link #setupRender} for the current - * level render. This method is guaranteed to be called on the render thread. + * {@link #renderCrumbling} are called. This method is guaranteed to be called on the render thread. * * @param context The context for the current level render. */ @@ -82,7 +71,7 @@ public interface Engine { /** * Render the given instances as a crumbling overlay. * - *

This method is guaranteed to be called after {@link #setupRender} for the current + *

This method is guaranteed to be called after {@link #render} for the current * level render. This method is guaranteed to be called on the render thread. * * @param context The context for the current level render. diff --git a/common/src/api/java/dev/engine_room/flywheel/api/visualization/VisualizationManager.java b/common/src/api/java/dev/engine_room/flywheel/api/visualization/VisualizationManager.java index 512ea1439..e7f831dbc 100644 --- a/common/src/api/java/dev/engine_room/flywheel/api/visualization/VisualizationManager.java +++ b/common/src/api/java/dev/engine_room/flywheel/api/visualization/VisualizationManager.java @@ -47,10 +47,31 @@ public interface VisualizationManager { @ApiStatus.NonExtendable interface RenderDispatcher { + /** + * Prepare visuals for render. + * + *

Guaranteed to be called before {@link #afterEntities} and {@link #beforeCrumbling}. + *
Guaranteed to be called after the render thread has processed all light updates. + *
The caller is otherwise free to choose an invocation site, but it is recommended to call + * this as early as possible to give the VisualizationManager time to process things off-thread. + */ void onStartLevelRender(RenderContext ctx); + /** + * Render instances. + * + *

Guaranteed to be called after {@link #onStartLevelRender} and before {@link #beforeCrumbling}. + *
The caller is otherwise free to choose an invocation site, but it is recommended to call + * this between rendering entities and block entities. + */ void afterEntities(RenderContext ctx); + /** + * Render crumbling block entities. + * + *

Guaranteed to be called after {@link #onStartLevelRender} and {@link #afterEntities} + * @param destructionProgress The destruction progress map from {@link net.minecraft.client.renderer.LevelRenderer LevelRenderer}. + */ void beforeCrumbling(RenderContext ctx, Long2ObjectMap> destructionProgress); } } diff --git a/common/src/backend/java/dev/engine_room/flywheel/backend/engine/DrawManager.java b/common/src/backend/java/dev/engine_room/flywheel/backend/engine/DrawManager.java index bf4d10bee..996c900a2 100644 --- a/common/src/backend/java/dev/engine_room/flywheel/backend/engine/DrawManager.java +++ b/common/src/backend/java/dev/engine_room/flywheel/backend/engine/DrawManager.java @@ -38,7 +38,7 @@ public abstract class DrawManager> { /** * A list of instancers that have not yet been initialized. *
- * All new instancers land here before having resources allocated in {@link #flush}. + * All new instancers land here before having resources allocated in {@link #render}. */ protected final Queue> initializationQueue = new ConcurrentLinkedQueue<>(); @@ -56,7 +56,7 @@ public abstract class DrawManager> { return ForEachPlan.of(() -> new ArrayList<>(instancers.values()), AbstractInstancer::parallelUpdate); } - public void flush(LightStorage lightStorage, EnvironmentStorage environmentStorage) { + public void render(LightStorage lightStorage, EnvironmentStorage environmentStorage) { // Thread safety: flush is called from the render thread after all visual updates have been made, // so there are no:tm: threads we could be racing with. for (var init : initializationQueue) { @@ -75,8 +75,6 @@ public abstract class DrawManager> { .forEach(AbstractInstancer::clear); } - public abstract void render(); - public abstract void renderCrumbling(List crumblingBlocks); protected abstract N create(InstancerKey type); diff --git a/common/src/backend/java/dev/engine_room/flywheel/backend/engine/EngineImpl.java b/common/src/backend/java/dev/engine_room/flywheel/backend/engine/EngineImpl.java index 330915bd9..a5c83d33d 100644 --- a/common/src/backend/java/dev/engine_room/flywheel/backend/engine/EngineImpl.java +++ b/common/src/backend/java/dev/engine_room/flywheel/backend/engine/EngineImpl.java @@ -89,23 +89,13 @@ public class EngineImpl implements Engine { } @Override - public void setupRender(RenderContext context) { + public void render(RenderContext context) { try (var state = GlStateTracker.getRestoreState()) { // Process the render queue for font updates RenderSystem.replayQueue(); Uniforms.update(context); environmentStorage.flush(); - drawManager.flush(lightStorage, environmentStorage); - } catch (ShaderException e) { - FlwBackend.LOGGER.error("Falling back", e); - triggerFallback(); - } - } - - @Override - public void render(RenderContext context) { - try (var state = GlStateTracker.getRestoreState()) { - drawManager.render(); + drawManager.render(lightStorage, environmentStorage); } catch (ShaderException e) { FlwBackend.LOGGER.error("Falling back", e); triggerFallback(); diff --git a/common/src/backend/java/dev/engine_room/flywheel/backend/engine/indirect/IndirectDrawManager.java b/common/src/backend/java/dev/engine_room/flywheel/backend/engine/indirect/IndirectDrawManager.java index 6ac442dab..5cc280750 100644 --- a/common/src/backend/java/dev/engine_room/flywheel/backend/engine/indirect/IndirectDrawManager.java +++ b/common/src/backend/java/dev/engine_room/flywheel/backend/engine/indirect/IndirectDrawManager.java @@ -48,8 +48,6 @@ public class IndirectDrawManager extends DrawManager> { private final DepthPyramid depthPyramid; - private boolean needsBarrier = false; - public IndirectDrawManager(IndirectPrograms programs) { this.programs = programs; programs.acquire(); @@ -78,30 +76,9 @@ public class IndirectDrawManager extends DrawManager> { group.add((IndirectInstancer) instancer, key, meshPool); } - public void render() { - TextureBinder.bindLightAndOverlay(); - - vertexArray.bindForDraw(); - lightBuffers.bind(); - matrixBuffer.bind(); - Uniforms.bindAll(); - - if (needsBarrier) { - glMemoryBarrier(GL_SHADER_STORAGE_BARRIER_BIT); - needsBarrier = false; - } - - for (var group : cullingGroups.values()) { - group.submit(); - } - - MaterialRenderState.reset(); - TextureBinder.resetLightAndOverlay(); - } - @Override - public void flush(LightStorage lightStorage, EnvironmentStorage environmentStorage) { - super.flush(lightStorage, environmentStorage); + public void render(LightStorage lightStorage, EnvironmentStorage environmentStorage) { + super.render(lightStorage, environmentStorage); for (var group : cullingGroups.values()) { group.flushInstancers(); @@ -151,7 +128,21 @@ public class IndirectDrawManager extends DrawManager> { group.dispatchApply(); } - needsBarrier = true; + glMemoryBarrier(GL_SHADER_STORAGE_BARRIER_BIT); + + TextureBinder.bindLightAndOverlay(); + + vertexArray.bindForDraw(); + lightBuffers.bind(); + matrixBuffer.bind(); + Uniforms.bindAll(); + + for (var group : cullingGroups.values()) { + group.submit(); + } + + MaterialRenderState.reset(); + TextureBinder.resetLightAndOverlay(); } @Override diff --git a/common/src/backend/java/dev/engine_room/flywheel/backend/engine/instancing/InstancedDrawManager.java b/common/src/backend/java/dev/engine_room/flywheel/backend/engine/instancing/InstancedDrawManager.java index 7a798e0d3..e2a97ddf7 100644 --- a/common/src/backend/java/dev/engine_room/flywheel/backend/engine/instancing/InstancedDrawManager.java +++ b/common/src/backend/java/dev/engine_room/flywheel/backend/engine/instancing/InstancedDrawManager.java @@ -51,8 +51,8 @@ public class InstancedDrawManager extends DrawManager> { } @Override - public void flush(LightStorage lightStorage, EnvironmentStorage environmentStorage) { - super.flush(lightStorage, environmentStorage); + public void render(LightStorage lightStorage, EnvironmentStorage environmentStorage) { + super.render(lightStorage, environmentStorage); this.instancers.values() .removeIf(instancer -> { @@ -71,13 +71,8 @@ public class InstancedDrawManager extends DrawManager> { meshPool.flush(); light.flush(lightStorage); - } - @Override - public void render() { - var stage = draws; - - if (stage.isEmpty()) { + if (draws.isEmpty()) { return; } @@ -86,7 +81,7 @@ public class InstancedDrawManager extends DrawManager> { TextureBinder.bindLightAndOverlay(); light.bind(); - stage.draw(instanceTexture, programs); + draws.draw(instanceTexture, programs); MaterialRenderState.reset(); TextureBinder.resetLightAndOverlay(); diff --git a/common/src/main/java/dev/engine_room/flywheel/impl/visualization/VisualizationManagerImpl.java b/common/src/main/java/dev/engine_room/flywheel/impl/visualization/VisualizationManagerImpl.java index ba91bc73e..5eeadf043 100644 --- a/common/src/main/java/dev/engine_room/flywheel/impl/visualization/VisualizationManagerImpl.java +++ b/common/src/main/java/dev/engine_room/flywheel/impl/visualization/VisualizationManagerImpl.java @@ -73,8 +73,6 @@ public class VisualizationManagerImpl implements VisualizationManager { private final Plan framePlan; private final Plan tickPlan; - private boolean canEngineRender; - private VisualizationManagerImpl(LevelAccessor level) { taskExecutor = FlwTaskExecutor.get(); engine = BackendManager.currentBackend() @@ -241,24 +239,15 @@ public class VisualizationManagerImpl implements VisualizationManager { frameFlag.lower(); frameLimiter.tick(); - canEngineRender = false; framePlan.execute(taskExecutor, context); } - private void ensureCanRender(RenderContext context) { - taskExecutor.syncUntil(frameFlag::isRaised); - if (!canEngineRender) { - engine.setupRender(context); - canEngineRender = true; - } - } - /** * Draw all visuals of the given type. */ private void render(RenderContext context) { - ensureCanRender(context); + taskExecutor.syncUntil(frameFlag::isRaised); engine.render(context); } @@ -267,8 +256,6 @@ public class VisualizationManagerImpl implements VisualizationManager { return; } - ensureCanRender(context); - List crumblingBlocks = new ArrayList<>(); for (var entry : destructionProgress.long2ObjectEntrySet()) {