From d83e19905e14fb6eb5384990774195c0c092bc35 Mon Sep 17 00:00:00 2001 From: TheJudge156 Date: Wed, 3 Apr 2024 22:27:55 -0400 Subject: [PATCH 01/11] Fix shader attributes being offset by 1 unnecessarily embeddedt note: Despite Iris/Oculus copying these constants, the original binaries appear to still work without modification, so this seems safe to ship --- .../render/chunk/shader/ChunkShaderBindingPoints.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/main/java/me/jellysquid/mods/sodium/client/render/chunk/shader/ChunkShaderBindingPoints.java b/src/main/java/me/jellysquid/mods/sodium/client/render/chunk/shader/ChunkShaderBindingPoints.java index 3f87ecf90..f1c322e71 100644 --- a/src/main/java/me/jellysquid/mods/sodium/client/render/chunk/shader/ChunkShaderBindingPoints.java +++ b/src/main/java/me/jellysquid/mods/sodium/client/render/chunk/shader/ChunkShaderBindingPoints.java @@ -1,10 +1,10 @@ package me.jellysquid.mods.sodium.client.render.chunk.shader; public class ChunkShaderBindingPoints { - public static final int ATTRIBUTE_POSITION_ID = 1; - public static final int ATTRIBUTE_COLOR = 2; - public static final int ATTRIBUTE_BLOCK_TEXTURE = 3; - public static final int ATTRIBUTE_LIGHT_TEXTURE = 4; + public static final int ATTRIBUTE_POSITION_ID = 0; + public static final int ATTRIBUTE_COLOR = 1; + public static final int ATTRIBUTE_BLOCK_TEXTURE = 2; + public static final int ATTRIBUTE_LIGHT_TEXTURE = 3; public static final int FRAG_COLOR = 0; } From 4c996a3f2745f0fd75578189c0c3d94d09a2c6fa Mon Sep 17 00:00:00 2001 From: Nolij Date: Tue, 9 Apr 2024 18:18:10 -0400 Subject: [PATCH 02/11] add DCO to PR policy --- .github/workflows/dco.yml | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 .github/workflows/dco.yml diff --git a/.github/workflows/dco.yml b/.github/workflows/dco.yml new file mode 100644 index 000000000..50b074efe --- /dev/null +++ b/.github/workflows/dco.yml @@ -0,0 +1,32 @@ +name: "DCO Assistant" +on: + issue_comment: + types: [created] + pull_request_target: + types: [opened, closed, synchronize] + +jobs: + DCO: + runs-on: ubuntu-latest + steps: + - name: "DCO Assistant" + if: ( + github.event.comment.body == "recheck" || + github.event.comment.body == "I have read, and hereby affirm the entire contents of the Developer Certificate of Origin." + ) || github.event_name == "pull_request_target" + uses: cla-assistant/github-action@v2.3.0 + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + with: + path-to-signatures: "signatures.json" + path-to-document: "https://github.com/embeddedt/embeddium/blob/dco/DCO.txt" + branch: "dco" + create-file-commit-message: "Create DCO signature list" + signed-commit-message: "Add $contributorName to the DCO signature list" + custom-notsigned-prcomment: + "Before we can merge your submission, we require that you read and affirm the contents of the + [Developer Certificate of Origin]($pathToCLADocument) by adding a comment containing the below text. + Otherwise, please close this PR." + custom-pr-sign-comment: "I have read, and hereby affirm the entire contents of the Developer Certificate of Origin." + custom-allsigned-prcomment: "All contributors have read and affirmed the entire contents of the Developer Certificate of Origin." + use-dco-flag: true From 129439d4cb32a9b020a2b10be73b6eab2afa9d5b Mon Sep 17 00:00:00 2001 From: Nolij Date: Tue, 9 Apr 2024 18:30:48 -0400 Subject: [PATCH 03/11] update DCO workflow to conform with poor syntax choices --- .github/workflows/dco.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/dco.yml b/.github/workflows/dco.yml index 50b074efe..bff441048 100644 --- a/.github/workflows/dco.yml +++ b/.github/workflows/dco.yml @@ -11,9 +11,9 @@ jobs: steps: - name: "DCO Assistant" if: ( - github.event.comment.body == "recheck" || - github.event.comment.body == "I have read, and hereby affirm the entire contents of the Developer Certificate of Origin." - ) || github.event_name == "pull_request_target" + github.event.comment.body == 'recheck' || + github.event.comment.body == 'I have read, and hereby affirm the entire contents of the Developer Certificate of Origin.' + ) || github.event_name == 'pull_request_target' uses: cla-assistant/github-action@v2.3.0 env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} From c8f01c85117368dff3dee69a0eb009f57cc0ec20 Mon Sep 17 00:00:00 2001 From: Nolij Date: Tue, 9 Apr 2024 18:32:08 -0400 Subject: [PATCH 04/11] fix DCO workflow permissions --- .github/workflows/dco.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.github/workflows/dco.yml b/.github/workflows/dco.yml index bff441048..7934fd016 100644 --- a/.github/workflows/dco.yml +++ b/.github/workflows/dco.yml @@ -5,6 +5,12 @@ on: pull_request_target: types: [opened, closed, synchronize] +permissions: + actions: write + contents: write + pull-requests: write + statuses: write + jobs: DCO: runs-on: ubuntu-latest From 876e373d9e3f265db984565ad40bb84dda88b123 Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Tue, 9 Apr 2024 18:35:20 -0400 Subject: [PATCH 05/11] Fix broken link in DCO comment --- .github/workflows/dco.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/dco.yml b/.github/workflows/dco.yml index 7934fd016..d63c9b968 100644 --- a/.github/workflows/dco.yml +++ b/.github/workflows/dco.yml @@ -31,7 +31,7 @@ jobs: signed-commit-message: "Add $contributorName to the DCO signature list" custom-notsigned-prcomment: "Before we can merge your submission, we require that you read and affirm the contents of the - [Developer Certificate of Origin]($pathToCLADocument) by adding a comment containing the below text. + [Developer Certificate of Origin](https://github.com/embeddedt/embeddium/blob/dco/DCO.txt) by adding a comment containing the below text. Otherwise, please close this PR." custom-pr-sign-comment: "I have read, and hereby affirm the entire contents of the Developer Certificate of Origin." custom-allsigned-prcomment: "All contributors have read and affirmed the entire contents of the Developer Certificate of Origin." From 7e7ca3f7768c72fe77989fac7b81673e1f6315fc Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Tue, 9 Apr 2024 18:49:35 -0400 Subject: [PATCH 06/11] Update CONTRIBUTING.md to mention DCO --- CONTRIBUTING.md | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 5122efbd4..c1c9cb0be 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,5 +1,11 @@ ## Contribution Guidelines +### Developer Certificate of Origin + +We require contributors to Embeddium to acknowledge they have followed the terms of the [Developer Certificate of Origin](https://github.com/embeddedt/embeddium/blob/dco/DCO.txt) when making contributions. This is a simple mechanism for us to ensure all contributions are appropriately licensed and attributed. + +When you open a PR, our CI will automatically check if you have previously affirmed the DCO, and if you have not, give you a prompt to read & acknowledge it. + ### Code Style When contributing source code to the project, ensure that you make consistent use of our code style guidelines. These @@ -51,4 +57,4 @@ style guidelines. If you're adding new Mixin patches to the project, please ensure that you have created appropriate entries to disable them in the config file. Mixins should always be self-contained and grouped into "patch sets" which are easy to isolate, -and where that is not possible, they should be placed into the "core" package. \ No newline at end of file +and where that is not possible, they should be placed into the "core" package. From 563668b6286c96be2c620784ad2a356a20fbc672 Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Thu, 11 Apr 2024 21:12:17 -0400 Subject: [PATCH 07/11] Workaround for world disappearing when walking into an unloaded chunk In this situation, we use the diamond spiral iterator used at the top/bottom of the world, and also disable the occlusion culler (because the results it produces are nonsensical when the camera isn't inside the graph) --- .../chunk/occlusion/OcclusionCuller.java | 23 ++++++++++++++----- 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/src/main/java/me/jellysquid/mods/sodium/client/render/chunk/occlusion/OcclusionCuller.java b/src/main/java/me/jellysquid/mods/sodium/client/render/chunk/occlusion/OcclusionCuller.java index a427aaa8f..d2405d738 100644 --- a/src/main/java/me/jellysquid/mods/sodium/client/render/chunk/occlusion/OcclusionCuller.java +++ b/src/main/java/me/jellysquid/mods/sodium/client/render/chunk/occlusion/OcclusionCuller.java @@ -12,12 +12,16 @@ import net.minecraft.world.level.Level; import org.jetbrains.annotations.NotNull; +import java.util.Objects; + public class OcclusionCuller { private final Long2ReferenceMap sections; private final Level world; private final DoubleBufferedQueue queue = new DoubleBufferedQueue<>(); + private boolean isCameraInUnloadedSection; + public OcclusionCuller(Long2ReferenceMap sections, Level world) { this.sections = sections; this.world = world; @@ -32,7 +36,11 @@ public void findVisible(Visitor visitor, final var queues = this.queue; queues.reset(); + this.isCameraInUnloadedSection = false; this.init(visitor, queues.write(), viewport, searchDistance, useOcclusionCulling, frame); + if(this.isCameraInUnloadedSection) { + useOcclusionCulling = false; + } while (queues.flip()) { processQueue(visitor, viewport, searchDistance, useOcclusionCulling, frame, queues.read(), queues.write()); @@ -197,11 +205,16 @@ private void init(Visitor visitor, if (origin.getY() < this.world.getMinSection()) { // below the world this.initOutsideWorldHeight(queue, viewport, searchDistance, frame, - this.world.getMinSection(), GraphDirection.DOWN); + this.world.getMinSection(), GraphDirectionSet.of(GraphDirection.DOWN)); } else if (origin.getY() >= this.world.getMaxSection()) { // above the world this.initOutsideWorldHeight(queue, viewport, searchDistance, frame, - this.world.getMaxSection() - 1, GraphDirection.UP); + this.world.getMaxSection() - 1, GraphDirectionSet.of(GraphDirection.UP)); + } else if(this.getRenderSection(origin.getX(), origin.getY(), origin.getZ()) == null) { + // inside the world height-wise, but in an unloaded section + this.initOutsideWorldHeight(queue, viewport, searchDistance, frame, + origin.getY(), GraphDirectionSet.of(GraphDirection.UP) | GraphDirectionSet.of(GraphDirection.DOWN)); + this.isCameraInUnloadedSection = true; } else { this.initWithinWorld(visitor, queue, viewport, useOcclusionCulling, frame); } @@ -211,9 +224,7 @@ private void initWithinWorld(Visitor visitor, WriteQueue queue, V var origin = viewport.getChunkCoord(); var section = this.getRenderSection(origin.getX(), origin.getY(), origin.getZ()); - if (section == null) { - return; - } + Objects.requireNonNull(section); section.setLastVisibleFrame(frame); section.setIncomingDirections(GraphDirectionSet.NONE); @@ -296,7 +307,7 @@ private void tryVisitNode(WriteQueue queue, int x, int y, int z, return; } - visitNode(queue, section, GraphDirectionSet.of(direction), frame); + visitNode(queue, section, direction, frame); } private RenderSection getRenderSection(int x, int y, int z) { From 0894d3fd91f28eb50495395c4b8062f388d235ef Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Fri, 12 Apr 2024 10:41:04 -0400 Subject: [PATCH 08/11] Fix license files not being added to jar --- build.gradle.kts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/build.gradle.kts b/build.gradle.kts index d089a8781..921ecb8e5 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -185,9 +185,7 @@ java { } tasks.jar { - from("LICENSE") { - rename { "${it}_${"archives_base_name"()}"} - } + from("COPYING", "COPYING.LESSER") from(sourceSets["compat"].output.classesDirs) from(sourceSets["compat"].output.resourcesDir) from(sourceSets["api"].output.classesDirs) From 7a9a7c16f4cd2fedd14d7059fde195724bf0519d Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Fri, 12 Apr 2024 17:27:22 -0400 Subject: [PATCH 09/11] Fix buffer uploads failing above a certain size due to integer overflow --- .../sodium/client/gl/arena/GlBufferArena.java | 24 +++++++++++++------ .../render/chunk/RenderSectionManager.java | 4 ++-- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/src/main/java/me/jellysquid/mods/sodium/client/gl/arena/GlBufferArena.java b/src/main/java/me/jellysquid/mods/sodium/client/gl/arena/GlBufferArena.java index f82c4db7d..96206f518 100644 --- a/src/main/java/me/jellysquid/mods/sodium/client/gl/arena/GlBufferArena.java +++ b/src/main/java/me/jellysquid/mods/sodium/client/gl/arena/GlBufferArena.java @@ -41,7 +41,7 @@ public GlBufferArena(CommandList commands, int initialCapacity, int stride, Stag this.head.setFree(true); this.arenaBuffer = commands.createMutableBuffer(); - commands.allocateStorage(this.arenaBuffer, this.capacity * stride, BUFFER_USAGE); + commands.allocateStorage(this.arenaBuffer, (long)this.capacity * stride, BUFFER_USAGE); this.stagingBuffer = stagingBuffer; } @@ -121,13 +121,13 @@ private void transferSegments(CommandList commandList, Collection getUsedSegments() { return used; } + @Deprecated public int getDeviceUsedMemory() { return this.used * this.stride; } + @Deprecated public int getDeviceAllocatedMemory() { return this.capacity * this.stride; } + public long getDeviceUsedMemoryL() { + return (long)this.used * this.stride; + } + + public long getDeviceAllocatedMemoryL() { + return (long)this.capacity * this.stride; + } + private GlBufferSegment alloc(int size) { GlBufferSegment a = this.findFree(size); @@ -306,7 +316,7 @@ private boolean tryUpload(CommandList commandList, PendingUpload upload) { } // Copy the data into our staging buffer, then copy it into the arena's buffer - this.stagingBuffer.enqueueCopy(commandList, data, this.arenaBuffer, dst.getOffset() * this.stride); + this.stagingBuffer.enqueueCopy(commandList, data, this.arenaBuffer, (long)dst.getOffset() * this.stride); upload.setResult(dst); @@ -316,7 +326,7 @@ private boolean tryUpload(CommandList commandList, PendingUpload upload) { public void ensureCapacity(CommandList commandList, int elementCount) { // Re-sizing the arena results in a compaction, so any free space in the arena will be // made into one contiguous segment, joined with the new segment of free space we're asking for - // We calculate the number of free elements in our arena and then subtract that from the total requested + // We calculate the number of free elements in our arena and then subtract that frozm the total requested int elementsNeeded = elementCount - (this.capacity - this.used); // Try to allocate some extra buffer space unless this is an unusually large allocation diff --git a/src/main/java/me/jellysquid/mods/sodium/client/render/chunk/RenderSectionManager.java b/src/main/java/me/jellysquid/mods/sodium/client/render/chunk/RenderSectionManager.java index ac6a4c61f..1817c13cb 100644 --- a/src/main/java/me/jellysquid/mods/sodium/client/render/chunk/RenderSectionManager.java +++ b/src/main/java/me/jellysquid/mods/sodium/client/render/chunk/RenderSectionManager.java @@ -646,8 +646,8 @@ public Collection getDebugStrings() { var buffer = resources.getGeometryArena(); - deviceUsed += buffer.getDeviceUsedMemory(); - deviceAllocated += buffer.getDeviceAllocatedMemory(); + deviceUsed += buffer.getDeviceUsedMemoryL(); + deviceAllocated += buffer.getDeviceAllocatedMemoryL(); count++; } From 09c6306ca99a24b2b0aca5198899cbdd24b66fe2 Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Fri, 12 Apr 2024 17:42:31 -0400 Subject: [PATCH 10/11] Fix arena growth being calculated with bytes instead of elements This caused excessively large arenas in some test scenarios, with most of the allocated space unused --- .../mods/sodium/client/gl/arena/GlBufferArena.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/me/jellysquid/mods/sodium/client/gl/arena/GlBufferArena.java b/src/main/java/me/jellysquid/mods/sodium/client/gl/arena/GlBufferArena.java index 96206f518..95b7ca2b8 100644 --- a/src/main/java/me/jellysquid/mods/sodium/client/gl/arena/GlBufferArena.java +++ b/src/main/java/me/jellysquid/mods/sodium/client/gl/arena/GlBufferArena.java @@ -277,9 +277,9 @@ public boolean upload(CommandList commandList, Stream stream) { // If we weren't able to upload some buffers, they will have been left behind in the queue if (!queue.isEmpty()) { // Calculate the amount of memory needed for the remaining uploads - int remainingElements = queue.stream() - .mapToInt(upload -> upload.getDataBuffer().getLength()) - .sum(); + int remainingElements = (int)(queue.stream() + .mapToLong(upload -> upload.getDataBuffer().getLength()) + .sum() / this.stride); // Ask the arena to grow to accommodate the remaining uploads // This will force a re-allocation and compaction, which will leave us a continuous free segment From ece7936967834fc663b6e0dc14ea286ff23b1700 Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Sat, 13 Apr 2024 19:00:06 -0400 Subject: [PATCH 11/11] Fix fluid renderer occluding when it shouldn't There are two bugs fixed by this commit: * scratchPos gets clobbered which causes isFaceSturdy to be invoked with the wrong position. Fixed by refactoring the method body. * It is not enough to check isFaceSturdy to determine that a fluid is occluded, the occlusion shape must be checked too --- .../chunk/compile/pipeline/FluidRenderer.java | 27 ++++++++++++++++--- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/src/main/java/me/jellysquid/mods/sodium/client/render/chunk/compile/pipeline/FluidRenderer.java b/src/main/java/me/jellysquid/mods/sodium/client/render/chunk/compile/pipeline/FluidRenderer.java index 98665545a..36d0a5886 100644 --- a/src/main/java/me/jellysquid/mods/sodium/client/render/chunk/compile/pipeline/FluidRenderer.java +++ b/src/main/java/me/jellysquid/mods/sodium/client/render/chunk/compile/pipeline/FluidRenderer.java @@ -32,6 +32,7 @@ import net.minecraft.tags.FluidTags; import net.minecraft.util.Mth; import net.minecraft.world.level.BlockAndTintGetter; +import net.minecraft.world.level.block.Block; import net.minecraft.world.level.block.SupportType; import net.minecraft.world.level.block.state.BlockState; import net.minecraft.world.level.material.Fluid; @@ -75,14 +76,32 @@ public FluidRenderer(ColorProviderRegistry colorProviderRegistry, LightPipelineP } private boolean isFluidOccluded(BlockAndTintGetter world, int x, int y, int z, Direction dir, Fluid fluid) { + // Check if the fluid adjacent to us in the given direction is the same + if (world.getFluidState(this.scratchPos.set(x + dir.getStepX(), y + dir.getStepY(), z + dir.getStepZ())).getType().isSame(fluid)) { + return true; + } + + // Stricter than vanilla: check whether the containing block can occlude, has a sturdy face on the given side, + // and has a solid occlusion shape. If so, assume the fluid inside is not visible on that side. + // This avoids rendering the top face of water inside an upper waterlogged slab, for instance. BlockPos pos = this.scratchPos.set(x, y, z); BlockState blockState = world.getBlockState(pos); - BlockPos adjPos = this.scratchPos.set(x + dir.getStepX(), y + dir.getStepY(), z + dir.getStepZ()); - if (blockState.canOcclude()) { - return world.getFluidState(adjPos).getType().isSame(fluid) || blockState.isFaceSturdy(world, pos, dir, SupportType.FULL); + if (!blockState.canOcclude() || !blockState.isFaceSturdy(world, pos, dir, SupportType.FULL)) { + return false; + } + + VoxelShape sideShape = blockState.getFaceOcclusionShape(world, pos, dir); + if (sideShape == Shapes.block()) { + // The face fills the 1x1 area, so the fluid is occluded + return true; + } else if (sideShape == Shapes.empty()) { + // The face does not exist, so the fluid is not occluded + return false; + } else { + // Check if the face fills the 1x1 area + return Block.isShapeFullBlock(sideShape); } - return world.getFluidState(adjPos).getType().isSame(fluid); } private boolean isSideExposed(BlockAndTintGetter world, int x, int y, int z, Direction dir, float height) {