From f771b0cf907c1457c71c9d9f580563645a2526af Mon Sep 17 00:00:00 2001 From: Pierre Maurice Schwang Date: Wed, 11 Sep 2024 22:27:12 +0200 Subject: [PATCH] fix: don't process out of bound section while trimming Y sections (#2902) * fix: don't process out of bound section while trimming Y sections * fix: handle upper sections * "fix" macos tests? * cleanup imports * update test case(s), fix upper bound(?) * chore: simplify trim logic --- .../core/queue/IBatchProcessor.java | 59 ++++---- .../core/queue/IBatchProcessorTest.java | 136 ++++++++++++++++++ 2 files changed, 166 insertions(+), 29 deletions(-) create mode 100644 worldedit-core/src/test/java/com/fastasyncworldedit/core/queue/IBatchProcessorTest.java diff --git a/worldedit-core/src/main/java/com/fastasyncworldedit/core/queue/IBatchProcessor.java b/worldedit-core/src/main/java/com/fastasyncworldedit/core/queue/IBatchProcessor.java index 4ba91a4f3..6b81b61f7 100644 --- a/worldedit-core/src/main/java/com/fastasyncworldedit/core/queue/IBatchProcessor.java +++ b/worldedit-core/src/main/java/com/fastasyncworldedit/core/queue/IBatchProcessor.java @@ -62,40 +62,41 @@ public interface IBatchProcessor { * @return false if chunk is empty of blocks */ default boolean trimY(IChunkSet set, int minY, int maxY, final boolean keepInsideRange) { - int minLayer = (minY - 1) >> 4; - int maxLayer = (maxY + 1) >> 4; + int minLayer = minY >> 4; + int maxLayer = maxY >> 4; if (keepInsideRange) { - for (int layer = set.getMinSectionPosition(); layer <= minLayer; layer++) { - if (set.hasSection(layer)) { - if (layer == minLayer) { - char[] arr = set.loadIfPresent(layer); - if (arr != null) { - int index = (minY & 15) << 8; - for (int i = 0; i < index; i++) { - arr[i] = BlockTypesCache.ReservedIDs.__RESERVED__; - } - set.setBlocks(layer, arr); - } - } else { - set.setBlocks(layer, null); + for (int layer = set.getMinSectionPosition(); layer <= set.getMaxSectionPosition(); layer++) { + if (!set.hasSection(layer)) { + continue; + } + // wipe all data from chunk layers above or below the max / min layer + if (layer < minLayer || layer > maxLayer) { + set.setBlocks(layer, null); + continue; + } + // if chunk layer / section is fully enclosed by minY to maxY, keep as is + if (layer > minLayer && layer < maxLayer) { + continue; + } + char[] blocks = set.loadIfPresent(layer); + if (blocks == null) { + continue; + } + // When on the minimum layer (as defined by minY), remove blocks up to minY (exclusive) + if (layer == minLayer) { + int index = (minY & 15) << 8; + for (int i = 0; i < index; i++) { + blocks[i] = BlockTypesCache.ReservedIDs.__RESERVED__; } } - } - for (int layer = maxLayer; layer <= set.getMaxSectionPosition(); layer++) { - if (set.hasSection(layer)) { - if (layer == maxLayer) { - char[] arr = set.loadIfPresent(layer); - if (arr != null) { - int index = ((maxY + 1) & 15) << 8; - for (int i = index; i < arr.length; i++) { - arr[i] = BlockTypesCache.ReservedIDs.__RESERVED__; - } - set.setBlocks(layer, arr); - } - } else { - set.setBlocks(layer, null); + // When on the maximum layer (as defined by maxY), remove blocks above maxY (exclusive) + if (layer == maxLayer) { + int index = ((maxY & 15) + 1) << 8; + for (int i = index; i < blocks.length; i++) { + blocks[i] = BlockTypesCache.ReservedIDs.__RESERVED__; } } + set.setBlocks(layer, blocks); } try { int layer = (minY - 15) >> 4; diff --git a/worldedit-core/src/test/java/com/fastasyncworldedit/core/queue/IBatchProcessorTest.java b/worldedit-core/src/test/java/com/fastasyncworldedit/core/queue/IBatchProcessorTest.java new file mode 100644 index 000000000..e28d0d236 --- /dev/null +++ b/worldedit-core/src/test/java/com/fastasyncworldedit/core/queue/IBatchProcessorTest.java @@ -0,0 +1,136 @@ +package com.fastasyncworldedit.core.queue; + +import com.sk89q.worldedit.extent.Extent; +import com.sk89q.worldedit.world.block.BlockTypesCache; +import org.jetbrains.annotations.Nullable; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.parallel.Isolated; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +import java.util.Arrays; +import java.util.stream.Stream; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.*; + +class IBatchProcessorTest { + + @Nested + @Isolated + class trimY { + + private static final char[] CHUNK_DATA = new char[16 * 16 * 16]; + private static final char[] SLICE_AIR = new char[16 * 16]; + private static final char[] SLICE_RESERVED = new char[16 * 16]; + private final IBatchProcessor processor = new NoopBatchProcessor(); + + static { + Arrays.fill(CHUNK_DATA, (char) BlockTypesCache.ReservedIDs.AIR); + Arrays.fill(SLICE_AIR, (char) BlockTypesCache.ReservedIDs.AIR); + Arrays.fill(SLICE_RESERVED, (char) BlockTypesCache.ReservedIDs.__RESERVED__); + } + + @ParameterizedTest + @MethodSource("provideTrimYInBoundsParameters") + void testFullChunkSelectedInBoundedRegion(int minY, int maxY, int minSection, int maxSection) { + final IChunkSet set = mock(); + + char[][] sections = new char[(320 + 64) >> 4][CHUNK_DATA.length]; + for (final char[] chars : sections) { + System.arraycopy(CHUNK_DATA, 0, chars, 0, CHUNK_DATA.length); + } + + when(set.getMinSectionPosition()).thenReturn(-64 >> 4); + when(set.getMaxSectionPosition()).thenReturn(319 >> 4); + when(set.hasSection(anyInt())).thenReturn(true); + when(set.loadIfPresent(anyInt())).thenAnswer(invocationOnMock -> sections[invocationOnMock.getArgument(0) + 4]); + doAnswer(invocationOnMock -> { + sections[invocationOnMock.getArgument(0) + 4] = invocationOnMock.getArgument(1); + return null; + }).when(set).setBlocks(anyInt(), any()); + + processor.trimY(set, minY, maxY, true); + + + for (int section = -64 >> 4; section < 320 >> 4; section++) { + int sectionIndex = section + 4; + char[] palette = sections[sectionIndex]; + if (section < minSection) { + assertNull(palette, "expected section below minimum section to be null"); + continue; + } + if (section > maxSection) { + assertNull(palette, "expected section above maximum section to be null"); + continue; + } + if (section == minSection) { + for (int slice = 0; slice < 16; slice++) { + boolean shouldContainBlocks = slice >= (minY % 16); + // If boundaries only span one section, the upper constraints have to be checked explicitly + if (section == maxSection) { + shouldContainBlocks &= slice <= (maxY % 16); + } + assertArrayEquals( + shouldContainBlocks ? SLICE_AIR : SLICE_RESERVED, + Arrays.copyOfRange(palette, slice << 8, (slice + 1) << 8), + ("[lower] slice %d (y=%d) expected to contain " + (shouldContainBlocks ? "air" : "nothing")) + .formatted(slice, ((section << 4) + slice)) + ); + } + continue; + } + if (section == maxSection) { + for (int slice = 0; slice < 16; slice++) { + boolean shouldContainBlocks = slice <= (maxY % 16); + assertArrayEquals( + shouldContainBlocks ? SLICE_AIR : SLICE_RESERVED, + Arrays.copyOfRange(palette, slice << 8, (slice + 1) << 8), + ("[upper] slice %d (y=%d) expected to contain " + (shouldContainBlocks ? "air" : "nothing")) + .formatted(slice, ((section << 4) + slice)) + ); + } + continue; + } + assertArrayEquals(CHUNK_DATA, palette, "full captured chunk @ %d should contain full data".formatted(section)); + } + + } + + /** + * Arguments explained: + * 1. minimum y coordinate (inclusive) + * 2. maximum y coordinate (inclusive) + * 3. chunk section which contains minimum y coordinate + * 4. chunk section which contains maximum y coordinate + */ + private static Stream provideTrimYInBoundsParameters() { + return Stream.of( + Arguments.of(64, 72, 4, 4), + Arguments.of(-64, 0, -4, 0), + Arguments.of(0, 128, 0, 8), + Arguments.of(16, 132, 1, 8), + Arguments.of(4, 144, 0, 9), + Arguments.of(12, 255, 0, 15), + Arguments.of(24, 103, 1, 6) + ); + } + + } + + private static final class NoopBatchProcessor implements IBatchProcessor { + + @Override + public IChunkSet processSet(final IChunk chunk, final IChunkGet get, final IChunkSet set) { + return set; + } + + @Override + public @Nullable Extent construct(final Extent child) { + return null; + } + + } + +}