Little things

- Do not store a list of initialized instancers.
- Remove AbstractInstancer#delete. Only InstancedInstancer was using it
  so move instancer deletion to InstancedDrawManager.
- Improve CompilationHarness builder pattern and reusability.
  - Pass compilation keys directly to compileAndReportErrors.
  - Build the harness at the end of the builder chain rather than at the
    beginning.
  - Use same CompilationHarness for apply shader and scatter shader.
- Remove _ prefix from packed struct fields.
- Make element type's byte size 4-aligned.
- Remove byteSize method from Element.

src/main/java/com/jozufozu/flywheel/api/layout/Layout.java

@@ -24,7 +24,5 @@ public interface Layout {
 		ElementType type();
 
 		int offset();
-
-		int byteSize();
 	}
 }

src/main/java/com/jozufozu/flywheel/backend/compile/CompilationHarness.java

@@ -1,11 +1,11 @@
 package com.jozufozu.flywheel.backend.compile;
 
+import java.util.Collection;
 import java.util.HashMap;
 import java.util.Map;
 
 import org.jetbrains.annotations.Nullable;
 
-import com.google.common.collect.ImmutableList;
 import com.jozufozu.flywheel.Flywheel;
 import com.jozufozu.flywheel.backend.compile.core.CompilerStats;
 import com.jozufozu.flywheel.backend.compile.core.ProgramLinker;
