Stripping instances

- Remove #removeNow and #getWorldPosition from Instance
- Add #distanceSquared for use in update limiting
- Refactor DistanceUpdateLimiter to directly accept distance squared
- Remove proper name from backend
- Misc. cleanup
  - ifs without braces
  - some method names

src/main/java/com/jozufozu/flywheel/Flywheel.java

@@ -115,7 +115,7 @@ public class Flywheel {
 
 		VanillaInstances.init();
 
-		CrashReportCallables.registerCrashCallable("Flywheel Backend", BackendManager::getBackendDescriptor);
+		CrashReportCallables.registerCrashCallable("Flywheel Backend", BackendManager::getBackendNameForCrashReport);
 
 		// https://github.com/Jozufozu/Flywheel/issues/69
 		// Weird issue with accessor loading.

src/main/java/com/jozufozu/flywheel/api/backend/Backend.java

@@ -11,15 +11,24 @@ import net.minecraft.network.chat.Component;
 public interface Backend {
 	static IdRegistry<Backend> REGISTRY = IdRegistryImpl.create();
 
-	// TODO: remove and use ID instead? Currently this is only used for the crash log string.
+	/**
-	String getProperName();
+	 * Get a message to display to the user when the engine is enabled.
-
+	 */
-	Component getEngineMessage();
+	Component engineMessage();
 
+	/**
+	 * Create a new engine instance.
+	 */
 	Engine createEngine();
 
+	/**
+	 * Get a fallback backend in case this backend is not supported.
+	 */
 	Backend findFallback();
 
+	/**
+	 * Check if this backend is supported.
+	 */
 	boolean isSupported();
 
 	@Nullable Pipeline pipelineShader();

src/main/java/com/jozufozu/flywheel/api/backend/BackendManager.java

@@ -16,8 +16,8 @@ public final class BackendManager {
 	/**
 	 * Get a string describing the current backend.
 	 */
-	public static String getBackendDescriptor() {
+	public static String getBackendNameForCrashReport() {
-		return BackendManagerImpl.getBackendDescriptor();
+		return BackendManagerImpl.getBackendNameForCrashReport();
 	}
 
 	public static boolean isOn() {

src/main/java/com/jozufozu/flywheel/api/instance/Instance.java

@@ -2,13 +2,10 @@ package com.jozufozu.flywheel.api.instance;
 
 import org.joml.FrustumIntersection;
 
-import net.minecraft.core.BlockPos;
-
 /**
  * A general interface providing information about any type of thing that could use Flywheel's instanced rendering.
  */
 public interface Instance {
-    BlockPos getWorldPosition();
 
 	/**
 	 * Initialize parts here.
@@ -39,17 +36,25 @@ public interface Instance {
 	/**
 	 * Check this instance against a frustum.<p>
 	 * An implementor may choose to return a constant to skip the frustum check.
+	 *
 	 * @param frustum A frustum intersection tester for the current frame.
 	 * @return {@code true} if this instance should be considered for updates.
 	 */
 	boolean checkFrustum(FrustumIntersection frustum);
 
+	/**
+	 * Calculate the distance squared between this instance and the given <em>world</em> position.
+	 *
+	 * @param x The x coordinate.
+	 * @param y The y coordinate.
+	 * @param z The z coordinate.
+	 * @return The distance squared between this instance and the given position.
+	 */
+	double distanceSquared(double x, double y, double z);
+
 	/**
 	 * Free any acquired resources.
 	 */
 	void delete();
 
-	// TODO
-	@Deprecated
-	void removeNow();
 }

src/main/java/com/jozufozu/flywheel/backend/instancing/manager/EffectInstanceManager.java

@@ -65,7 +65,7 @@ public class EffectInstanceManager extends InstanceManager<Effect> {
 			this.tickableInstances.removeAll(instances);
 			this.dynamicInstances.removeAll(instances);
 			for (Instance instance : instances) {
-				instance.removeNow();
+				instance.delete();
 			}
 		}
 

src/main/java/com/jozufozu/flywheel/backend/instancing/manager/InstanceManager.java

@@ -21,8 +21,6 @@ import com.jozufozu.flywheel.backend.instancing.ratelimit.NonLimiter;
 import com.jozufozu.flywheel.backend.instancing.storage.Storage;
 import com.jozufozu.flywheel.config.FlwConfig;
 
-import net.minecraft.core.BlockPos;
-
 public abstract class InstanceManager<T> {
 	private final Set<T> queuedAdditions = new HashSet<>(64);
 	private final Set<T> queuedUpdates = new HashSet<>(64);
@@ -144,7 +142,7 @@ public abstract class InstanceManager<T> {
 
 	public void delete() {
 		for (Instance instance : getStorage().getAllInstances()) {
-			instance.removeNow();
+			instance.delete();
 		}
 	}
 
@@ -191,28 +189,19 @@ public abstract class InstanceManager<T> {
 		tickLimiter.tick();
 		processQueuedUpdates();
 
-		// integer camera pos as a micro-optimization
-		int cX = (int) cameraX;
-		int cY = (int) cameraY;
-		int cZ = (int) cameraZ;
-
 		var instances = getStorage().getInstancesForTicking();
-		distributeWork(executor, instances, instance -> tickInstance(instance, cX, cY, cZ));
+		distributeWork(executor, instances, instance -> tickInstance(instance, cameraX, cameraY, cameraZ));
 	}
 
-	protected void tickInstance(TickableInstance instance, int cX, int cY, int cZ) {
+	protected void tickInstance(TickableInstance instance, double cX, double cY, double cZ) {
 		if (!instance.decreaseTickRateWithDistance()) {
 			instance.tick();
 			return;
 		}
 
-		BlockPos pos = instance.getWorldPosition();
+		var dsq = instance.distanceSquared(cX, cY, cZ);
 
-		int dX = pos.getX() - cX;
+		if (!tickLimiter.shouldUpdate(dsq)) {
-		int dY = pos.getY() - cY;
-		int dZ = pos.getZ() - cZ;
-
-		if (!tickLimiter.shouldUpdate(dX, dY, dZ)) {
 			return;
 		}
 
@@ -223,11 +212,11 @@ public abstract class InstanceManager<T> {
 		frameLimiter.tick();
 		processQueuedAdditions();
 
-		// integer camera pos
+		var cameraPos = context.camera()
-		BlockPos cameraIntPos = context.camera().getBlockPosition();
+				.getPosition();
-		int cX = cameraIntPos.getX();
+		double cX = cameraPos.x;
-		int cY = cameraIntPos.getY();
+		double cY = cameraPos.y;
-		int cZ = cameraIntPos.getZ();
+		double cZ = cameraPos.z;
 		FrustumIntersection culler = context.culler();
 
 		var instances = getStorage().getInstancesForUpdate();
@@ -251,18 +240,13 @@ public abstract class InstanceManager<T> {
 		}
 	}
 
-	protected void updateInstance(DynamicInstance instance, FrustumIntersection frustum, int cX, int cY, int cZ) {
+	protected void updateInstance(DynamicInstance instance, FrustumIntersection frustum, double cX, double cY, double cZ) {
 		if (!instance.decreaseFramerateWithDistance()) {
 			instance.beginFrame();
 			return;
 		}
 
-		BlockPos worldPos = instance.getWorldPosition();
+		if (!frameLimiter.shouldUpdate(instance.distanceSquared(cX, cY, cZ))) {
-		int dX = worldPos.getX() - cX;
-		int dY = worldPos.getY() - cY;
-		int dZ = worldPos.getZ() - cZ;
-
-		if (!frameLimiter.shouldUpdate(dX, dY, dZ)) {
 			return;
 		}
 

src/main/java/com/jozufozu/flywheel/backend/instancing/ratelimit/BandedPrimeLimiter.java

@@ -14,12 +14,12 @@ public class BandedPrimeLimiter implements DistanceUpdateLimiter {
 	}
 
 	@Override
-	public boolean shouldUpdate(int dX, int dY, int dZ) {
+	public boolean shouldUpdate(double distanceSquared) {
-		return (tickCount % getUpdateDivisor(dX, dY, dZ)) == 0;
+		return (tickCount % getUpdateDivisor(distanceSquared)) == 0;
 	}
 
-	protected int getUpdateDivisor(int dX, int dY, int dZ) {
+	protected int getUpdateDivisor(double distanceSquared) {
-		int dSq = dX * dX + dY * dY + dZ * dZ;
+		int dSq = Mth.ceil(distanceSquared);
 
 		int i = (dSq / 2048);
 

src/main/java/com/jozufozu/flywheel/backend/instancing/ratelimit/DistanceUpdateLimiter.java

@@ -11,10 +11,9 @@ public interface DistanceUpdateLimiter {
 
 	/**
 	 * Check to see if an object at the given position relative to the camera should be updated.
-	 * @param dX The X distance from the camera.
+	 *
-	 * @param dY The Y distance from the camera.
+	 * @param distanceSquared
-	 * @param dZ The Z distance from the camera.
 	 * @return {@code true} if the object should be updated, {@code false} otherwise.
 	 */
-	boolean shouldUpdate(int dX, int dY, int dZ);
+	boolean shouldUpdate(double distanceSquared);
 }

src/main/java/com/jozufozu/flywheel/backend/instancing/ratelimit/NonLimiter.java

@@ -6,7 +6,7 @@ public class NonLimiter implements DistanceUpdateLimiter {
 	}
 
 	@Override
-	public boolean shouldUpdate(int dX, int dY, int dZ) {
+	public boolean shouldUpdate(double distanceSquared) {
 		return true;
 	}
 }

src/main/java/com/jozufozu/flywheel/backend/instancing/storage/One2OneStorage.java

@@ -46,7 +46,6 @@ public abstract class One2OneStorage<T> extends AbstractStorage<T> {
 		instance.delete();
 		dynamicInstances.remove(instance);
 		tickableInstances.remove(instance);
-		instance.removeNow();
 	}
 
 	@Override

src/main/java/com/jozufozu/flywheel/config/FlwCommands.java

@@ -53,7 +53,7 @@ public class FlwCommands {
 							return 0;
 						}
 
-						Component message = backend.getEngineMessage();
+						Component message = backend.engineMessage();
 						player.displayClientMessage(message, false);
 					}
 					return Command.SINGLE_SUCCESS;
@@ -65,7 +65,7 @@ public class FlwCommands {
 							Backend backend = context.getArgument("id", Backend.class);
 							value.set(Backend.REGISTRY.getIdOrThrow(backend).toString());
 
-							Component message = backend.getEngineMessage();
+							Component message = backend.engineMessage();
 							player.displayClientMessage(message, false);
 
 							BackendUtil.reloadWorldRenderers();

src/main/java/com/jozufozu/flywheel/impl/BackendManagerImpl.java

@@ -18,8 +18,15 @@ public final class BackendManagerImpl {
 		return backend;
 	}
 
-	public static String getBackendDescriptor() {
+	public static String getBackendNameForCrashReport() {
-		return backend == null ? "Uninitialized" : backend.getProperName();
+		if (backend == null) {
+			return "Uninitialized";
+		}
+		var backendId = Backend.REGISTRY.getId(backend);
+		if (backendId == null) {
+			return "Unregistered";
+		}
+		return backendId.toString();
 	}
 
 	public static boolean isOn() {

src/main/java/com/jozufozu/flywheel/lib/backend/Backends.java

@@ -15,7 +15,6 @@ import net.minecraft.network.chat.TextComponent;
 
 public class Backends {
 	public static final Backend OFF = SimpleBackend.builder()
-			.properName("Off")
 			.engineMessage(new TextComponent("Disabled Flywheel").withStyle(ChatFormatting.RED))
 			.engineSupplier(() -> {
 				throw new IllegalStateException("Cannot create engine when backend is off.");
@@ -28,7 +27,6 @@ public class Backends {
 	 * Use a thread pool to buffer instances in parallel on the CPU.
 	 */
 	public static final Backend BATCHING = SimpleBackend.builder()
-			.properName("Parallel Batching")
 			.engineMessage(new TextComponent("Using Batching Engine").withStyle(ChatFormatting.GREEN))
 			.engineSupplier(BatchingEngine::new)
 			.fallback(() -> Backends.OFF)
@@ -39,7 +37,6 @@ public class Backends {
 	 * Use GPU instancing to render everything.
 	 */
 	public static final Backend INSTANCING = SimpleBackend.builder()
-			.properName("GL33 Instanced Arrays")
 			.engineMessage(new TextComponent("Using Instancing Engine").withStyle(ChatFormatting.GREEN))
 			.engineSupplier(() -> new InstancingEngine(Contexts.WORLD, 100 * 100))
 			.fallback(() -> Backends.BATCHING)
@@ -52,7 +49,6 @@ public class Backends {
 	 * Use Compute shaders to cull instances.
 	 */
 	public static final Backend INDIRECT = SimpleBackend.builder()
-			.properName("GL46 Compute Culling")
 			.engineMessage(new TextComponent("Using Indirect Engine").withStyle(ChatFormatting.GREEN))
 			.engineSupplier(() -> new IndirectEngine(Contexts.WORLD, 100 * 100))
 			.fallback(() -> Backends.INSTANCING)

src/main/java/com/jozufozu/flywheel/lib/backend/SimpleBackend.java

@@ -13,15 +13,13 @@ import net.minecraft.network.chat.Component;
 import net.minecraft.resources.ResourceLocation;
 
 public class SimpleBackend implements Backend {
-	private final String properName;
 	private final Component engineMessage;
 	private final Supplier<Engine> engineSupplier;
 	private final Supplier<Backend> fallback;
 	private final BooleanSupplier isSupported;
 	private final Pipeline pipelineShader;
 
-	public SimpleBackend(String properName, Component engineMessage, Supplier<Engine> engineSupplier, Supplier<Backend> fallback, BooleanSupplier isSupported, @Nullable Pipeline pipelineShader) {
+	public SimpleBackend(Component engineMessage, Supplier<Engine> engineSupplier, Supplier<Backend> fallback, BooleanSupplier isSupported, @Nullable Pipeline pipelineShader) {
-		this.properName = properName;
 		this.engineMessage = engineMessage;
 		this.engineSupplier = engineSupplier;
 		this.fallback = fallback;
@@ -34,12 +32,7 @@ public class SimpleBackend implements Backend {
 	}
 
 	@Override
-	public String getProperName() {
+	public Component engineMessage() {
-		return properName;
-	}
-
-	@Override
-	public Component getEngineMessage() {
 		return engineMessage;
 	}
 
@@ -69,18 +62,12 @@ public class SimpleBackend implements Backend {
 	}
 
 	public static class Builder {
-		private String properName;
 		private Component engineMessage;
 		private Supplier<Engine> engineSupplier;
 		private Supplier<Backend> fallback;
 		private BooleanSupplier isSupported;
 		private Pipeline pipelineShader;
 
-		public Builder properName(String properName) {
-			this.properName = properName;
-			return this;
-		}
-
 		public Builder engineMessage(Component engineMessage) {
 			this.engineMessage = engineMessage;
 			return this;
@@ -107,7 +94,7 @@ public class SimpleBackend implements Backend {
 		}
 
 		public Backend register(ResourceLocation id) {
-			return Backend.REGISTRY.registerAndGet(id, new SimpleBackend(properName, engineMessage, engineSupplier, fallback, isSupported, pipelineShader));
+			return Backend.REGISTRY.registerAndGet(id, new SimpleBackend(engineMessage, engineSupplier, fallback, isSupported, pipelineShader));
 		}
 	}
 }

src/main/java/com/jozufozu/flywheel/lib/instance/AbstractBlockEntityInstance.java

@@ -63,7 +63,7 @@ public abstract class AbstractBlockEntityInstance<T extends BlockEntity> extends
 
 	/**
 	 * Just before {@link #update()} would be called, {@code shouldReset()} is checked.
-	 * If this function returns {@code true}, then this instance will be {@link #remove removed},
+	 * If this function returns {@code true}, then this instance will be {@link #delete removed},
 	 * and another instance will be constructed to replace it. This allows for more sane resource
 	 * acquisition compared to trying to update everything within the lifetime of an instance.
 	 *
@@ -85,11 +85,6 @@ public abstract class AbstractBlockEntityInstance<T extends BlockEntity> extends
 		return pos.subtract(instancerManager.getOriginCoordinate());
 	}
 
-	@Override
-	public BlockPos getWorldPosition() {
-		return pos;
-	}
-
 	@Override
 	public ImmutableBox getVolume() {
 		return MutableBox.from(pos);
@@ -99,4 +94,9 @@ public abstract class AbstractBlockEntityInstance<T extends BlockEntity> extends
 	public boolean checkFrustum(FrustumIntersection frustum) {
 		return frustum.testAab(pos.getX(), pos.getY(), pos.getZ(), pos.getX() + 1, pos.getY() + 1, pos.getZ() + 1);
 	}
+
+	@Override
+	public double distanceSquared(double x, double y, double z) {
+		return pos.distToCenterSqr(x, y, z);
+	}
 }

src/main/java/com/jozufozu/flywheel/lib/instance/AbstractEntityInstance.java

@@ -11,7 +11,6 @@ import com.jozufozu.flywheel.lib.box.MutableBox;
 import com.jozufozu.flywheel.lib.light.TickingLightListener;
 import com.mojang.math.Vector3f;
 
-import net.minecraft.core.BlockPos;
 import net.minecraft.core.Vec3i;
 import net.minecraft.util.Mth;
 import net.minecraft.world.entity.Entity;
@@ -88,20 +87,17 @@ public abstract class AbstractEntityInstance<E extends Entity> extends AbstractI
 		Vec3i origin = instancerManager.getOriginCoordinate();
 		return new Vector3f(
 				(float) (Mth.lerp(partialTicks, entity.xOld, pos.x) - origin.getX()),
-				(float) (Mth.lerp(partialTicks, entity.yOld, pos.y) - origin.getY()),
+				(float) (Mth.lerp(partialTicks, entity.yOld, pos.y) - origin.getY()), (float) (Mth.lerp(partialTicks, entity.zOld, pos.z) - origin.getZ()));
-				(float) (Mth.lerp(partialTicks, entity.zOld, pos.z) - origin.getZ())
-		);
-	}
-
-	@Override
-	public BlockPos getWorldPosition() {
-		return entity.blockPosition();
 	}
 
 	@Override
 	public boolean checkFrustum(FrustumIntersection frustum) {
 		AABB aabb = entity.getBoundingBox();
-		return frustum.testAab((float) aabb.minX, (float) aabb.minY, (float) aabb.minZ,
+		return frustum.testAab((float) aabb.minX, (float) aabb.minY, (float) aabb.minZ, (float) aabb.maxX, (float) aabb.maxY, (float) aabb.maxZ);
-				(float) aabb.maxX, (float) aabb.maxY, (float) aabb.maxZ);
+	}
+
+	@Override
+	public double distanceSquared(double x, double y, double z) {
+		return entity.distanceToSqr(x, y, z);
 	}
 }

src/main/java/com/jozufozu/flywheel/lib/instance/AbstractInstance.java

@@ -86,8 +86,4 @@ public abstract class AbstractInstance implements Instance, LightListener {
 				.setSkyLight(sky));
 	}
 
-	@Override
-	public final void removeNow() {
-		LightUpdater.get(level).removeListener(this);
-	}
 }

src/main/java/com/jozufozu/flywheel/vanilla/BellInstance.java

@@ -41,15 +41,18 @@ public class BellInstance extends AbstractBlockEntityInstance<BellBlockEntity> i
 
 	@Override
 	public void beginFrame() {
-		float ringTime = (float)blockEntity.ticks + AnimationTickHolder.getPartialTicks();
+		float ringTime = (float) blockEntity.ticks + AnimationTickHolder.getPartialTicks();
 
-		if (ringTime == lastRingTime) return;
+		if (ringTime == lastRingTime) {
+			return;
+		}
 		lastRingTime = ringTime;
 
 		if (blockEntity.shaking) {
 			float angle = Mth.sin(ringTime / (float) Math.PI) / (4.0F + ringTime / 3.0F);
 
-			Vector3f ringAxis = blockEntity.clickDirection.getCounterClockWise().step();
+			Vector3f ringAxis = blockEntity.clickDirection.getCounterClockWise()
+					.step();
 
 			bell.setRotation(ringAxis.rotation(angle));
 		} else {
@@ -59,7 +62,7 @@ public class BellInstance extends AbstractBlockEntityInstance<BellBlockEntity> i
 
 	@Override
 	public void updateLight() {
-		relight(getWorldPosition(), bell);
+		relight(pos, bell);
 	}
 
 	@Override

src/main/java/com/jozufozu/flywheel/vanilla/ChestInstance.java

@@ -71,11 +71,9 @@ public class ChestInstance<T extends BlockEntity & LidBlockEntity> extends Abstr
 
 			body.setRotation(baseRotation);
 
-			DoubleBlockCombiner.NeighborCombineResult<? extends ChestBlockEntity> wrapper = chestBlock.combine(blockState, level, getWorldPosition(), true);
+			DoubleBlockCombiner.NeighborCombineResult<? extends ChestBlockEntity> wrapper = chestBlock.combine(blockState, level, pos, true);
 
 			this.lidProgress = wrapper.apply(ChestBlock.opennessCombiner(blockEntity));
-
-
 		} else {
 			baseRotation = Quaternion.ONE;
 			lidProgress = $ -> 0f;
@@ -86,7 +84,9 @@ public class ChestInstance<T extends BlockEntity & LidBlockEntity> extends Abstr
 	public void beginFrame() {
 		float progress = lidProgress.get(AnimationTickHolder.getPartialTicks());
 
-		if (lastProgress == progress) return;
+		if (lastProgress == progress) {
+			return;
+		}
 
 		lastProgress = progress;
 
@@ -108,7 +108,7 @@ public class ChestInstance<T extends BlockEntity & LidBlockEntity> extends Abstr
 
 	@Override
 	public void updateLight() {
-		relight(getWorldPosition(), body, lid);
+		relight(pos, body, lid);
 	}
 
 	@Override
@@ -123,13 +123,11 @@ public class ChestInstance<T extends BlockEntity & LidBlockEntity> extends Abstr
 	}
 
 	private OrientedPart baseInstance() {
-
 		return instancerManager.getInstancer(StructTypes.ORIENTED, BASE.apply(chestType, sprite), RenderStage.AFTER_BLOCK_ENTITIES)
 				.createInstance();
 	}
 
 	private TransformedPart lidInstance() {
-
 		return instancerManager.getInstancer(StructTypes.TRANSFORMED, LID.apply(chestType, sprite), RenderStage.AFTER_BLOCK_ENTITIES)
 				.createInstance();
 	}

src/main/java/com/jozufozu/flywheel/vanilla/MinecartInstance.java

@@ -59,7 +59,7 @@ public class MinecartInstance<T extends AbstractMinecart> extends AbstractEntity
 			contents.delete();
 			contents = getContents();
 			if (contents != null) {
-				relight(getWorldPosition(), contents);
+				relight(entity.blockPosition(), contents);
 			}
 		}
 	}
@@ -143,16 +143,19 @@ public class MinecartInstance<T extends AbstractMinecart> extends AbstractEntity
 
 	@Override
 	public void updateLight() {
-		if (contents == null)
+		if (contents == null) {
-			relight(getWorldPosition(), body);
+			relight(entity.blockPosition(), body);
-		else
+		} else {
-			relight(getWorldPosition(), body, contents);
+			relight(entity.blockPosition(), body, contents);
+		}
 	}
 
 	@Override
 	protected void _delete() {
 		body.delete();
-		if (contents != null) contents.delete();
+		if (contents != null) {
+			contents.delete();
+		}
 	}
 
 	private TransformedPart getContents() {
@@ -165,8 +168,9 @@ public class MinecartInstance<T extends AbstractMinecart> extends AbstractEntity
 		}
 		active = true;
 
-		if (shape == RenderShape.INVISIBLE)
+		if (shape == RenderShape.INVISIBLE) {
 			return null;
+		}
 
         return instancerManager.getInstancer(StructTypes.TRANSFORMED, Models.block(blockState), RenderStage.AFTER_ENTITIES)
 				.createInstance();

src/main/java/com/jozufozu/flywheel/vanilla/effect/ExampleEffect.java

@@ -256,11 +256,6 @@ public class ExampleEffect implements Effect {
 					.setSkyLight(15);
 		}
 
-		@Override
-		public BlockPos getWorldPosition() {
-			return blockPos;
-		}
-
 		@Override
 		protected void _delete() {
 			instance.delete();
@@ -304,5 +299,10 @@ public class ExampleEffect implements Effect {
 		public boolean checkFrustum(FrustumIntersection frustum) {
 			return true;
 		}
+
+		@Override
+		public double distanceSquared(double x, double y, double z) {
+			return 0;
+		}
 	}
 }