diff --git a/src/main/java/com/jozufozu/flywheel/backend/gl/GlStateTracker.java b/src/main/java/com/jozufozu/flywheel/backend/gl/GlStateTracker.java index e63897fc6..77749e8c7 100644 --- a/src/main/java/com/jozufozu/flywheel/backend/gl/GlStateTracker.java +++ b/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]); } } diff --git a/src/main/java/com/jozufozu/flywheel/backend/instancing/instancing/GPUInstancer.java b/src/main/java/com/jozufozu/flywheel/backend/instancing/instancing/GPUInstancer.java index 4b469c02f..a127b8e30 100644 --- a/src/main/java/com/jozufozu/flywheel/backend/instancing/instancing/GPUInstancer.java +++ b/src/main/java/com/jozufozu/flywheel/backend/instancing/instancing/GPUInstancer.java @@ -45,6 +45,7 @@ public class GPUInstancer extends AbstractInstancer { 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 extends AbstractInstancer { 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 extends AbstractInstancer { removeDeletedInstances(); } + // XXX ARRAY_BUFFER is bound and reset instanceVBO.bind(); if (!realloc()) { diff --git a/src/main/java/com/jozufozu/flywheel/backend/instancing/instancing/InstancedMaterialGroup.java b/src/main/java/com/jozufozu/flywheel/backend/instancing/instancing/InstancedMaterialGroup.java index 2bc228ed9..b132596ca 100644 --- a/src/main/java/com/jozufozu/flywheel/backend/instancing/instancing/InstancedMaterialGroup.java +++ b/src/main/java/com/jozufozu/flywheel/backend/instancing/instancing/InstancedMaterialGroup.java @@ -68,15 +68,23 @@ public class InstancedMaterialGroup

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

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

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

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) } } diff --git a/src/main/java/com/jozufozu/flywheel/backend/instancing/instancing/InstancingEngine.java b/src/main/java/com/jozufozu/flywheel/backend/instancing/instancing/InstancingEngine.java index 5e8c30a54..2708f474d 100644 --- a/src/main/java/com/jozufozu/flywheel/backend/instancing/instancing/InstancingEngine.java +++ b/src/main/java/com/jozufozu/flywheel/backend/instancing/instancing/InstancingEngine.java @@ -74,6 +74,7 @@ public class InstancingEngine

