From 5a75fe972f418dc5b86edc7bae8cffcbca47b2fe Mon Sep 17 00:00:00 2001 From: Jozufozu Date: Wed, 18 Sep 2024 23:37:19 -0700 Subject: [PATCH] First try - Implement instance hiding by deleting/stealing - Work around instancer persistence by storing a recreation supplier in the instance handle - Rework instancer ctors to just take an InstancerKey - Parameterize InstanceHandle by I extends Instance so the steal method and the supplier can be safely assigned --- .../flywheel/api/instance/InstanceHandle.java | 4 ++ .../backend/engine/AbstractInstancer.java | 44 ++++++++++++------- .../flywheel/backend/engine/DrawManager.java | 15 ++++--- .../backend/engine/InstanceHandleImpl.java | 41 ++++++++++++++--- .../engine/indirect/IndirectDrawManager.java | 2 +- .../engine/indirect/IndirectInstancer.java | 11 +++-- .../instancing/InstancedDrawManager.java | 2 +- .../engine/instancing/InstancedInstancer.java | 8 ++-- 8 files changed, 87 insertions(+), 40 deletions(-) diff --git a/common/src/api/java/dev/engine_room/flywheel/api/instance/InstanceHandle.java b/common/src/api/java/dev/engine_room/flywheel/api/instance/InstanceHandle.java index ea2065dd0..3815f5f7e 100644 --- a/common/src/api/java/dev/engine_room/flywheel/api/instance/InstanceHandle.java +++ b/common/src/api/java/dev/engine_room/flywheel/api/instance/InstanceHandle.java @@ -7,4 +7,8 @@ public interface InstanceHandle { void setChanged(); void setDeleted(); + + void setVisible(boolean visible); + + boolean isVisible(); } 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 16aa88b64..8938fe080 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,6 +1,7 @@ package dev.engine_room.flywheel.backend.engine; import java.util.ArrayList; +import java.util.function.Supplier; import org.jetbrains.annotations.Nullable; @@ -13,26 +14,32 @@ public abstract class AbstractInstancer implements Instancer { public final InstanceType type; public final Environment environment; + private final Supplier> 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 ArrayList> handles = new ArrayList<>(); protected final AtomicBitSet changed = new AtomicBitSet(); protected final AtomicBitSet deleted = new AtomicBitSet(); - protected AbstractInstancer(InstanceType type, Environment environment) { - this.type = type; - this.environment = environment; + protected AbstractInstancer(InstancerKey key, Supplier> recreate) { + this.type = key.type(); + this.environment = key.environment(); + this.recreate = recreate; } @Override public I createInstance() { synchronized (lock) { var i = instances.size(); - var handle = new InstanceHandleImpl(this, i); + var handle = new InstanceHandleImpl(); + handle.instancer = this; + handle.recreate = recreate; + handle.index = i; I instance = type.create(handle); + handle.instance = instance; addLocked(instance, handle); return instance; @@ -47,12 +54,15 @@ public void stealInstance(@Nullable I instance) { var instanceHandle = instance.handle(); - if (!(instanceHandle instanceof InstanceHandleImpl handle)) { + if (!(instanceHandle instanceof InstanceHandleImpl)) { // UB: do nothing return; } - if (handle.instancer == this) { + // Should InstanceType have an isInstance method? + var handle = (InstanceHandleImpl) instanceHandle; + + if (handle.instancer == this && handle.visible) { return; } @@ -65,19 +75,23 @@ public void stealInstance(@Nullable I instance) { // is filtering deleted instances later, so is safe. handle.setDeleted(); - // Only lock now that we'll be mutating our state. - synchronized (lock) { - // Add the instance to this instancer. - handle.instancer = this; - handle.index = instances.size(); - addLocked(instance, handle); + // Add the instance to this instancer. + handle.instancer = this; + handle.recreate = recreate; + + if (handle.visible) { + // Only lock now that we'll be mutating our state. + synchronized (lock) { + handle.index = instances.size(); + addLocked(instance, handle); + } } } /** * Calls must be synchronized on {@link #lock}. */ - private void addLocked(I instance, InstanceHandleImpl handle) { + private void addLocked(I instance, InstanceHandleImpl handle) { instances.add(instance); handles.add(handle); changed.set(handle.index); @@ -163,7 +177,7 @@ protected void setRangeChanged(int start, int end) { * Clear all instances without freeing resources. */ public void clear() { - for (InstanceHandleImpl handle : handles) { + 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. diff --git a/common/src/backend/java/dev/engine_room/flywheel/backend/engine/DrawManager.java b/common/src/backend/java/dev/engine_room/flywheel/backend/engine/DrawManager.java index 476a9abc4..993046659 100644 --- a/common/src/backend/java/dev/engine_room/flywheel/backend/engine/DrawManager.java +++ b/common/src/backend/java/dev/engine_room/flywheel/backend/engine/DrawManager.java @@ -11,7 +11,6 @@ import dev.engine_room.flywheel.api.backend.Engine; 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.api.model.Model; import dev.engine_room.flywheel.api.visualization.VisualType; import dev.engine_room.flywheel.backend.FlwBackend; @@ -36,9 +35,13 @@ public abstract class DrawManager> { */ protected final Queue> initializationQueue = new ConcurrentLinkedQueue<>(); + public AbstractInstancer getInstancer(Environment environment, InstanceType type, Model model, VisualType visualType, int bias) { + return getInstancer(new InstancerKey<>(environment, type, model, visualType, bias)); + } + @SuppressWarnings("unchecked") - public Instancer getInstancer(Environment environment, InstanceType type, Model model, VisualType visualType, int bias) { - return (Instancer) instancers.computeIfAbsent(new InstancerKey<>(environment, type, model, visualType, bias), this::createAndDeferInit); + public AbstractInstancer getInstancer(InstancerKey key) { + return (AbstractInstancer) instancers.computeIfAbsent(key, this::createAndDeferInit); } public void flush(LightStorage lightStorage, EnvironmentStorage environmentStorage) { @@ -94,8 +97,8 @@ private static boolean checkAndWarnEmptyModel(Model model) { return false; } - protected static > Map, Int2ObjectMap>>> doCrumblingSort(Class clazz, List crumblingBlocks) { - Map, Int2ObjectMap>>> byType = new HashMap<>(); + protected static > Map, Int2ObjectMap>>>> doCrumblingSort(Class clazz, List crumblingBlocks) { + Map, Int2ObjectMap>>>> byType = new HashMap<>(); for (Engine.CrumblingBlock block : crumblingBlocks) { int progress = block.progress(); @@ -107,7 +110,7 @@ protected static > Map, Int2ObjectMap // Filter out instances that weren't created by this engine. // If all is well, we probably shouldn't take the `continue` // branches but better to do checked casts. - if (!(instance.handle() instanceof InstanceHandleImpl impl)) { + if (!(instance.handle() instanceof InstanceHandleImpl impl)) { continue; } 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 3a1de3200..5641cb96c 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 @@ -1,16 +1,22 @@ 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 implements InstanceHandle { - public AbstractInstancer instancer; +public class InstanceHandleImpl implements InstanceHandle { + @UnknownNullability + public AbstractInstancer instancer; + @UnknownNullability + public I instance; + @UnknownNullability + public Supplier> recreate; + public boolean visible = true; public int index; - public InstanceHandleImpl(AbstractInstancer instancer, int index) { - this.instancer = instancer; - this.index = index; - } - @Override public void setChanged() { instancer.notifyDirty(index); @@ -23,6 +29,27 @@ public void setDeleted() { clear(); } + @Override + public void setVisible(boolean visible) { + if (this.visible == visible) { + return; + } + + this.visible = visible; + + if (visible) { + recreate.get().stealInstance(instance); + } else { + instancer.notifyRemoval(index); + clear(); + } + } + + @Override + public boolean isVisible() { + return visible; + } + public void clear() { index = -1; } diff --git a/common/src/backend/java/dev/engine_room/flywheel/backend/engine/indirect/IndirectDrawManager.java b/common/src/backend/java/dev/engine_room/flywheel/backend/engine/indirect/IndirectDrawManager.java index 99ff35707..cb029600e 100644 --- a/common/src/backend/java/dev/engine_room/flywheel/backend/engine/indirect/IndirectDrawManager.java +++ b/common/src/backend/java/dev/engine_room/flywheel/backend/engine/indirect/IndirectDrawManager.java @@ -66,7 +66,7 @@ public IndirectDrawManager(IndirectPrograms programs) { @Override protected IndirectInstancer create(InstancerKey key) { - return new IndirectInstancer<>(key.type(), key.environment(), key.model()); + return new IndirectInstancer<>(key, () -> getInstancer(key)); } @SuppressWarnings("unchecked") 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 7e3ef62c3..26ada98bb 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 @@ -2,17 +2,16 @@ import java.util.ArrayList; import java.util.List; +import java.util.function.Supplier; import org.jetbrains.annotations.UnknownNullability; import org.joml.Vector4fc; import org.lwjgl.system.MemoryUtil; import dev.engine_room.flywheel.api.instance.Instance; -import dev.engine_room.flywheel.api.instance.InstanceType; import dev.engine_room.flywheel.api.instance.InstanceWriter; -import dev.engine_room.flywheel.api.model.Model; import dev.engine_room.flywheel.backend.engine.AbstractInstancer; -import dev.engine_room.flywheel.backend.engine.embed.Environment; +import dev.engine_room.flywheel.backend.engine.InstancerKey; import dev.engine_room.flywheel.backend.util.AtomicBitSet; import dev.engine_room.flywheel.lib.math.MoreMath; @@ -29,12 +28,12 @@ public class IndirectInstancer extends AbstractInstancer private int modelIndex = -1; private int baseInstance = -1; - public IndirectInstancer(InstanceType type, Environment environment, Model model) { - super(type, environment); + public IndirectInstancer(InstancerKey key, Supplier> recreate) { + super(key, recreate); instanceStride = MoreMath.align4(type.layout() .byteSize()); writer = this.type.writer(); - boundingSphere = model.boundingSphere(); + boundingSphere = key.model().boundingSphere(); } @Override diff --git a/common/src/backend/java/dev/engine_room/flywheel/backend/engine/instancing/InstancedDrawManager.java b/common/src/backend/java/dev/engine_room/flywheel/backend/engine/instancing/InstancedDrawManager.java index 8b241f6c0..f7c55d43f 100644 --- a/common/src/backend/java/dev/engine_room/flywheel/backend/engine/instancing/InstancedDrawManager.java +++ b/common/src/backend/java/dev/engine_room/flywheel/backend/engine/instancing/InstancedDrawManager.java @@ -128,7 +128,7 @@ public void delete() { @Override protected InstancedInstancer create(InstancerKey key) { - return new InstancedInstancer<>(key.type(), key.environment()); + return new InstancedInstancer<>(key, () -> getInstancer(key)); } @Override 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 a5dff27d9..d72117e7d 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 @@ -2,14 +2,14 @@ import java.util.ArrayList; import java.util.List; +import java.util.function.Supplier; 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.InstanceWriter; import dev.engine_room.flywheel.backend.engine.AbstractInstancer; -import dev.engine_room.flywheel.backend.engine.embed.Environment; +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; import dev.engine_room.flywheel.backend.gl.buffer.GlBufferUsage; @@ -25,8 +25,8 @@ public class InstancedInstancer extends AbstractInstancer private final List draws = new ArrayList<>(); - public InstancedInstancer(InstanceType type, Environment environment) { - super(type, environment); + public InstancedInstancer(InstancerKey key, Supplier> recreate) { + super(key, recreate); var layout = type.layout(); // Align to one texel in the texture buffer instanceStride = MoreMath.align16(layout.byteSize());