@@ -18,12 +18,9 @@ public class CompilationHarness<K> {
 	private final SourceLoader sourceLoader;
 	private final ShaderCompiler shaderCompiler;
 	private final ProgramLinker programLinker;
-	private final ImmutableList<K> keys;
 	private final CompilerStats stats = new CompilerStats();
 
-	public CompilationHarness(ShaderSources sources, ImmutableList<K> keys, KeyCompiler<K> compiler) {
+	public CompilationHarness(ShaderSources sources, KeyCompiler<K> compiler) {
-		this.keys = keys;
-
 		this.compiler = compiler;
 		sourceLoader = new SourceLoader(sources, stats);
 		shaderCompiler = new ShaderCompiler(stats);
@@ -31,7 +28,7 @@ public class CompilationHarness<K> {
 	}
 
 	@Nullable
-	public Map<K, GlProgram> compileAndReportErrors() {
+	public Map<K, GlProgram> compileAndReportErrors(Collection<K> keys) {
 		stats.start();
 		Map<K, GlProgram> out = new HashMap<>();
 		for (var key : keys) {
@@ -59,28 +56,4 @@ public class CompilationHarness<K> {
 	public interface KeyCompiler<K> {
 		@Nullable GlProgram compile(K key, SourceLoader loader, ShaderCompiler shaderCompiler, ProgramLinker programLinker);
 	}
-
-	public static class Builder<K> {
-		private final ShaderSources sources;
-		private ImmutableList<K> keys;
-		private KeyCompiler<K> compiler;
-
-		public Builder(ShaderSources sources) {
-			this.sources = sources;
-		}
-
-		public Builder<K> keys(ImmutableList<K> keys) {
-			this.keys = keys;
-			return this;
-		}
-
-		public Builder<K> compiler(KeyCompiler<K> compiler) {
-			this.compiler = compiler;
-			return this;
-		}
-
-		public CompilationHarness<K> build() {
-			return new CompilationHarness<>(sources, keys, compiler);
-		}
-	}
 }

src/main/java/com/jozufozu/flywheel/backend/compile/Compile.java

@@ -44,15 +44,15 @@ public class Compile<K> {
 		return new ProgramLinkBuilder<>();
 	}
 
-	public CompilationHarness.Builder<K> harness(ShaderSources sources) {
-		return new CompilationHarness.Builder<>(sources);
-	}
-
 	public static class ProgramLinkBuilder<K> implements CompilationHarness.KeyCompiler<K> {
 		private final Map<ShaderType, ShaderCompilerBuilder<K>> compilers = new EnumMap<>(ShaderType.class);
 		private BiConsumer<K, GlProgram> onLink = (k, p) -> {
 		};
 
+		public CompilationHarness<K> harness(ShaderSources sources) {
+			return new CompilationHarness<>(sources, this);
+		}
+
 		public ProgramLinkBuilder<K> link(ShaderCompilerBuilder<K> compilerBuilder) {
 			if (compilers.containsKey(compilerBuilder.shaderType)) {
 				throw new IllegalArgumentException("Duplicate shader type: " + compilerBuilder.shaderType);

src/main/java/com/jozufozu/flywheel/backend/compile/IndirectPrograms.java

@@ -17,7 +17,6 @@ import com.jozufozu.flywheel.backend.gl.shader.ShaderType;
 import com.jozufozu.flywheel.backend.glsl.GlslVersion;
 import com.jozufozu.flywheel.backend.glsl.ShaderSources;
 import com.jozufozu.flywheel.backend.glsl.SourceComponent;
-import com.jozufozu.flywheel.lib.util.Unit;
 
 import net.minecraft.resources.ResourceLocation;
 
@@ -28,7 +27,7 @@ public class IndirectPrograms {
 
 	public static IndirectPrograms instance;
 	private static final Compile<InstanceType<?>> CULL = new Compile<>();
-	private static final Compile<Unit> UNIT = new Compile<>();
+	private static final Compile<ResourceLocation> UTIL = new Compile<>();
 	private final Map<PipelineProgramKey, GlProgram> pipeline;
 	private final Map<InstanceType<?>, GlProgram> culling;
 	private final GlProgram apply;
@@ -43,19 +42,17 @@ public class IndirectPrograms {
 
 	static void reload(ShaderSources sources, ImmutableList<PipelineProgramKey> pipelineKeys, UniformComponent uniformComponent, List<SourceComponent> vertexComponents, List<SourceComponent> fragmentComponents) {
 		_delete();
-		var pipelineCompiler = PipelineCompiler.create(sources, Pipelines.INDIRECT, pipelineKeys, uniformComponent, vertexComponents, fragmentComponents);
+		var pipelineCompiler = PipelineCompiler.create(sources, Pipelines.INDIRECT, uniformComponent, vertexComponents, fragmentComponents);
 		var cullingCompiler = createCullingCompiler(uniformComponent, sources);
-		var applyCompiler = createApplyCompiler(sources);
+		var applyCompiler = createUtilCompiler(sources);
-		var scatterCompiler = createScatterCompiler(sources);
 
 		try {
-			var pipelineResult = pipelineCompiler.compileAndReportErrors();
+			var pipelineResult = pipelineCompiler.compileAndReportErrors(pipelineKeys);
-			var cullingResult = cullingCompiler.compileAndReportErrors();
+			var cullingResult = cullingCompiler.compileAndReportErrors(createCullingKeys());
-			var applyResult = applyCompiler.compileAndReportErrors();
+			var utils = applyCompiler.compileAndReportErrors(List.of(APPLY_SHADER_MAIN, SCATTER_SHADER_MAIN));
-			var scatterResult = scatterCompiler.compileAndReportErrors();
 
-			if (pipelineResult != null && cullingResult != null && applyResult != null && scatterResult != null) {
+			if (pipelineResult != null && cullingResult != null && utils != null) {
-				instance = new IndirectPrograms(pipelineResult, cullingResult, applyResult.get(Unit.INSTANCE), scatterResult.get(Unit.INSTANCE));
+				instance = new IndirectPrograms(pipelineResult, cullingResult, utils.get(APPLY_SHADER_MAIN), utils.get(SCATTER_SHADER_MAIN));
 			}
 		} catch (Throwable e) {
 			Flywheel.LOGGER.error("Failed to compile indirect programs", e);
@@ -90,37 +87,23 @@ public class IndirectPrograms {
 	}
 
 	private static CompilationHarness<InstanceType<?>> createCullingCompiler(UniformComponent uniformComponent, ShaderSources sources) {
-		return CULL.harness(sources)
+		return CULL.program()
-				.keys(createCullingKeys())
+				.link(CULL.shader(GlslVersion.V460, ShaderType.COMPUTE)
-				.compiler(CULL.program()
+						.define("_FLW_SUBGROUP_SIZE", GlCompat.SUBGROUP_SIZE)
-						.link(CULL.shader(GlslVersion.V460, ShaderType.COMPUTE)
+						.withComponent(uniformComponent)
-								.define("_FLW_SUBGROUP_SIZE", GlCompat.SUBGROUP_SIZE)
+						.withComponent(IndirectComponent::create)
-								.withComponent(uniformComponent)
+						.withResource(InstanceType::cullShader)
-								.withComponent(IndirectComponent::create)
+						.withResource(CULL_SHADER_MAIN))
-								.withResource(InstanceType::cullShader)
+				.then((key, program) -> program.setUniformBlockBinding("FlwUniforms", 0))
-								.withResource(CULL_SHADER_MAIN))
+				.harness(sources);
-						.then((key, program) -> program.setUniformBlockBinding("FlwUniforms", 0)))
-				.build();
 	}
 
-	private static CompilationHarness<Unit> createApplyCompiler(ShaderSources sources) {
+	private static CompilationHarness<ResourceLocation> createUtilCompiler(ShaderSources sources) {
-		return UNIT.harness(sources)
+		return UTIL.program()
-				.keys(ImmutableList.of(Unit.INSTANCE))
+				.link(UTIL.shader(GlslVersion.V460, ShaderType.COMPUTE)
-				.compiler(UNIT.program()
+						.define("_FLW_SUBGROUP_SIZE", GlCompat.SUBGROUP_SIZE)
-						.link(UNIT.shader(GlslVersion.V460, ShaderType.COMPUTE)
+						.withResource(s -> s))
-								.define("_FLW_SUBGROUP_SIZE", GlCompat.SUBGROUP_SIZE)
+				.harness(sources);
-								.withResource(APPLY_SHADER_MAIN)))
-				.build();
-	}
-
-	private static CompilationHarness<Unit> createScatterCompiler(ShaderSources sources) {
-		return UNIT.harness(sources)
-				.keys(ImmutableList.of(Unit.INSTANCE))
-				.compiler(UNIT.program()
-						.link(UNIT.shader(GlslVersion.V460, ShaderType.COMPUTE)
-								.define("_FLW_SUBGROUP_SIZE", GlCompat.SUBGROUP_SIZE)
-								.withResource(SCATTER_SHADER_MAIN)))
-				.build();
 	}
 
 	public GlProgram getIndirectProgram(InstanceType<?> instanceType, Context contextShader) {

src/main/java/com/jozufozu/flywheel/backend/compile/InstancingPrograms.java

@@ -24,10 +24,10 @@ public class InstancingPrograms {
 
 	static void reload(ShaderSources sources, ImmutableList<PipelineProgramKey> pipelineKeys, UniformComponent uniformComponent, List<SourceComponent> vertexComponents, List<SourceComponent> fragmentComponents) {
 		_delete();
-		var instancingCompiler = PipelineCompiler.create(sources, Pipelines.INSTANCED_ARRAYS, pipelineKeys, uniformComponent, vertexComponents, fragmentComponents);
+		var instancingCompiler = PipelineCompiler.create(sources, Pipelines.INSTANCED_ARRAYS, uniformComponent, vertexComponents, fragmentComponents);
 
 		try {
-			var result = instancingCompiler.compileAndReportErrors();
+			var result = instancingCompiler.compileAndReportErrors(pipelineKeys);
 
 			if (result != null) {
 				instance = new InstancingPrograms(result);

src/main/java/com/jozufozu/flywheel/backend/compile/PipelineCompiler.java

@@ -2,7 +2,6 @@ package com.jozufozu.flywheel.backend.compile;
 
 import java.util.List;
 
-import com.google.common.collect.ImmutableList;
 import com.jozufozu.flywheel.backend.InternalVertex;
 import com.jozufozu.flywheel.backend.compile.component.UniformComponent;
 import com.jozufozu.flywheel.backend.gl.shader.ShaderType;
@@ -12,35 +11,33 @@ import com.jozufozu.flywheel.backend.glsl.SourceComponent;
 public class PipelineCompiler {
 	private static final Compile<PipelineProgramKey> PIPELINE = new Compile<>();
 
-	static CompilationHarness<PipelineProgramKey> create(ShaderSources sources, Pipeline pipeline, ImmutableList<PipelineProgramKey> pipelineKeys, UniformComponent uniformComponent, List<SourceComponent> vertexComponents, List<SourceComponent> fragmentComponents) {
+	static CompilationHarness<PipelineProgramKey> create(ShaderSources sources, Pipeline pipeline, UniformComponent uniformComponent, List<SourceComponent> vertexComponents, List<SourceComponent> fragmentComponents) {
-		return PIPELINE.harness(sources)
+		return PIPELINE.program()
-				.keys(pipelineKeys)
+				.link(PIPELINE.shader(pipeline.glslVersion(), ShaderType.VERTEX)
-				.compiler(PIPELINE.program()
+						.withComponent(uniformComponent)
-						.link(PIPELINE.shader(pipeline.glslVersion(), ShaderType.VERTEX)
+						.withResource(pipeline.vertexApiImpl())
-								.withComponent(uniformComponent)
+						.withResource(InternalVertex.LAYOUT_SHADER)
-								.withResource(pipeline.vertexApiImpl())
+						.withComponent(key -> pipeline.assembler()
-								.withResource(InternalVertex.LAYOUT_SHADER)
+								.assemble(new Pipeline.InstanceAssemblerContext(InternalVertex.ATTRIBUTE_COUNT, key.instanceType())))
-								.withComponent(key -> pipeline.assembler()
+						.withComponents(vertexComponents)
-										.assemble(new Pipeline.InstanceAssemblerContext(InternalVertex.ATTRIBUTE_COUNT, key.instanceType())))
+						.withResource(key -> key.instanceType()
-								.withComponents(vertexComponents)
+								.vertexShader())
-								.withResource(key -> key.instanceType()
+						.withResource(key -> key.contextShader()
-										.vertexShader())
+								.vertexShader())
-								.withResource(key -> key.contextShader()
+						.withResource(pipeline.vertexMain()))
-										.vertexShader())
+				.link(PIPELINE.shader(pipeline.glslVersion(), ShaderType.FRAGMENT)
-								.withResource(pipeline.vertexMain()))
+						.enableExtension("GL_ARB_conservative_depth")
-						.link(PIPELINE.shader(pipeline.glslVersion(), ShaderType.FRAGMENT)
+						.withComponent(uniformComponent)
-								.enableExtension("GL_ARB_conservative_depth")
+						.withResource(pipeline.fragmentApiImpl())
-								.withComponent(uniformComponent)
+						.withComponents(fragmentComponents)
-								.withResource(pipeline.fragmentApiImpl())
+						.withResource(key -> key.contextShader()
-								.withComponents(fragmentComponents)
+								.fragmentShader())
-								.withResource(key -> key.contextShader()
+						.withResource(pipeline.fragmentMain()))
-										.fragmentShader())
+				.then((key, program) -> {
-								.withResource(pipeline.fragmentMain()))
+					key.contextShader()
-						.then((key, program) -> {
+							.onProgramLink(program);
-							key.contextShader()
+					program.setUniformBlockBinding("FlwUniforms", 0);
-									.onProgramLink(program);
+				})
-							program.setUniformBlockBinding("FlwUniforms", 0);
+				.harness(sources);
-						}))
-				.build();
 	}
 }

src/main/java/com/jozufozu/flywheel/backend/compile/component/IndirectComponent.java

@@ -113,7 +113,7 @@ public class IndirectComponent implements SourceComponent {
 		// FIXME: I don't think we're unpacking signed byte/short values correctly
 		// FIXME: we definitely don't consider endianness. this all assumes little endian which works on my machine.
 		var type = element.type();
-		var name = "_" + element.name();
+		var name = element.name();
 
 		if (type instanceof ScalarElementType scalar) {
 			return unpackScalar(name, packed, scalar);

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

@@ -150,9 +150,6 @@ public abstract class AbstractInstancer<I extends Instance> implements Instancer
 		deleted.clear();
 	}
 
-	public void delete() {
-	}
-
 	@Override
 	public String toString() {
 		return "AbstractInstancer[" + getInstanceCount() + ']';

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

@@ -20,25 +20,18 @@ public abstract class InstancerStorage<N extends AbstractInstancer<?>> {
 	 * <br>
 	 * See {@link #getInstancer} for insertion details.
 	 */
-	private final Map<InstancerKey<?>, N> instancers = new HashMap<>();
+	protected final Map<InstancerKey<?>, N> instancers = new HashMap<>();
 	/**
 	 * A list of instancers that have not yet been initialized.
 	 * <br>
 	 * All new instancers land here before having resources allocated in {@link #flush}.
 	 * Write access to this list must be synchronized on {@link #creationLock}.
 	 */
-	private final List<UninitializedInstancer<N, ?>> uninitializedInstancers = new ArrayList<>();
+	protected final List<UninitializedInstancer<N, ?>> uninitializedInstancers = new ArrayList<>();
 	/**
 	 * Mutex for {@link #instancers} and {@link #uninitializedInstancers}.
 	 */
-	private final Object creationLock = new Object();
+	protected final Object creationLock = new Object();
-
-	/**
-	 * A list of initialized instancers.
-	 * <br>
-	 * These are instancers that may need to be cleared or deleted.
-	 */
-	private final List<N> initializedInstancers = new ArrayList<>();
 
 	@SuppressWarnings("unchecked")
 	public <I extends Instance> Instancer<I> getInstancer(InstanceType<I> type, Model model, RenderStage stage) {
@@ -69,21 +62,18 @@ public abstract class InstancerStorage<N extends AbstractInstancer<?>> {
 	public void delete() {
 		instancers.clear();
 		uninitializedInstancers.clear();
-
-		initializedInstancers.forEach(AbstractInstancer::delete);
-		initializedInstancers.clear();
 	}
 
 	public void flush() {
 		for (var instancer : uninitializedInstancers) {
 			add(instancer.key(), instancer.instancer(), instancer.model(), instancer.stage());
-			initializedInstancers.add(instancer.instancer());
 		}
 		uninitializedInstancers.clear();
 	}
 
 	public void onRenderOriginChanged() {
-		initializedInstancers.forEach(AbstractInstancer::clear);
+		instancers.values()
+				.forEach(AbstractInstancer::clear);
 	}
 
 	protected abstract <I extends Instance> N create(InstanceType<I> type);

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

@@ -40,6 +40,9 @@ public class InstancedDrawManager extends InstancerStorage<InstancedInstancer<?>
 	}
 
 	public void delete() {
+		instancers.values()
+				.forEach(InstancedInstancer::delete);
+
 		super.delete();
 
 		meshPool.delete();

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

@@ -119,7 +119,6 @@ public class InstancedInstancer<I extends Instance> extends AbstractInstancer<I>
 		vao.bindAttributes(1, startAttrib, instanceAttributes);
 	}
 
-	@Override
 	public void delete() {
 		vbo.delete();
 		vbo = null;

src/main/java/com/jozufozu/flywheel/impl/layout/LayoutBuilderImpl.java

@@ -14,7 +14,6 @@ import com.jozufozu.flywheel.api.layout.Layout.Element;
 import com.jozufozu.flywheel.api.layout.LayoutBuilder;
 import com.jozufozu.flywheel.api.layout.ValueRepr;
 import com.jozufozu.flywheel.impl.layout.LayoutImpl.ElementImpl;
-import com.jozufozu.flywheel.lib.math.MoreMath;
 
 import it.unimi.dsi.fastutil.objects.Object2IntMap;
 import it.unimi.dsi.fastutil.objects.Object2IntOpenHashMap;
@@ -126,9 +125,9 @@ public class LayoutBuilderImpl implements LayoutBuilder {
 	}
 
 	private LayoutBuilder element(String name, ElementType type) {
-		elements.add(new ElementImpl(name, type, offset, MoreMath.align4(type.byteSize())));
+		elements.add(new ElementImpl(name, type, offset));
+		// type.byteSize() is guaranteed to be 4-aligned.
 		offset += type.byteSize();
-		offset = MoreMath.align4(offset);
 		return this;
 	}
 

src/main/java/com/jozufozu/flywheel/impl/layout/LayoutImpl.java

@@ -71,6 +71,6 @@ final class LayoutImpl implements Layout {
 		return elements.equals(other.elements);
 	}
 
-	record ElementImpl(String name, ElementType type, int offset, int byteSize) implements Element {
+	record ElementImpl(String name, ElementType type, int offset) implements Element {
 	}
 }

src/main/java/com/jozufozu/flywheel/impl/layout/MatrixElementTypeImpl.java

@@ -4,6 +4,7 @@ import org.jetbrains.annotations.Range;
 
 import com.jozufozu.flywheel.api.layout.FloatRepr;
 import com.jozufozu.flywheel.api.layout.MatrixElementType;
+import com.jozufozu.flywheel.lib.math.MoreMath;
 
 record MatrixElementTypeImpl(FloatRepr repr, @Range(from = 2, to = 4) int rows, @Range(from = 2, to = 4) int columns,
 							 int byteSize) implements MatrixElementType {
@@ -14,7 +15,7 @@ record MatrixElementTypeImpl(FloatRepr repr, @Range(from = 2, to = 4) int rows,
 		if (columns < 2 || columns > 4) {
 			throw new IllegalArgumentException("Matrix element column count must be in range [2, 4]!");
 		}
-		int byteSize = repr.byteSize() * rows * columns;
+		int byteSize = MoreMath.align4(repr.byteSize() * rows * columns);
 		return new MatrixElementTypeImpl(repr, rows, columns, byteSize);
 	}
 }

src/main/java/com/jozufozu/flywheel/impl/layout/ScalarElementTypeImpl.java

@@ -2,9 +2,10 @@ package com.jozufozu.flywheel.impl.layout;
 
 import com.jozufozu.flywheel.api.layout.ScalarElementType;
 import com.jozufozu.flywheel.api.layout.ValueRepr;
+import com.jozufozu.flywheel.lib.math.MoreMath;
 
 record ScalarElementTypeImpl(ValueRepr repr, int byteSize) implements ScalarElementType {
 	static ScalarElementTypeImpl create(ValueRepr repr) {
-		return new ScalarElementTypeImpl(repr, repr.byteSize());
+		return new ScalarElementTypeImpl(repr, MoreMath.align4(repr.byteSize()));
 	}
 }

src/main/java/com/jozufozu/flywheel/impl/layout/VectorElementTypeImpl.java

@@ -4,6 +4,7 @@ import org.jetbrains.annotations.Range;
 
 import com.jozufozu.flywheel.api.layout.ValueRepr;
 import com.jozufozu.flywheel.api.layout.VectorElementType;
+import com.jozufozu.flywheel.lib.math.MoreMath;
 
 record VectorElementTypeImpl(ValueRepr repr, @Range(from = 2, to = 4) int size,
 							 int byteSize) implements VectorElementType {
@@ -13,7 +14,7 @@ record VectorElementTypeImpl(ValueRepr repr, @Range(from = 2, to = 4) int size,
 			throw new IllegalArgumentException("Vector element size must be in range [2, 4]!");
 		}
 
-		int byteSize = repr.byteSize() * size;
+		int byteSize = MoreMath.align4(repr.byteSize() * size);
 		return new VectorElementTypeImpl(repr, size, byteSize);
 	}
 }

src/main/java/com/jozufozu/flywheel/lib/math/MoreMath.java

@@ -9,12 +9,12 @@ public final class MoreMath {
 	 */
 	public static final float SQRT_3_OVER_2 = (float) (Math.sqrt(3.0) / 2.0);
 
-	public static int align16(int numToRound) {
+	public static int align16(int size) {
-		return (numToRound + 15) & ~15;
+		return (size + 15) & ~15;
 	}
 
-	public static int align4(int offset1) {
+	public static int align4(int size) {
-		return (offset1 + 3) & ~3;
+		return (size + 3) & ~3;
 	}
 
 	public static int ceilingDiv(int numerator, int denominator) {