More hooks more problems

- Remove Engine#setupRender
- Combine DrawManager#flush and DrawManager#render to simplify engines
- Document the hooks in RenderDispatcher

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

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.
+		 *
+		 * <p>Guaranteed to be called before {@link #afterEntities} and {@link #beforeCrumbling}.
+		 * <br>Guaranteed to be called after the render thread has processed all light updates.
+		 * <br>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.
+		 *
+		 * <p>Guaranteed to be called after {@link #onStartLevelRender} and before {@link #beforeCrumbling}.
+		 * <br>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.
+		 *
+		 * <p>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<SortedSet<BlockDestructionProgress>> destructionProgress);
 	}
 }

common/src/backend/java/dev/engine_room/flywheel/backend/engine/DrawManager.java

@@ -38,7 +38,7 @@ public abstract class DrawManager<N extends AbstractInstancer<?>> {
 	/**
 	 * A list of instancers that have not yet been initialized.
 	 * <br>
-	 * 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<UninitializedInstancer<N, ?>> initializationQueue = new ConcurrentLinkedQueue<>();
 
@@ -56,7 +56,7 @@ public abstract class DrawManager<N extends AbstractInstancer<?>> {
 		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<N extends AbstractInstancer<?>> {
 				.forEach(AbstractInstancer::clear);
 	}
 
-	public abstract void render();
-
 	public abstract void renderCrumbling(List<Engine.CrumblingBlock> crumblingBlocks);
 
 	protected abstract <I extends Instance> N create(InstancerKey<I> type);

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);
+			drawManager.render(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();
 		} catch (ShaderException e) {
 			FlwBackend.LOGGER.error("Falling back", e);
 			triggerFallback();

common/src/backend/java/dev/engine_room/flywheel/backend/engine/indirect/IndirectDrawManager.java

@@ -48,8 +48,6 @@ public class IndirectDrawManager extends DrawManager<IndirectInstancer<?>> {
 
 	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<IndirectInstancer<?>> {
 		group.add((IndirectInstancer<I>) 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) {
+	public void render(LightStorage lightStorage, EnvironmentStorage environmentStorage) {
-		super.flush(lightStorage, environmentStorage);
+		super.render(lightStorage, environmentStorage);
 
 		for (var group : cullingGroups.values()) {
 			group.flushInstancers();
@@ -151,7 +128,21 @@ public class IndirectDrawManager extends DrawManager<IndirectInstancer<?>> {
 			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

common/src/backend/java/dev/engine_room/flywheel/backend/engine/instancing/InstancedDrawManager.java

@@ -51,8 +51,8 @@ public class InstancedDrawManager extends DrawManager<InstancedInstancer<?>> {
 	}
 
 	@Override
-	public void flush(LightStorage lightStorage, EnvironmentStorage environmentStorage) {
+	public void render(LightStorage lightStorage, EnvironmentStorage environmentStorage) {
-		super.flush(lightStorage, environmentStorage);
+		super.render(lightStorage, environmentStorage);
 
 		this.instancers.values()
 				.removeIf(instancer -> {
@@ -71,13 +71,8 @@ public class InstancedDrawManager extends DrawManager<InstancedInstancer<?>> {
 		meshPool.flush();
 
 		light.flush(lightStorage);
-	}
 
-	@Override
+		if (draws.isEmpty()) {
-	public void render() {
-		var stage = draws;
-
-		if (stage.isEmpty()) {
 			return;
 		}
 
@@ -86,7 +81,7 @@ public class InstancedDrawManager extends DrawManager<InstancedInstancer<?>> {
 		TextureBinder.bindLightAndOverlay();
 		light.bind();
 
-		stage.draw(instanceTexture, programs);
+		draws.draw(instanceTexture, programs);
 
 		MaterialRenderState.reset();
 		TextureBinder.resetLightAndOverlay();

common/src/main/java/dev/engine_room/flywheel/impl/visualization/VisualizationManagerImpl.java

@@ -73,8 +73,6 @@ public class VisualizationManagerImpl implements VisualizationManager {
 	private final Plan<RenderContext> framePlan;
 	private final Plan<TickableVisual.Context> 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<Engine.CrumblingBlock> crumblingBlocks = new ArrayList<>();
 
 		for (var entry : destructionProgress.long2ObjectEntrySet()) {