State your state

- Address a few bugs with rubidium
 - Ignore EBOs when restoring state
 - ElementBuffer deals in raw gl handles
 - Document all state changes/restores/resets

Co-authored-by: PepperCode1 <44146161+peppercode1@users.noreply.github.com>

src/main/java/com/jozufozu/flywheel/backend/gl/GlStateTracker.java

@@ -39,7 +39,7 @@ public class GlStateTracker {
 		return new State(BUFFERS.clone(), vao, program);
 	}
 
-	public static record State(int[] buffers, int vao, int program) {
+	public record State(int[] buffers, int vao, int program) {
 		public void restore() {
 			if (vao != GlStateTracker.vao) {
 				GlStateManager._glBindVertexArray(vao);
@@ -48,7 +48,7 @@ public class GlStateTracker {
 			GlBufferType[] values = GlBufferType.values();
 
 			for (int i = 0; i < values.length; i++) {
-				if (buffers[i] != GlStateTracker.BUFFERS[i]) {
+				if (buffers[i] != GlStateTracker.BUFFERS[i] && values[i] != GlBufferType.ELEMENT_ARRAY_BUFFER) {
 					GlStateManager._glBindBuffer(values[i].glEnum, buffers[i]);
 				}
 			}

src/main/java/com/jozufozu/flywheel/backend/instancing/instancing/GPUInstancer.java

@@ -45,6 +45,7 @@ public class GPUInstancer<D extends InstanceData> extends AbstractInstancer<D> {
 	public void render() {
 		if (invalid()) return;
 
+		// XXX VAO is bound and not reset or restored
 		vao.bind();
 
 		renderSetup();
@@ -68,12 +69,15 @@ public class GPUInstancer<D extends InstanceData> extends AbstractInstancer<D> {
 
 		vao = new GlVertexArray();
 
+		// XXX Callback seems unnecessary. Remove and extract code to run after alloc call?
 		model = modelAllocator.alloc(modelData, arenaModel -> {
+			// XXX VAO is bound and not reset or restored
 			vao.bind();
 
 			arenaModel.setupState(vao);
 		});
 
+		// XXX VAO is already guaranteed to be bound in model callback
 		vao.bind();
 		vao.enableArrays(model.getAttributeCount() + instanceFormat.getAttributeCount());
 
@@ -108,6 +112,7 @@ public class GPUInstancer<D extends InstanceData> extends AbstractInstancer<D> {
 			removeDeletedInstances();
 		}
 
+		// XXX ARRAY_BUFFER is bound and reset
 		instanceVBO.bind();
 		if (!realloc()) {
 

src/main/java/com/jozufozu/flywheel/backend/instancing/instancing/InstancedMaterialGroup.java

@@ -68,15 +68,23 @@ public class InstancedMaterialGroup<P extends WorldProgram> implements MaterialG
 		return vertexCount;
 	}
 
+	// XXX Overriden in CrumblingGroup
+	// XXX Runs inside of restore state
 	public void render(Matrix4f viewProjection, double camX, double camY, double camZ, RenderLayer layer) {
 		type.setupRenderState();
-		Textures.bindActiveTextures();
-		renderAll(viewProjection, camX, camY, camZ, layer);
+		Textures.bindActiveTextures(); // XXX Changes active unit and bound textures
+		renderAll(viewProjection, camX, camY, camZ, layer); // XXX May change ARRAY_BUFFER binding (reset or not reset), VAO binding (not reset), shader binding (not reset), call Model.createEBO
 		type.clearRenderState();
+		// XXX Should texture bindings be reset or restored?
+		// XXX Should the active unit be reset or restored?
+		// XXX Should the VAO binding be reset or restored?
+		// XXX Should the ARRAY_BUFFER binding be reset or restored?
+		// XXX Should the shader binding be reset or restored?
 	}
 
+	// XXX Internal GL state changes are inconsistent; sometimes bindings are reset to 0, sometimes not
 	protected void renderAll(Matrix4f viewProjection, double camX, double camY, double camZ, RenderLayer layer) {
-		initializeInstancers();
+		initializeInstancers(); // XXX May change ARRAY_BUFFER binding (reset or not reset), VAO binding (not reset), call Model.createEBO
 
 		vertexCount = 0;
 		instanceCount = 0;
@@ -88,6 +96,7 @@ public class InstancedMaterialGroup<P extends WorldProgram> implements MaterialG
 			P program = owner.context.getProgram(ProgramContext.create(entry.getKey()
 					.getProgramSpec(), Formats.POS_TEX_NORMAL, layer));
 
+			// XXX Shader is bound and not reset or restored
 			program.bind();
 			program.uploadViewProjection(viewProjection);
 			program.uploadCameraPos(camX, camY, camZ);
@@ -95,7 +104,7 @@ public class InstancedMaterialGroup<P extends WorldProgram> implements MaterialG
 			setup(program);
 
 			for (GPUInstancer<?> instancer : material.getAllInstancers()) {
-				instancer.render();
+				instancer.render(); // XXX May change VAO binding (not reset), ARRAY_BUFFER binding (reset)
 				vertexCount += instancer.getVertexCount();
 				instanceCount += instancer.getInstanceCount();
 			}
@@ -103,19 +112,19 @@ public class InstancedMaterialGroup<P extends WorldProgram> implements MaterialG
 	}
 
 	private void initializeInstancers() {
-		ModelAllocator allocator = getModelAllocator();
+		ModelAllocator allocator = getModelAllocator(); // XXX May change ARRAY_BUFFER binding (not reset)
 
 		// initialize all uninitialized instancers...
 		for (InstancedMaterial<?> material : materials.values()) {
 			for (GPUInstancer<?> instancer : material.uninitialized) {
-				instancer.init(allocator);
+				instancer.init(allocator); // XXX May change VAO binding (not reset), ARRAY_BUFFER binding (not reset), call Model.createEBO
 			}
 			material.uninitialized.clear();
 		}
 
 		if (allocator instanceof ModelPool pool) {
 			// ...and then flush the model arena in case anything was marked for upload
-			pool.flush();
+			pool.flush(); // XXX May change ARRAY_BUFFER binding (reset)
 		}
 	}
 

src/main/java/com/jozufozu/flywheel/backend/instancing/instancing/InstancingEngine.java

@@ -74,6 +74,7 @@ public class InstancingEngine<P extends WorldProgram> implements Engine {
 	 */
 	@Override
 	public void render(TaskEngine taskEngine, RenderLayerEvent event) {
+		// XXX Restore state
 		GlStateTracker.State restoreState = GlStateTracker.getRestoreState();
 
 		double camX;

src/main/java/com/jozufozu/flywheel/backend/model/ElementBuffer.java

@@ -1,27 +1,29 @@
 package com.jozufozu.flywheel.backend.model;
 
-import com.jozufozu.flywheel.backend.gl.GlNumericType;
-import com.jozufozu.flywheel.backend.gl.buffer.GlBuffer;
-import com.mojang.blaze3d.vertex.VertexBuffer;
+import com.jozufozu.flywheel.backend.gl.buffer.GlBufferType;
 import com.mojang.blaze3d.vertex.VertexFormat;
 
 public class ElementBuffer {
 
-	private final GlBuffer buffer;
-	public final int elementCount;
-	public final VertexFormat.IndexType eboIndexType;
+	protected final int elementCount;
+	protected final VertexFormat.IndexType eboIndexType;
+	private final int glBuffer;
 
-	public ElementBuffer(GlBuffer backing, int elementCount, VertexFormat.IndexType indexType) {
-		this.buffer = backing;
-		this.eboIndexType = indexType;
+	public ElementBuffer(int backing, int elementCount, VertexFormat.IndexType indexType) {
 		this.elementCount = elementCount;
+		this.eboIndexType = indexType;
+		this.glBuffer = backing;
 	}
 
 	public void bind() {
-		buffer.bind();
+		GlBufferType.ELEMENT_ARRAY_BUFFER.bind(glBuffer);
 	}
 
-	public void unbind() {
-		buffer.unbind();
+	public int getElementCount() {
+		return elementCount;
+	}
+
+	public VertexFormat.IndexType getEboIndexType() {
+		return eboIndexType;
 	}
 }

src/main/java/com/jozufozu/flywheel/backend/model/IndexedModel.java

@@ -59,27 +59,26 @@ public class IndexedModel implements BufferedModel {
 	 * The VBO/VAO should be bound externally.
 	 */
 	public void setupState(GlVertexArray vao) {
+		// XXX ARRAY_BUFFER is bound and not reset or restored
 		vbo.bind();
 		vao.enableArrays(getAttributeCount());
 		vao.bindAttributes(0, getType().getLayout());
+		ebo.bind();
 	}
 
 	@Override
 	public void drawCall() {
-		ebo.bind();
-		GL20.glDrawElements(primitiveMode.glEnum, ebo.elementCount, ebo.eboIndexType.asGLType, 0);
+		GL20.glDrawElements(primitiveMode.glEnum, ebo.getElementCount(), ebo.getEboIndexType().asGLType, 0);
 	}
 
 	/**
-		 * Draws many instances of this model, assuming the appropriate state is already bound.
-		 */
+	 * Draws many instances of this model, assuming the appropriate state is already bound.
+	 */
 	@Override
 	public void drawInstances(int instanceCount) {
 		if (!valid()) return;
 
-		ebo.bind();
-
-		GL31.glDrawElementsInstanced(primitiveMode.glEnum, ebo.elementCount, ebo.eboIndexType.asGLType, 0, instanceCount);
+		GL31.glDrawElementsInstanced(primitiveMode.glEnum, ebo.getElementCount(), ebo.getEboIndexType().asGLType, 0, instanceCount);
 	}
 
 	public boolean isDeleted() {

src/main/java/com/jozufozu/flywheel/backend/model/ModelPool.java

@@ -42,6 +42,7 @@ public class ModelPool implements ModelAllocator {
 
 		vbo = new MappedGlBuffer(GlBufferType.ARRAY_BUFFER);
 
+		// XXX ARRAY_BUFFER is bound and not reset or restored
 		vbo.bind();
 		vbo.setGrowthMargin(stride * 64);
 	}
@@ -68,6 +69,7 @@ public class ModelPool implements ModelAllocator {
 		if (dirty) {
 			if (anyToRemove) processDeletions();
 
+			// XXX ARRAY_BUFFER is bound and reset
 			vbo.bind();
 			if (realloc()) {
 				uploadAll();
@@ -182,25 +184,25 @@ public class ModelPool implements ModelAllocator {
 
 		@Override
 		public void setupState(GlVertexArray vao) {
+			// XXX ARRAY_BUFFER is bound and not reset or restored
 			vbo.bind();
 			vao.enableArrays(getAttributeCount());
 			vao.bindAttributes(0, vertexType.getLayout());
+			ebo.bind();
 		}
 
 		@Override
 		public void drawCall() {
-			GL32.glDrawElementsBaseVertex(GlPrimitive.TRIANGLES.glEnum, ebo.elementCount, ebo.eboIndexType.asGLType, 0, first);
+			GL32.glDrawElementsBaseVertex(GlPrimitive.TRIANGLES.glEnum, ebo.getElementCount(), ebo.getEboIndexType().asGLType, 0, first);
 		}
 
 		@Override
 		public void drawInstances(int instanceCount) {
 			if (!valid()) return;
 
-			ebo.bind();
-
 			//Backend.log.info(StringUtil.args("drawElementsInstancedBaseVertex", GlPrimitive.TRIANGLES, ebo.elementCount, ebo.eboIndexType, 0, instanceCount, first));
 
-			GL32.glDrawElementsInstancedBaseVertex(GlPrimitive.TRIANGLES.glEnum, ebo.elementCount, ebo.eboIndexType.asGLType, 0, instanceCount, first);
+			GL32.glDrawElementsInstancedBaseVertex(GlPrimitive.TRIANGLES.glEnum, ebo.getElementCount(), ebo.getEboIndexType().asGLType, 0, instanceCount, first);
 		}
 
 		@Override

src/main/java/com/jozufozu/flywheel/backend/model/VBOModel.java

@@ -58,6 +58,7 @@ public class VBOModel implements BufferedModel {
 	 * The VBO/VAO should be bound externally.
 	 */
 	public void setupState(GlVertexArray vao) {
+		// XXX ARRAY_BUFFER is bound and not reset or restored
 		vbo.bind();
 		vao.enableArrays(getAttributeCount());
 		vao.bindAttributes(0, getLayout());

src/main/java/com/jozufozu/flywheel/core/QuadConverter.java

@@ -1,21 +1,21 @@
 package com.jozufozu.flywheel.core;
 
-import java.nio.Buffer;
-import java.nio.ByteBuffer;
-import java.nio.ByteOrder;
-
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
+import org.lwjgl.opengl.GL32;
+import org.lwjgl.opengl.GL32C;
 import org.lwjgl.system.MemoryUtil;
 
 import com.jozufozu.flywheel.backend.gl.GlNumericType;
 import com.jozufozu.flywheel.backend.gl.buffer.GlBufferType;
-import com.jozufozu.flywheel.backend.gl.buffer.MappedBuffer;
-import com.jozufozu.flywheel.backend.gl.buffer.MappedGlBuffer;
+import com.jozufozu.flywheel.backend.gl.buffer.GlBufferUsage;
 import com.jozufozu.flywheel.backend.model.ElementBuffer;
 import com.jozufozu.flywheel.event.ReloadRenderersEvent;
 import com.mojang.blaze3d.vertex.VertexFormat;
 
+import it.unimi.dsi.fastutil.ints.Int2ReferenceArrayMap;
+import it.unimi.dsi.fastutil.ints.Int2ReferenceMap;
+
 /**
  * A class to manage EBOs that index quads as triangles.
  */
@@ -37,76 +37,74 @@ public class QuadConverter {
 		return INSTANCE;
 	}
 
-	private final MappedGlBuffer ebo;
+	private final Int2ReferenceMap<ElementBuffer> cache = new Int2ReferenceArrayMap<>();
+	private final int ebo;
 	private int quadCapacity;
 
 	public QuadConverter() {
-		this.ebo = new MappedGlBuffer(GlBufferType.ELEMENT_ARRAY_BUFFER);
+		this.ebo = GL32.glGenBuffers();
 		this.quadCapacity = 0;
 	}
 
 	public ElementBuffer quads2Tris(int quads) {
-		int indexCount = quads * 6;
-
 		if (quads > quadCapacity) {
-			ebo.bind();
-			ebo.ensureCapacity((long) indexCount * GlNumericType.UINT.getByteWidth());
-
-			try (MappedBuffer map = ebo.getBuffer()) {
-				ByteBuffer indices = map.unwrap();
-
-				fillBuffer(indices, quads);
-			}
-			ebo.unbind();
-
-			this.quadCapacity = quads;
+			grow(quads * 2);
 		}
 
-		return new ElementBuffer(ebo, indexCount, VertexFormat.IndexType.INT);
+		return cache.computeIfAbsent(quads, this::createElementBuffer);
+	}
+
+	@NotNull
+	private ElementBuffer createElementBuffer(int quads) {
+		return new ElementBuffer(ebo, quads * 6, VertexFormat.IndexType.INT);
+	}
+
+	private void grow(int quads) {
+		int byteSize = quads * 6 * GlNumericType.UINT.getByteWidth();
+		final long ptr = MemoryUtil.nmemAlloc(byteSize);
+
+		fillBuffer(ptr, quads);
+
+		// XXX ARRAY_BUFFER is bound and reset
+		final var bufferType = GlBufferType.ARRAY_BUFFER;
+		final int oldBuffer = bufferType.getBoundBuffer();
+		bufferType.bind(ebo);
+		GL32C.nglBufferData(bufferType.glEnum, byteSize, ptr, GlBufferUsage.STATIC_DRAW.glEnum);
+		bufferType.bind(oldBuffer);
+
+		MemoryUtil.nmemFree(ptr);
+
+		this.quadCapacity = quads;
 	}
 
 	public void delete() {
-		ebo.delete();
+		GL32.glDeleteBuffers(ebo);
 		this.quadCapacity = 0;
 	}
 
-	private void fillBuffer(ByteBuffer indices, int quads) {
-		long addr = MemoryUtil.memAddress(indices);
+	private void fillBuffer(long ptr, int quads) {
 		int numVertices = 4 * quads;
 		int baseVertex = 0;
 		while (baseVertex < numVertices) {
-			// writeQuadIndices(indices, baseVertex);
-			writeQuadIndicesUnsafe(addr, baseVertex);
+			writeQuadIndicesUnsafe(ptr, baseVertex);
 
 			baseVertex += 4;
-			addr += 6 * 4;
+			ptr += 6 * 4;
 		}
-		// ((Buffer) indices).flip();
 	}
 
-	private void writeQuadIndices(ByteBuffer indices, int baseVertex) {
+	private void writeQuadIndicesUnsafe(long ptr, int baseVertex) {
 		// triangle a
-		indices.putInt(baseVertex);
-		indices.putInt(baseVertex + 1);
-		indices.putInt(baseVertex + 2);
+		MemoryUtil.memPutInt(ptr, baseVertex);
+		MemoryUtil.memPutInt(ptr + 4, baseVertex + 1);
+		MemoryUtil.memPutInt(ptr + 8, baseVertex + 2);
 		// triangle b
-		indices.putInt(baseVertex);
-		indices.putInt(baseVertex + 2);
-		indices.putInt(baseVertex + 3);
+		MemoryUtil.memPutInt(ptr + 12, baseVertex);
+		MemoryUtil.memPutInt(ptr + 16, baseVertex + 2);
+		MemoryUtil.memPutInt(ptr + 20, baseVertex + 3);
 	}
 
-	private void writeQuadIndicesUnsafe(long addr, int baseVertex) {
-		// triangle a
-		MemoryUtil.memPutInt(addr, baseVertex);
-		MemoryUtil.memPutInt(addr + 4, baseVertex + 1);
-		MemoryUtil.memPutInt(addr + 8, baseVertex + 2);
-		// triangle b
-		MemoryUtil.memPutInt(addr + 12, baseVertex);
-		MemoryUtil.memPutInt(addr + 16, baseVertex + 2);
-		MemoryUtil.memPutInt(addr + 20, baseVertex + 3);
-	}
-
-	// make sure this gets reset first so it has a chance to repopulate
+	// make sure this gets reset first, so it has a chance to repopulate
 	public static void onRendererReload(ReloadRenderersEvent event) {
 		if (INSTANCE != null) {
 			INSTANCE.delete();

src/main/java/com/jozufozu/flywheel/core/crumbling/CrumblingGroup.java

@@ -18,6 +18,7 @@ public class CrumblingGroup<P extends CrumblingProgram> extends InstancedMateria
 		super(owner, type);
 	}
 
+	// XXX See notes of overriden method
 	@Override
 	public void render(Matrix4f viewProjection, double camX, double camY, double camZ, RenderLayer layer) {
 		type.setupRenderState();

src/main/java/com/jozufozu/flywheel/core/crumbling/CrumblingRenderer.java

@@ -65,6 +65,7 @@ public class CrumblingRenderer {
 
 		Vec3 cameraPos = camera.getPosition();
 
+		// XXX Restore state
 		GlStateTracker.State restoreState = GlStateTracker.getRestoreState();
 		CrumblingRenderer.renderBreaking(activeStages, new RenderLayerEvent(level, null, stack, null, cameraPos.x, cameraPos.y, cameraPos.z));
 		restoreState.restore();
@@ -87,6 +88,7 @@ public class CrumblingRenderer {
 
 				instanceManager.beginFrame(SerialTaskEngine.INSTANCE, info);
 
+				// XXX Each call applies another restore state even though we are already inside of a restore state
 				materials.render(SerialTaskEngine.INSTANCE, event);
 
 				instanceManager.invalidate();
@@ -94,6 +96,9 @@ public class CrumblingRenderer {
 
 		}
 
+		// XXX Inconsistent GL state cleanup
+		// If texture binding and active unit need to be restored, store them in variables before GL state is changed
+		// instead of guessing that unit 0 and crumbling tex 0 are correct
 		GlTextureUnit.T0.makeActive();
 		AbstractTexture breaking = textureManager.getTexture(ModelBakery.BREAKING_LOCATIONS.get(0));
 		if (breaking != null) RenderSystem.bindTexture(breaking.getId());

src/main/java/com/jozufozu/flywheel/core/model/BlockModel.java

@@ -3,13 +3,14 @@ package com.jozufozu.flywheel.core.model;
 import java.nio.ByteBuffer;
 import java.util.function.Supplier;
 
+import org.lwjgl.opengl.GL15;
+import org.lwjgl.opengl.GL32;
 import org.lwjgl.system.MemoryUtil;
 
 import com.jozufozu.flywheel.api.vertex.VertexList;
 import com.jozufozu.flywheel.api.vertex.VertexType;
 import com.jozufozu.flywheel.backend.gl.buffer.GlBufferType;
 import com.jozufozu.flywheel.backend.gl.buffer.GlBufferUsage;
-import com.jozufozu.flywheel.backend.gl.buffer.MappedGlBuffer;
 import com.jozufozu.flywheel.backend.model.ElementBuffer;
 import com.jozufozu.flywheel.core.Formats;
 import com.jozufozu.flywheel.core.QuadConverter;
@@ -64,10 +65,15 @@ public class BlockModel implements Model {
 			ByteBuffer indexBuffer = MemoryTracker.create(src.capacity());
 			MemoryUtil.memCopy(src, indexBuffer);
 			eboSupplier = () -> {
+				int vbo = GL32.glGenBuffers();
 
-				MappedGlBuffer vbo = new MappedGlBuffer(GlBufferType.ELEMENT_ARRAY_BUFFER, GlBufferUsage.STATIC_DRAW);
-
-				vbo.upload(indexBuffer);
+				// XXX ARRAY_BUFFER is bound and restored
+				var bufferType = GlBufferType.ARRAY_BUFFER;
+				var oldBuffer = bufferType.getBoundBuffer();
+				bufferType.bind(vbo);
+				GL15.glBufferData(bufferType.glEnum, indexBuffer, GlBufferUsage.STATIC_DRAW.glEnum);
+				bufferType.bind(oldBuffer);
+				MemoryUtil.memFree(indexBuffer);
 
 				return new ElementBuffer(vbo, drawState.indexCount(), drawState.indexType());
 			};

src/main/java/com/jozufozu/flywheel/core/model/Model.java

@@ -45,6 +45,7 @@ public interface Model {
 		return Formats.POS_TEX_NORMAL;
 	}
 
+	// XXX Since this is public API (technically) we cannot make assumptions about what GL state this method can use or modify unless a contract is established.
 	/**
 	 * Create an element buffer object that indexes the vertices of this model.
 	 *