implements Engine { */ @Override public void render(TaskEngine taskEngine, RenderLayerEvent event) { + // XXX Restore state GlStateTracker.State restoreState = GlStateTracker.getRestoreState(); double camX; diff --git a/src/main/java/com/jozufozu/flywheel/backend/model/ElementBuffer.java b/src/main/java/com/jozufozu/flywheel/backend/model/ElementBuffer.java index 8227ad019..07ce772d3 100644 --- a/src/main/java/com/jozufozu/flywheel/backend/model/ElementBuffer.java +++ b/src/main/java/com/jozufozu/flywheel/backend/model/ElementBuffer.java @@ -1,25 +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.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 GlNumericType eboIndexType; + protected final int elementCount; + protected final VertexFormat.IndexType eboIndexType; + private final int glBuffer; - public ElementBuffer(GlBuffer backing, int elementCount, GlNumericType 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; } } diff --git a/src/main/java/com/jozufozu/flywheel/backend/model/IndexedModel.java b/src/main/java/com/jozufozu/flywheel/backend/model/IndexedModel.java index 7a372b0c8..3d3e79814 100644 --- a/src/main/java/com/jozufozu/flywheel/backend/model/IndexedModel.java +++ b/src/main/java/com/jozufozu/flywheel/backend/model/IndexedModel.java @@ -52,27 +52,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.getGlEnum(), 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.getGlEnum(), 0, instanceCount); + GL31.glDrawElementsInstanced(primitiveMode.glEnum, ebo.getElementCount(), ebo.getEboIndexType().asGLType, 0, instanceCount); } public boolean isDeleted() { diff --git a/src/main/java/com/jozufozu/flywheel/backend/model/ModelPool.java b/src/main/java/com/jozufozu/flywheel/backend/model/ModelPool.java index 5c1d426c7..3d7d330e5 100644 --- a/src/main/java/com/jozufozu/flywheel/backend/model/ModelPool.java +++ b/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.getGlEnum(), 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.getGlEnum(), 0, instanceCount, first); + GL32.glDrawElementsInstancedBaseVertex(GlPrimitive.TRIANGLES.glEnum, ebo.getElementCount(), ebo.getEboIndexType().asGLType, 0, instanceCount, first); } @Override diff --git a/src/main/java/com/jozufozu/flywheel/backend/model/VBOModel.java b/src/main/java/com/jozufozu/flywheel/backend/model/VBOModel.java index 54dccbd28..e51832a63 100644 --- a/src/main/java/com/jozufozu/flywheel/backend/model/VBOModel.java +++ b/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()); diff --git a/src/main/java/com/jozufozu/flywheel/core/QuadConverter.java b/src/main/java/com/jozufozu/flywheel/core/QuadConverter.java index d1871224b..37ba431b7 100644 --- a/src/main/java/com/jozufozu/flywheel/core/QuadConverter.java +++ b/src/main/java/com/jozufozu/flywheel/core/QuadConverter.java @@ -1,43 +1,32 @@ package com.jozufozu.flywheel.core; -import java.nio.Buffer; -import java.nio.ByteBuffer; -import java.nio.ByteOrder; -import java.util.EnumMap; -import java.util.Map; - -import javax.annotation.Nonnull; -import javax.annotation.Nullable; - -import org.lwjgl.system.MemoryStack; +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.GlBuffer; import com.jozufozu.flywheel.backend.gl.buffer.GlBufferType; -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 net.minecraftforge.api.distmarker.Dist; -import net.minecraftforge.eventbus.api.EventPriority; -import net.minecraftforge.eventbus.api.SubscribeEvent; -import net.minecraftforge.fml.common.Mod; +import it.unimi.dsi.fastutil.ints.Int2ReferenceArrayMap; +import it.unimi.dsi.fastutil.ints.Int2ReferenceMap; /** * A class to manage EBOs that index quads as triangles. */ -@Mod.EventBusSubscriber(Dist.CLIENT) public class QuadConverter { - public static final int STARTING_CAPACITY = 42; // 255 / 6 = 42 - private static QuadConverter INSTANCE; - @Nonnull + @NotNull public static QuadConverter getInstance() { if (INSTANCE == null) { - INSTANCE = new QuadConverter(STARTING_CAPACITY); + INSTANCE = new QuadConverter(); } return INSTANCE; @@ -48,130 +37,78 @@ public class QuadConverter { return INSTANCE; } - Map ebos; - int[] capacities; + private final Int2ReferenceMap cache = new Int2ReferenceArrayMap<>(); + private final int ebo; + private int quadCapacity; - public QuadConverter(int initialCapacity) { - this.ebos = new EnumMap<>(GlNumericType.class); - initCapacities(); - - fillBuffer(initialCapacity); + public QuadConverter() { + this.ebo = GL32.glGenBuffers(); + this.quadCapacity = 0; } public ElementBuffer quads2Tris(int quads) { - int indexCount = quads * 6; - GlNumericType type = getSmallestIndexType(indexCount); - - if (quads > getCapacity(type)) { - fillBuffer(quads, indexCount, type); + if (quads > quadCapacity) { + grow(quads * 2); } - return new ElementBuffer(getBuffer(type), indexCount, type); + return cache.computeIfAbsent(quads, this::createElementBuffer); } - private void initCapacities() { - this.capacities = new int[GlNumericType.values().length]; + @NotNull + private ElementBuffer createElementBuffer(int quads) { + return new ElementBuffer(ebo, quads * 6, VertexFormat.IndexType.INT); } - private int getCapacity(GlNumericType type) { - return capacities[type.ordinal()]; - } + private void grow(int quads) { + int byteSize = quads * 6 * GlNumericType.UINT.getByteWidth(); + final long ptr = MemoryUtil.nmemAlloc(byteSize); - private void updateCapacity(GlNumericType type, int capacity) { - if (getCapacity(type) < capacity) { - capacities[type.ordinal()] = capacity; - } + 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() { - ebos.values() - .forEach(GlBuffer::delete); - ebos.clear(); - initCapacities(); + GL32.glDeleteBuffers(ebo); + this.quadCapacity = 0; } - private void fillBuffer(int quads) { - int indexCount = quads * 6; + private void fillBuffer(long ptr, int quads) { + int numVertices = 4 * quads; + int baseVertex = 0; + while (baseVertex < numVertices) { + writeQuadIndicesUnsafe(ptr, baseVertex); - fillBuffer(quads, indexCount, getSmallestIndexType(indexCount)); - } - - private void fillBuffer(int quads, int indexCount, GlNumericType type) { - MemoryStack stack = MemoryStack.stackPush(); - int bytes = indexCount * type.getByteWidth(); - - ByteBuffer indices; - if (bytes > stack.getSize()) { - indices = MemoryUtil.memAlloc(bytes); // not enough space on the preallocated stack - } else { - stack.push(); - indices = stack.malloc(bytes); + baseVertex += 4; + ptr += 6 * 4; } - - indices.order(ByteOrder.nativeOrder()); - - fillBuffer(indices, type, quads); - - GlBuffer buffer = getBuffer(type); - - buffer.bind(); - buffer.upload(indices); - buffer.unbind(); - - if (bytes > stack.getSize()) { - MemoryUtil.memFree(indices); - } else { - stack.pop(); - } - - updateCapacity(type, quads); } - private void fillBuffer(ByteBuffer indices, GlNumericType type, int quads) { - for (int i = 0, max = 4 * quads; i < max; i += 4) { - // triangle a - type.castAndBuffer(indices, i); - type.castAndBuffer(indices, i + 1); - type.castAndBuffer(indices, i + 2); - // triangle b - type.castAndBuffer(indices, i); - type.castAndBuffer(indices, i + 2); - type.castAndBuffer(indices, i + 3); - } - ((Buffer) indices).flip(); + private void writeQuadIndicesUnsafe(long ptr, int baseVertex) { + // triangle a + MemoryUtil.memPutInt(ptr, baseVertex); + MemoryUtil.memPutInt(ptr + 4, baseVertex + 1); + MemoryUtil.memPutInt(ptr + 8, baseVertex + 2); + // triangle b + MemoryUtil.memPutInt(ptr + 12, baseVertex); + MemoryUtil.memPutInt(ptr + 16, baseVertex + 2); + MemoryUtil.memPutInt(ptr + 20, baseVertex + 3); } - private GlBuffer getBuffer(GlNumericType type) { - return ebos.computeIfAbsent(type, $ -> new MappedGlBuffer(GlBufferType.ELEMENT_ARRAY_BUFFER)); - } - - /** - * Given the needed number of indices, what is the smallest bit width type that can index everything?
- * - * 

-	 * | indexCount   | type  |
-	 * |--------------|-------|
-	 * | [0, 255)     | byte  |
-	 * | [256, 65536)	| short	|
-	 * | [65537, )	| int	|
-	 *
- */ - private static GlNumericType getSmallestIndexType(int indexCount) { -// indexCount = indexCount >>> 8; -// if (indexCount == 0) { -// return GlNumericType.UBYTE; -// } -// indexCount = indexCount >>> 8; -// if (indexCount == 0) { -// return GlNumericType.USHORT; -// } - - return GlNumericType.UINT; - } - - // make sure this gets reset first so it has a chance to repopulate - @SubscribeEvent(priority = EventPriority.HIGHEST) + // make sure this gets reset first, so it has a chance to repopulate public static void onRendererReload(ReloadRenderersEvent event) { - if (INSTANCE != null) INSTANCE.delete(); + if (INSTANCE != null) { + INSTANCE.delete(); + INSTANCE = null; + } } } diff --git a/src/main/java/com/jozufozu/flywheel/core/crumbling/CrumblingGroup.java b/src/main/java/com/jozufozu/flywheel/core/crumbling/CrumblingGroup.java index ee113de17..0378b6ee6 100644 --- a/src/main/java/com/jozufozu/flywheel/core/crumbling/CrumblingGroup.java +++ b/src/main/java/com/jozufozu/flywheel/core/crumbling/CrumblingGroup.java @@ -18,6 +18,7 @@ public class CrumblingGroup

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(); diff --git a/src/main/java/com/jozufozu/flywheel/core/crumbling/CrumblingRenderer.java b/src/main/java/com/jozufozu/flywheel/core/crumbling/CrumblingRenderer.java index e15e91a4e..1455eb25e 100644 --- a/src/main/java/com/jozufozu/flywheel/core/crumbling/CrumblingRenderer.java +++ b/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()); diff --git a/src/main/java/com/jozufozu/flywheel/core/model/Model.java b/src/main/java/com/jozufozu/flywheel/core/model/Model.java index eacfd3394..e94d983ec 100644 --- a/src/main/java/com/jozufozu/flywheel/core/model/Model.java +++ b/src/main/java/com/jozufozu/flywheel/core/model/Model.java @@ -46,6 +46,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. *