From a7e709086662ffdec21be7a4e7859b4aa0a33692 Mon Sep 17 00:00:00 2001 From: Jozufozu Date: Mon, 28 Oct 2024 21:53:38 -0700 Subject: [PATCH] Finding an old bookmark - Make AbstractInstancer much more slim and move logic to BaseInstancer - Extend paging concept to the indirect instancer - Extend ObjectStorage to support more interesting layouts - Instance creation on indirect is now entirely lock free and deletions no longer require re-uploading the entire instancer --- .../backend/engine/AbstractInstancer.java | 229 +-------------- .../backend/engine/BaseInstancer.java | 175 +++++++++++ .../backend/engine/InstanceHandleImpl.java | 3 +- .../engine/indirect/IndirectInstancer.java | 273 ++++++++++++++++-- .../engine/indirect/ObjectStorage.java | 27 +- .../engine/instancing/InstancedInstancer.java | 58 +++- .../flywheel/internal/indirect/cull.glsl | 10 +- 7 files changed, 511 insertions(+), 264 deletions(-) create mode 100644 common/src/backend/java/dev/engine_room/flywheel/backend/engine/BaseInstancer.java diff --git a/common/src/backend/java/dev/engine_room/flywheel/backend/engine/AbstractInstancer.java b/common/src/backend/java/dev/engine_room/flywheel/backend/engine/AbstractInstancer.java index 3215d12d2..95bc88e58 100644 --- a/common/src/backend/java/dev/engine_room/flywheel/backend/engine/AbstractInstancer.java +++ b/common/src/backend/java/dev/engine_room/flywheel/backend/engine/AbstractInstancer.java @@ -1,27 +1,14 @@ package dev.engine_room.flywheel.backend.engine; -import java.util.ArrayList; - -import org.jetbrains.annotations.Nullable; - import dev.engine_room.flywheel.api.instance.Instance; import dev.engine_room.flywheel.api.instance.InstanceType; import dev.engine_room.flywheel.api.instance.Instancer; import dev.engine_room.flywheel.backend.engine.embed.Environment; -import dev.engine_room.flywheel.backend.util.AtomicBitSet; -public abstract class AbstractInstancer implements Instancer, InstanceHandleImpl.State { +public abstract class AbstractInstancer implements Instancer { public final InstanceType type; public final Environment environment; - private final Recreate recreate; - - // Lock for all instances, only needs to be used in methods that may run on the TaskExecutor. - protected final Object lock = new Object(); - protected final ArrayList instances = new ArrayList<>(); - protected final ArrayList> handles = new ArrayList<>(); - - protected final AtomicBitSet changed = new AtomicBitSet(); - protected final AtomicBitSet deleted = new AtomicBitSet(); + public final Recreate recreate; protected AbstractInstancer(InstancerKey key, Recreate recreate) { this.type = key.type(); @@ -29,218 +16,16 @@ public abstract class AbstractInstancer implements Instancer this.recreate = recreate; } - @Override - public InstanceHandleImpl.State setChanged(int index) { - notifyDirty(index); - return this; - } + public abstract InstanceHandleImpl.State revealInstance(InstanceHandleImpl handle, I instance); - @Override - public InstanceHandleImpl.State setDeleted(int index) { - notifyRemoval(index); - return InstanceHandleImpl.Deleted.instance(); - } + public abstract int instanceCount(); - @Override - public InstanceHandleImpl.State setVisible(InstanceHandleImpl handle, int index, boolean visible) { - if (visible) { - return this; - } - - notifyRemoval(index); - - 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); - } - - return new InstanceHandleImpl.Hidden<>(recreate, instance); - } - - @Override - public I createInstance() { - var handle = new InstanceHandleImpl<>(this); - I instance = type.create(handle); - - synchronized (lock) { - handle.index = instances.size(); - addLocked(instance, handle); - return instance; - } - } - - public void revealInstance(InstanceHandleImpl handle, I instance) { - synchronized (lock) { - handle.index = instances.size(); - addLocked(instance, handle); - } - } - - @Override - public void stealInstance(@Nullable I instance) { - if (instance == null) { - return; - } - - var instanceHandle = instance.handle(); - - if (!(instanceHandle instanceof InstanceHandleImpl)) { - // UB: do nothing - return; - } - - // Should InstanceType have an isInstance method? - @SuppressWarnings("unchecked") - var handle = (InstanceHandleImpl) instanceHandle; - - // 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 hidden && recreate.equals(hidden.recreate())) { - return; - } - - // FIXME: in theory there could be a race condition here if the instance - // is somehow being stolen by 2 different instancers between threads. - // That seems kinda impossible so I'm fine leaving it as is for now. - - // Add the instance to this instancer. - if (handle.state instanceof AbstractInstancer 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); - } - } else if (handle.state instanceof InstanceHandleImpl.Hidden) { - handle.state = new InstanceHandleImpl.Hidden<>(recreate, instance); - } - } - - /** - * Calls must be synchronized on {@link #lock}. - */ - private void addLocked(I instance, InstanceHandleImpl handle) { - instances.add(instance); - handles.add(handle); - setIndexChanged(handle.index); - } - - public int instanceCount() { - return instances.size(); - } - - public void notifyDirty(int index) { - if (index < 0 || index >= instanceCount()) { - return; - } - setIndexChanged(index); - } - - protected void setIndexChanged(int index) { - changed.set(index); - } - - public void notifyRemoval(int index) { - if (index < 0 || index >= instanceCount()) { - return; - } - deleted.set(index); - } - - public void removeDeletedInstances() { - if (deleted.isEmpty()) { - return; - } - - // Figure out which elements are to be removed. - final int oldSize = this.instances.size(); - int removeCount = deleted.cardinality(); - - if (oldSize == removeCount) { - clear(); - return; - } - - final int newSize = oldSize - removeCount; - - // Start from the first deleted index. - int writePos = deleted.nextSetBit(0); - - if (writePos < newSize) { - // Since we'll be shifting everything into this space we can consider it all changed. - setRangeChanged(writePos, newSize); - } - - // We definitely shouldn't consider the deleted instances as changed though, - // else we might try some out of bounds accesses later. - changed.clear(newSize, oldSize); - - // Punch out the deleted instances, shifting over surviving instances to fill their place. - for (int scanPos = writePos; (scanPos < oldSize) && (writePos < newSize); scanPos++, writePos++) { - // Find next non-deleted element. - scanPos = deleted.nextClearBit(scanPos); - - if (scanPos != writePos) { - // Grab the old instance/handle from scanPos... - var handle = handles.get(scanPos); - I instance = instances.get(scanPos); - - // ... and move it to writePos. - handles.set(writePos, handle); - instances.set(writePos, instance); - - // Make sure the handle knows it's been moved - handle.index = writePos; - } - } - - deleted.clear(); - instances.subList(newSize, oldSize) - .clear(); - handles.subList(newSize, oldSize) - .clear(); - } - - protected void setRangeChanged(int start, int end) { - changed.set(start, end); - } - - /** - * Clear all instances without freeing resources. - */ - public void clear() { - for (InstanceHandleImpl handle : handles) { - // Only clear instances that belong to this instancer. - // If one of these handles was stolen by another 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.clear(); - handle.state = InstanceHandleImpl.Deleted.instance(); - } - } - instances.clear(); - handles.clear(); - changed.clear(); - deleted.clear(); - } + public abstract void removeDeletedInstances(); public abstract void delete(); + public abstract void clear(); + @Override public String toString() { return "AbstractInstancer[" + instanceCount() + ']'; diff --git a/common/src/backend/java/dev/engine_room/flywheel/backend/engine/BaseInstancer.java b/common/src/backend/java/dev/engine_room/flywheel/backend/engine/BaseInstancer.java new file mode 100644 index 000000000..eae6d73cb --- /dev/null +++ b/common/src/backend/java/dev/engine_room/flywheel/backend/engine/BaseInstancer.java @@ -0,0 +1,175 @@ +package dev.engine_room.flywheel.backend.engine; + +import java.util.ArrayList; + +import org.jetbrains.annotations.Nullable; + +import dev.engine_room.flywheel.api.instance.Instance; +import dev.engine_room.flywheel.backend.util.AtomicBitSet; + +public abstract class BaseInstancer extends AbstractInstancer implements InstanceHandleImpl.State { + // Lock for all instances, only needs to be used in methods that may run on the TaskExecutor. + protected final Object lock = new Object(); + protected final ArrayList instances = new ArrayList<>(); + protected final ArrayList> handles = new ArrayList<>(); + + protected final AtomicBitSet changed = new AtomicBitSet(); + protected final AtomicBitSet deleted = new AtomicBitSet(); + + protected BaseInstancer(InstancerKey key, Recreate recreate) { + super(key, recreate); + } + + + @Override + public InstanceHandleImpl.State setChanged(int index) { + notifyDirty(index); + return this; + } + + @Override + public InstanceHandleImpl.State setDeleted(int index) { + notifyRemoval(index); + return InstanceHandleImpl.Deleted.instance(); + } + + @Override + public InstanceHandleImpl.State setVisible(InstanceHandleImpl handle, int index, boolean visible) { + if (visible) { + return this; + } + + notifyRemoval(index); + + 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); + } + + return new InstanceHandleImpl.Hidden<>(recreate, instance); + } + + @Override + public I createInstance() { + var handle = new InstanceHandleImpl<>(this); + I instance = type.create(handle); + + synchronized (lock) { + handle.index = instances.size(); + addLocked(instance, handle); + return instance; + } + } + + public InstanceHandleImpl.State revealInstance(InstanceHandleImpl handle, I instance) { + synchronized (lock) { + handle.index = instances.size(); + addLocked(instance, handle); + } + return this; + } + + @Override + public void stealInstance(@Nullable I instance) { + if (instance == null) { + return; + } + + var instanceHandle = instance.handle(); + + if (!(instanceHandle instanceof InstanceHandleImpl)) { + // UB: do nothing + return; + } + + // Should InstanceType have an isInstance method? + @SuppressWarnings("unchecked") var handle = (InstanceHandleImpl) instanceHandle; + + // 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 hidden && recreate.equals(hidden.recreate())) { + return; + } + + // FIXME: in theory there could be a race condition here if the instance + // is somehow being stolen by 2 different instancers between threads. + // That seems kinda impossible so I'm fine leaving it as is for now. + + // Add the instance to this instancer. + if (handle.state instanceof BaseInstancer 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); + } + } else if (handle.state instanceof InstanceHandleImpl.Hidden) { + handle.state = new InstanceHandleImpl.Hidden<>(recreate, instance); + } + } + + /** + * Calls must be synchronized on {@link #lock}. + */ + private void addLocked(I instance, InstanceHandleImpl handle) { + instances.add(instance); + handles.add(handle); + setIndexChanged(handle.index); + } + + public int instanceCount() { + return instances.size(); + } + + public void notifyDirty(int index) { + if (index < 0 || index >= instanceCount()) { + return; + } + setIndexChanged(index); + } + + protected void setIndexChanged(int index) { + changed.set(index); + } + + public void notifyRemoval(int index) { + if (index < 0 || index >= instanceCount()) { + return; + } + deleted.set(index); + } + + /** + * Clear all instances without freeing resources. + */ + public void clear() { + for (InstanceHandleImpl handle : handles) { + // Only clear instances that belong to this instancer. + // If one of these handles was stolen by another 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.clear(); + handle.state = InstanceHandleImpl.Deleted.instance(); + } + } + instances.clear(); + handles.clear(); + changed.clear(); + deleted.clear(); + } +} diff --git a/common/src/backend/java/dev/engine_room/flywheel/backend/engine/InstanceHandleImpl.java b/common/src/backend/java/dev/engine_room/flywheel/backend/engine/InstanceHandleImpl.java index d5932f45e..445f5dfba 100644 --- a/common/src/backend/java/dev/engine_room/flywheel/backend/engine/InstanceHandleImpl.java +++ b/common/src/backend/java/dev/engine_room/flywheel/backend/engine/InstanceHandleImpl.java @@ -62,8 +62,7 @@ public class InstanceHandleImpl implements InstanceHandle { return this; } var instancer = recreate.recreate(); - instancer.revealInstance(handle, instance); - return instancer; + return instancer.revealInstance(handle, instance); } } diff --git a/common/src/backend/java/dev/engine_room/flywheel/backend/engine/indirect/IndirectInstancer.java b/common/src/backend/java/dev/engine_room/flywheel/backend/engine/indirect/IndirectInstancer.java index ee11999a8..c09a3e9e9 100644 --- a/common/src/backend/java/dev/engine_room/flywheel/backend/engine/indirect/IndirectInstancer.java +++ b/common/src/backend/java/dev/engine_room/flywheel/backend/engine/indirect/IndirectInstancer.java @@ -1,8 +1,12 @@ package dev.engine_room.flywheel.backend.engine.indirect; +import java.lang.reflect.Array; import java.util.ArrayList; import java.util.List; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; +import org.jetbrains.annotations.Nullable; import org.jetbrains.annotations.UnknownNullability; import org.joml.Vector4fc; import org.lwjgl.system.MemoryUtil; @@ -10,17 +14,22 @@ import org.lwjgl.system.MemoryUtil; import dev.engine_room.flywheel.api.instance.Instance; import dev.engine_room.flywheel.api.instance.InstanceWriter; import dev.engine_room.flywheel.backend.engine.AbstractInstancer; +import dev.engine_room.flywheel.backend.engine.InstanceHandleImpl; import dev.engine_room.flywheel.backend.engine.InstancerKey; import dev.engine_room.flywheel.backend.util.AtomicBitSet; import dev.engine_room.flywheel.lib.math.MoreMath; public class IndirectInstancer extends AbstractInstancer { + private final AtomicReference pages; + private final long instanceStride; private final InstanceWriter writer; private final List associatedDraws = new ArrayList<>(); private final Vector4fc boundingSphere; private final AtomicBitSet changedPages = new AtomicBitSet(); + private final AtomicBitSet fullPages = new AtomicBitSet(); + private final Class instanceClass; public ObjectStorage.@UnknownNullability Mapping mapping; @@ -33,18 +42,93 @@ public class IndirectInstancer extends AbstractInstancer .byteSize()); writer = this.type.writer(); boundingSphere = key.model().boundingSphere(); + + instanceClass = (Class) type.create(new InstanceHandleImpl(null)) + .getClass(); + pages = new AtomicReference<>((InstancePage[]) Array.newInstance(InstancePage.class, 0)); } - @Override - public void setIndexChanged(int index) { - changedPages.set(ObjectStorage.objectIndex2PageIndex(index)); - } + public final class InstancePage implements InstanceHandleImpl.State { + private final int pageNo; + private final I[] instances; + private final InstanceHandleImpl[] handles; + /** + * A bitset describing which indices in the instances/handles arrays contain live instances. + */ + private final AtomicInteger valid; - @Override - protected void setRangeChanged(int start, int end) { - super.setRangeChanged(start, end); + InstancePage(Class clazz, int pageNo) { + this.pageNo = pageNo; + this.instances = (I[]) Array.newInstance(clazz, ObjectStorage.PAGE_SIZE); + this.handles = (InstanceHandleImpl[]) new InstanceHandleImpl[ObjectStorage.PAGE_SIZE]; + this.valid = new AtomicInteger(0); + } - changedPages.set(ObjectStorage.objectIndex2PageIndex(start), ObjectStorage.objectIndex2PageIndex(end) + 1); + public boolean add(I instance, InstanceHandleImpl handle) { + // Thread safety: we loop until we either win the race and add the given instance, or we + // run out of space because other threads trying to add at the same time. + while (true) { + int currentValue = valid.get(); + if (currentValue == 0xFFFFFFFF) { + // The page is full, must search elsewhere + return false; + } + + // determine what the new long value will be after we set the appropriate bit. + int index = Integer.numberOfTrailingZeros(~currentValue); + + int newValue = currentValue | (1 << index); + + // if no other thread has modified the value since we read it, we won the race and we are done. + if (valid.compareAndSet(currentValue, newValue)) { + instances[index] = instance; + handles[index] = handle; + handle.state = this; + handle.index = (pageNo << ObjectStorage.LOG_2_PAGE_SIZE) + index; + + changedPages.set(pageNo); + if (newValue == 0xFFFFFFFF) { + fullPages.set(pageNo); + } + return true; + } + } + } + + @Override + public InstanceHandleImpl.State setChanged(int index) { + changedPages.set(pageNo); + return this; + } + + @Override + public InstanceHandleImpl.State setDeleted(int index) { + int localIndex = index % ObjectStorage.PAGE_SIZE; + + instances[localIndex] = null; + handles[localIndex] = null; + + while (true) { + int currentValue = valid.get(); + int newValue = currentValue & ~(1 << localIndex); + + if (valid.compareAndSet(currentValue, newValue)) { + fullPages.clear(pageNo); + return InstanceHandleImpl.Deleted.instance(); + } + } + } + + @Override + public InstanceHandleImpl.State setVisible(InstanceHandleImpl handle, int index, boolean visible) { + if (visible) { + return this; + } + + int localIndex = index % ObjectStorage.PAGE_SIZE; + + return new InstanceHandleImpl.Hidden<>(recreate, instances[localIndex]); + } } public void addDraw(IndirectDraw draw) { @@ -56,9 +140,18 @@ public class IndirectInstancer extends AbstractInstancer } public void update(int modelIndex, int baseInstance) { - this.modelIndex = modelIndex; this.baseInstance = baseInstance; - mapping.update(modelIndex, instanceCount()); + + if (this.modelIndex == modelIndex && changedPages.isEmpty()) { + return; + } + this.modelIndex = modelIndex; + var pages = this.pages.get(); + mapping.updateCount(pages.length); + + for (int i = 0; i < pages.length; i++) { + mapping.updatePage(i, modelIndex, pages[i].valid.get()); + } } public void writeModel(long ptr) { @@ -76,21 +169,12 @@ public class IndirectInstancer extends AbstractInstancer return; } - int numPages = mapping.pageCount(); - - var instanceCount = instances.size(); - - for (int page = changedPages.nextSetBit(0); page >= 0 && page < numPages; page = changedPages.nextSetBit(page + 1)) { - int startObject = ObjectStorage.pageIndex2ObjectIndex(page); - - if (startObject >= instanceCount) { - break; - } - - int endObject = Math.min(instanceCount, ObjectStorage.pageIndex2ObjectIndex(page + 1)); + var pages = this.pages.get(); + for (int page = changedPages.nextSetBit(0); page >= 0 && page < pages.length; page = changedPages.nextSetBit(page + 1)) { + var instances = pages[page].instances; long baseByte = mapping.page2ByteOffset(page); - long size = (endObject - startObject) * instanceStride; + long size = ObjectStorage.PAGE_SIZE * instanceStride; // Because writes are broken into pages, we end up with significantly more calls into // StagingBuffer#enqueueCopy and the allocations for the writer got out of hand. Here @@ -101,9 +185,10 @@ public class IndirectInstancer extends AbstractInstancer long direct = stagingBuffer.reserveForCopy(size, instanceVbo, baseByte); if (direct != MemoryUtil.NULL) { - for (int i = startObject; i < endObject; i++) { - var instance = instances.get(i); - writer.write(direct, instance); + for (I instance : instances) { + if (instance != null) { + writer.write(direct, instance); + } direct += instanceStride; } continue; @@ -112,9 +197,10 @@ public class IndirectInstancer extends AbstractInstancer // Otherwise, write to a scratch buffer and enqueue a copy. var block = stagingBuffer.getScratch(size); var ptr = block.ptr(); - for (int i = startObject; i < endObject; i++) { - var instance = instances.get(i); - writer.write(ptr, instance); + for (I instance : instances) { + if (instance != null) { + writer.write(ptr, instance); + } ptr += instanceStride; } stagingBuffer.enqueueCopy(block.ptr(), size, instanceVbo, baseByte); @@ -123,6 +209,10 @@ public class IndirectInstancer extends AbstractInstancer changedPages.clear(); } + public void removeDeletedInstances() { + + } + @Override public void delete() { for (IndirectDraw draw : draws()) { @@ -143,4 +233,129 @@ public class IndirectInstancer extends AbstractInstancer public int local2GlobalInstanceIndex(int instanceIndex) { return mapping.objectIndex2GlobalIndex(instanceIndex); } + + @Override + public I createInstance() { + var handle = new InstanceHandleImpl(null); + I instance = type.create(handle); + + addInner(instance, handle); + + return instance; + } + + public InstanceHandleImpl.State revealInstance(InstanceHandleImpl handle, I instance) { + addInner(instance, handle); + return handle.state; + } + + @Override + public void stealInstance(@Nullable I instance) { + if (instance == null) { + return; + } + + var instanceHandle = instance.handle(); + + if (!(instanceHandle instanceof InstanceHandleImpl)) { + // UB: do nothing + return; + } + + // Should InstanceType have an isInstance method? + @SuppressWarnings("unchecked") var handle = (InstanceHandleImpl) instanceHandle; + + // 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 hidden && recreate.equals(hidden.recreate())) { + return; + } + + // FIXME: in theory there could be a race condition here if the instance + // is somehow being stolen by 2 different instancers between threads. + // That seems kinda impossible so I'm fine leaving it as is for now. + + // Add the instance to this instancer. + if (handle.state instanceof InstancePage other) { + // TODO: shortcut here if we already own the instance + + // 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.setDeleted(handle.index); + + // Only lock now that we'll be mutating our state. + addInner(instance, handle); + } else if (handle.state instanceof InstanceHandleImpl.Hidden) { + handle.state = new InstanceHandleImpl.Hidden<>(recreate, instance); + } + } + + private void addInner(I instance, InstanceHandleImpl handle) { + // Outer loop: + // - try to find an empty space + // - or grow the page array if we can't + // - add the instance to the new page, or try again + while (true) { + var pages = this.pages.get(); + + // First, try to find a page with space. + for (int i = fullPages.nextClearBit(0); i < pages.length; i = fullPages.nextClearBit(i + 1)) { + // It may have been filled in while we were searching, but hopefully not. + if (pages[i].add(instance, handle)) { + return; + } + } + + // If we're here, all other pages are full + // If we hit this on the second iteration of the outer loop then `pages` is once again full. + var desiredLength = pages.length + 1; + + // Inner loop: grow the page array. This is very similar to the logic in AtomicBitSet. + while (pages.length < desiredLength) { + // Thread safety: segments contains all pages from the currently visible pages, plus extra. + // all pages in the currently visible pages are canonical and will not change. + // Can't just `new InstancePage[]` because it has a generic parameter. + InstancePage[] newPages = (InstancePage[]) Array.newInstance(InstancePage.class, desiredLength); + + System.arraycopy(pages, 0, newPages, 0, pages.length); + newPages[pages.length] = new InstancePage(instanceClass, pages.length); + + // because we are using a compareAndSet, if this thread "wins the race" and successfully sets this variable, then the new page becomes canonical. + if (this.pages.compareAndSet(pages, newPages)) { + pages = newPages; + } else { + // If we "lose the race" and are growing the AtomicBitset segments larger, + // then we will gather the new canonical pages from the update which we missed on the next iteration of this loop. + // The new page will be discarded and never seen again. + pages = this.pages.get(); + } + } + + // Shortcut: try to add the instance to the last page. + // Technically we could just let the outer loop go again, but that + // involves a good bit of work just to likely get back here. + if (pages[pages.length - 1].add(instance, handle)) { + return; + } + // It may be the case that many other instances were added in the same instant. + // We can still lose this race, though it is very unlikely. + } + } + + public int instanceCount() { + // Not exactly accurate but it's an upper bound. + // TODO: maybe this could be tracked with an AtomicInteger? + return pages.get().length << ObjectStorage.LOG_2_PAGE_SIZE; + } + + /** + * Clear all instances without freeing resources. + */ + public void clear() { + + } } diff --git a/common/src/backend/java/dev/engine_room/flywheel/backend/engine/indirect/ObjectStorage.java b/common/src/backend/java/dev/engine_room/flywheel/backend/engine/indirect/ObjectStorage.java index 56d39613e..bf12c46a1 100644 --- a/common/src/backend/java/dev/engine_room/flywheel/backend/engine/indirect/ObjectStorage.java +++ b/common/src/backend/java/dev/engine_room/flywheel/backend/engine/indirect/ObjectStorage.java @@ -14,6 +14,7 @@ public class ObjectStorage extends AbstractArena { public static final int PAGE_MASK = PAGE_SIZE - 1; public static final int INITIAL_PAGES_ALLOCATED = 4; + public static final int DESCRIPTOR_SIZE_BYTES = Integer.BYTES * 2; /** * The GPU side buffer containing all the objects, logically divided into page frames. @@ -37,8 +38,8 @@ public class ObjectStorage extends AbstractArena { this.frameDescriptorBuffer = new ResizableStorageBuffer(); objectBuffer.ensureCapacity(INITIAL_PAGES_ALLOCATED * elementSizeBytes); - frameDescriptorBuffer.ensureCapacity(INITIAL_PAGES_ALLOCATED * Integer.BYTES); - frameDescriptors = MemoryBlock.malloc(INITIAL_PAGES_ALLOCATED * Integer.BYTES); + frameDescriptorBuffer.ensureCapacity(INITIAL_PAGES_ALLOCATED * DESCRIPTOR_SIZE_BYTES); + frameDescriptors = MemoryBlock.malloc(INITIAL_PAGES_ALLOCATED * DESCRIPTOR_SIZE_BYTES); } public Mapping createMapping() { @@ -79,7 +80,7 @@ public class ObjectStorage extends AbstractArena { } private long ptrForPage(int page) { - return frameDescriptors.ptr() + (long) page * Integer.BYTES; + return frameDescriptors.ptr() + (long) page * DESCRIPTOR_SIZE_BYTES; } public static int objectIndex2PageIndex(int objectIndex) { @@ -100,6 +101,14 @@ public class ObjectStorage extends AbstractArena { private int modelIndex = -1; private int objectCount = 0; + public void updatePage(int i, int modelIndex, int i1) { + var ptr = ptrForPage(pages[i]); + MemoryUtil.memPutInt(ptr, modelIndex); + MemoryUtil.memPutInt(ptr + 4, i1); + + ObjectStorage.this.needsUpload = true; + } + /** * Adjust this allocation to the given model index and object count. * @@ -155,6 +164,18 @@ public class ObjectStorage extends AbstractArena { } } + public void updateCount(int newLength) { + var oldLength = pages.length; + if (oldLength > newLength) { + // Eagerly free the now unnecessary pages. + // shrink will zero out the pageTable entries for the freed pages. + shrink(oldLength, newLength); + } else if (oldLength < newLength) { + // Allocate new pages to fit the new object count. + grow(newLength, oldLength); + } + } + public int pageCount() { return pages.length; } diff --git a/common/src/backend/java/dev/engine_room/flywheel/backend/engine/instancing/InstancedInstancer.java b/common/src/backend/java/dev/engine_room/flywheel/backend/engine/instancing/InstancedInstancer.java index 2895195a5..833e90ddf 100644 --- a/common/src/backend/java/dev/engine_room/flywheel/backend/engine/instancing/InstancedInstancer.java +++ b/common/src/backend/java/dev/engine_room/flywheel/backend/engine/instancing/InstancedInstancer.java @@ -7,7 +7,7 @@ import org.jetbrains.annotations.Nullable; import dev.engine_room.flywheel.api.instance.Instance; import dev.engine_room.flywheel.api.instance.InstanceWriter; -import dev.engine_room.flywheel.backend.engine.AbstractInstancer; +import dev.engine_room.flywheel.backend.engine.BaseInstancer; import dev.engine_room.flywheel.backend.engine.InstancerKey; import dev.engine_room.flywheel.backend.gl.TextureBuffer; import dev.engine_room.flywheel.backend.gl.buffer.GlBuffer; @@ -15,7 +15,7 @@ import dev.engine_room.flywheel.backend.gl.buffer.GlBufferUsage; import dev.engine_room.flywheel.lib.math.MoreMath; import dev.engine_room.flywheel.lib.memory.MemoryBlock; -public class InstancedInstancer extends AbstractInstancer { +public class InstancedInstancer extends BaseInstancer { private final int instanceStride; private final InstanceWriter writer; @@ -109,6 +109,60 @@ public class InstancedInstancer extends AbstractInstancer return capacity > vbo.size(); } + public void removeDeletedInstances() { + if (deleted.isEmpty()) { + return; + } + + // Figure out which elements are to be removed. + final int oldSize = this.instances.size(); + int removeCount = deleted.cardinality(); + + if (oldSize == removeCount) { + clear(); + return; + } + + final int newSize = oldSize - removeCount; + + // Start from the first deleted index. + int writePos = deleted.nextSetBit(0); + + if (writePos < newSize) { + // Since we'll be shifting everything into this space we can consider it all changed. + changed.set(writePos, newSize); + } + + // We definitely shouldn't consider the deleted instances as changed though, + // else we might try some out of bounds accesses later. + changed.clear(newSize, oldSize); + + // Punch out the deleted instances, shifting over surviving instances to fill their place. + for (int scanPos = writePos; (scanPos < oldSize) && (writePos < newSize); scanPos++, writePos++) { + // Find next non-deleted element. + scanPos = deleted.nextClearBit(scanPos); + + if (scanPos != writePos) { + // Grab the old instance/handle from scanPos... + var handle = handles.get(scanPos); + I instance = instances.get(scanPos); + + // ... and move it to writePos. + handles.set(writePos, handle); + instances.set(writePos, instance); + + // Make sure the handle knows it's been moved + handle.index = writePos; + } + } + + deleted.clear(); + instances.subList(newSize, oldSize) + .clear(); + handles.subList(newSize, oldSize) + .clear(); + } + public void delete() { if (vbo == null) { return; diff --git a/common/src/backend/resources/assets/flywheel/flywheel/internal/indirect/cull.glsl b/common/src/backend/resources/assets/flywheel/flywheel/internal/indirect/cull.glsl index 395979d5c..1869e33b8 100644 --- a/common/src/backend/resources/assets/flywheel/flywheel/internal/indirect/cull.glsl +++ b/common/src/backend/resources/assets/flywheel/flywheel/internal/indirect/cull.glsl @@ -126,24 +126,22 @@ bool _flw_isVisible(uint instanceIndex, uint modelIndex) { } void main() { - uint pageIndex = gl_WorkGroupID.x; + uint pageIndex = gl_WorkGroupID.x << 1u; if (pageIndex >= _flw_pageFrameDescriptors.length()) { return; } - uint packedModelIndexAndCount = _flw_pageFrameDescriptors[pageIndex]; + uint modelIndex = _flw_pageFrameDescriptors[pageIndex]; - uint pageInstanceCount = packedModelIndexAndCount >> _FLW_PAGE_COUNT_OFFSET; + uint pageValidity = _flw_pageFrameDescriptors[pageIndex + 1]; - if (gl_LocalInvocationID.x >= pageInstanceCount) { + if (((1u << gl_LocalInvocationID.x) & pageValidity) == 0) { return; } uint instanceIndex = gl_GlobalInvocationID.x; - uint modelIndex = packedModelIndexAndCount & _FLW_MODEL_INDEX_MASK; - if (_flw_isVisible(instanceIndex, modelIndex)) { uint localIndex = atomicAdd(_flw_models[modelIndex].instanceCount, 1); uint targetIndex = _flw_models[modelIndex].baseInstance + localIndex;