More sane models

- BufferedModel is no longer abstract.
 - InstancedModel no longer inherits from BufferedModel, it accepts one as input.
 - Replace usage of IndexedModel with BufferedModel

src/main/java/com/jozufozu/flywheel/backend/core/BufferedModel.java

@@ -1,90 +1,98 @@
 package com.jozufozu.flywheel.backend.core;
 
+import static org.lwjgl.opengl.GL20.GL_COLOR_ARRAY;
+import static org.lwjgl.opengl.GL20.GL_INDEX_ARRAY;
+import static org.lwjgl.opengl.GL20.GL_NORMAL_ARRAY;
+import static org.lwjgl.opengl.GL20.GL_QUADS;
+import static org.lwjgl.opengl.GL20.GL_TEXTURE_COORD_ARRAY;
+import static org.lwjgl.opengl.GL20.GL_VERTEX_ARRAY;
+import static org.lwjgl.opengl.GL20.glDisableClientState;
+import static org.lwjgl.opengl.GL20.glDrawArrays;
+
 import java.nio.ByteBuffer;
 
-import org.lwjgl.opengl.GL20;
-
-import com.jozufozu.flywheel.backend.RenderWork;
 import com.jozufozu.flywheel.backend.gl.attrib.VertexFormat;
 import com.jozufozu.flywheel.backend.gl.buffer.GlBuffer;
 import com.jozufozu.flywheel.backend.gl.buffer.GlBufferType;
-import com.jozufozu.flywheel.backend.gl.buffer.MappedBuffer;
+import com.jozufozu.flywheel.util.AttribUtil;
 
