Fluid rendering fixes (#3593)

* Fluid rendering fixes

- Fix default overlay block check using TransparentBlock instead of TranslucentBlock
- Fix setBlockTransparency/isBlockTransparent not being thread-safe
- Reuse the same render handler objects for water and lava

* Fix implementation issues

- Fix custom geometry being buffered twice if FluidRenderHandler#renderFluid is invoked directly
- Fix calling FluidRenderHandler.super.renderFluid not using passed arguments to calculate color
- Fix calling FluidRenderHandler.super.renderFluid more than once producing incorrect geometry
- Fix fluids with no handler never receiving water overlay instead of using default behavior

* Add way to render fluid with non-vanilla default

- Fix testmod

fabric-rendering-fluids-v1/src/client/java/net/fabricmc/fabric/api/client/render/fluid/v1/FluidRenderHandler.java

@@ -20,13 +20,14 @@
 
 import net.minecraft.block.BlockState;
 import net.minecraft.client.render.VertexConsumer;
+import net.minecraft.client.render.block.FluidRenderer;
 import net.minecraft.client.texture.Sprite;
 import net.minecraft.client.texture.SpriteAtlasTexture;
 import net.minecraft.fluid.FluidState;
 import net.minecraft.util.math.BlockPos;
 import net.minecraft.world.BlockRenderView;
 
-import net.fabricmc.fabric.impl.client.rendering.fluid.FluidRenderHandlerRegistryImpl;
+import net.fabricmc.fabric.impl.client.rendering.fluid.FluidRenderingImpl;
 
 /**
  * Interface for handling the rendering of a FluidState.
@@ -67,14 +68,13 @@ default int getFluidColor(@Nullable BlockRenderView view, @Nullable BlockPos pos
 	}
 
 	/**
-	 * Tessellate your fluid. This method will be invoked before the default
-	 * fluid renderer. By default, it will call the default fluid renderer. Call
-	 * {@code FluidRenderHandler.super.renderFluid} if you want to render over
-	 * the default fluid renderer.
-	 *
-	 * <p>Note that this method must *only* return {@code true} if at least one
-	 * face is tessellated. If no faces are tessellated this method must return
-	 * {@code false}.
+	 * Tessellate your fluid. By default, this method will call the default
+	 * fluid renderer. Call {@code FluidRenderHandler.super.renderFluid} if
+	 * you want to render over the default fluid renderer. This is the
+	 * intended way to render default geometry; calling
+	 * {@link FluidRenderer#render} is not supported. When rendering default
+	 * geometry, the current handler will be used instead of looking up
+	 * a new one for the passed fluid state.
 	 *
 	 * @param pos The position in the world, of the fluid to render.
 	 * @param world The world the fluid is in
@@ -83,7 +83,7 @@ default int getFluidColor(@Nullable BlockRenderView view, @Nullable BlockPos pos
 	 * @param fluidState The fluid state being rendered.
 	 */
 	default void renderFluid(BlockPos pos, BlockRenderView world, VertexConsumer vertexConsumer, BlockState blockState, FluidState fluidState) {
-		((FluidRenderHandlerRegistryImpl) FluidRenderHandlerRegistry.INSTANCE).renderFluid(pos, world, vertexConsumer, blockState, fluidState);
+		FluidRenderingImpl.renderDefault(this, world, pos, vertexConsumer, blockState, fluidState);
 	}
 
 	/**

fabric-rendering-fluids-v1/src/client/java/net/fabricmc/fabric/api/client/render/fluid/v1/FluidRendering.java

@@ -0,0 +1,72 @@
+/*
+ * Copyright (c) 2016, 2017, 2018, 2019 FabricMC
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package net.fabricmc.fabric.api.client.render.fluid.v1;
+
+import net.minecraft.block.BlockState;
+import net.minecraft.client.render.VertexConsumer;
+import net.minecraft.client.render.block.FluidRenderer;
+import net.minecraft.fluid.FluidState;
+import net.minecraft.util.math.BlockPos;
+import net.minecraft.world.BlockRenderView;
+
+import net.fabricmc.fabric.impl.client.rendering.fluid.FluidRenderingImpl;
+
+/**
+ * A class containing some utilities for rendering fluids.
+ */
+public final class FluidRendering {
+	private FluidRendering() {
+	}
+
+	/**
+	 * Renders a fluid using the given handler, default renderer, and context. Internally, this just invokes
+	 * {@link FluidRenderHandler#renderFluid}, but the passed default renderer is invoked instead of the vanilla
+	 * renderer whenever the handler requests default geometry to be rendered.
+	 *
+	 * @param handler the render handler to invoke {@link FluidRenderHandler#renderFluid} on
+	 * @param world the world
+	 * @param pos the pos
+	 * @param vertexConsumer the vertex consumer
+	 * @param blockState the block state
+	 * @param fluidState the fluid state
+	 * @param defaultRenderer the renderer to use whenever the handler requests default geometry
+	 */
+	public static void render(FluidRenderHandler handler, BlockRenderView world, BlockPos pos, VertexConsumer vertexConsumer, BlockState blockState, FluidState fluidState, DefaultRenderer defaultRenderer) {
+		FluidRenderingImpl.render(handler, world, pos, vertexConsumer, blockState, fluidState, defaultRenderer);
+	}
+
+	public interface DefaultRenderer {
+		/**
+		 * Render the default geometry when it is requested by {@link FluidRenderHandler#renderFluid}. The default
+		 * implementation invokes the vanilla renderer. Calling {@link FluidRenderer#render} directly is not supported
+		 * but using {@code DefaultRenderer.super.render} is supported. Note that the parameter values passed to this
+		 * call are provided by the render handler, meaning they are not necessarily the same as those provided to the
+		 * initial rendering call. As per the documentation of {@link FluidRenderHandler#renderFluid}, a new handler
+		 * should not be retrieved and only the passed one should be used.
+		 *
+		 * @param handler the handler that {@link FluidRenderHandler#renderFluid} was invoked on
+		 * @param world the world
+		 * @param pos the pos
+		 * @param vertexConsumer the vertex consumer
+		 * @param blockState the block state
+		 * @param fluidState the fluid state
+		 */
+		default void render(FluidRenderHandler handler, BlockRenderView world, BlockPos pos, VertexConsumer vertexConsumer, BlockState blockState, FluidState fluidState) {
+			FluidRenderingImpl.renderVanillaDefault(handler, world, pos, vertexConsumer, blockState, fluidState);
+		}
+	}
+}

fabric-rendering-fluids-v1/src/client/java/net/fabricmc/fabric/api/client/render/fluid/v1/SimpleFluidRenderHandler.java

@@ -85,7 +85,7 @@ public class SimpleFluidRenderHandler implements FluidRenderHandler {
 	 */
 	public SimpleFluidRenderHandler(Identifier stillTexture, Identifier flowingTexture, @Nullable Identifier overlayTexture, int tint) {
 		this.stillTexture = Objects.requireNonNull(stillTexture, "stillTexture");
-		this.flowingTexture = Objects.requireNonNull(flowingTexture, "flowingTexture");;
+		this.flowingTexture = Objects.requireNonNull(flowingTexture, "flowingTexture");
 		this.overlayTexture = overlayTexture;
 		this.sprites = new Sprite[overlayTexture == null ? 2 : 3];
 		this.tint = tint;

fabric-rendering-fluids-v1/src/client/java/net/fabricmc/fabric/impl/client/rendering/fluid/FluidRendererHookContainer.java → fabric-rendering-fluids-v1/src/client/java/net/fabricmc/fabric/impl/client/rendering/fluid/FluidRenderHandlerInfo.java

@@ -16,49 +16,41 @@
 
 package net.fabricmc.fabric.impl.client.rendering.fluid;
 
-import net.minecraft.block.BlockState;
+import org.jetbrains.annotations.Nullable;
+
 import net.minecraft.client.texture.Sprite;
 import net.minecraft.fluid.FluidState;
 import net.minecraft.util.math.BlockPos;
 import net.minecraft.world.BlockRenderView;
 
 import net.fabricmc.fabric.api.client.render.fluid.v1.FluidRenderHandler;
 
-public class FluidRendererHookContainer {
-	public BlockRenderView view;
-	public BlockPos pos;
-	public BlockState blockState;
-	public FluidState fluidState;
-	public FluidRenderHandler handler;
+public class FluidRenderHandlerInfo {
 	public final Sprite[] sprites = new Sprite[2];
-	public Sprite overlay;
+	@Nullable
+	public FluidRenderHandler handler;
 	public boolean hasOverlay;
+	public Sprite overlaySprite;
 
-	public void getSprites(BlockRenderView world, BlockPos pos, FluidState fluidState) {
-		if (handler != null) {
-			Sprite[] sprites = handler.getFluidSprites(world, pos, fluidState);
+	public void setup(FluidRenderHandler handler, BlockRenderView world, BlockPos pos, FluidState fluidState) {
+		this.handler = handler;
 
-			this.sprites[0] = sprites[0];
-			this.sprites[1] = sprites[1];
+		Sprite[] sprites = handler.getFluidSprites(world, pos, fluidState);
 
-			if (sprites.length > 2) {
-				hasOverlay = true;
-				overlay = sprites[2];
-			}
-		} else {
-			hasOverlay = false;
+		this.sprites[0] = sprites[0];
+		this.sprites[1] = sprites[1];
+
+		if (sprites.length > 2) {
+			hasOverlay = true;
+			overlaySprite = sprites[2];
 		}
 	}
 
 	public void clear() {
-		view = null;
-		pos = null;
-		blockState = null;
-		fluidState = null;
-		handler = null;
 		sprites[0] = null;
 		sprites[1] = null;
-		overlay = null;
+		handler = null;
 		hasOverlay = false;
+		overlaySprite = null;
 	}
 }

fabric-rendering-fluids-v1/src/client/java/net/fabricmc/fabric/impl/client/rendering/fluid/FluidRenderHandlerRegistryImpl.java

@@ -18,16 +18,16 @@
 
 import java.util.IdentityHashMap;
 import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
 
 import org.jetbrains.annotations.Nullable;
 
 import net.minecraft.block.Block;
-import net.minecraft.block.BlockState;
 import net.minecraft.block.LeavesBlock;
-import net.minecraft.block.TransparentBlock;
+import net.minecraft.block.TranslucentBlock;
 import net.minecraft.client.MinecraftClient;
 import net.minecraft.client.color.world.BiomeColors;
-import net.minecraft.client.render.VertexConsumer;
 import net.minecraft.client.render.block.FluidRenderer;
 import net.minecraft.client.texture.Sprite;
 import net.minecraft.client.texture.SpriteAtlasTexture;
@@ -43,15 +43,16 @@
 import net.fabricmc.fabric.api.client.render.fluid.v1.FluidRenderHandlerRegistry;
 
 public class FluidRenderHandlerRegistryImpl implements FluidRenderHandlerRegistry {
-	/**
-	 * The water color of {@link BiomeKeys#OCEAN}.
-	 */
-	private static final int DEFAULT_WATER_COLOR = 0x3f76e4;
 	private final Map<Fluid, FluidRenderHandler> handlers = new IdentityHashMap<>();
 	private final Map<Fluid, FluidRenderHandler> modHandlers = new IdentityHashMap<>();
-	private final Map<Block, Boolean> overlayBlocks = new IdentityHashMap<>();
+	private final ConcurrentMap<Block, Boolean> overlayBlocks = new ConcurrentHashMap<>();
 
-	private FluidRenderer fluidRenderer;
+	{
+		handlers.put(Fluids.WATER, WaterRenderHandler.INSTANCE);
+		handlers.put(Fluids.FLOWING_WATER, WaterRenderHandler.INSTANCE);
+		handlers.put(Fluids.LAVA, LavaRenderHandler.INSTANCE);
+		handlers.put(Fluids.FLOWING_LAVA, LavaRenderHandler.INSTANCE);
+	}
 
 	public FluidRenderHandlerRegistryImpl() {
 	}
@@ -81,42 +82,14 @@ public void setBlockTransparency(Block block, boolean transparent) {
 
 	@Override
 	public boolean isBlockTransparent(Block block) {
-		return overlayBlocks.computeIfAbsent(block, k -> k instanceof TransparentBlock || k instanceof LeavesBlock);
+		return overlayBlocks.computeIfAbsent(block, k -> k instanceof TranslucentBlock || k instanceof LeavesBlock);
 	}
 
 	public void onFluidRendererReload(FluidRenderer renderer, Sprite[] waterSprites, Sprite[] lavaSprites, Sprite waterOverlay) {
-		fluidRenderer = renderer;
-
-		Sprite[] waterSpritesFull = {waterSprites[0], waterSprites[1], waterOverlay};
-		FluidRenderHandler waterHandler = new FluidRenderHandler() {
-			@Override
-			public Sprite[] getFluidSprites(BlockRenderView view, BlockPos pos, FluidState state) {
-				return waterSpritesFull;
-			}
-
-			@Override
-			public int getFluidColor(BlockRenderView view, BlockPos pos, FluidState state) {
-				if (view != null && pos != null) {
-					return BiomeColors.getWaterColor(view, pos);
-				} else {
-					return DEFAULT_WATER_COLOR;
-				}
-			}
-		};
-
-		//noinspection Convert2Lambda
-		FluidRenderHandler lavaHandler = new FluidRenderHandler() {
-			@Override
-			public Sprite[] getFluidSprites(BlockRenderView view, BlockPos pos, FluidState state) {
-				return lavaSprites;
-			}
-		};
+		FluidRenderingImpl.setVanillaRenderer(renderer);
 
-		handlers.put(Fluids.WATER, waterHandler);
-		handlers.put(Fluids.FLOWING_WATER, waterHandler);
-		handlers.put(Fluids.LAVA, lavaHandler);
-		handlers.put(Fluids.FLOWING_LAVA, lavaHandler);
-		handlers.putAll(modHandlers);
+		WaterRenderHandler.INSTANCE.updateSprites(waterSprites, waterOverlay);
+		LavaRenderHandler.INSTANCE.updateSprites(lavaSprites);
 
 		SpriteAtlasTexture texture = MinecraftClient.getInstance()
 				.getBakedModelManager()
@@ -127,7 +100,49 @@ public Sprite[] getFluidSprites(BlockRenderView view, BlockPos pos, FluidState s
 		}
 	}
 
-	public void renderFluid(BlockPos pos, BlockRenderView world, VertexConsumer vertexConsumer, BlockState blockState, FluidState fluidState) {
-		fluidRenderer.render(world, pos, vertexConsumer, blockState, fluidState);
+	private static class WaterRenderHandler implements FluidRenderHandler {
+		public static final WaterRenderHandler INSTANCE = new WaterRenderHandler();
+
+		/**
+		 * The water color of {@link BiomeKeys#OCEAN}.
+		 */
+		private static final int DEFAULT_WATER_COLOR = 0x3f76e4;
+
+		private final Sprite[] sprites = new Sprite[3];
+
+		@Override
+		public Sprite[] getFluidSprites(@Nullable BlockRenderView view, @Nullable BlockPos pos, FluidState state) {
+			return sprites;
+		}
+
+		@Override
+		public int getFluidColor(@Nullable BlockRenderView view, @Nullable BlockPos pos, FluidState state) {
+			if (view != null && pos != null) {
+				return BiomeColors.getWaterColor(view, pos);
+			} else {
+				return DEFAULT_WATER_COLOR;
+			}
+		}
+
+		public void updateSprites(Sprite[] waterSprites, Sprite waterOverlay) {
+			sprites[0] = waterSprites[0];
+			sprites[1] = waterSprites[1];
+			sprites[2] = waterOverlay;
+		}
+	}
+
+	private static class LavaRenderHandler implements FluidRenderHandler {
+		public static final LavaRenderHandler INSTANCE = new LavaRenderHandler();
+
+		private Sprite[] sprites;
+
+		@Override
+		public Sprite[] getFluidSprites(@Nullable BlockRenderView view, @Nullable BlockPos pos, FluidState state) {
+			return sprites;
+		}
+
+		public void updateSprites(Sprite[] lavaSprites) {
+			sprites = lavaSprites;
+		}
 	}
 }