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 de69900fa..20f06db10 100644 --- a/src/main/java/com/jozufozu/flywheel/backend/engine/InstancerStorage.java +++ b/src/main/java/com/jozufozu/flywheel/backend/engine/InstancerStorage.java @@ -1,9 +1,9 @@ package com.jozufozu.flywheel.backend.engine; import java.util.ArrayList; -import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; import com.jozufozu.flywheel.Flywheel; import com.jozufozu.flywheel.api.event.RenderStage; @@ -17,78 +17,32 @@ public abstract class InstancerStorage> { * A map of instancer keys to instancers. *
* This map is populated as instancers are requested and contains both initialized and uninitialized instancers. - * Write access to this map must be synchronized on {@link #creationLock}. - *
- * See {@link #getInstancer} for insertion details. */ - protected final Map, N> instancers = new HashMap<>(); + protected final Map, N> instancers = new ConcurrentHashMap<>(); /** * A list of instancers that have not yet been initialized. *
* All new instancers land here before having resources allocated in {@link #flush}. - * Write access to this list must be synchronized on {@link #creationLock}. */ - protected final List> uninitializedInstancers = new ArrayList<>(); - /** - * Mutex for {@link #instancers} and {@link #uninitializedInstancers}. - */ - protected final Object creationLock = new Object(); + protected final List> initializationQueue = new ArrayList<>(); @SuppressWarnings("unchecked") public Instancer getInstancer(InstanceType type, Model model, RenderStage stage) { - InstancerKey key = new InstancerKey<>(type, model, stage); - - N instancer = instancers.get(key); - // Happy path: instancer is already initialized. - if (instancer != null) { - return (Instancer) instancer; - } - - // Unhappy path: instancer is not initialized, need to sync to make sure we don't create duplicates. - synchronized (creationLock) { - // Someone else might have initialized it while we were waiting for the lock. - instancer = instancers.get(key); - if (instancer != null) { - return (Instancer) instancer; - } - - maybeWarnEmptyModel(model); - - // Create a new instancer and add it to the uninitialized list. - instancer = create(type); - instancers.put(key, instancer); - uninitializedInstancers.add(new UninitializedInstancer<>(key, instancer, model, stage)); - return (Instancer) instancer; - } - } - - private static void maybeWarnEmptyModel(Model model) { - if (!model.meshes() - .isEmpty()) { - // All is good. - return; - } - - StringBuilder builder = new StringBuilder(); - builder.append("Creating an instancer for a model with no meshes! Stack trace:"); - StackWalker.getInstance() - .walk(s -> s.skip(3)) // skip 3 to get back to the caller of InstancerProvider#instancer - .forEach(f -> builder.append("\n\t") - .append(f.toString())); - - Flywheel.LOGGER.warn(builder.toString()); + return (Instancer) instancers.computeIfAbsent(new InstancerKey<>(type, model, stage), this::createAndDeferInit); } public void delete() { instancers.clear(); - uninitializedInstancers.clear(); + initializationQueue.clear(); } public void flush() { - for (var instancer : uninitializedInstancers) { - add(instancer.key(), instancer.instancer(), instancer.model(), instancer.stage()); + // Thread safety: flush is called from the render thread after all visual updates have been made, + // so there are no:tm: threads we could be racing with. + for (var instancer : initializationQueue) { + initialize(instancer.key(), instancer.instancer()); } - uninitializedInstancers.clear(); + initializationQueue.clear(); } public void onRenderOriginChanged() { @@ -98,8 +52,37 @@ public abstract class InstancerStorage> { protected abstract N create(InstanceType type); - protected abstract void add(InstancerKey key, N instancer, Model model, RenderStage stage); + protected abstract void initialize(InstancerKey key, N instancer); - private record UninitializedInstancer(InstancerKey key, N instancer, Model model, RenderStage stage) { + private N createAndDeferInit(InstancerKey key) { + var out = create(key.type()); + + // Only queue the instancer for initialization if it has anything to render. + if (key.model() + .meshes() + .isEmpty()) { + warnEmptyModel(); + } else { + // 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; + } + + protected record UninitializedInstancer(InstancerKey key, N instancer) { + + } + + private static void warnEmptyModel() { + StringBuilder builder = new StringBuilder(); + builder.append("Creating an instancer for a model with no meshes! Stack trace:"); + + StackWalker.getInstance() + // .walk(s -> s.skip(3)) // this causes forEach to crash for some reason + .forEach(f -> builder.append("\n\t") + .append(f.toString())); + + Flywheel.LOGGER.warn(builder.toString()); } } diff --git a/src/main/java/com/jozufozu/flywheel/backend/engine/indirect/IndirectDrawManager.java b/src/main/java/com/jozufozu/flywheel/backend/engine/indirect/IndirectDrawManager.java index 1f92f8976..cd7c75795 100644 --- a/src/main/java/com/jozufozu/flywheel/backend/engine/indirect/IndirectDrawManager.java +++ b/src/main/java/com/jozufozu/flywheel/backend/engine/indirect/IndirectDrawManager.java @@ -17,7 +17,6 @@ import com.jozufozu.flywheel.api.backend.Engine; import com.jozufozu.flywheel.api.event.RenderStage; import com.jozufozu.flywheel.api.instance.Instance; import com.jozufozu.flywheel.api.instance.InstanceType; -import com.jozufozu.flywheel.api.model.Model; import com.jozufozu.flywheel.backend.compile.IndirectPrograms; import com.jozufozu.flywheel.backend.engine.CommonCrumbling; import com.jozufozu.flywheel.backend.engine.InstanceHandleImpl; @@ -52,15 +51,9 @@ public class IndirectDrawManager extends InstancerStorage> @SuppressWarnings("unchecked") @Override - protected void add(InstancerKey key, IndirectInstancer instancer, Model model, RenderStage stage) { - if (model.meshes() - .isEmpty()) { - // Don't bother allocating resources for models with no meshes. - return; - } - + protected void initialize(InstancerKey key, IndirectInstancer instancer) { var group = (IndirectCullingGroup) cullingGroups.computeIfAbsent(key.type(), t -> new IndirectCullingGroup<>(t, programs)); - group.add((IndirectInstancer) instancer, model, stage); + group.add((IndirectInstancer) instancer, key.model(), key.stage()); } public boolean hasStage(RenderStage stage) { 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 9901ab354..a0ea3f517 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 @@ -14,7 +14,6 @@ import com.jozufozu.flywheel.api.event.RenderStage; import com.jozufozu.flywheel.api.instance.Instance; import com.jozufozu.flywheel.api.instance.InstanceType; import com.jozufozu.flywheel.api.model.Mesh; -import com.jozufozu.flywheel.api.model.Model; import com.jozufozu.flywheel.backend.engine.InstancerKey; import com.jozufozu.flywheel.backend.engine.InstancerStorage; @@ -64,18 +63,13 @@ public class InstancedDrawManager extends InstancerStorage } @Override - protected void add(InstancerKey key, InstancedInstancer instancer, Model model, RenderStage stage) { - if (model.meshes() - .isEmpty()) { - // Don't bother allocating resources for models with no meshes. - return; - } - + protected void initialize(InstancerKey key, InstancedInstancer instancer) { instancer.init(); - DrawSet drawSet = drawSets.computeIfAbsent(stage, DrawSet::new); + DrawSet drawSet = drawSets.computeIfAbsent(key.stage(), DrawSet::new); - var meshes = model.meshes(); + var meshes = key.model() + .meshes(); for (var entry : meshes.entrySet()) { var mesh = alloc(entry.getValue());