More statefuler

- Hidden state now tracks the Instance object to keep the handle small
- Make the recreate supplier an explicit record to allow comparisons
- Add setVisible method to Instance

common/src/api/java/dev/engine_room/flywheel/api/instance/Instance.java

@@ -12,4 +12,8 @@ public interface Instance {
 	default void delete() {
 		handle().setDeleted();
 	}
+
+	default void setVisible(boolean visible) {
+		handle().setVisible(visible);
+	}
 }

common/src/backend/java/dev/engine_room/flywheel/backend/engine/AbstractInstancer.java

@@ -1,7 +1,6 @@
 package dev.engine_room.flywheel.backend.engine;
 
 import java.util.ArrayList;
-import java.util.function.Supplier;
 
 import org.jetbrains.annotations.Nullable;
 
@@ -14,7 +13,7 @@ import dev.engine_room.flywheel.backend.util.AtomicBitSet;
 public abstract class AbstractInstancer<I extends Instance> implements Instancer<I>, InstanceHandleImpl.State<I> {
 	public final InstanceType<I> type;
 	public final Environment environment;
-	private final InstanceHandleImpl.Hidden<I> hidden;
+	private final Recreate<I> recreate;
 
 	// Lock for all instances, only needs to be used in methods that may run on the TaskExecutor.
 	protected final Object lock = new Object();
@@ -24,10 +23,10 @@ public abstract class AbstractInstancer<I extends Instance> implements Instancer
 	protected final AtomicBitSet changed = new AtomicBitSet();
 	protected final AtomicBitSet deleted = new AtomicBitSet();
 
-	protected AbstractInstancer(InstancerKey<I> key, Supplier<AbstractInstancer<I>> recreate) {
+	protected AbstractInstancer(InstancerKey<I> key, Recreate<I> recreate) {
 		this.type = key.type();
 		this.environment = key.environment();
-		this.hidden = new InstanceHandleImpl.Hidden<>(recreate);
+		this.recreate = recreate;
 	}
 
 	@Override
@@ -43,26 +42,26 @@ public abstract class AbstractInstancer<I extends Instance> implements Instancer
 	}
 
 	@Override
-	public InstanceHandleImpl.State<I> setVisible(int index, I instance, boolean visible) {
+	public InstanceHandleImpl.State<I> setVisible(InstanceHandleImpl<I> handle, int index, boolean visible) {
 		if (visible) {
 			return this;
 		}
 
 		notifyRemoval(index);
 
-		return hidden;
-	}
+		I instance;
+		synchronized (lock) {
+			// I think we need to lock to prevent wacky stuff from happening if the array gets resized.
+			instance = instances.get(index);
+		}
 
-	@Override
-	public InstanceHandleImpl.Status status() {
-		return InstanceHandleImpl.Status.VISIBLE;
+		return new InstanceHandleImpl.Hidden<>(recreate, instance);
 	}
 
 	@Override
 	public I createInstance() {
 		var handle = new InstanceHandleImpl<>(this);
 		I instance = type.create(handle);
-		handle.instance = instance;
 
 		synchronized (lock) {
 			handle.index = instances.size();
@@ -71,6 +70,13 @@ public abstract class AbstractInstancer<I extends Instance> implements Instancer
 		}
 	}
 
+	public void revealInstance(InstanceHandleImpl<I> handle, I instance) {
+		synchronized (lock) {
+			handle.index = instances.size();
+			addLocked(instance, handle);
+		}
+	}
+
 	@Override
 	public void stealInstance(@Nullable I instance) {
 		if (instance == null) {
@@ -85,10 +91,19 @@ public abstract class AbstractInstancer<I extends Instance> implements Instancer
 		}
 
 		// Should InstanceType have an isInstance method?
+		@SuppressWarnings("unchecked")
 		var handle = (InstanceHandleImpl<I>) instanceHandle;
 
-		// Should you be allowed to steal deleted instances?
-		if (handle.state == this || handle.state.status() == InstanceHandleImpl.Status.DELETED) {
+		// No need to steal if this instance is already owned by this instancer.
+		if (handle.state == this) {
+			return;
+		}
+		// Not allowed to steal deleted instances.
+		if (handle.state instanceof InstanceHandleImpl.Deleted) {
+			return;
+		}
+		// No need to steal if the instance will recreate to us.
+		if (handle.state instanceof InstanceHandleImpl.Hidden<I> hidden && recreate.equals(hidden.recreate())) {
 			return;
 		}
 
@@ -96,24 +111,21 @@ public abstract class AbstractInstancer<I extends Instance> implements Instancer
 		//  is somehow being stolen by 2 different instancers between threads.
 		//  That seems kinda impossible so I'm fine leaving it as is for now.
 
-		// Remove the instance from its old instancer.
-		// This won't have any unwanted effect when the old instancer
-		// is filtering deleted instances later, so is safe.
-		handle.setDeleted();
-
 		// Add the instance to this instancer.
-		switch (handle.state.status()) {
-		case VISIBLE:
+		if (handle.state instanceof AbstractInstancer<I> other) {
+			// Remove the instance from its old instancer.
+			// This won't have any unwanted effect when the old instancer
+			// is filtering deleted instances later, so is safe.
+			other.notifyRemoval(handle.index);
+
 			handle.state = this;
 			// Only lock now that we'll be mutating our state.
 			synchronized (lock) {
 				handle.index = instances.size();
 				addLocked(instance, handle);
 			}
-			break;
-		case HIDDEN:
-			handle.state = hidden;
-			break;
+		} else if (handle.state instanceof InstanceHandleImpl.Hidden<I>) {
+			handle.state = new InstanceHandleImpl.Hidden<>(recreate, instance);
 		}
 	}
 
@@ -216,7 +228,7 @@ public abstract class AbstractInstancer<I extends Instance> implements Instancer
 			// clearing it here would cause significant visual artifacts and instance leaks.
 			// At the same time, we need to clear handles we own to prevent
 			// instances from changing/deleting positions in this instancer that no longer exist.
-			if (handle.state == this || handle.state == hidden) {
+			if (handle.state == this) {
 				handle.clear();
 				handle.state = InstanceHandleImpl.Deleted.instance();
 			}
@@ -233,4 +245,10 @@ public abstract class AbstractInstancer<I extends Instance> implements Instancer
 	public String toString() {
 		return "AbstractInstancer[" + instanceCount() + ']';
 	}
+
+	public record Recreate<I extends Instance>(InstancerKey<I> key, DrawManager<?> drawManager) {
+		public AbstractInstancer<I> recreate() {
+			return drawManager.getInstancer(key);
+		}
+	}
 }

common/src/backend/java/dev/engine_room/flywheel/backend/engine/InstanceHandleImpl.java

@@ -1,16 +1,10 @@
 package dev.engine_room.flywheel.backend.engine;
 
-import java.util.function.Supplier;
-
-import org.jetbrains.annotations.UnknownNullability;
-
 import dev.engine_room.flywheel.api.instance.Instance;
 import dev.engine_room.flywheel.api.instance.InstanceHandle;
 
 public class InstanceHandleImpl<I extends Instance> implements InstanceHandle {
 	public State<I> state;
-	@UnknownNullability
-	public I instance;
 	public int index;
 
 	public InstanceHandleImpl(State<I> state) {
@@ -31,35 +25,27 @@ public class InstanceHandleImpl<I extends Instance> implements InstanceHandle {
 
 	@Override
 	public void setVisible(boolean visible) {
-		state = state.setVisible(index, instance, visible);
+		state = state.setVisible(this, index, visible);
 	}
 
 	@Override
 	public boolean isVisible() {
-		return state.status() == Status.VISIBLE;
+		return state instanceof AbstractInstancer<?>;
 	}
 
 	public void clear() {
 		index = -1;
 	}
 
-	public enum Status {
-		HIDDEN,
-		DELETED,
-		VISIBLE
-	}
-
 	public interface State<I extends Instance> {
 		State<I> setChanged(int index);
 
 		State<I> setDeleted(int index);
 
-		State<I> setVisible(int index, I instance, boolean visible);
-
-		Status status();
+		State<I> setVisible(InstanceHandleImpl<I> handle, int index, boolean visible);
 	}
 
-	public record Hidden<I extends Instance>(Supplier<AbstractInstancer<I>> supplier) implements State<I> {
+	public record Hidden<I extends Instance>(AbstractInstancer.Recreate<I> recreate, I instance) implements State<I> {
 		@Override
 		public State<I> setChanged(int index) {
 			return this;
@@ -71,19 +57,14 @@ public class InstanceHandleImpl<I extends Instance> implements InstanceHandle {
 		}
 
 		@Override
-		public State<I> setVisible(int index, I instance, boolean visible) {
+		public State<I> setVisible(InstanceHandleImpl<I> handle, int index, boolean visible) {
 			if (!visible) {
 				return this;
 			}
-			var instancer = supplier.get();
-			instancer.stealInstance(instance);
+			var instancer = recreate.recreate();
+			instancer.revealInstance(handle, instance);
 			return instancer;
 		}
-
-		@Override
-		public Status status() {
-			return Status.HIDDEN;
-		}
 	}
 
 	public record Deleted<I extends Instance>() implements State<I> {
@@ -105,13 +86,8 @@ public class InstanceHandleImpl<I extends Instance> implements InstanceHandle {
 		}
 
 		@Override
-		public State<I> setVisible(int index, I instance, boolean visible) {
+		public State<I> setVisible(InstanceHandleImpl<I> handle, int index, boolean visible) {
 			return this;
 		}
-
-		@Override
-		public Status status() {
-			return Status.DELETED;
-		}
 	}
 }

common/src/backend/java/dev/engine_room/flywheel/backend/engine/indirect/IndirectDrawManager.java

@@ -19,6 +19,7 @@ import dev.engine_room.flywheel.api.visualization.VisualType;
 import dev.engine_room.flywheel.backend.Samplers;
 import dev.engine_room.flywheel.backend.compile.ContextShader;
 import dev.engine_room.flywheel.backend.compile.IndirectPrograms;
+import dev.engine_room.flywheel.backend.engine.AbstractInstancer;
 import dev.engine_room.flywheel.backend.engine.CommonCrumbling;
 import dev.engine_room.flywheel.backend.engine.DrawManager;
 import dev.engine_room.flywheel.backend.engine.GroupKey;
@@ -67,7 +68,7 @@ public class IndirectDrawManager extends DrawManager<IndirectInstancer<?>> {
 
 	@Override
 	protected <I extends Instance> IndirectInstancer<?> create(InstancerKey<I> key) {
-		return new IndirectInstancer<>(key, () -> getInstancer(key));
+		return new IndirectInstancer<>(key, new AbstractInstancer.Recreate<>(key, this));
 	}
 
 	@SuppressWarnings("unchecked")

common/src/backend/java/dev/engine_room/flywheel/backend/engine/indirect/IndirectInstancer.java

@@ -2,7 +2,6 @@ package dev.engine_room.flywheel.backend.engine.indirect;
 
 import java.util.ArrayList;
 import java.util.List;
-import java.util.function.Supplier;
 
 import org.jetbrains.annotations.UnknownNullability;
 import org.joml.Vector4fc;
@@ -28,7 +27,7 @@ public class IndirectInstancer<I extends Instance> extends AbstractInstancer<I>
 	private int modelIndex = -1;
 	private int baseInstance = -1;
 
-	public IndirectInstancer(InstancerKey<I> key, Supplier<AbstractInstancer<I>> recreate) {
+	public IndirectInstancer(InstancerKey<I> key, Recreate<I> recreate) {
 		super(key, recreate);
 		instanceStride = MoreMath.align4(type.layout()
 				.byteSize());

common/src/backend/java/dev/engine_room/flywheel/backend/engine/instancing/InstancedDrawManager.java

@@ -14,6 +14,7 @@ import dev.engine_room.flywheel.backend.MaterialShaderIndices;
 import dev.engine_room.flywheel.backend.Samplers;
 import dev.engine_room.flywheel.backend.compile.ContextShader;
 import dev.engine_room.flywheel.backend.compile.InstancingPrograms;
+import dev.engine_room.flywheel.backend.engine.AbstractInstancer;
 import dev.engine_room.flywheel.backend.engine.CommonCrumbling;
 import dev.engine_room.flywheel.backend.engine.DrawManager;
 import dev.engine_room.flywheel.backend.engine.GroupKey;
@@ -125,7 +126,7 @@ public class InstancedDrawManager extends DrawManager<InstancedInstancer<?>> {
 
 	@Override
 	protected <I extends Instance> InstancedInstancer<I> create(InstancerKey<I> key) {
-		return new InstancedInstancer<>(key, () -> getInstancer(key));
+		return new InstancedInstancer<>(key, new AbstractInstancer.Recreate<>(key, this));
 	}
 
 	@Override

common/src/backend/java/dev/engine_room/flywheel/backend/engine/instancing/InstancedInstancer.java

@@ -2,7 +2,6 @@ package dev.engine_room.flywheel.backend.engine.instancing;
 
 import java.util.ArrayList;
 import java.util.List;
-import java.util.function.Supplier;
 
 import org.jetbrains.annotations.Nullable;
 
@@ -25,7 +24,7 @@ public class InstancedInstancer<I extends Instance> extends AbstractInstancer<I>
 
 	private final List<InstancedDraw> draws = new ArrayList<>();
 
-	public InstancedInstancer(InstancerKey<I> key, Supplier<AbstractInstancer<I>> recreate) {
+	public InstancedInstancer(InstancerKey<I> key, Recreate<I> recreate) {
 		super(key, recreate);
 		var layout = type.layout();
 		// Align to one texel in the texture buffer

common/src/lib/java/dev/engine_room/flywheel/lib/instance/AbstractInstance.java

@@ -34,4 +34,10 @@ public abstract class AbstractInstance implements Instance {
 		// Override to mark final.
 		handle.setDeleted();
 	}
+
+	@Override
+	public final void setVisible(boolean visible) {
+		// Override to mark final.
+		handle.setVisible(visible);
+	}
 }