Robust reload on pipeline state change

- Revert LightDataManager.reload() change from 2 days ago

src/main/java/grondag/canvas/apiimpl/CanvasState.java

@@ -34,6 +34,7 @@
 import grondag.canvas.material.property.TextureMaterialState;
 import grondag.canvas.perf.ChunkRebuildCounters;
 import grondag.canvas.perf.Timekeeper;
+import grondag.canvas.pipeline.Pipeline;
 import grondag.canvas.pipeline.PipelineManager;
 import grondag.canvas.pipeline.config.PipelineLoader;
 import grondag.canvas.shader.GlMaterialProgramManager;
@@ -46,27 +47,69 @@
 import grondag.canvas.terrain.util.ChunkColorCache;
 
 public class CanvasState {
-	public static void recompileIfNeeded(boolean forceRecompile) {
-		while (CanvasMod.RECOMPILE.consumeClick()) {
-			forceRecompile = true;
+	public static void handleRecompileKeybind() {
+		final boolean recompilePressed = CanvasMod.RECOMPILE.consumeClick();
+
+		while (true) {
+			// consume all clicks
+            if (!CanvasMod.RECOMPILE.consumeClick()) {
+				break;
+			}
+		}
+
+		if (recompilePressed) {
+			recompile(false);
+		}
+	}
+
+	public static void recompile() {
+		recompile(false);
+	}
+
+	private static int loopCounter = 0;
+
+	private static boolean recompilePipeline() {
+		final boolean prevColorLightsState = Pipeline.coloredLightsEnabled();
+		final boolean prevAdvancedCullingState = Pipeline.advancedTerrainCulling();
+
+		PipelineLoader.reload(Minecraft.getInstance().getResourceManager());
+		PipelineManager.reload();
+
+		final boolean coloredLightsChanged = Pipeline.coloredLightsEnabled() && !prevColorLightsState;
+		final boolean requireCullingRebuild = Pipeline.advancedTerrainCulling() && !prevAdvancedCullingState;
+
+		return coloredLightsChanged || requireCullingRebuild;
+	}
+
+	private static void recompile(boolean alreadyReloaded) {
+		CanvasMod.LOG.info(I18n.get("info.canvas.recompile"));
+
+		final boolean requireReload = recompilePipeline();
+
+		if (!alreadyReloaded && loopCounter < 2 && requireReload) {
+			CanvasMod.LOG.info(I18n.get("info.canvas.recompile_needs_reload"));
+			loopCounter++;
+			Minecraft.getInstance().levelRenderer.allChanged();
+			return;
 		}
 
-		if (forceRecompile) {
-			CanvasMod.LOG.info(I18n.get("info.canvas.recompile"));
-			PipelineLoader.reload(Minecraft.getInstance().getResourceManager());
-			PipelineManager.reload();
-			LightDataManager.reload();
-			PreReleaseShaderCompat.reload();
-			MaterialProgram.reload();
-			GlShaderManager.INSTANCE.reload();
-			GlProgramManager.INSTANCE.reload();
-			GlMaterialProgramManager.INSTANCE.reload();
-			// LightmapHdTexture.reload();
-			// LightmapHd.reload();
-			TextureMaterialState.reload();
-			ShaderDataManager.reload();
-			Timekeeper.configOrPipelineReload();
+		LightDataManager.reload();
+		PreReleaseShaderCompat.reload();
+		MaterialProgram.reload();
+		GlShaderManager.INSTANCE.reload();
+		GlProgramManager.INSTANCE.reload();
+		GlMaterialProgramManager.INSTANCE.reload();
+		// LightmapHdTexture.reload();
+		// LightmapHd.reload();
+		TextureMaterialState.reload();
+		ShaderDataManager.reload();
+		Timekeeper.configOrPipelineReload();
+
+		if (loopCounter > 1) {
+			CanvasMod.LOG.warn("Reloading recursively twice or more. This isn't supposed to happen.");
 		}
+
+		loopCounter = 0;
 	}
 
 	public static void reload() {
@@ -79,6 +122,6 @@ public static void reload() {
 		ChunkColorCache.invalidate();
 		AoFace.clampExteriorVertices(Configurator.clampExteriorVertices);
 
-		recompileIfNeeded(true);
+		recompile(true);
 	}
 }

src/main/java/grondag/canvas/config/CanvasConfigScreen.java

@@ -528,7 +528,7 @@ private void save() {
 			Timekeeper.configOrPipelineReload();
 		}
 
-		ConfigManager.saveUserInput(reload ? RELOAD_EVERYTHING : DONT_RELOAD);
+		ConfigManager.saveCanvasConfig(reload ? RELOAD_EVERYTHING : DONT_RELOAD);
 
 		if (requiresRestart) {
 			this.minecraft.setScreen(new ConfigRestartScreen(this.parent));

src/main/java/grondag/canvas/config/ConfigManager.java

@@ -120,7 +120,7 @@ public static void savePipelineOptions(OptionConfig[] options) {
 			CanvasMod.LOG.error("Error loading pipeline config. Using default values.");
 		}
 
-		CanvasState.recompileIfNeeded(true);
+		CanvasState.recompile();
 	}
 
 	private static void saveConfig() {
@@ -163,12 +163,12 @@ static void loadConfig() {
 		Configurator.readFromConfig(config, true);
 	}
 
-	static void saveUserInput(Reload reload) {
+	static void saveCanvasConfig(Reload reload) {
 		saveConfig();
 
 		switch (reload) {
 			case RELOAD_EVERYTHING -> Minecraft.getInstance().levelRenderer.allChanged();
-			case RELOAD_PIPELINE -> CanvasState.recompileIfNeeded(true);
+			case RELOAD_PIPELINE -> CanvasState.recompile();
 			case DONT_RELOAD -> { }
 		}
 	}

src/main/java/grondag/canvas/config/PipelineOptionScreen.java

@@ -20,7 +20,6 @@
 
 package grondag.canvas.config;
 
-import static grondag.canvas.config.ConfigManager.Reload.RELOAD_EVERYTHING;
 import static grondag.canvas.config.ConfigManager.Reload.RELOAD_PIPELINE;
 
 import java.util.List;
@@ -40,7 +39,6 @@
 import grondag.canvas.config.gui.BaseButton;
 import grondag.canvas.config.gui.BaseScreen;
 import grondag.canvas.config.gui.ListWidget;
-import grondag.canvas.pipeline.Pipeline;
 import grondag.canvas.pipeline.config.PipelineConfig;
 import grondag.canvas.pipeline.config.PipelineConfigBuilder;
 import grondag.canvas.pipeline.config.PipelineLoader;
@@ -118,14 +116,8 @@ protected void init() {
 	}
 
 	private void savePipelineSelection(ResourceLocation newPipelineId) {
-		final var newPipeline = PipelineLoader.get(newPipelineId.toString());
-
-		boolean shadowsChanged = Pipeline.shadowsEnabled() != newPipeline.shadowsEnabled;
-		boolean coloredLightsChanged = Pipeline.coloredLightsEnabled() != newPipeline.coloredLightsEnabled;
-		boolean needRegionsReloaded = (shadowsChanged && !Configurator.advancedTerrainCulling) || coloredLightsChanged;
-
 		Configurator.pipelineId = newPipelineId.toString();
-		ConfigManager.saveUserInput(needRegionsReloaded ? RELOAD_EVERYTHING : RELOAD_PIPELINE);
+		ConfigManager.saveCanvasConfig(RELOAD_PIPELINE);
 	}
 
 	private void save() {

src/main/java/grondag/canvas/light/color/LightDataManager.java

@@ -50,16 +50,21 @@ public static LightRegionAccess allocate(BlockPos regionOrigin) {
 	}
 
 	public static void reload() {
-		if (INSTANCE != null) {
-			INSTANCE.close();
-			INSTANCE = null;
-		}
-
 		if (Pipeline.coloredLightsEnabled()) {
 			assert Pipeline.config().coloredLights != null;
 
-			INSTANCE = new LightDataManager();
+			// NB: this is a shader recompile, not chunk storage reload. DON'T destroy existing instance.
+			if (INSTANCE == null) {
+				INSTANCE = new LightDataManager();
+			}
+
 			INSTANCE.useOcclusionData = Pipeline.config().coloredLights.useOcclusionData;
+		} else {
+			// If colored lights state change, we can clean this up as the next state change will trigger chunk storage reload.
+			if (INSTANCE != null) {
+				INSTANCE.close();
+				INSTANCE = null;
+			}
 		}
 	}
 
@@ -341,7 +346,7 @@ public void end() {
 			minUpdateTimeNanos = Math.min(elapsedNanos, minUpdateTimeNanos);
 			maxUpdateTimeNanos = Math.max(elapsedNanos, maxUpdateTimeNanos);
 			totalUpdateTimeNanos += elapsedNanos;
-			totalUpdatePerformed ++;
+			totalUpdatePerformed++;
 
 			if (System.nanoTime() - startTimeOverall > 10000000000L) {
 				CanvasMod.LOG.info("Colored Lights profiler output:");

src/main/java/grondag/canvas/render/world/CanvasWorldRenderer.java

@@ -851,7 +851,7 @@ public void renderLevel(PoseStack viewMatrixStack, float tickDelta, long frameSt
 		BufferSynchronizer.checkPoint();
 		DirectBufferAllocator.update();
 		TransferBuffers.update();
-		CanvasState.recompileIfNeeded(false);
+		CanvasState.handleRecompileKeybind();
 		FlawlessFramesController.handleToggle();
 
 		if (wasFabulous != Pipeline.isFabulous()) {

src/main/java/grondag/canvas/shader/data/AccessibilityData.java

@@ -41,7 +41,7 @@ public class AccessibilityData {
 
 	public static void onCloseOptionScreen() {
 		if (AccessibilityData.checkChanged() && Minecraft.getInstance().level != null) {
-			CanvasState.recompileIfNeeded(true);
+			CanvasState.recompile();
 		}
 	}
 

src/main/resources/assets/canvas/lang/en_us.json

@@ -204,5 +204,6 @@
   "config.canvas.help.trace_texture_load": "Log significant events of texture/sprite atlas loading.;For debugging use. Will spam the log.",
   "config.canvas.value.bloom_toggle": "Enable Bloom",
   "config.canvas.help.bloom_toggle": "Renders glow effect around light sources.;Modest impact on performance.",
-  "info.canvas.recompile": "Recompiling shaders"
+  "info.canvas.recompile": "Recompiling shaders",
+  "info.canvas.recompile_needs_reload": "Found a state change that requires reload during recompilation. Reloading renderer..."
 }