From 7d59fdc86c416a4615af0e9c68f851973ba51859 Mon Sep 17 00:00:00 2001 From: Jozufozu Date: Mon, 19 Feb 2024 10:53:52 -0600 Subject: [PATCH] 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 --- .../api/instance/InstancerProvider.java | 6 +- .../com/jozufozu/flywheel/api/model/Mesh.java | 8 -- .../backend/engine/InstancerStorage.java | 16 +-- .../engine/indirect/IndirectCullingGroup.java | 2 +- .../engine/indirect/IndirectMeshPool.java | 7 +- .../backend/engine/instancing/DrawCall.java | 36 +++--- .../engine/instancing/InstancedCrumbling.java | 6 +- .../instancing/InstancedDrawManager.java | 34 +++-- .../engine/instancing/InstancedInstancer.java | 32 ++--- .../engine/instancing/InstancedMeshPool.java | 118 +++++++++--------- .../engine/instancing/InstancingEngine.java | 2 - 11 files changed, 132 insertions(+), 135 deletions(-) diff --git a/src/main/java/com/jozufozu/flywheel/api/instance/InstancerProvider.java b/src/main/java/com/jozufozu/flywheel/api/instance/InstancerProvider.java index 5c064355f..4c4ddf92b 100644 --- a/src/main/java/com/jozufozu/flywheel/api/instance/InstancerProvider.java +++ b/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. * - *

Calling this method twice with the same arguments will return the same instancer.

+ *

Calling this method twice with the same arguments in the + * same frame will return the same instancer.

+ * + *

It is not safe to store instancers between frames. Each + * time you need an instancer, you should call this method.

* * @return An instancer for the given instance type rendering the given model. */ diff --git a/src/main/java/com/jozufozu/flywheel/api/model/Mesh.java b/src/main/java/com/jozufozu/flywheel/api/model/Mesh.java index 46f8ba3ef..8b84d5ba5 100644 --- a/src/main/java/com/jozufozu/flywheel/api/model/Mesh.java +++ b/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. diff --git a/src/main/java/com/jozufozu/flywheel/backend/engine/InstancerStorage.java b/src/main/java/com/jozufozu/flywheel/backend/engine/InstancerStorage.java index 66050c00b..aafb42d81 100644 --- a/src/main/java/com/jozufozu/flywheel/backend/engine/InstancerStorage.java +++ b/src/main/java/com/jozufozu/flywheel/backend/engine/InstancerStorage.java @@ -59,23 +59,23 @@ public abstract class InstancerStorage> { 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(InstancerKey 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> { .append(f.toString())); Flywheel.LOGGER.warn(builder.toString()); + + return false; } } diff --git a/src/main/java/com/jozufozu/flywheel/backend/engine/indirect/IndirectCullingGroup.java b/src/main/java/com/jozufozu/flywheel/backend/engine/indirect/IndirectCullingGroup.java index 53f5093e8..730b43fcd 100644 --- a/src/main/java/com/jozufozu/flywheel/backend/engine/indirect/IndirectCullingGroup.java +++ b/src/main/java/com/jozufozu/flywheel/backend/engine/indirect/IndirectCullingGroup.java @@ -83,7 +83,7 @@ public class IndirectCullingGroup { 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); diff --git a/src/main/java/com/jozufozu/flywheel/backend/engine/indirect/IndirectMeshPool.java b/src/main/java/com/jozufozu/flywheel/backend/engine/indirect/IndirectMeshPool.java index 269c0568f..9cf4c3167 100644 --- a/src/main/java/com/jozufozu/flywheel/backend/engine/indirect/IndirectMeshPool.java +++ b/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; diff --git a/src/main/java/com/jozufozu/flywheel/backend/engine/instancing/DrawCall.java b/src/main/java/com/jozufozu/flywheel/backend/engine/instancing/DrawCall.java index 86aa9c8bf..a8801fa20 100644 --- a/src/main/java/com/jozufozu/flywheel/backend/engine/instancing/DrawCall.java +++ b/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; } } diff --git a/src/main/java/com/jozufozu/flywheel/backend/engine/instancing/InstancedCrumbling.java b/src/main/java/com/jozufozu/flywheel/backend/engine/instancing/InstancedCrumbling.java index 62c1629d0..27adecff2 100644 --- a/src/main/java/com/jozufozu/flywheel/backend/engine/instancing/InstancedCrumbling.java +++ b/src/main/java/com/jozufozu/flywheel/backend/engine/instancing/InstancedCrumbling.java @@ -98,11 +98,7 @@ public class InstancedCrumbling { continue; } - List 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)); diff --git a/src/main/java/com/jozufozu/flywheel/backend/engine/instancing/InstancedDrawManager.java b/src/main/java/com/jozufozu/flywheel/backend/engine/instancing/InstancedDrawManager.java index 24abbf9de..f7950aae9 100644 --- a/src/main/java/com/jozufozu/flywheel/backend/engine/instancing/InstancedDrawManager.java +++ b/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 * The set of draw calls to make in each {@link RenderStage}. */ private final Map 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 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 eboCache.invalidate(); } - private InstancedMeshPool.BufferedMesh alloc(Mesh mesh) { - return meshPool.alloc(mesh, eboCache); - } - @Override protected InstancedInstancer create(InstancerKey key) { return new InstancedInstancer<>(key.type(), key.context()); @@ -68,7 +81,7 @@ public class InstancedDrawManager extends InstancerStorage 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 .entrySet() .iterator(); } + + public void prune() { + drawCalls.values() + .removeIf(DrawCall::deleted); + } } } diff --git a/src/main/java/com/jozufozu/flywheel/backend/engine/instancing/InstancedInstancer.java b/src/main/java/com/jozufozu/flywheel/backend/engine/instancing/InstancedInstancer.java index 8dabe219f..99ffbd455 100644 --- a/src/main/java/com/jozufozu/flywheel/backend/engine/instancing/InstancedInstancer.java +++ b/src/main/java/com/jozufozu/flywheel/backend/engine/instancing/InstancedInstancer.java @@ -39,14 +39,6 @@ public class InstancedInstancer extends AbstractInstancer 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 extends AbstractInstancer 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 extends AbstractInstancer } vbo.delete(); vbo = null; + + for (DrawCall drawCall : drawCalls) { + drawCall.delete(); + } } public void addDrawCall(DrawCall drawCall) { diff --git a/src/main/java/com/jozufozu/flywheel/backend/engine/instancing/InstancedMeshPool.java b/src/main/java/com/jozufozu/flywheel/backend/engine/instancing/InstancedMeshPool.java index 3afc5065d..502797a1c 100644 --- a/src/main/java/com/jozufozu/flywheel/backend/engine/instancing/InstancedMeshPool.java +++ b/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 meshes = new HashMap<>(); - private final List allBuffered = new ArrayList<>(); - private final List pendingUpload = new ArrayList<>(); + private final Map byMesh = new HashMap<>(); + private final List 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 calculateByteSizeAndGetMeshesForUpload() { + List 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 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 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; + } } } } diff --git a/src/main/java/com/jozufozu/flywheel/backend/engine/instancing/InstancingEngine.java b/src/main/java/com/jozufozu/flywheel/backend/engine/instancing/InstancingEngine.java index f7596922d..eed31cffc 100644 --- a/src/main/java/com/jozufozu/flywheel/backend/engine/instancing/InstancingEngine.java +++ b/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; }