Fix framebuffer leaks in shader init callbacks

changelog.md

@@ -1,3 +1,13 @@
+------------------------------------------------------
+Version 1.18.0
+------------------------------------------------------
+**Additions**
+- Added `WorldRendererReloadCallback`, an event that gets triggered when e.g. video settings are updated or the player joins a world
+
+**Changes**
+- Shaders' init callbacks now also run during the above event
+  - This fixes resource leaks caused by setting a sampler uniform to a vanilla Framebuffer in those callbacks
+
 ------------------------------------------------------
 Version 1.17.0
 ------------------------------------------------------

gradle.properties

@@ -3,14 +3,14 @@ org.gradle.jvmargs=-Xmx2G
 
 # Fabric Properties
 # check these on https://fabricmc.net/develop
-minecraft_version=1.20.5
-yarn_mappings=1.20.5+build.1
-loader_version=0.15.10
+minecraft_version=1.20.6
+yarn_mappings=1.20.6+build.1
+loader_version=0.15.11
 #Fabric api
-fabric_version=0.97.5+1.20.5
+fabric_version=0.98.0+1.20.6
 
 # Mod Properties
-mod_version = 1.17.0
+mod_version = 1.18.0
 owners = Ladysnake
 maven_group = org.ladysnake
 archives_base_name = satin
@@ -27,7 +27,7 @@ display_name = Satin
 license_header = LGPL
 gpl_version = 3
 curseforge_id = 385463
-curseforge_versions = 1.20.5
+curseforge_versions = 1.20.5; 1.20.6
 cf_requirements = fabric-api
 modrinth_id = fRbqPLg4
 release_type = release

src/main/java/ladysnake/satin/Satin.java

@@ -18,6 +18,7 @@
 package ladysnake.satin;
 
 import ladysnake.satin.api.event.ResolutionChangeCallback;
+import ladysnake.satin.api.event.WorldRendererReloadCallback;
 import ladysnake.satin.impl.ReloadableShaderEffectManager;
 import net.fabricmc.api.ClientModInitializer;
 import net.fabricmc.loader.api.FabricLoader;
@@ -44,9 +45,9 @@ public static boolean areShadersDisabled() {
     @Override
     public void onInitializeClient() {
         ResolutionChangeCallback.EVENT.register(ReloadableShaderEffectManager.INSTANCE);
+        WorldRendererReloadCallback.EVENT.register(ReloadableShaderEffectManager.INSTANCE);
         if (FabricLoader.getInstance().isModLoaded("optifabric")) {
             LOGGER.warn("[Satin] Optifine present in the instance, custom entity post process shaders will not work");
         }
     }
-
 }

src/main/java/ladysnake/satin/api/event/WorldRendererReloadCallback.java

@@ -0,0 +1,37 @@
+/*
+ * Satin
+ * Copyright (C) 2019-2024 Ladysnake
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; If not, see <https://www.gnu.org/licenses>.
+ */
+package ladysnake.satin.api.event;
+
+import net.fabricmc.fabric.api.event.Event;
+import net.fabricmc.fabric.api.event.EventFactory;
+import net.minecraft.client.render.WorldRenderer;
+
+@FunctionalInterface
+public interface WorldRendererReloadCallback {
+    /**
+     * Fired in {@link WorldRenderer#reload()}, typically called when video settings are updated or the player joins a world
+     */
+    Event<WorldRendererReloadCallback> EVENT = EventFactory.createArrayBacked(WorldRendererReloadCallback.class,
+            (listeners) -> (renderer) -> {
+                for (WorldRendererReloadCallback event : listeners) {
+                    event.onRendererReload(renderer);
+                }
+            });
+
+    void onRendererReload(WorldRenderer renderer);
+}

src/main/java/ladysnake/satin/impl/ReloadableShaderEffectManager.java

@@ -20,11 +20,15 @@
 import it.unimi.dsi.fastutil.objects.ReferenceOpenHashSet;
 import ladysnake.satin.Satin;
 import ladysnake.satin.api.event.ResolutionChangeCallback;
+import ladysnake.satin.api.event.WorldRendererReloadCallback;
 import ladysnake.satin.api.managed.ManagedCoreShader;
 import ladysnake.satin.api.managed.ManagedShaderEffect;
 import ladysnake.satin.api.managed.ShaderEffectManager;
+import net.minecraft.client.MinecraftClient;
 import net.minecraft.client.render.VertexFormat;
 import net.minecraft.client.render.VertexFormats;
