Cache ALL the things

README.md

@@ -8,17 +8,17 @@
 
 **This mod is open source and under a permissive license.** As such, it can be included in any modpack on any platform without prior permission. We appreciate hearing about people using our mods, but you do not need to ask to use them. See the [LICENSE file](LICENSE) for more details.
 
-Templates is an API for Carpenter's Blocks-like templated blocks. Currently, plain slopes are the only built-in template blocks.
-
 Template blocks can be placed in the world, then right-clicked with a full-size block to set the textures for the template. Template blocks will inherit light and redstone values from the blocks they're given, or they can have light or redstone output added to any given block by right-clicking the template with glowstone dust or a redstone torch, respectively.
 
+While Templates itself adds a handful of common shapes, it's not too hard for other mods to interface with Templates and add their own templatable blocks.
+
 # quat was here
 
+Todo move this into the main readme section
+
 ## Todo
 
-* `templates:block/slope_base` needs a suspicious amount of custom rotations. Maybe the model is pointing the wrong way.
 * `uvlock` in a blockstate will not work for `RetexturedMeshTemplateUnbakedModel`s. Can it be fixed?
-* Upside-down slopes would be nice...
 * More templates !!
 
 # For addon developers

src/main/java/io/github/cottonmc/templates/model/RetexturedJsonModelBakedModel.java

@@ -46,14 +46,14 @@ public class RetexturedJsonModelBakedModel extends ForwardingBakedModel {
 		}
 	}
 	
-	//TODO: Check that TemplateAppearance equals() behavior is what i want, and also that it's fast
-	private record CacheKey(BlockState state, TemplateAppearance appearance) {}
-	
 	private final TemplateAppearanceManager tam;
-	private final ConcurrentHashMap<CacheKey, Mesh> meshCache = new ConcurrentHashMap<>();
 	private final Sprite[] specialSprites = new Sprite[DIRECTIONS.length];
 	private final BlockState itemModelState;
 	
