From 00bd05af88440bfa7899299df69b31194af1584b Mon Sep 17 00:00:00 2001 From: Jozufozu Date: Sat, 16 Nov 2024 14:48:35 -0800 Subject: [PATCH 1/6] Who's counting? - Atomically count instances in indirect instancer - Fixes indirect draws/instancers never being deleted - Fix baseDraw being set incorrectly on intel - Handle setting baseDraw in GlCompat#safeMultiDrawElementsIndirect --- .../engine/indirect/IndirectCullingGroup.java | 10 +++------- .../backend/engine/indirect/IndirectInstancer.java | 10 +++++++--- .../engine_room/flywheel/backend/gl/GlCompat.java | 14 ++++++++++---- 3 files changed, 20 insertions(+), 14 deletions(-) diff --git a/common/src/backend/java/dev/engine_room/flywheel/backend/engine/indirect/IndirectCullingGroup.java b/common/src/backend/java/dev/engine_room/flywheel/backend/engine/indirect/IndirectCullingGroup.java index 9060b5363..0f6e8ef57 100644 --- a/common/src/backend/java/dev/engine_room/flywheel/backend/engine/indirect/IndirectCullingGroup.java +++ b/common/src/backend/java/dev/engine_room/flywheel/backend/engine/indirect/IndirectCullingGroup.java @@ -199,7 +199,6 @@ public void submit(VisualType visualType) { drawBarrier(); GlProgram lastProgram = null; - int baseDrawUniformLoc = -1; for (var multiDraw : multiDraws.get(visualType)) { var drawProgram = programs.getIndirectProgram(instanceType, multiDraw.embedded ? ContextShader.EMBEDDED : ContextShader.DEFAULT, multiDraw.material); @@ -208,14 +207,11 @@ public void submit(VisualType visualType) { // Don't need to do this unless the program changes. drawProgram.bind(); - baseDrawUniformLoc = drawProgram.getUniformLocation("_flw_baseDraw"); } - glUniform1ui(baseDrawUniformLoc, multiDraw.start); - MaterialRenderState.setup(multiDraw.material); - multiDraw.submit(); + multiDraw.submit(drawProgram); } } @@ -290,8 +286,8 @@ public boolean checkEmptyAndDelete() { } private record MultiDraw(Material material, boolean embedded, int start, int end) { - private void submit() { - GlCompat.safeMultiDrawElementsIndirect(GL_TRIANGLES, GL_UNSIGNED_INT, this.start * IndirectBuffers.DRAW_COMMAND_STRIDE, this.end - this.start, (int) IndirectBuffers.DRAW_COMMAND_STRIDE); + private void submit(GlProgram drawProgram) { + GlCompat.safeMultiDrawElementsIndirect(drawProgram, GL_TRIANGLES, GL_UNSIGNED_INT, this.start, this.end, IndirectBuffers.DRAW_COMMAND_STRIDE); } } } diff --git a/common/src/backend/java/dev/engine_room/flywheel/backend/engine/indirect/IndirectInstancer.java b/common/src/backend/java/dev/engine_room/flywheel/backend/engine/indirect/IndirectInstancer.java index 1291e9a77..aa9b372f1 100644 --- a/common/src/backend/java/dev/engine_room/flywheel/backend/engine/indirect/IndirectInstancer.java +++ b/common/src/backend/java/dev/engine_room/flywheel/backend/engine/indirect/IndirectInstancer.java @@ -25,6 +25,9 @@ public class IndirectInstancer extends AbstractInstancer private final Vector4fc boundingSphere; private final AtomicReference[]> pages = new AtomicReference<>(pageArray(0)); + + private final AtomicInteger instanceCount = new AtomicInteger(0); + /** * The set of pages whose count changed and thus need their descriptor re-uploaded. */ @@ -145,6 +148,8 @@ public boolean add(I instance, InstanceHandleImpl handle) { // We just filled the 17th instance, so we are no longer mergeable. parent.mergeablePages.clear(pageNo); } + + parent.instanceCount.incrementAndGet(); return true; } } @@ -203,6 +208,7 @@ private void clear(int localIndex) { } // Set full page last so that other threads don't race to set the other bitsets. parent.fullPages.clear(pageNo); + parent.instanceCount.decrementAndGet(); break; } } @@ -538,9 +544,7 @@ private void addInner(I instance, InstanceHandleImpl handle) { } public int instanceCount() { - // Not exactly accurate but it's an upper bound. - // TODO: maybe this could be tracked with an AtomicInteger? - return pages.get().length << ObjectStorage.LOG_2_PAGE_SIZE; + return instanceCount.get(); } /** diff --git a/common/src/backend/java/dev/engine_room/flywheel/backend/gl/GlCompat.java b/common/src/backend/java/dev/engine_room/flywheel/backend/gl/GlCompat.java index b7efa23ed..636b9ac4c 100644 --- a/common/src/backend/java/dev/engine_room/flywheel/backend/gl/GlCompat.java +++ b/common/src/backend/java/dev/engine_room/flywheel/backend/gl/GlCompat.java @@ -16,6 +16,7 @@ import dev.engine_room.flywheel.backend.FlwBackend; import dev.engine_room.flywheel.backend.compile.core.Compilation; +import dev.engine_room.flywheel.backend.gl.shader.GlProgram; import dev.engine_room.flywheel.backend.glsl.GlslVersion; import dev.engine_room.flywheel.lib.math.MoreMath; @@ -78,15 +79,20 @@ public static void safeShaderSource(int glId, CharSequence source) { * but uses consecutive DI instead of MDI if MDI is known to not work well with the current driver. * Unlike the original function, stride cannot be equal to 0. */ - public static void safeMultiDrawElementsIndirect(int mode, int type, long indirect, int drawcount, int stride) { + public static void safeMultiDrawElementsIndirect(GlProgram drawProgram, int mode, int type, int start, int end, long stride) { + var count = end - start; + long indirect = start * stride; + if (GlCompat.DRIVER == Driver.INTEL) { - // Intel renders garbage with MDI, but consecutive DI works fine. - for (int i = 0; i < drawcount; i++) { + // Intel renders garbage with MDI, but consecutive DI works "fine". + for (int i = 0; i < count; i++) { + drawProgram.setUInt("_flw_baseDraw", start + i); GL40.glDrawElementsIndirect(mode, type, indirect); indirect += stride; } } else { - GL43.glMultiDrawElementsIndirect(mode, type, indirect, drawcount, stride); + drawProgram.setUInt("_flw_baseDraw", start); + GL43.glMultiDrawElementsIndirect(mode, type, indirect, count, (int) stride); } } From 3b7f9c77704fe65a51803b49210d721cff80c9fc Mon Sep 17 00:00:00 2001 From: Jozufozu Date: Sat, 16 Nov 2024 15:10:31 -0800 Subject: [PATCH 2/6] Lights, camera... - Hopefully fix gh actions by bumping setup-gradle version --- .github/workflows/build.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index cf5b1c501..4b270af22 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -23,7 +23,7 @@ jobs: restore-keys: "${{ runner.os }}-gradle-" - name: Setup Gradle - uses: gradle/actions/setup-gradle@v3 + uses: gradle/actions/setup-gradle@v4.2.0 with: gradle-home-cache-cleanup: true cache-read-only: ${{ !endsWith(github.ref_name, '/dev') }} From 7148ff3f3124fec6ceea7117c12c8a95b87dfe6a Mon Sep 17 00:00:00 2001 From: Jozufozu Date: Sun, 17 Nov 2024 12:24:14 -0800 Subject: [PATCH 3/6] Breaking bright - Fix crumbling having an incorrect draw buffer bound, causing it to appear too bright on indirect --- .../flywheel/backend/engine/indirect/IndirectBuffers.java | 6 ++---- .../backend/engine/indirect/IndirectCullingGroup.java | 8 +++----- .../backend/engine/indirect/IndirectDrawManager.java | 3 +-- 3 files changed, 6 insertions(+), 11 deletions(-) diff --git a/common/src/backend/java/dev/engine_room/flywheel/backend/engine/indirect/IndirectBuffers.java b/common/src/backend/java/dev/engine_room/flywheel/backend/engine/indirect/IndirectBuffers.java index 866150347..6280dfc6e 100644 --- a/common/src/backend/java/dev/engine_room/flywheel/backend/engine/indirect/IndirectBuffers.java +++ b/common/src/backend/java/dev/engine_room/flywheel/backend/engine/indirect/IndirectBuffers.java @@ -109,11 +109,9 @@ public void bindForDraw() { GlBufferType.DRAW_INDIRECT_BUFFER.bind(draw.handle()); } - /** - * Bind all buffers except the draw command buffer. - */ public void bindForCrumbling() { - multiBind(1, 4); + // All we need is the instance buffer. Crumbling uses its own draw buffer. + multiBind(BufferBindings.INSTANCE, 1); } private void multiBind(int base, int count) { diff --git a/common/src/backend/java/dev/engine_room/flywheel/backend/engine/indirect/IndirectCullingGroup.java b/common/src/backend/java/dev/engine_room/flywheel/backend/engine/indirect/IndirectCullingGroup.java index 0f6e8ef57..9b56968d2 100644 --- a/common/src/backend/java/dev/engine_room/flywheel/backend/engine/indirect/IndirectCullingGroup.java +++ b/common/src/backend/java/dev/engine_room/flywheel/backend/engine/indirect/IndirectCullingGroup.java @@ -2,7 +2,6 @@ import static org.lwjgl.opengl.GL11.GL_TRIANGLES; import static org.lwjgl.opengl.GL11.GL_UNSIGNED_INT; -import static org.lwjgl.opengl.GL30.glUniform1ui; import static org.lwjgl.opengl.GL42.GL_COMMAND_BARRIER_BIT; import static org.lwjgl.opengl.GL42.glMemoryBarrier; import static org.lwjgl.opengl.GL43.glDispatchCompute; @@ -215,8 +214,8 @@ public void submit(VisualType visualType) { } } - public void bindWithContextShader(ContextShader override, Material material) { - var program = programs.getIndirectProgram(instanceType, override, material); + public void bindForCrumbling(Material material) { + var program = programs.getIndirectProgram(instanceType, ContextShader.CRUMBLING, material); program.bind(); @@ -224,8 +223,7 @@ public void bindWithContextShader(ContextShader override, Material material) { drawBarrier(); - var flwBaseDraw = program.getUniformLocation("_flw_baseDraw"); - glUniform1ui(flwBaseDraw, 0); + program.setUInt("_flw_baseDraw", 0); } private void drawBarrier() { diff --git a/common/src/backend/java/dev/engine_room/flywheel/backend/engine/indirect/IndirectDrawManager.java b/common/src/backend/java/dev/engine_room/flywheel/backend/engine/indirect/IndirectDrawManager.java index 65c0d4aba..7fab5d55e 100644 --- a/common/src/backend/java/dev/engine_room/flywheel/backend/engine/indirect/IndirectDrawManager.java +++ b/common/src/backend/java/dev/engine_room/flywheel/backend/engine/indirect/IndirectDrawManager.java @@ -17,7 +17,6 @@ import dev.engine_room.flywheel.api.instance.InstanceType; import dev.engine_room.flywheel.api.visualization.VisualType; import dev.engine_room.flywheel.backend.Samplers; -import dev.engine_room.flywheel.backend.compile.ContextShader; import dev.engine_room.flywheel.backend.compile.IndirectPrograms; import dev.engine_room.flywheel.backend.engine.AbstractInstancer; import dev.engine_room.flywheel.backend.engine.CommonCrumbling; @@ -235,7 +234,7 @@ public void renderCrumbling(List crumblingBlocks) { // Transform the material to be suited for crumbling. CommonCrumbling.applyCrumblingProperties(crumblingMaterial, draw.material()); - cullingGroup.bindWithContextShader(ContextShader.CRUMBLING, crumblingMaterial); + cullingGroup.bindForCrumbling(crumblingMaterial); MaterialRenderState.setup(crumblingMaterial); From 9e60045d43a05f4b211ea0a9d51e77ce226af66c Mon Sep 17 00:00:00 2001 From: Jozufozu Date: Mon, 18 Nov 2024 16:13:25 -0800 Subject: [PATCH 4/6] Dimensional storage - Fix exception thrown in light storage when vising nether and end dimensions - Try to make section collection more tolerant, but more work on a fallback path is likely needed --- .../flywheel/backend/engine/LightStorage.java | 52 ++++++++++++++----- 1 file changed, 38 insertions(+), 14 deletions(-) diff --git a/common/src/backend/java/dev/engine_room/flywheel/backend/engine/LightStorage.java b/common/src/backend/java/dev/engine_room/flywheel/backend/engine/LightStorage.java index c9539ca95..ada3ed24c 100644 --- a/common/src/backend/java/dev/engine_room/flywheel/backend/engine/LightStorage.java +++ b/common/src/backend/java/dev/engine_room/flywheel/backend/engine/LightStorage.java @@ -1,6 +1,7 @@ package dev.engine_room.flywheel.backend.engine; import java.util.BitSet; +import java.util.Objects; import org.jetbrains.annotations.Nullable; import org.lwjgl.system.MemoryUtil; @@ -34,6 +35,7 @@ import net.minecraft.world.level.LevelAccessor; import net.minecraft.world.level.LightLayer; import net.minecraft.world.level.chunk.DataLayer; +import net.minecraft.world.level.lighting.LayerLightEventListener; /** * A managed arena of light sections for uploading to the GPU. @@ -57,8 +59,8 @@ public class LightStorage implements Effect { private static final int DEFAULT_ARENA_CAPACITY_SECTIONS = 64; private static final int INVALID_SECTION = -1; - private static final ConstantDataLayer EMPTY_BLOCK_DATA = new ConstantDataLayer(0); - private static final ConstantDataLayer EMPTY_SKY_DATA = new ConstantDataLayer(15); + private static final ConstantDataLayer ALWAYS_0 = new ConstantDataLayer(0); + private static final ConstantDataLayer ALWAYS_15 = new ConstantDataLayer(15); private final LevelAccessor level; private final LightLut lut; @@ -268,29 +270,51 @@ private void collectSolidData(long ptr, long section) { } private DataLayer getSkyData(long section) { - var sky = level.getLightEngine() + var layerListener = level.getLightEngine() .getLayerListener(LightLayer.SKY); - var skyStorage = (SkyLightSectionStorageExtension) ((LightEngineAccessor) sky).flywheel$storage(); - var out = skyStorage.flywheel$skyDataLayer(section); + if (layerListener == LayerLightEventListener.DummyLightLayerEventListener.INSTANCE) { + // The dummy listener always returns 0. + // In vanilla this happens in the nether and end, + // and the light texture is simply updated + // to be invariant on sky light. + return ALWAYS_0; + } + + if (layerListener instanceof LightEngineAccessor accessor) { + // Sky storage has a fancy way to get the sky light at a given block position, but the logic is not + // implemented in vanilla for fetching data layers directly. We need to re-implement it here. The simplest + // way to do it was to expose the same logic via an extension method. Re-implementing it external to the + // SkyLightSectionStorage class would require many more accessors. + if (accessor.flywheel$storage() instanceof SkyLightSectionStorageExtension skyStorage) { + var out = skyStorage.flywheel$skyDataLayer(section); - if (out == null) { - return EMPTY_SKY_DATA; + // Null section here means there are no blocks above us to darken this section. + return Objects.requireNonNullElse(out, ALWAYS_15); + } } - return out; + // FIXME: We're likely in some exotic dimension that needs special handling. + return ALWAYS_0; } private DataLayer getBlockData(long section) { - var out = ((LightEngineAccessor) level.getLightEngine() - .getLayerListener(LightLayer.BLOCK)).flywheel$storage() - .getDataLayerData(section); + var layerListener = level.getLightEngine() + .getLayerListener(LightLayer.BLOCK); - if (out == null) { - return EMPTY_BLOCK_DATA; + if (layerListener == LayerLightEventListener.DummyLightLayerEventListener.INSTANCE) { + return ALWAYS_0; } - return out; + if (layerListener instanceof LightEngineAccessor accessor) { + var out = accessor.flywheel$storage() + .getDataLayerData(section); + + return Objects.requireNonNullElse(out, ALWAYS_0); + } + + // FIXME: We're likely in some exotic dimension that needs special handling. + return ALWAYS_0; } private void collectXStrip(long ptr, long section, SectionEdge y, SectionEdge z) { From e1ddd7c2fcf5cb63e9ddf05214308036e5c3c7b0 Mon Sep 17 00:00:00 2001 From: Jozufozu Date: Mon, 2 Dec 2024 17:19:00 -0800 Subject: [PATCH 5/6] You merely adopted it - Default light to 0 in DefaultVertexList - Take the max of mesh and instance light in the instance shader --- .../dev/engine_room/flywheel/lib/vertex/DefaultVertexList.java | 3 +-- .../resources/assets/flywheel/flywheel/instance/oriented.vert | 2 +- .../lib/resources/assets/flywheel/flywheel/instance/posed.vert | 2 +- .../assets/flywheel/flywheel/instance/transformed.vert | 2 +- 4 files changed, 4 insertions(+), 5 deletions(-) diff --git a/common/src/lib/java/dev/engine_room/flywheel/lib/vertex/DefaultVertexList.java b/common/src/lib/java/dev/engine_room/flywheel/lib/vertex/DefaultVertexList.java index b6361854b..9d5f9c8bf 100644 --- a/common/src/lib/java/dev/engine_room/flywheel/lib/vertex/DefaultVertexList.java +++ b/common/src/lib/java/dev/engine_room/flywheel/lib/vertex/DefaultVertexList.java @@ -1,7 +1,6 @@ package dev.engine_room.flywheel.lib.vertex; import dev.engine_room.flywheel.api.vertex.MutableVertexList; -import net.minecraft.client.renderer.LightTexture; import net.minecraft.client.renderer.texture.OverlayTexture; public interface DefaultVertexList extends MutableVertexList { @@ -57,7 +56,7 @@ default int overlay(int index) { @Override default int light(int index) { - return LightTexture.FULL_BRIGHT; + return 0; } @Override diff --git a/common/src/lib/resources/assets/flywheel/flywheel/instance/oriented.vert b/common/src/lib/resources/assets/flywheel/flywheel/instance/oriented.vert index 6d113f0a8..f7cf36dc0 100644 --- a/common/src/lib/resources/assets/flywheel/flywheel/instance/oriented.vert +++ b/common/src/lib/resources/assets/flywheel/flywheel/instance/oriented.vert @@ -6,5 +6,5 @@ void flw_instanceVertex(in FlwInstance i) { flw_vertexColor *= i.color; flw_vertexOverlay = i.overlay; // Some drivers have a bug where uint over float division is invalid, so use an explicit cast. - flw_vertexLight = vec2(i.light) / 256.0; + flw_vertexLight = max(vec2(i.light) / 256.0, flw_vertexLight); } diff --git a/common/src/lib/resources/assets/flywheel/flywheel/instance/posed.vert b/common/src/lib/resources/assets/flywheel/flywheel/instance/posed.vert index 918883e84..deae24c42 100644 --- a/common/src/lib/resources/assets/flywheel/flywheel/instance/posed.vert +++ b/common/src/lib/resources/assets/flywheel/flywheel/instance/posed.vert @@ -4,5 +4,5 @@ void flw_instanceVertex(in FlwInstance i) { flw_vertexColor *= i.color; flw_vertexOverlay = i.overlay; // Some drivers have a bug where uint over float division is invalid, so use an explicit cast. - flw_vertexLight = vec2(i.light) / 256.0; + flw_vertexLight = max(vec2(i.light) / 256.0, flw_vertexLight); } diff --git a/common/src/lib/resources/assets/flywheel/flywheel/instance/transformed.vert b/common/src/lib/resources/assets/flywheel/flywheel/instance/transformed.vert index e0abf722b..73097069f 100644 --- a/common/src/lib/resources/assets/flywheel/flywheel/instance/transformed.vert +++ b/common/src/lib/resources/assets/flywheel/flywheel/instance/transformed.vert @@ -4,5 +4,5 @@ void flw_instanceVertex(in FlwInstance i) { flw_vertexColor *= i.color; flw_vertexOverlay = i.overlay; // Some drivers have a bug where uint over float division is invalid, so use an explicit cast. - flw_vertexLight = vec2(i.light) / 256.0; + flw_vertexLight = max(vec2(i.light) / 256.0, flw_vertexLight); } From 0c195fef9fef010539043db1dee43f4695826441 Mon Sep 17 00:00:00 2001 From: Jozufozu Date: Mon, 2 Dec 2024 17:45:08 -0800 Subject: [PATCH 6/6] Manual light updates - Update the BakedModelBufferers to default to zero sky light --- .../flywheel/lib/model/baked/BakedModelBufferer.java | 2 +- .../flywheel/lib/model/baked/BakedModelBufferer.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/fabric/src/lib/java/dev/engine_room/flywheel/lib/model/baked/BakedModelBufferer.java b/fabric/src/lib/java/dev/engine_room/flywheel/lib/model/baked/BakedModelBufferer.java index 474e31fbe..23c63a041 100644 --- a/fabric/src/lib/java/dev/engine_room/flywheel/lib/model/baked/BakedModelBufferer.java +++ b/fabric/src/lib/java/dev/engine_room/flywheel/lib/model/baked/BakedModelBufferer.java @@ -144,7 +144,7 @@ public interface ResultConsumer { } private static class ThreadLocalObjects { - public final FabricOriginBlockAndTintGetter level = new FabricOriginBlockAndTintGetter(p -> 0, p -> 15); + public final FabricOriginBlockAndTintGetter level = new FabricOriginBlockAndTintGetter(p -> 0, p -> 0); public final PoseStack identityPoseStack = new PoseStack(); public final RandomSource random = RandomSource.createNewThreadLocalInstance(); diff --git a/forge/src/lib/java/dev/engine_room/flywheel/lib/model/baked/BakedModelBufferer.java b/forge/src/lib/java/dev/engine_room/flywheel/lib/model/baked/BakedModelBufferer.java index b90004b91..b67ac8b7f 100644 --- a/forge/src/lib/java/dev/engine_room/flywheel/lib/model/baked/BakedModelBufferer.java +++ b/forge/src/lib/java/dev/engine_room/flywheel/lib/model/baked/BakedModelBufferer.java @@ -145,7 +145,7 @@ public interface ResultConsumer { } private static class ThreadLocalObjects { - public final OriginBlockAndTintGetter level = new OriginBlockAndTintGetter(p -> 0, p -> 15); + public final OriginBlockAndTintGetter level = new OriginBlockAndTintGetter(p -> 0, p -> 0); public final PoseStack identityPoseStack = new PoseStack(); public final RandomSource random = RandomSource.createNewThreadLocalInstance();