Eviction notice

- For instancing, remove empty instancers and delete meshes
- Specify in InstancerProvider's contract that instancers should not be
  kept around, but reusing them within one frame in guaranteed to be
  safe
- Do all instancer updates in flush, and remove unnecessary checks that
  would be made later in the frame
- Remove isEmpty from Mesh
- Remove staging buffer param from indirect mesh pool

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

@@ -9,7 +9,11 @@ public interface InstancerProvider {
 	/**
 	 * Get an instancer for the given instance type rendering the given model.
 	 *
-	 * <p>Calling this method twice with the same arguments will return the same instancer.</p>
+	 * <p>Calling this method twice with the same arguments in the
+	 * same frame will return the same instancer.</p>
+	 *
+	 * <p>It is not safe to store instancers between frames. Each
+	 * time you need an instancer, you should call this method.</p>
 	 *
 	 * @return An instancer for the given instance type rendering the given model.
 	 */

src/main/java/com/jozufozu/flywheel/api/model/Mesh.java

@@ -13,14 +13,6 @@ public interface Mesh {
 	 */
 	int vertexCount();
 
-	/**
-	 * Is there nothing to render?
-	 * @return true if there are no vertices.
-	 */
-	default boolean isEmpty() {
-		return vertexCount() == 0;
-	}
-
 	/**
 	 * Write this mesh into a vertex list. Vertices with index {@literal <}0 or {@literal >=}{@link #vertexCount()} will not be
 	 * read or modified.

src/main/java/com/jozufozu/flywheel/backend/engine/InstancerStorage.java

@@ -59,23 +59,23 @@ public abstract class InstancerStorage<N extends AbstractInstancer<?>> {
 		var out = create(key);
 
 		// Only queue the instancer for initialization if it has anything to render.
-		if (key.model()
-				.meshes()
-				.isEmpty()) {
-			warnEmptyModel();
-		} else {
+        if (checkAndWarnEmptyModel(key.model())) {
 			// Thread safety: this method is called atomically from within computeIfAbsent,
 			// so we don't need extra synchronization to protect the queue.
 			initializationQueue.add(new UninitializedInstancer<>(key, out));
 		}
-		return out;
+        return out;
 	}
 
 	protected record UninitializedInstancer<N, I extends Instance>(InstancerKey<I> key, N instancer) {
 
 	}
 
-	private static void warnEmptyModel() {
+	private static boolean checkAndWarnEmptyModel(Model model) {
+		if (!model.meshes().isEmpty()) {
+			return true;
+		}
+
 		StringBuilder builder = new StringBuilder();
 		builder.append("Creating an instancer for a model with no meshes! Stack trace:");
 
@@ -85,5 +85,7 @@ public abstract class InstancerStorage<N extends AbstractInstancer<?>> {
 						.append(f.toString()));
 
 		Flywheel.LOGGER.warn(builder.toString());
+
+		return false;
 	}
 }

src/main/java/com/jozufozu/flywheel/backend/engine/indirect/IndirectCullingGroup.java

@@ -83,7 +83,7 @@ public class IndirectCullingGroup<I extends Instance> {
 		buffers.updateCounts(instanceCountThisFrame, indirectModels.size(), indirectDraws.size());
 
 		// Must flush the mesh pool first so everything else has the right baseVertex and baseIndex.
-		meshPool.flush(stagingBuffer);
+		meshPool.flush();
 
 		// Upload only objects that have changed.
 		uploadObjects(stagingBuffer);

src/main/java/com/jozufozu/flywheel/backend/engine/indirect/IndirectMeshPool.java

@@ -62,15 +62,14 @@ public class IndirectMeshPool {
 		return meshes.get(mesh);
 	}
 
-	public void flush(StagingBuffer stagingBuffer) {
+	public void flush() {
 		if (dirty) {
-			// TODO: use the staging buffer and be smarter about allocation in general.
-			uploadAll(stagingBuffer);
+			uploadAll();
 			dirty = false;
 		}
 	}
 
-	private void uploadAll(StagingBuffer stagingBuffer) {
+	private void uploadAll() {
 		long neededSize = 0;
 		int maxQuadIndexCount = 0;
 		int nonQuadIndexCount = 0;

src/main/java/com/jozufozu/flywheel/backend/engine/instancing/DrawCall.java

@@ -11,32 +11,27 @@ public class DrawCall {
 	private final InstancedInstancer<?> instancer;
 	private final InstancedMeshPool.BufferedMesh mesh;
 
-	@Nullable
-	private GlVertexArray vao;
+	private final GlVertexArray vao;
 	@Nullable
 	private GlVertexArray vaoScratch;
+	private boolean deleted;
 
 	public DrawCall(InstancedInstancer<?> instancer, InstancedMeshPool.BufferedMesh mesh, ShaderState shaderState) {
 		this.instancer = instancer;
 		this.mesh = mesh;
 		this.shaderState = shaderState;
 
+		mesh.acquire();
+
 		vao = GlVertexArray.create();
 	}
 
-	public boolean isInvalid() {
-		return instancer.isInvalid() || vao == null;
+	public boolean deleted() {
+		return deleted;
 	}
 
 	public void render() {
-		if (isInvalid() || mesh.isEmpty()) {
-			return;
-		}
-
-		instancer.update();
-
-		int instanceCount = instancer.getInstanceCount();
-		if (instanceCount <= 0) {
+		if (mesh.invalid()) {
 			return;
 		}
 
@@ -45,16 +40,14 @@ public class DrawCall {
 
 		vao.bindForDraw();
 
-		mesh.draw(instanceCount);
+		mesh.draw(instancer.getInstanceCount());
 	}
 
 	public void renderOne(InstanceHandleImpl impl) {
-		if (isInvalid() || mesh.isEmpty()) {
+		if (mesh.invalid()) {
 			return;
 		}
 
-		instancer.update();
-
 		int instanceCount = instancer.getInstanceCount();
 		if (instanceCount <= 0 || impl.index >= instanceCount) {
 			return;
@@ -78,14 +71,19 @@ public class DrawCall {
 	}
 
 	public void delete() {
-		if (vao != null) {
-			vao.delete();
-			vao = null;
+		if (deleted) {
+			return;
 		}
 
+		vao.delete();
+
 		if (vaoScratch != null) {
 			vaoScratch.delete();
 			vaoScratch = null;
 		}
+
+		mesh.drop();
+
+		deleted = true;
 	}
 }

src/main/java/com/jozufozu/flywheel/backend/engine/instancing/InstancedCrumbling.java

@@ -98,11 +98,7 @@ public class InstancedCrumbling {
 					continue;
 				}
 
-				List<DrawCall> draws = instancer.drawCalls();
-
-				draws.removeIf(DrawCall::isInvalid);
-
-				for (DrawCall draw : draws) {
+                for (DrawCall draw : instancer.drawCalls()) {
 					out.computeIfAbsent(draw.shaderState, $ -> new Int2ObjectArrayMap<>())
 							.computeIfAbsent(progress, $ -> new ArrayList<>())
 							.add(() -> draw.renderOne(impl));

src/main/java/com/jozufozu/flywheel/backend/engine/instancing/InstancedDrawManager.java

@@ -10,7 +10,6 @@ import com.google.common.collect.ImmutableListMultimap;
 import com.google.common.collect.ListMultimap;
 import com.jozufozu.flywheel.api.event.RenderStage;
 import com.jozufozu.flywheel.api.instance.Instance;
-import com.jozufozu.flywheel.api.model.Mesh;
 import com.jozufozu.flywheel.backend.engine.InstancerKey;
 import com.jozufozu.flywheel.backend.engine.InstancerStorage;
 
@@ -19,11 +18,11 @@ public class InstancedDrawManager extends InstancerStorage<InstancedInstancer<?>
 	 * The set of draw calls to make in each {@link RenderStage}.
 	 */
 	private final Map<RenderStage, DrawSet> drawSets = new EnumMap<>(RenderStage.class);
+	private final EboCache eboCache = new EboCache();
 	/**
 	 * A map of vertex types to their mesh pools.
 	 */
-	private final InstancedMeshPool meshPool = new InstancedMeshPool();
-	private final EboCache eboCache = new EboCache();
+	private final InstancedMeshPool meshPool = new InstancedMeshPool(eboCache);
 
 	public DrawSet get(RenderStage stage) {
 		return drawSets.getOrDefault(stage, DrawSet.EMPTY);
@@ -32,6 +31,24 @@ public class InstancedDrawManager extends InstancerStorage<InstancedInstancer<?>
 	public void flush() {
 		super.flush();
 
+		var instancers = this.instancers.values();
+		instancers.removeIf(instancer -> {
+			// Update the instancers and remove any that are empty.
+			instancer.update();
+
+			if (instancer.getInstanceCount() == 0) {
+				instancer.delete();
+				return true;
+			} else {
+				return false;
+			}
+		});
+
+		for (DrawSet drawSet : drawSets.values()) {
+			// Remove the draw calls for any instancers we deleted.
+			drawSet.prune();
+		}
+
 		meshPool.flush();
 	}
 
@@ -50,10 +67,6 @@ public class InstancedDrawManager extends InstancerStorage<InstancedInstancer<?>
 		eboCache.invalidate();
 	}
 
-	private InstancedMeshPool.BufferedMesh alloc(Mesh mesh) {
-		return meshPool.alloc(mesh, eboCache);
-	}
-
 	@Override
 	protected <I extends Instance> InstancedInstancer<I> create(InstancerKey<I> key) {
 		return new InstancedInstancer<>(key.type(), key.context());
@@ -68,7 +81,7 @@ public class InstancedDrawManager extends InstancerStorage<InstancedInstancer<?>
 		var meshes = key.model()
 				.meshes();
 		for (var entry : meshes.entrySet()) {
-			var mesh = alloc(entry.getValue());
+			var mesh = meshPool.alloc(entry.getValue());
 
 			ShaderState shaderState = new ShaderState(entry.getKey(), key.type(), key.context());
 			DrawCall drawCall = new DrawCall(instancer, mesh, shaderState);
@@ -111,5 +124,10 @@ public class InstancedDrawManager extends InstancerStorage<InstancedInstancer<?>
 					.entrySet()
 					.iterator();
 		}
+
+		public void prune() {
+            drawCalls.values()
+                    .removeIf(DrawCall::deleted);
+		}
 	}
 }

src/main/java/com/jozufozu/flywheel/backend/engine/instancing/InstancedInstancer.java

@@ -39,14 +39,6 @@ public class InstancedInstancer<I extends Instance> extends AbstractInstancer<I>
 		writer = type.writer();
 	}
 