-public abstract class BufferedModel {
+public class BufferedModel {
 
 	protected final ByteBuffer data;
-	protected final VertexFormat modelFormat;
+	protected final VertexFormat format;
 	protected final int vertexCount;
-	protected GlBuffer modelVBO;
+	protected GlBuffer vbo;
-	private boolean initialized; // lazy init
 	private boolean removed;
 
-	protected BufferedModel(VertexFormat modelFormat, ByteBuffer data, int vertices) {
+	public BufferedModel(VertexFormat format, ByteBuffer data, int vertices) {
 		this.data = data;
-		this.modelFormat = modelFormat;
+		this.format = format;
 		this.vertexCount = vertices;
+
+		vbo = new GlBuffer(GlBufferType.ARRAY_BUFFER);
+
+		vbo.bind();
+		// allocate the buffer on the gpu
+		vbo.alloc(this.data.capacity());
+
+		// mirror it in system memory so we can write to it, and upload our model.
+		vbo.getBuffer(0, this.data.capacity())
+				.put(this.data)
+				.flush();
+		vbo.unbind();
+	}
+
+	public VertexFormat getFormat() {
+		return format;
+	}
+
+	public int getVertexCount() {
+		return vertexCount;
+	}
+
+	public void bindBuffer() {
+		vbo.bind();
+	}
+
+	public void unbindBuffer() {
+		vbo.unbind();
 	}
 
 	/**
 	 * Renders this model, checking first if there is anything to render.
 	 */
-	public final void render() {
+	public void render() {
 		if (vertexCount <= 0 || removed) return;
 
-		if (!initialized) {
+		// TODO: minecraft sometimes leaves its state dirty on launch. this is a hack
-			// Lazily acquire resources in order to get around initialization order, as #getTotalShaderAttributeCount
+		glDisableClientState(GL_VERTEX_ARRAY);
-			// might depend on fields in subclasses.
+		glDisableClientState(GL_NORMAL_ARRAY);
-			init();
+		glDisableClientState(GL_COLOR_ARRAY);
-			initialized = true;
+		glDisableClientState(GL_INDEX_ARRAY);
+		glDisableClientState(GL_TEXTURE_COORD_ARRAY);
+
+		vbo.bind();
+
+		AttribUtil.enableArrays(getAttributeCount());
+		format.vertexAttribPointers(0);
+
+		glDrawArrays(GL_QUADS, 0, vertexCount);
+
+		AttribUtil.disableArrays(getAttributeCount());
+
+		vbo.unbind();
 	}
 
-		doRender();
+	public void delete() {
-	}
+		if (removed) return;
 
-	/**
-	 * Set up any state and make the draw calls.
-	 */
-	protected abstract void doRender();
-
-	public final void delete() {
 		removed = true;
-		if (initialized) {
+		vbo.delete();
-			RenderWork.enqueue(this::deleteInternal);
-		}
 	}
 
-	protected void init() {
+	public int getAttributeCount() {
-		modelVBO = new GlBuffer(GlBufferType.ARRAY_BUFFER);
+		return format.getAttributeCount();
-
-		modelVBO.bind();
-		initModel();
-		modelVBO.unbind();
 	}
 
-	protected void initModel() {
-		// allocate the buffer on the gpu
-		modelVBO.alloc(data.capacity());
-
-		// mirror it in system memory so we can write to it
-		MappedBuffer buffer = modelVBO.getBuffer(0, data.capacity());
-		buffer.put(data);
-		buffer.flush();
-	}
-
-	protected int getTotalShaderAttributeCount() {
-		return modelFormat.getShaderAttributeCount();
-	}
-
-	protected void setupAttributes() {
-		int numAttributes = getTotalShaderAttributeCount();
-		for (int i = 0; i <= numAttributes; i++) {
-			GL20.glEnableVertexAttribArray(i);
-		}
-
-		modelFormat.vertexAttribPointers(0);
-	}
-
-	protected void deleteInternal() {
-		modelVBO.delete();
-	}
 }
+

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

@@ -8,62 +8,49 @@ import com.jozufozu.flywheel.backend.gl.GlPrimitiveType;
 import com.jozufozu.flywheel.backend.gl.attrib.VertexFormat;
 import com.jozufozu.flywheel.backend.gl.buffer.GlBuffer;
 import com.jozufozu.flywheel.backend.gl.buffer.GlBufferType;
-import com.jozufozu.flywheel.backend.gl.buffer.MappedBuffer;
+import com.jozufozu.flywheel.util.AttribUtil;
 
 public class IndexedModel extends BufferedModel {
 
 	protected GlPrimitiveType eboIndexType;
 	protected GlBuffer ebo;
 
-	public IndexedModel(VertexFormat modelFormat, ByteBuffer buf, int vertices) {
+	public IndexedModel(VertexFormat modelFormat, ByteBuffer buf, int vertices, ByteBuffer indices, GlPrimitiveType indexType) {
 		super(modelFormat, buf, vertices);
-	}
 
-	@Override
-	protected void init() {
-		super.init();
-
-		createEBO();
-	}
-
-	@Override
-    protected void doRender() {
-        modelVBO.bind();
-        ebo.bind();
-
-        setupAttributes();
-        GL20.glDrawElements(GL20.GL_QUADS, vertexCount, eboIndexType.getGlConstant(), 0);
-
-        int numAttributes = getTotalShaderAttributeCount();
-        for (int i = 0; i <= numAttributes; i++) {
-            GL20.glDisableVertexAttribArray(i);
-        }
-
-        ebo.unbind();
-        modelVBO.unbind();
-    }
-
-    protected final void createEBO() {
 		ebo = new GlBuffer(GlBufferType.ELEMENT_ARRAY_BUFFER);
-		eboIndexType = GlPrimitiveType.UINT; // TODO: choose this based on the number of vertices
+		this.eboIndexType = indexType;
 
-		int indicesSize = vertexCount * eboIndexType.getSize();
+		int indicesSize = vertexCount * indexType.getSize();
 
 		ebo.bind();
 
 		ebo.alloc(indicesSize);
-		MappedBuffer indices = ebo.getBuffer(0, indicesSize);
+		ebo.getBuffer(0, indicesSize)
-		for (int i = 0; i < vertexCount; i++) {
+				.put(indices)
-			indices.putInt(i);
+				.flush();
-		}
-		indices.flush();
 
 		ebo.unbind();
 	}
 
+	public void render() {
+		vbo.bind();
+		ebo.bind();
+
+		AttribUtil.enableArrays(getAttributeCount());
+		format.vertexAttribPointers(0);
+
+		GL20.glDrawElements(GL20.GL_QUADS, vertexCount, eboIndexType.getGlConstant(), 0);
+
+		AttribUtil.disableArrays(getAttributeCount());
+
+		ebo.unbind();
+		vbo.unbind();
+	}
+
 	@Override
-    protected void deleteInternal() {
+	public void delete() {
-        super.deleteInternal();
+		super.delete();
 		ebo.delete();
 	}
 }

src/main/java/com/jozufozu/flywheel/backend/gl/attrib/VertexFormat.java

@@ -22,7 +22,7 @@ public class VertexFormat {
 		this.stride = stride;
 	}
 
-	public int getShaderAttributeCount() {
+	public int getAttributeCount() {
 		return numAttributes;
 	}
 

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

@@ -1,7 +1,6 @@
 package com.jozufozu.flywheel.backend.instancing;
 
 
-import java.nio.ByteBuffer;
 import java.util.ArrayList;
 import java.util.BitSet;
 
@@ -14,31 +13,64 @@ import com.jozufozu.flywheel.backend.gl.attrib.VertexFormat;
 import com.jozufozu.flywheel.backend.gl.buffer.GlBuffer;
 import com.jozufozu.flywheel.backend.gl.buffer.GlBufferType;
 import com.jozufozu.flywheel.backend.gl.buffer.MappedBuffer;
+import com.jozufozu.flywheel.util.AttribUtil;
 
-public class InstancedModel<D extends InstanceData> extends BufferedModel {
+public class InstancedModel<D extends InstanceData> {
 
 	public final InstancedTileRenderer<?> renderer;
 
+	protected final BufferedModel model;
+
 	protected final VertexFormat instanceFormat;
 	protected final InstanceFactory<D> factory;
 	protected GlVertexArray vao;
 	protected GlBuffer instanceVBO;
 	protected int glBufferSize = -1;
 	protected int glInstanceCount = 0;
+	private boolean deleted;
 
 	protected final ArrayList<D> data = new ArrayList<>();
 
 	boolean anyToRemove;
 	boolean anyToUpdate;
 
-	public InstancedModel(VertexFormat modelFormat, ByteBuffer buf, int vertices, InstancedTileRenderer<?> renderer, VertexFormat instanceFormat, InstanceFactory<D> factory) {
+	public InstancedModel(BufferedModel model, InstancedTileRenderer<?> renderer, VertexFormat instanceFormat, InstanceFactory<D> factory) {
-		super(modelFormat, buf, vertices);
+		this.model = model;
 		this.factory = factory;
 		this.instanceFormat = instanceFormat;
 		this.renderer = renderer;
+
+		if (model.getVertexCount() <= 0)
+			throw new IllegalArgumentException("Refusing to instance a model with no vertices.");
+
+		vao = new GlVertexArray();
+		instanceVBO = new GlBuffer(GlBufferType.ARRAY_BUFFER);
+
+		vao.bind();
+
+		// bind the model's vbo to our vao
+		model.bindBuffer();
+		model.getFormat().vertexAttribPointers(0);
+		model.unbindBuffer();
+
+		// enable all the attribute arrays in our vao. we only need to do this once
+		AttribUtil.enableArrays(model.getAttributeCount() + this.instanceFormat.getAttributeCount());
+		vao.unbind();
 	}
 
-	public synchronized D createInstance() {
+	public void render() {
+		if (deleted) return;
+
+		vao.bind();
+		renderSetup();
+
+		if (glInstanceCount > 0)
+			Backend.compat.drawInstanced.drawArraysInstanced(GL11.GL_QUADS, 0, model.getVertexCount(), glInstanceCount);
+
+		vao.unbind();
+	}
+
+	public D createInstance() {
 		D instanceData = factory.create(this);
 		instanceData.dirty = true;
 		anyToUpdate = true;
@@ -47,39 +79,17 @@ public class InstancedModel<D extends InstanceData> extends BufferedModel {
 		return instanceData;
 	}
 
-	@Override
+	public void delete() {
-	protected void init() {
+		if (deleted) return;
-		vao = new GlVertexArray();
-		instanceVBO = new GlBuffer(GlBufferType.ARRAY_BUFFER);
 
-		vao.bind();
+		deleted = true;
-		super.init();
-		vao.unbind();
-	}
 
-	@Override
+		model.delete();
-	protected void initModel() {
-		super.initModel();
-		setupAttributes();
-	}
-
-	protected void deleteInternal() {
-		super.deleteInternal();
 
 		instanceVBO.delete();
 		vao.delete();
 	}
 
-	protected void doRender() {
-		vao.bind();
-		renderSetup();
-
-		if (glInstanceCount > 0)
-			Backend.compat.drawInstanced.drawArraysInstanced(GL11.GL_QUADS, 0, vertexCount, glInstanceCount);
-
-		vao.unbind();
-	}
-
 	protected void renderSetup() {
 		if (anyToRemove) {
 			removeDeletedInstances();
@@ -96,23 +106,12 @@ public class InstancedModel<D extends InstanceData> extends BufferedModel {
 				updateBuffer();
 			}
 
+			glInstanceCount = data.size();
 		}
 
-		glInstanceCount = data.size();
-		informAttribDivisors();
 		instanceVBO.unbind();
 
-		this.anyToRemove = false;
+		anyToRemove = anyToUpdate = false;
-		this.anyToUpdate = false;
-	}
-
-	private void informAttribDivisors() {
-		int staticAttributes = modelFormat.getShaderAttributeCount();
-		instanceFormat.vertexAttribPointers(staticAttributes);
-
-		for (int i = 0; i < instanceFormat.getShaderAttributeCount(); i++) {
-			Backend.compat.instancedArrays.vertexAttribDivisor(i + staticAttributes, 1);
-		}
 	}
 
 	private void clearBufferTail() {
@@ -185,6 +184,9 @@ public class InstancedModel<D extends InstanceData> extends BufferedModel {
 			buffer.flush();
 
 			glInstanceCount = size;
+
+			informAttribDivisors();
+
 			return true;
 		}
 		return false;
@@ -222,7 +224,13 @@ public class InstancedModel<D extends InstanceData> extends BufferedModel {
 
 	}
 
-	protected int getTotalShaderAttributeCount() {
+	private void informAttribDivisors() {
-		return instanceFormat.getShaderAttributeCount() + super.getTotalShaderAttributeCount();
+		int staticAttributes = model.getAttributeCount();
+		instanceFormat.vertexAttribPointers(staticAttributes);
+
+		for (int i = 0; i < instanceFormat.getAttributeCount(); i++) {
+			Backend.compat.instancedArrays.vertexAttribDivisor(i + staticAttributes, 1);
 		}
+	}
+
 }

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

@@ -12,6 +12,8 @@ import org.lwjgl.opengl.GL11;
 
 import com.google.common.cache.Cache;
 import com.google.common.cache.CacheBuilder;
+import com.jozufozu.flywheel.backend.RenderWork;
+import com.jozufozu.flywheel.backend.core.BufferedModel;
 import com.jozufozu.flywheel.backend.core.PartialModel;
 import com.jozufozu.flywheel.backend.core.shader.ShaderCallback;
 import com.jozufozu.flywheel.backend.core.shader.WorldProgram;
@@ -45,7 +47,10 @@ public class RenderMaterial<P extends WorldProgram, D extends InstanceData> {
 		this.spec = spec;
 
 		this.models = CacheBuilder.newBuilder()
-				.removalListener(notification -> ((InstancedModel<?>) notification.getValue()).delete())
+				.removalListener(notification -> {
+					InstancedModel<?> model = (InstancedModel<?>) notification.getValue();
+					RenderWork.enqueue(model::delete);
+				})
 				.build();
 	}
 
@@ -140,7 +145,9 @@ public class RenderMaterial<P extends WorldProgram, D extends InstanceData> {
 
 		to.rewind();
 
-		return new InstancedModel<>(format, to, vertexCount, renderer, spec.getInstanceFormat(), spec.getInstanceFactory());
+		BufferedModel bufferedModel = new BufferedModel(format, to, vertexCount);
+
+		return new InstancedModel<>(bufferedModel, renderer, spec.getInstanceFormat(), spec.getInstanceFactory());
 	}
 
 	private static final Direction[] dirs;

src/main/java/com/jozufozu/flywheel/util/AttribUtil.java

@@ -0,0 +1,26 @@
+package com.jozufozu.flywheel.util;
+
+import org.lwjgl.opengl.GL20;
+
+public class AttribUtil {
+
+	public static void enableArrays(int count) {
+		enableArrays(0, count);
+	}
+
+	public static void enableArrays(int start, int end) {
+		for (int i = start; i <= end; i++) {
+			GL20.glEnableVertexAttribArray(i);
+		}
+	}
+
+	public static void disableArrays(int count) {
+		disableArrays(0, count);
+	}
+
+	public static void disableArrays(int start, int end) {
+		for (int i = start; i <= end; i++) {
+			GL20.glDisableVertexAttribArray(i);
+		}
+	}
+}

src/main/java/com/simibubi/create/content/contraptions/components/structureMovement/render/RenderedContraption.java

@@ -10,7 +10,7 @@ import java.util.Map;
 import javax.annotation.Nullable;
 
 import com.jozufozu.flywheel.backend.Backend;
-import com.jozufozu.flywheel.backend.core.IndexedModel;
+import com.jozufozu.flywheel.backend.core.BufferedModel;
 import com.jozufozu.flywheel.backend.gl.attrib.CommonAttributes;
 import com.jozufozu.flywheel.backend.gl.attrib.VertexFormat;
 import com.jozufozu.flywheel.backend.instancing.IInstanceRendered;
@@ -45,7 +45,7 @@ public class RenderedContraption extends ContraptionWorldHolder {
 	private final ContraptionLighter<?> lighter;
 	public final ContraptionKineticRenderer kinetics;
 
-	private final Map<RenderType, IndexedModel> renderLayers = new HashMap<>();
+	private final Map<RenderType, BufferedModel> renderLayers = new HashMap<>();
 
 	private Matrix4f model;
 	private AxisAlignedBB lightBox;
@@ -67,7 +67,7 @@ public class RenderedContraption extends ContraptionWorldHolder {
 	}
 
 	public void doRenderLayer(RenderType layer, ContraptionProgram shader) {
-		IndexedModel structure = renderLayers.get(layer);
+		BufferedModel structure = renderLayers.get(layer);
 		if (structure != null) {
 			setup(shader);
 			structure.render();
@@ -108,7 +108,7 @@ public class RenderedContraption extends ContraptionWorldHolder {
 	}
 
 	void invalidate() {
-		for (IndexedModel buffer : renderLayers.values()) {
+		for (BufferedModel buffer : renderLayers.values()) {
 			buffer.delete();
 		}
 		renderLayers.clear();
@@ -119,7 +119,7 @@ public class RenderedContraption extends ContraptionWorldHolder {
 	}
 
 	private void buildLayers() {
-		for (IndexedModel buffer : renderLayers.values()) {
+		for (BufferedModel buffer : renderLayers.values()) {
 			buffer.delete();
 		}
 
@@ -128,7 +128,7 @@ public class RenderedContraption extends ContraptionWorldHolder {
 		List<RenderType> blockLayers = RenderType.getBlockLayers();
 
 		for (RenderType layer : blockLayers) {
-			IndexedModel layerModel = buildStructureModel(renderWorld, contraption, layer);
+			BufferedModel layerModel = buildStructureModel(renderWorld, contraption, layer);
 			if (layerModel != null) renderLayers.put(layer, layerModel);
 		}
 	}
@@ -153,7 +153,7 @@ public class RenderedContraption extends ContraptionWorldHolder {
 	}
 
 	@Nullable
-	private static IndexedModel buildStructureModel(PlacementSimulationWorld renderWorld, Contraption c, RenderType layer) {
+	private static BufferedModel buildStructureModel(PlacementSimulationWorld renderWorld, Contraption c, RenderType layer) {
 		BufferBuilderReader reader = new BufferBuilderReader(ContraptionRenderDispatcher.buildStructure(renderWorld, c, layer));
 
 		int vertexCount = reader.getVertexCount();
@@ -192,6 +192,6 @@ public class RenderedContraption extends ContraptionWorldHolder {
 
 		to.rewind();
 
-		return new IndexedModel(format, to, vertexCount);
+		return new BufferedModel(format, to, vertexCount);
 	}
 }