Improve manual culling efficiency

- Use int bitmaps instead of a Boolean[] for cull caching
- Refine config code
- Make mixin annotation format consistent
- Modify build workflow to not run on pull request
- Update build dependencies

.github/workflows/build.yml

@@ -1,6 +1,6 @@
 name: Java CI with Gradle
 
-on: [ push, pull_request ]
+on: [ push ]
 
 jobs:
   build:

build.gradle

@@ -20,8 +20,7 @@ repositories {
 		url 'https://api.modrinth.com/maven'
 	}
 	maven {
-		name 'dblsaiko'
-		url 'https://maven.dblsaiko.net'
+		url 'https://maven.vram.io'
 	}
 	maven {
 		name 'Shedaniel'
@@ -41,12 +40,12 @@ dependencies {
 	// Fabric API
 	modImplementation "net.fabricmc.fabric-api:fabric-api:${project.fabric_version}"
 
-	modImplementation('com.terraformersmc:modmenu:2.0.10') {
+	modImplementation('com.terraformersmc:modmenu:2.0.14') {
 		exclude group: 'net.fabricmc.fabric-api'
 	}
 
 	modCompileOnly 'maven.modrinth:sodium:mc1.17.1-0.3.2'
-	modCompileOnly('grondag:canvas-mc117-1.17:1.0.2035-snapshot') {
+	modCompileOnly('io.vram:canvas-fabric-mc117-1.17:1.0.2124') {
 		exclude group: '*', module: '*'
 	}
 }

gradle.properties

@@ -5,7 +5,7 @@ org.gradle.jvmargs = -Xmx1G
 	loom_version = 0.10-SNAPSHOT
 	minecraft_version = 1.17.1
 	yarn_mappings = 1.17.1+build.61
-	loader_version = 0.11.6
+	loader_version = 0.11.7
 
 # Mod Properties
 	mod_version = 1.0.1
@@ -14,4 +14,4 @@ org.gradle.jvmargs = -Xmx1G
 	archives_base_name = continuity
 
 # Dependencies
-	fabric_version = 0.40.1+1.17
+	fabric_version = 0.41.0+1.17

src/main/java/me/pepperbell/continuity/client/config/ContinuityConfig.java

@@ -75,15 +75,19 @@ protected void fromJson(JsonElement json) throws JsonParseException {
 				Object2ObjectMap.Entry<String, Option<?>> entry = iterator.next();
 				JsonElement element = object.get(entry.getKey());
 				if (element != null) {
-					entry.getValue().fromJson(element);
+					try {
+						entry.getValue().fromJson(element);
+					} catch (JsonParseException e) {
+						LOGGER.error("Could not read option '" + entry.getKey() + "'", e);
+					}
 				}
 			}
 		} else {
 			throw new JsonParseException("Json must be an object");
 		}
 	}
 
-	protected JsonElement toJson() throws JsonParseException {
+	protected JsonElement toJson() {
 		JsonObject object = new JsonObject();
 		ObjectBidirectionalIterator<Object2ObjectMap.Entry<String, Option<?>>> iterator = optionMap.object2ObjectEntrySet().fastIterator();
 		while (iterator.hasNext()) {

src/main/java/me/pepperbell/continuity/client/config/Option.java

@@ -11,7 +11,7 @@ public interface Option<T> {
 
 	void set(T value);
 
-	JsonElement toJson() throws JsonParseException;
+	JsonElement toJson();
 
 	void fromJson(JsonElement json) throws JsonParseException;
 
@@ -51,8 +51,12 @@ public JsonElement toJson() {
 		}
 
 		@Override
-		public void fromJson(JsonElement json) {
-			set(json.getAsBoolean());
+		public void fromJson(JsonElement json) throws JsonParseException {
+			if (json.isJsonPrimitive()) {
+				set(json.getAsBoolean());
+			} else {
+				throw new JsonParseException("Json must be a primitive");
+			}
 		}
 	}
 }

src/main/java/me/pepperbell/continuity/client/mixin/BlockModelsMixin.java

@@ -10,7 +10,7 @@
 
 @Mixin(BlockModels.class)
 public class BlockModelsMixin {
-	@Inject(at = @At("HEAD"), method = "reload()V")
+	@Inject(method = "reload()V", at = @At("HEAD"))
 	private void onHeadReload(CallbackInfo ci) {
 		SpriteCalculator.clearCache();
 	}

src/main/java/me/pepperbell/continuity/client/mixin/FallingBlockEntityRendererMixin.java

@@ -13,12 +13,12 @@
 
 @Mixin(FallingBlockEntityRenderer.class)
 public class FallingBlockEntityRendererMixin {
-	@Inject(at = @At(value = "INVOKE", target = "Lnet/minecraft/client/render/block/BlockModelRenderer;render(Lnet/minecraft/world/BlockRenderView;Lnet/minecraft/client/render/model/BakedModel;Lnet/minecraft/block/BlockState;Lnet/minecraft/util/math/BlockPos;Lnet/minecraft/client/util/math/MatrixStack;Lnet/minecraft/client/render/VertexConsumer;ZLjava/util/Random;JI)Z"), method = "render(Lnet/minecraft/entity/FallingBlockEntity;FFLnet/minecraft/client/util/math/MatrixStack;Lnet/minecraft/client/render/VertexConsumerProvider;I)V")
+	@Inject(method = "render(Lnet/minecraft/entity/FallingBlockEntity;FFLnet/minecraft/client/util/math/MatrixStack;Lnet/minecraft/client/render/VertexConsumerProvider;I)V", at = @At(value = "INVOKE", target = "Lnet/minecraft/client/render/block/BlockModelRenderer;render(Lnet/minecraft/world/BlockRenderView;Lnet/minecraft/client/render/model/BakedModel;Lnet/minecraft/block/BlockState;Lnet/minecraft/util/math/BlockPos;Lnet/minecraft/client/util/math/MatrixStack;Lnet/minecraft/client/render/VertexConsumer;ZLjava/util/Random;JI)Z"))
 	private void beforeRenderModel(FallingBlockEntity fallingBlockEntity, float yaw, float tickDelta, MatrixStack matrices, VertexConsumerProvider vertexConsumers, int light, CallbackInfo ci) {
 		CTMStateManager.INSTANCE.disableCTM();
 	}
 
-	@Inject(at = @At(value = "INVOKE", target = "Lnet/minecraft/client/render/block/BlockModelRenderer;render(Lnet/minecraft/world/BlockRenderView;Lnet/minecraft/client/render/model/BakedModel;Lnet/minecraft/block/BlockState;Lnet/minecraft/util/math/BlockPos;Lnet/minecraft/client/util/math/MatrixStack;Lnet/minecraft/client/render/VertexConsumer;ZLjava/util/Random;JI)Z", shift = At.Shift.AFTER), method = "render(Lnet/minecraft/entity/FallingBlockEntity;FFLnet/minecraft/client/util/math/MatrixStack;Lnet/minecraft/client/render/VertexConsumerProvider;I)V")
+	@Inject(method = "render(Lnet/minecraft/entity/FallingBlockEntity;FFLnet/minecraft/client/util/math/MatrixStack;Lnet/minecraft/client/render/VertexConsumerProvider;I)V", at = @At(value = "INVOKE", target = "Lnet/minecraft/client/render/block/BlockModelRenderer;render(Lnet/minecraft/world/BlockRenderView;Lnet/minecraft/client/render/model/BakedModel;Lnet/minecraft/block/BlockState;Lnet/minecraft/util/math/BlockPos;Lnet/minecraft/client/util/math/MatrixStack;Lnet/minecraft/client/render/VertexConsumer;ZLjava/util/Random;JI)Z", shift = At.Shift.AFTER))
 	private void afterRenderModel(FallingBlockEntity fallingBlockEntity, float yaw, float tickDelta, MatrixStack matrices, VertexConsumerProvider vertexConsumers, int light, CallbackInfo ci) {
 		CTMStateManager.INSTANCE.enableCTM();
 	}

src/main/java/me/pepperbell/continuity/client/mixin/IdentifierMixin.java

@@ -10,7 +10,7 @@
 
 @Mixin(Identifier.class)
 public class IdentifierMixin {
-	@Inject(at = @At("HEAD"), method = "isPathValid(Ljava/lang/String;)Z", cancellable = true)
+	@Inject(method = "isPathValid(Ljava/lang/String;)Z", at = @At("HEAD"), cancellable = true)
 	private static void onIsPathValid(String path, CallbackInfoReturnable<Boolean> cir) {
 		if (InvalidIdentifierHandler.areInvalidPathsEnabled()) {
 			cir.setReturnValue(true);

src/main/java/me/pepperbell/continuity/client/mixin/ModelLoaderMixin.java

@@ -36,32 +36,32 @@ public class ModelLoaderMixin {
 	@Unique
 	private BlockState currentBlockState;
 
-	@Inject(at = @At(value = "INVOKE_STRING", target = "Lnet/minecraft/util/profiler/Profiler;push(Ljava/lang/String;)V", args = "ldc=missing_model", shift = At.Shift.BEFORE), method = "<init>(Lnet/minecraft/resource/ResourceManager;Lnet/minecraft/client/color/block/BlockColors;Lnet/minecraft/util/profiler/Profiler;I)V")
+	@Inject(method = "<init>(Lnet/minecraft/resource/ResourceManager;Lnet/minecraft/client/color/block/BlockColors;Lnet/minecraft/util/profiler/Profiler;I)V", at = @At(value = "INVOKE_STRING", target = "Lnet/minecraft/util/profiler/Profiler;push(Ljava/lang/String;)V", args = "ldc=missing_model", shift = At.Shift.BEFORE))
 	private void afterStoreArgs(ResourceManager resourceManager, BlockColors blockColors, Profiler profiler, int mipmap, CallbackInfo ci) {
 		BiomeHolderManager.clearCache(); // TODO: move BiomeHolderManager calls elsewhere?
 		CTMPropertiesLoader.loadAll(resourceManager);
 	}
 
-	@Inject(at = @At("TAIL"), method = "<init>(Lnet/minecraft/resource/ResourceManager;Lnet/minecraft/client/color/block/BlockColors;Lnet/minecraft/util/profiler/Profiler;I)V")
+	@Inject(method = "<init>(Lnet/minecraft/resource/ResourceManager;Lnet/minecraft/client/color/block/BlockColors;Lnet/minecraft/util/profiler/Profiler;I)V", at = @At("TAIL"))
 	private void onTailInit(ResourceManager resourceManager, BlockColors blockColors, Profiler profiler, int mipmap, CallbackInfo ci) {
 		BiomeHolderManager.refreshHolders(); // TODO: move BiomeHolderManager calls elsewhere?
 		CTMPropertiesLoader.clearAll();
 	}
 
-	@Inject(at = @At("HEAD"), method = "method_4716(Lnet/minecraft/block/BlockState;)V")
+	@Inject(method = "method_4716(Lnet/minecraft/block/BlockState;)V", at = @At("HEAD"))
 	private void onAddBlockStateModel(BlockState state, CallbackInfo ci) {
 		currentBlockState = state;
 	}
 
-	@Inject(at = @At(value = "TAIL"), method = "addModel(Lnet/minecraft/client/util/ModelIdentifier;)V", locals = LocalCapture.CAPTURE_FAILHARD)
+	@Inject(method = "addModel(Lnet/minecraft/client/util/ModelIdentifier;)V", at = @At(value = "TAIL"), locals = LocalCapture.CAPTURE_FAILHARD)
 	private void afterAddModel(ModelIdentifier id, CallbackInfo ci, UnbakedModel model) {
 		if (currentBlockState != null) {
 			AddBlockStateModelCallback.EVENT.invoker().onAddBlockStateModel(id, currentBlockState, model, (ModelLoader) (Object) this);
 			currentBlockState = null;
 		}
 	}
 
-	@Inject(at = @At(value = "INVOKE_STRING", target = "Lnet/minecraft/util/profiler/Profiler;swap(Ljava/lang/String;)V", args = "ldc=textures"), method = "<init>(Lnet/minecraft/resource/ResourceManager;Lnet/minecraft/client/color/block/BlockColors;Lnet/minecraft/util/profiler/Profiler;I)V")
+	@Inject(method = "<init>(Lnet/minecraft/resource/ResourceManager;Lnet/minecraft/client/color/block/BlockColors;Lnet/minecraft/util/profiler/Profiler;I)V", at = @At(value = "INVOKE_STRING", target = "Lnet/minecraft/util/profiler/Profiler;swap(Ljava/lang/String;)V", args = "ldc=textures"))
 	private void onFinishAddingModels(ResourceManager resourceManager, BlockColors blockColors, Profiler profiler, int mipmap, CallbackInfo ci) {
 		ModelsAddedCallback.EVENT.invoker().onModelsAdded((ModelLoader) (Object) this, resourceManager, profiler, unbakedModels, modelsToBake);
 	}

src/main/java/me/pepperbell/continuity/client/mixin/NamespaceResourceManagerMixin.java

@@ -11,12 +11,12 @@
 
 @Mixin(NamespaceResourceManager.class)
 public class NamespaceResourceManagerMixin {
-	@Inject(at = @At("HEAD"), method = "getMetadataPath(Lnet/minecraft/util/Identifier;)Lnet/minecraft/util/Identifier;")
+	@Inject(method = "getMetadataPath(Lnet/minecraft/util/Identifier;)Lnet/minecraft/util/Identifier;", at = @At("HEAD"))
 	private static void onHeadGetMetadataPath(Identifier id, CallbackInfoReturnable<Identifier> cir) {
 		InvalidIdentifierHandler.enableInvalidPaths();
 	}
 
-	@Inject(at = @At("TAIL"), method = "getMetadataPath(Lnet/minecraft/util/Identifier;)Lnet/minecraft/util/Identifier;")
+	@Inject(method = "getMetadataPath(Lnet/minecraft/util/Identifier;)Lnet/minecraft/util/Identifier;", at = @At("TAIL"))
 	private static void onTailGetMetadataPath(Identifier id, CallbackInfoReturnable<Identifier> cir) {
 		InvalidIdentifierHandler.disableInvalidPaths();
 	}

src/main/java/me/pepperbell/continuity/client/mixin/ReloadableResourceManagerImplMixin.java

@@ -39,23 +39,23 @@ private Identifier redirect(Identifier id) {
 		return id;
 	}
 
-	@ModifyVariable(at = @At("HEAD"), method = "getResource")
+	@ModifyVariable(method = "getResource(Lnet/minecraft/util/Identifier;)Lnet/minecraft/resource/Resource;", at = @At("HEAD"))
 	private Identifier redirectGetResourceId(Identifier id) {
 		return redirect(id);
 	}
 
-	@ModifyVariable(at = @At("HEAD"), method = "containsResource")
+	@ModifyVariable(method = "containsResource(Lnet/minecraft/util/Identifier;)Z", at = @At("HEAD"))
 	private Identifier redirectContainsResourceId(Identifier id) {
 		return redirect(id);
 	}
 
-	@ModifyVariable(at = @At("HEAD"), method = "getAllResources")
+	@ModifyVariable(method = "getAllResources(Lnet/minecraft/util/Identifier;)Ljava/util/List;", at = @At("HEAD"))
 	private Identifier redirectGetAllResourcesId(Identifier id) {
 		return redirect(id);
 	}
 
-//	// TODO: what if redirect doesn't actually exist?
-//	@Inject(at = @At(value = "INVOKE", target = "Lcom/google/common/collect/Sets;newHashSet()Ljava/util/HashSet;", shift = At.Shift.BY, by = 2), method = "findResources", locals = LocalCapture.CAPTURE_FAILHARD)
+	// TODO: what if redirect doesn't actually exist?
+//	@Inject(method = "findResources(Ljava/lang/String;Ljava/util/function/Predicate;)Ljava/util/Collection;", at = @At(value = "INVOKE", target = "Lcom/google/common/collect/Sets;newHashSet()Ljava/util/HashSet;", shift = At.Shift.BY, by = 2), locals = LocalCapture.CAPTURE_FAILHARD)
 //	private void onFindResources(String startingPath, Predicate<String> pathPredicate, CallbackInfoReturnable<Collection<Identifier>> cir, Set<Identifier> set) {
 //		for (Identifier id : redirects.keySet()) {
 //			String path = id.getPath();
@@ -65,7 +65,7 @@ private Identifier redirectGetAllResourcesId(Identifier id) {
 //		}
 //	}
 
-	@Inject(at = @At("HEAD"), method = "reload(Ljava/util/concurrent/Executor;Ljava/util/concurrent/Executor;Ljava/util/concurrent/CompletableFuture;Ljava/util/List;)Lnet/minecraft/resource/ResourceReload;")
+	@Inject(method = "reload(Ljava/util/concurrent/Executor;Ljava/util/concurrent/Executor;Ljava/util/concurrent/CompletableFuture;Ljava/util/List;)Lnet/minecraft/resource/ResourceReload;", at = @At("HEAD"))
 	private void onHeadReload(Executor prepareExecutor, Executor applyExecutor, CompletableFuture<Unit> initialStage, List<ResourcePack> packs, CallbackInfoReturnable<ResourceReload> cir) {
 		redirects.clear();
 	}

src/main/java/me/pepperbell/continuity/client/mixin/RenderLayersMixin.java

@@ -12,7 +12,7 @@
 
 @Mixin(RenderLayers.class)
 public class RenderLayersMixin {
-	@Inject(at = @At("HEAD"), method = "getBlockLayer(Lnet/minecraft/block/BlockState;)Lnet/minecraft/client/render/RenderLayer;", cancellable = true)
+	@Inject(method = "getBlockLayer(Lnet/minecraft/block/BlockState;)Lnet/minecraft/client/render/RenderLayer;", at = @At("HEAD"), cancellable = true)
 	private static void onHeadGetBlockLayer(BlockState state, CallbackInfoReturnable<RenderLayer> cir) {
 		RenderLayer layer = CustomBlockLayers.getLayer(state);
 		if (layer != null) {

src/main/java/me/pepperbell/continuity/client/model/CTMBakedModel.java

@@ -85,7 +85,7 @@ protected static class ObjectContainer {
 	}
 
 	protected static class CTMQuadTransform implements RenderContext.QuadTransform {
-		protected final ManualCullingCache cullingCache = new ManualCullingCache();
+		protected final CullingCache cullingCache = new CullingCache();
 		protected final ProcessingContextImpl processingContext = new ProcessingContextImpl();
 
 		protected List<QuadProcessor> processors;
@@ -103,7 +103,7 @@ public boolean transform(MutableQuadView quad) {
 			if (useManualCulling) {
 				Direction cullFace = quad.cullFace();
 				if (cullFace != null) {
-					if (cullingCache.shouldCull(state, blockView, pos, cullFace)) {
+					if (cullingCache.shouldCull(blockView, pos, state, cullFace)) {
 						return false;
 					}
 				}
@@ -156,6 +156,8 @@ public void prepare(List<QuadProcessor> processors, List<QuadProcessor> multipas
 			this.randomSupplier = randomSupplier;
 			this.useManualCulling = useManualCulling;
 			spriteFinder = RenderUtil.getSpriteFinder();
+
+			cullingCache.prepare();
 		}
 
 		public void reset() {
@@ -168,29 +170,33 @@ public void reset() {
 			useManualCulling = false;
 			spriteFinder = null;
 
-			cullingCache.reset();
 			processingContext.reset();
 		}
 	}
 
-	protected static class ManualCullingCache {
-		protected static final Boolean[] EMPTY_CULL_CACHE = new Boolean[Direction.values().length];
+	protected static class CullingCache {
+		protected int completionFlags;
+		protected int resultFlags;
 
-		protected Boolean[] cullCache = new Boolean[Direction.values().length];
 		protected BlockPos.Mutable mutablePos = new BlockPos.Mutable();
 
-		public boolean shouldCull(BlockState state, BlockRenderView blockView, BlockPos pos, Direction cullFace) {
-			int ordinal = cullFace.ordinal();
-			Boolean cull = cullCache[ordinal];
-			if (cull == null) {
-				cull = !Block.shouldDrawSide(state, blockView, pos, cullFace, mutablePos.set(pos, cullFace));
-				cullCache[ordinal] = cull;
+		public boolean shouldCull(BlockRenderView blockView, BlockPos pos, BlockState state, Direction cullFace) {
+			int mask = 1 << cullFace.ordinal();
+			if ((completionFlags & mask) == 0) {
+				completionFlags |= mask;
+				if (Block.shouldDrawSide(state, blockView, pos, cullFace, mutablePos.set(pos, cullFace))) {
+					return false;
+				} else {
+					resultFlags |= mask;
+					return true;
+				}
+			} else {
+				return (resultFlags & mask) == 1;
 			}
-			return cull;
 		}
 
-		public void reset() {
-			System.arraycopy(EMPTY_CULL_CACHE, 0, cullCache, 0, EMPTY_CULL_CACHE.length);
+		public void prepare() {
+			completionFlags = 0;
 		}
 	}