-	public int getAttributeCount() {
-		return instanceAttributes.size();
-	}
-
-	public boolean isInvalid() {
-		return vbo == null;
-	}
-
 	public void init() {
 		if (vbo != null) {
 			return;
@@ -58,28 +50,22 @@ public class InstancedInstancer<I extends Instance> extends AbstractInstancer<I>
 
 	public void update() {
 		removeDeletedInstances();
-		ensureBufferCapacity();
 		updateBuffer();
 	}
 
-	private void ensureBufferCapacity() {
-		int count = instances.size();
-		int byteSize = instanceStride * count;
-		if (vbo.ensureCapacity(byteSize)) {
-			// The vbo has moved, so we need to re-bind attributes
-			boundTo.clear();
-		}
-	}
-
 	private void updateBuffer() {
 		if (changed.isEmpty() || vbo == null) {
 			return;
 		}
 
-		try (MappedBuffer buf = vbo.map()) {
-			long ptr = buf.ptr();
+		int byteSize = instanceStride * instances.size();
+		if (vbo.ensureCapacity(byteSize)) {
+			// The vbo has moved, so we need to re-bind attributes
+			boundTo.clear();
+		}
 
-			writeChanged(ptr);
+		try (MappedBuffer buf = vbo.map()) {
+            writeChanged(buf.ptr());
 
 			changed.clear();
 		} catch (Exception e) {
@@ -129,6 +115,10 @@ public class InstancedInstancer<I extends Instance> extends AbstractInstancer<I>
 		}
 		vbo.delete();
 		vbo = null;
+
+		for (DrawCall drawCall : drawCalls) {
+			drawCall.delete();
+		}
 	}
 
 	public void addDrawCall(DrawCall drawCall) {

src/main/java/com/jozufozu/flywheel/backend/engine/instancing/InstancedMeshPool.java

@@ -21,11 +21,11 @@ import com.jozufozu.flywheel.backend.gl.buffer.MappedBuffer;
 
 public class InstancedMeshPool {
 	private final VertexView vertexView;
-	private final Map<Mesh, BufferedMesh> meshes = new HashMap<>();
-	private final List<BufferedMesh> allBuffered = new ArrayList<>();
-	private final List<BufferedMesh> pendingUpload = new ArrayList<>();
+	private final Map<Mesh, BufferedMesh> byMesh = new HashMap<>();
+	private final List<BufferedMesh> ordered = new ArrayList<>();
 
 	private final GlBuffer vbo;
+	private final EboCache eboCache;
 	private long byteSize;
 
 	private boolean dirty;
@@ -34,35 +34,34 @@ public class InstancedMeshPool {
 	/**
 	 * Create a new mesh pool.
 	 */
-	public InstancedMeshPool() {
+	public InstancedMeshPool(EboCache eboCache) {
+		this.eboCache = eboCache;
 		vertexView = InternalVertex.createVertexView();
-		int stride = InternalVertex.STRIDE;
-		vbo = new GlBuffer();
-		vbo.growthFunction(l -> Math.max(l + stride * 128L, (long) (l * 1.6)));
+        vbo = new GlBuffer();
+		vbo.growthFunction(l -> Math.max(l + InternalVertex.STRIDE * 128L, (long) (l * 1.6)));
 	}
 
 	/**
 	 * Allocate a mesh in the arena.
 	 *
 	 * @param mesh     The mesh to allocate.
-	 * @param eboCache The EBO cache to use.
 	 * @return A handle to the allocated mesh.
 	 */
-	public BufferedMesh alloc(Mesh mesh, EboCache eboCache) {
-		return meshes.computeIfAbsent(mesh, m -> {
-			BufferedMesh bufferedMesh = new BufferedMesh(m, byteSize, eboCache);
-			byteSize += bufferedMesh.size();
-			allBuffered.add(bufferedMesh);
-			pendingUpload.add(bufferedMesh);
+	public BufferedMesh alloc(Mesh mesh) {
+		return byMesh.computeIfAbsent(mesh, this::_alloc);
+	}
 
-			dirty = true;
-			return bufferedMesh;
-		});
+	private BufferedMesh _alloc(Mesh m) {
+		BufferedMesh bufferedMesh = new BufferedMesh(m, this.eboCache);
+		ordered.add(bufferedMesh);
+
+		dirty = true;
+		return bufferedMesh;
 	}
 
 	@Nullable
 	public BufferedMesh get(Mesh mesh) {
-		return meshes.get(mesh);
+		return byMesh.get(mesh);
 	}
 
 	public void flush() {
@@ -71,53 +70,59 @@ public class InstancedMeshPool {
 		}
 
 		if (anyToRemove) {
+			anyToRemove = false;
 			processDeletions();
 		}
 
-		vbo.ensureCapacity(byteSize);
+		var forUpload = calculateByteSizeAndGetMeshesForUpload();
 
-		uploadPending();
+		if (!forUpload.isEmpty()) {
+			vbo.ensureCapacity(byteSize);
+
+			upload(forUpload);
+		}
 
 		dirty = false;
-		pendingUpload.clear();
 	}
 
 	private void processDeletions() {
 		// remove deleted meshes
-		allBuffered.removeIf(bufferedMesh -> {
-			boolean deleted = bufferedMesh.isDeleted();
+		ordered.removeIf(bufferedMesh -> {
+			boolean deleted = bufferedMesh.deleted();
 			if (deleted) {
-				meshes.remove(bufferedMesh.mesh);
+				byMesh.remove(bufferedMesh.mesh);
 			}
 			return deleted;
 		});
+	}
 
-		// re-evaluate first vertex for each mesh
-		int byteIndex = 0;
-		for (BufferedMesh mesh : allBuffered) {
+	private List<BufferedMesh> calculateByteSizeAndGetMeshesForUpload() {
+		List<BufferedMesh> out = new ArrayList<>();
+
+		long byteIndex = 0;
+		for (BufferedMesh mesh : ordered) {
 			if (mesh.byteIndex != byteIndex) {
-				pendingUpload.add(mesh);
+				out.add(mesh);
 			}
 
 			mesh.byteIndex = byteIndex;
 
-			byteIndex += mesh.size();
+			byteIndex += mesh.byteSize;
 		}
 
 		this.byteSize = byteIndex;
-		this.anyToRemove = false;
+
+		return out;
 	}
 
-	private void uploadPending() {
+	private void upload(List<BufferedMesh> meshes) {
 		try (MappedBuffer mapped = vbo.map()) {
 			long ptr = mapped.ptr();
 
-			for (BufferedMesh mesh : pendingUpload) {
+			for (BufferedMesh mesh : meshes) {
 				mesh.write(ptr, vertexView);
 				mesh.boundTo.clear();
 			}
-
-			pendingUpload.clear();
 		} catch (Exception e) {
 			Flywheel.LOGGER.error("Error uploading pooled meshes:", e);
 		}
@@ -125,14 +130,13 @@ public class InstancedMeshPool {
 
 	public void delete() {
 		vbo.delete();
-		meshes.clear();
-		allBuffered.clear();
-		pendingUpload.clear();
+		byMesh.clear();
+		ordered.clear();
 	}
 
 	@Override
 	public String toString() {
-		return "InstancedMeshPool{" + "byteSize=" + byteSize + ", meshCount=" + meshes.size() + '}';
+		return "InstancedMeshPool{" + "byteSize=" + byteSize + ", meshCount=" + byMesh.size() + '}';
 	}
 
 	public class BufferedMesh {
@@ -141,37 +145,28 @@ public class InstancedMeshPool {
 		private final int byteSize;
 		private final int ebo;
 
-		private long byteIndex;
-		private boolean deleted;
+		private long byteIndex = -1;
+		private int referenceCount = 0;
 
 		private final Set<GlVertexArray> boundTo = new HashSet<>();
 
-		private BufferedMesh(Mesh mesh, long byteIndex, EboCache eboCache) {
+		private BufferedMesh(Mesh mesh, EboCache eboCache) {
 			this.mesh = mesh;
 			vertexCount = mesh.vertexCount();
 			byteSize = vertexCount * InternalVertex.STRIDE;
-			this.byteIndex = byteIndex;
 			this.ebo = eboCache.get(mesh.indexSequence(), mesh.indexCount());
 		}
 
-		public int vertexCount() {
-			return vertexCount;
+		public boolean deleted() {
+			return referenceCount <= 0;
 		}
 
-		public int size() {
-			return byteSize;
-		}
-
-		public boolean isDeleted() {
-			return deleted;
-		}
-
-		public boolean isEmpty() {
-			return mesh.isEmpty() || isDeleted();
+		public boolean invalid() {
+			return mesh.vertexCount() == 0 || deleted() || byteIndex == -1;
 		}
 
 		private void write(long ptr, VertexView vertexView) {
-			if (isEmpty()) {
+			if (invalid()) {
 				return;
 			}
 
@@ -196,10 +191,15 @@ public class InstancedMeshPool {
 			}
 		}
 
-		public void delete() {
-			deleted = true;
-			InstancedMeshPool.this.dirty = true;
-			InstancedMeshPool.this.anyToRemove = true;
+		public void acquire() {
+			referenceCount++;
+		}
+
+		public void drop() {
+			if (--referenceCount == 0) {
+				InstancedMeshPool.this.dirty = true;
+				InstancedMeshPool.this.anyToRemove = true;
+			}
 		}
 	}
 }

src/main/java/com/jozufozu/flywheel/backend/engine/instancing/InstancingEngine.java

@@ -94,8 +94,6 @@ public class InstancingEngine extends AbstractEngine {
 			var shader = entry.getKey();
 			var drawCalls = entry.getValue();
 
-			drawCalls.removeIf(DrawCall::isInvalid);
-
 			if (drawCalls.isEmpty()) {
 				continue;
 			}