+	//TODO: Check that TemplateAppearance equals() behavior is what i want, and also that it's fast
+	private record CacheKey(BlockState state, TemplateAppearance appearance) {}
+	private final ConcurrentHashMap<CacheKey, Mesh> meshCache = new ConcurrentHashMap<>();
+	
 	@Override
 	public boolean isVanillaAdapter() {
 		return false;

src/main/java/io/github/cottonmc/templates/model/RetexturedMeshBakedModel.java

@@ -1,8 +1,11 @@
 package io.github.cottonmc.templates.model;
 
+import io.github.cottonmc.templates.TemplatesClient;
 import net.fabricmc.fabric.api.client.rendering.v1.ColorProviderRegistry;
 import net.fabricmc.fabric.api.renderer.v1.mesh.Mesh;
+import net.fabricmc.fabric.api.renderer.v1.mesh.MeshBuilder;
 import net.fabricmc.fabric.api.renderer.v1.mesh.MutableQuadView;
+import net.fabricmc.fabric.api.renderer.v1.mesh.QuadEmitter;
 import net.fabricmc.fabric.api.renderer.v1.model.ForwardingBakedModel;
 import net.fabricmc.fabric.api.renderer.v1.render.RenderContext;
 import net.fabricmc.fabric.api.rendering.data.v1.RenderAttachedBlockView;
@@ -20,12 +23,12 @@ import net.minecraft.util.math.BlockPos;
 import net.minecraft.util.math.Direction;
 import net.minecraft.util.math.random.Random;
 import net.minecraft.world.BlockRenderView;
-import org.jetbrains.annotations.NotNull;
 
 import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
 import java.util.function.Supplier;
 
-public final class RetexturedMeshBakedModel extends ForwardingBakedModel {
+public class RetexturedMeshBakedModel extends ForwardingBakedModel {
 	public RetexturedMeshBakedModel(BakedModel baseModel, TemplateAppearanceManager tam, AffineTransformation aff, Mesh baseMesh) {
 		this.wrapped = baseModel;
 		this.tam = tam;
@@ -37,6 +40,9 @@ public final class RetexturedMeshBakedModel extends ForwardingBakedModel {
 	private final Mesh baseMesh;
 	private final Map<Direction, Direction> facePermutation;
 	
+	//TODO: Check that TemplateAppearance equals() behavior is what i want, and also that it's fast
+	private final ConcurrentHashMap<TemplateAppearance, Mesh> meshCache = new ConcurrentHashMap<>();
+	
 	@Override
 	public boolean isVanillaAdapter() {
 		return false;
@@ -44,16 +50,43 @@ public final class RetexturedMeshBakedModel extends ForwardingBakedModel {
 	
 	@Override
 	public void emitBlockQuads(BlockRenderView blockView, BlockState state, BlockPos pos, Supplier<Random> randomSupplier, RenderContext context) {
-		context.pushTransform(retexturingBlockTransformer(blockView, state, pos, randomSupplier));
+		BlockState theme = (((RenderAttachedBlockView) blockView).getBlockEntityRenderAttachment(pos) instanceof BlockState s) ? s : null;
+		if(theme == null || theme.isAir()) {
+			context.meshConsumer().accept(getUntintedMesh(tam.getDefaultAppearance()));
+			return;
+		}
+		
+		TemplateAppearance ta = tam.getAppearance(theme);
+		
+		BlockColorProvider prov = ColorProviderRegistry.BLOCK.get(theme.getBlock());
+		int tint = prov == null ? 0xFFFFFFFF : (0xFF000000 | prov.getColor(theme, blockView, pos, 1));
+		
+		if(tint == 0xFFFFFFFF) {
+			//Cache this mesh indefinitely.
+			context.meshConsumer().accept(getUntintedMesh(ta));
+		} else {
+			//The specific tint might vary a lot; imagine grass color smoothly changing. Baking the tint into the cached mesh
+			//is likely unnecessary and will fill the cache with a ton of single-use meshes with only slighly different colors.
+			//We'd also have to percolate that tint color into the cache key, which is an allocation, blah blah blah.
+			//Let's fall back to a quad transform. In practice this is still nice and quick.
+			context.pushTransform(new RetexturingTransformer(ta, tint, facePermutation));
 			context.meshConsumer().accept(baseMesh);
 			context.popTransform();
 		}
+	}
 	
 	@Override
 	public void emitItemQuads(ItemStack stack, Supplier<Random> randomSupplier, RenderContext context) {
-		context.pushTransform(retexturingItemTransformer(stack, randomSupplier));
+		TemplateAppearance ta = tam.getDefaultAppearance();
-		context.meshConsumer().accept(baseMesh);
+		
-		context.popTransform();
+		//cheeky: if the item has NBT data, pluck out the blockstate from it
+		NbtCompound tag = BlockItem.getBlockEntityNbt(stack);
+		if(tag != null && tag.contains("BlockState")) {
+			BlockState state = NbtHelper.toBlockState(Registries.BLOCK.getReadOnlyWrapper(), tag.getCompound("BlockState"));
+			if(state != null && !state.isAir()) ta = tam.getAppearance(state);
+		}
+		
+		context.meshConsumer().accept(getUntintedMesh(ta));
 	}
 	
 	@Override
@@ -61,24 +94,12 @@ public final class RetexturedMeshBakedModel extends ForwardingBakedModel {
 		return tam.getDefaultAppearance().getParticleSprite();
 	}
 	
-	public @NotNull RenderContext.QuadTransform retexturingBlockTransformer(BlockRenderView blockView, BlockState state, BlockPos pos, Supplier<Random> randomSupplier) {
+	protected Mesh getUntintedMesh(TemplateAppearance ta) {
-		BlockState theme = (((RenderAttachedBlockView) blockView).getBlockEntityRenderAttachment(pos) instanceof BlockState s) ? s : null;
+		return meshCache.computeIfAbsent(ta, this::makeUntintedMesh);
-		if(theme == null || theme.isAir()) return new RetexturingTransformer(tam.getDefaultAppearance(), 0xFFFFFFFF, facePermutation);
-		
-		BlockColorProvider prov = ColorProviderRegistry.BLOCK.get(theme.getBlock());
-		int globalTint = prov != null ? prov.getColor(state, blockView, pos, 1) : 0xFFFFFFFF;
-		return new RetexturingTransformer(tam.getAppearance(theme), globalTint, facePermutation);
 	}
 	
-	public @NotNull RenderContext.QuadTransform retexturingItemTransformer(ItemStack stack, Supplier<Random> randomSupplier) {
+	protected Mesh makeUntintedMesh(TemplateAppearance appearance) {
-		//cheeky: if the item has NBT data, pluck out the blockstate from it
+		return new RetexturingTransformer(appearance, 0xFFFFFF, facePermutation).applyTo(baseMesh);
-		NbtCompound tag = BlockItem.getBlockEntityNbt(stack);
-		if(tag != null && tag.contains("BlockState")) {
-			BlockState state = NbtHelper.toBlockState(Registries.BLOCK.getReadOnlyWrapper(), tag.getCompound("BlockState"));
-			if(!state.isAir()) return new RetexturingTransformer(tam.getAppearance(state), 0xFFFFFFFF, facePermutation);
-		}
-		
-		return new RetexturingTransformer(tam.getDefaultAppearance(), 0xFFFFFFFF, facePermutation);
 	}
 	
 	public static record RetexturingTransformer(TemplateAppearance appearance, int color, Map<Direction, Direction> facePermutation) implements RenderContext.QuadTransform {
@@ -90,14 +111,27 @@ public final class RetexturedMeshBakedModel extends ForwardingBakedModel {
 			
 			//The quad tag numbers were selected so this magic trick works:
 			Direction dir = facePermutation.get(DIRECTIONS[quad.tag() - 1]);
-			
 			//TODO: this newly-simplified direction passing to hasColor is almost certainly incorrect
 			// I think hasColor was kinda incorrect in the first place tho
 			if(appearance.hasColor(dir)) quad.color(color, color, color, color);
-			Sprite sprite = appearance.getSprite(dir);
 			
+			Sprite sprite = appearance.getSprite(dir);
 			quad.spriteBake(sprite, MutableQuadView.BAKE_NORMALIZED);
+			
 			return true;
 		}
+		
+		//Pass a Mesh through a QuadTransform all at once, instead of at render time
+		private Mesh applyTo(Mesh original) {
+			MeshBuilder builder = TemplatesClient.getFabricRenderer().meshBuilder();
+			QuadEmitter emitter = builder.getEmitter();
+			
+			original.forEach(quad -> {
+				emitter.copyFrom(quad);
+				if(transform(emitter)) emitter.emit();
+			});
+			
+			return builder.build();
+		}
 	}
 }

src/main/java/io/github/cottonmc/templates/model/TemplateAppearanceManager.java

@@ -18,9 +18,11 @@ import net.minecraft.util.math.Direction;
 import net.minecraft.util.math.random.Random;
 import org.jetbrains.annotations.NotNull;
 
+import java.util.Arrays;
 import java.util.EnumMap;
 import java.util.List;
 import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.atomic.AtomicInteger;
 import java.util.function.Function;
 
 public class TemplateAppearanceManager {
@@ -32,17 +34,15 @@ public class TemplateAppearanceManager {
 		
 		Sprite defaultSprite = spriteLookup.apply(DEFAULT_SPRITE_ID);
 		if(defaultSprite == null) throw new IllegalStateException("Couldn't locate " + DEFAULT_SPRITE_ID + " !");
-		this.defaultAppearance = new SingleSpriteAppearance(defaultSprite, blockMaterials.get(BlendMode.CUTOUT));
+		this.defaultAppearance = new SingleSpriteAppearance(defaultSprite, blockMaterials.get(BlendMode.CUTOUT), serialNumber.getAndIncrement());
 	}
 	
 	public static final SpriteIdentifier DEFAULT_SPRITE_ID = new SpriteIdentifier(PlayerScreenHandler.BLOCK_ATLAS_TEXTURE, new Identifier("minecraft:block/scaffolding_top"));
 	private final TemplateAppearance defaultAppearance;
 	
-	//Mutable, append-only cache:
+	private final ConcurrentHashMap<BlockState, TemplateAppearance> appearanceCache = new ConcurrentHashMap<>(); //Mutable, append-only cache
-	private final ConcurrentHashMap<BlockState, TemplateAppearance> appearanceCache = new ConcurrentHashMap<>();
+	private final AtomicInteger serialNumber = new AtomicInteger(0); //Mutable
-	
+	private final EnumMap<BlendMode, RenderMaterial> blockMaterials = new EnumMap<>(BlendMode.class); //Immutable contents
-	//Immutable contents:
-	private final EnumMap<BlendMode, RenderMaterial> blockMaterials = new EnumMap<>(BlendMode.class);
 	
 	public TemplateAppearance getDefaultAppearance() {
 		return defaultAppearance;
@@ -52,6 +52,10 @@ public class TemplateAppearanceManager {
 		return appearanceCache.computeIfAbsent(state, this::computeAppearance);
 	}
 	
+	//I'm pretty sure ConcurrentHashMap semantics allow for this function to be called multiple times on the same key, on different threads.
+	//The computeIfAbsent map update will work without corrupting the map, but there will be some "wasted effort" computing the value twice.
+	//The results are going to be the same, apart from their serialNumbers differing (= their equals & hashCode differing).
+	//Tiny amount of wasted space in some caches if TemplateAppearances are used as a map key, then. IMO it's not a critical issue.
 	private TemplateAppearance computeAppearance(BlockState state) {
 		Random rand = Random.create();
 		BakedModel model = MinecraftClient.getInstance().getBlockRenderManager().getModel(state);
@@ -59,7 +63,7 @@ public class TemplateAppearanceManager {
 		Sprite[] sprites = new Sprite[7];
 		byte hasColorMask = 0b000000;
 		
-		//Read quads off the model.
+		//Read quads off the model by their `cullface`
 		for(Direction dir : Direction.values()) {
 			List<BakedQuad> sideQuads = model.getQuads(null, dir, rand);
 			if(sideQuads.isEmpty()) continue;
@@ -78,15 +82,33 @@ public class TemplateAppearanceManager {
 		//Just for space-usage purposes, we store the particle in sprites[6] instead of using another field.
 		sprites[6] = model.getParticleSprite();
 		
-		//Fill out any missing values in the sprites array. Failure to pick textures shouldn't lead to NPEs later on.
+		//Fill out any missing values in the sprites array, since failure to pick textures shouldn't lead to NPEs later on
 		for(int i = 0; i < sprites.length; i++) {
 			if(sprites[i] == null) sprites[i] = defaultAppearance.getParticleSprite();
 		}
 		
-		return new ComputedApperance(sprites, hasColorMask, blockMaterials.get(BlendMode.fromRenderLayer(RenderLayers.getBlockLayer(state))));
+		return new ComputedApperance(
+			sprites,
+			hasColorMask,
+			blockMaterials.get(BlendMode.fromRenderLayer(RenderLayers.getBlockLayer(state))),
+			serialNumber.getAndIncrement()
+		);
+	}
+	
+	@SuppressWarnings("ClassCanBeRecord")
+	private static final class ComputedApperance implements TemplateAppearance {
+		private final Sprite @NotNull[] sprites;
+		private final byte hasColorMask;
+		private final RenderMaterial mat;
+		private final int id;
+		
+		private ComputedApperance(@NotNull Sprite @NotNull[] sprites, byte hasColorMask, RenderMaterial mat, int id) {
+			this.sprites = sprites;
+			this.hasColorMask = hasColorMask;
+			this.mat = mat;
+			this.id = id;
 		}
 		
-	private static record ComputedApperance(@NotNull Sprite[] sprites, byte hasColorMask, RenderMaterial mat) implements TemplateAppearance {
 		@Override
 		public @NotNull Sprite getParticleSprite() {
 			return sprites[6];
@@ -106,9 +128,38 @@ public class TemplateAppearanceManager {
 		public boolean hasColor(Direction dir) {
 			return (hasColorMask & (1 << dir.ordinal())) != 0;
 		}
+		
+		@Override
+		public boolean equals(Object o) {
+			if(this == o) return true;
+			if(o == null || getClass() != o.getClass()) return false;
+			ComputedApperance that = (ComputedApperance) o;
+			return id == that.id;
+		}
+		
+		@Override
+		public int hashCode() {
+			return id;
+		}
+		
+		@Override
+		public String toString() {
+			return "ComputedApperance[sprites=%s, hasColorMask=%s, mat=%s, id=%d]".formatted(Arrays.toString(sprites), hasColorMask, mat, id);
+		}
+	}
+	
+	@SuppressWarnings("ClassCanBeRecord")
+	private static final class SingleSpriteAppearance implements TemplateAppearance {
+		private final @NotNull Sprite defaultSprite;
+		private final RenderMaterial mat;
+		private final int id;
+		
+		private SingleSpriteAppearance(@NotNull Sprite defaultSprite, RenderMaterial mat, int id) {
+			this.defaultSprite = defaultSprite;
+			this.mat = mat;
+			this.id = id;
 		}
 		
-	private static record SingleSpriteAppearance(@NotNull Sprite defaultSprite, RenderMaterial mat) implements TemplateAppearance {
 		@Override
 		public @NotNull Sprite getParticleSprite() {
 			return defaultSprite;
@@ -128,5 +179,23 @@ public class TemplateAppearanceManager {
 		public boolean hasColor(Direction dir) {
 			return false;
 		}
+		
+		@Override
+		public boolean equals(Object o) {
+			if(this == o) return true;
+			if(o == null || getClass() != o.getClass()) return false;
+			SingleSpriteAppearance that = (SingleSpriteAppearance) o;
+			return id == that.id;
+		}
+		
+		@Override
+		public int hashCode() {
+			return id;
+		}
+		
+		@Override
+		public String toString() {
+			return "SingleSpriteAppearance[defaultSprite=%s, mat=%s, id=%d]".formatted(defaultSprite, mat, id);
+		}
 	}
 }