+import net.minecraft.client.render.WorldRenderer;
+import net.minecraft.client.util.Window;
 import net.minecraft.resource.ResourceFactory;
 import net.minecraft.util.Identifier;
 
@@ -37,7 +41,7 @@
  * @see ShaderEffectManager
  * @see ResettableManagedShaderEffect
  */
-public final class ReloadableShaderEffectManager implements ShaderEffectManager, ResolutionChangeCallback {
+public final class ReloadableShaderEffectManager implements ShaderEffectManager, ResolutionChangeCallback, WorldRendererReloadCallback {
     public static final ReloadableShaderEffectManager INSTANCE = new ReloadableShaderEffectManager();
     public static final Identifier SHADER_RESOURCE_KEY = new Identifier("dissolution:shaders");
 
@@ -114,6 +118,16 @@ public void reload(ResourceFactory shaderResources) {
 
     @Override
     public void onResolutionChanged(int newWidth, int newHeight) {
+        runShaderSetup(newWidth, newHeight);
+    }
+
+    @Override
+    public void onRendererReload(WorldRenderer renderer) {
+        Window window = MinecraftClient.getInstance().getWindow();
+        runShaderSetup(window.getFramebufferWidth(), window.getFramebufferHeight());
+    }
+
+    private void runShaderSetup(int newWidth, int newHeight) {
         if (!Satin.areShadersDisabled() && !managedShaders.isEmpty()) {
             for (ResettableManagedShaderBase<?> ss : managedShaders) {
                 if (ss.isInitialized()) {

src/main/java/ladysnake/satin/mixin/client/render/WorldRendererMixin.java

@@ -0,0 +1,33 @@
+/*
+ * Satin
+ * Copyright (C) 2019-2024 Ladysnake
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; If not, see <https://www.gnu.org/licenses>.
+ */
+package ladysnake.satin.mixin.client.render;
+
+import ladysnake.satin.api.event.WorldRendererReloadCallback;
+import net.minecraft.client.render.WorldRenderer;
+import org.spongepowered.asm.mixin.Mixin;
+import org.spongepowered.asm.mixin.injection.At;
+import org.spongepowered.asm.mixin.injection.Inject;
+import org.spongepowered.asm.mixin.injection.callback.CallbackInfo;
+
+@Mixin(WorldRenderer.class)
+public abstract class WorldRendererMixin {
+    @Inject(method = "reload()V", at = @At("RETURN"))
+    private void reloadShaders(CallbackInfo ci) {
+        WorldRendererReloadCallback.EVENT.invoker().onRendererReload((WorldRenderer) (Object) this);
+    }
+}

src/main/resources/mixins.satin.client.json

@@ -11,11 +11,11 @@
     "event.MinecraftClientMixin",
     "event.WorldRendererMixin",
     "gl.CoreShaderMixin",
-    "gl.DepthGlFramebufferMixin",
     "gl.CustomFormatFramebufferMixin",
+    "gl.CustomFormatPostEffectProcessorMixin",
+    "gl.DepthGlFramebufferMixin",
     "gl.GlUniformMixin",
     "gl.JsonEffectGlShaderMixin",
-    "gl.CustomFormatPostEffectProcessorMixin",
     "render.RenderLayerAccessor",
     "render.RenderLayerMixin",
     "render.RenderLayerMixin$MultiPhaseParametersAccessor",
@@ -26,5 +26,6 @@
     "defaultRequire": 1
   },
   "client": [
+    "render.WorldRendererMixin"
   ]
 }

src/testmod/java/ladysnake/satinbasictest/SatinBasicTest.java

@@ -24,13 +24,18 @@
 import ladysnake.satintestcore.item.SatinTestItems;
 import net.fabricmc.api.ClientModInitializer;
 import net.minecraft.util.Identifier;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
 
 public final class SatinBasicTest implements ClientModInitializer {
     public static final String MOD_ID = "satinbasictest";
+    private static final Logger LOGGER = LogManager.getLogger(MOD_ID);
 
     private boolean renderingBlit = false;
     // literally the same as minecraft's blit, we are just checking that custom paths work
-    private final ManagedShaderEffect testShader = ShaderEffectManager.getInstance().manage(new Identifier(MOD_ID, "shaders/post/blit.json"));
+    private final ManagedShaderEffect testShader = ShaderEffectManager.getInstance().manage(new Identifier(MOD_ID, "shaders/post/blit.json"), (effect) -> {
+        LOGGER.info("Test shader got updated");
+    });
     private final Uniform4f color = testShader.findUniform4f("ColorModulate");
 
     @Override