Merge pull request #498 from EngineHub/feature/improved-chunk-batching

Buffered extent improvements

worldedit-core/src/main/java/com/sk89q/worldedit/command/tool/brush/GravityBrush.java

@@ -23,12 +23,13 @@ import com.sk89q.worldedit.EditSession;
 import com.sk89q.worldedit.MaxChangedBlocksException;
 import com.sk89q.worldedit.function.pattern.Pattern;
 import com.sk89q.worldedit.math.BlockVector3;
-import com.sk89q.worldedit.world.block.BlockState;
+import com.sk89q.worldedit.util.LocatedBlock;
+import com.sk89q.worldedit.util.collection.LocatedBlockList;
+import com.sk89q.worldedit.world.block.BaseBlock;
 import com.sk89q.worldedit.world.block.BlockTypes;
 
-import java.util.ArrayList;
+import java.util.LinkedHashSet;
-import java.util.Collections;
+import java.util.Set;
-import java.util.List;
 
 public class GravityBrush implements Brush {
 
@@ -40,27 +41,60 @@ public class GravityBrush implements Brush {
 
     @Override
     public void build(EditSession editSession, BlockVector3 position, Pattern pattern, double size) throws MaxChangedBlocksException {
-        final double startY = fullHeight ? editSession.getWorld().getMaxY() : position.getBlockY() + size;
+        double yMax = fullHeight ? editSession.getWorld().getMaxY() : position.getY() + size;
-        for (double x = position.getBlockX() + size; x > position.getBlockX() - size; --x) {
+        double yMin = Math.max(position.getY() - size, 0);
-            for (double z = position.getBlockZ() + size; z > position.getBlockZ() - size; --z) {
+        LocatedBlockList column = new LocatedBlockList();
-                double y = startY;
+        Set<BlockVector3> removedBlocks = new LinkedHashSet<>();
-                final List<BlockState> blockTypes = new ArrayList<>();
+        for (double x = position.getX() - size; x <= position.getX() + size; x++) {
-                for (; y > position.getBlockY() - size; --y) {
+            for (double z = position.getZ() - size; z <= position.getZ() + size; z++) {
-                    final BlockVector3 pt = BlockVector3.at(x, y, z);
+                /*
-                    final BlockState block = editSession.getBlock(pt);
+                 * Algorithm:
-                    if (!block.getBlockType().getMaterial().isAir()) {
+                 * 1. Find lowest air block in the selection -> $lowestAir = position
-                        blockTypes.add(block);
+                 * 2. Move the first non-air block above it down to $lowestAir
-                        editSession.setBlock(pt, BlockTypes.AIR.getDefaultState());
+                 * 3. Add 1 to $lowestAir's y-coord.
-                    }
+                 * 4. If more blocks above current position, repeat from 2
-                }
+                 */
+
+                BlockVector3 lowestAir = null;
+                for (double y = yMin; y <= yMax; y++) {
                     BlockVector3 pt = BlockVector3.at(x, y, z);
-                Collections.reverse(blockTypes);
+
-                for (int i = 0; i < blockTypes.size();) {
+                    BaseBlock block = editSession.getFullBlock(pt);
-                    if (editSession.getBlock(pt).getBlockType().getMaterial().isAir()) {
+
-                        editSession.setBlock(pt, blockTypes.get(i++));
+                    if (block.getBlockType().getMaterial().isAir()) {
+                        if (lowestAir == null) {
+                            // we found the lowest air block
+                            lowestAir = pt;
                         }
-                    pt = pt.add(0, 1, 0);
+                        continue;
                     }
+
+                    if (lowestAir == null) {
+                        // no place to move the block to
+                        continue;
+                    }
+
+                    BlockVector3 newPos = lowestAir;
+                    // we know the block above must be air,
+                    // since either this block is being moved into it,
+                    // or there has been more air before this block
+                    lowestAir = lowestAir.add(0, 1, 0);
+
+                    removedBlocks.remove(newPos);
+                    column.add(newPos, block);
+                    removedBlocks.add(pt);
+                }
+
+                for (LocatedBlock block : column) {
+                    editSession.setBlock(block.getLocation(), block.getBlock());
+                }
+
+                for (BlockVector3 removedBlock : removedBlocks) {
+                    editSession.setBlock(removedBlock, BlockTypes.AIR.getDefaultState());
+                }
+
+                column.clear();
+                removedBlocks.clear();
             }
         }
     }

worldedit-core/src/main/java/com/sk89q/worldedit/extent/AbstractBufferingExtent.java

@@ -0,0 +1,67 @@
+/*
+ * WorldEdit, a Minecraft world manipulation toolkit
+ * Copyright (C) sk89q <http://www.sk89q.com>
+ * Copyright (C) WorldEdit team and contributors
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU Lesser General Public License as published by the
+ * Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License
+ * for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+package com.sk89q.worldedit.extent;
+
+import com.sk89q.worldedit.WorldEditException;
+import com.sk89q.worldedit.math.BlockVector3;
+import com.sk89q.worldedit.world.block.BaseBlock;
+import com.sk89q.worldedit.world.block.BlockState;
+import com.sk89q.worldedit.world.block.BlockStateHolder;
+
+import java.util.Optional;
+
+/**
+ * Base extent class for buffering changes between {@link #setBlock(BlockVector3, BlockStateHolder)}
+ * and the delegate extent. This class ensures that {@link #getBlock(BlockVector3)} is properly
+ * handled, by returning buffered blocks.
+ */
+public abstract class AbstractBufferingExtent extends AbstractDelegateExtent {
+    /**
+     * Create a new instance.
+     *
+     * @param extent the extent
+     */
+    protected AbstractBufferingExtent(Extent extent) {
+        super(extent);
+    }
+
+    @Override
+    public abstract <T extends BlockStateHolder<T>> boolean setBlock(BlockVector3 location, T block) throws WorldEditException;
+
+    protected final <T extends BlockStateHolder<T>> boolean setDelegateBlock(BlockVector3 location, T block) throws WorldEditException {
+        return super.setBlock(location, block);
+    }
+
+    @Override
+    public BlockState getBlock(BlockVector3 position) {
+        return getBufferedBlock(position)
+            .map(BaseBlock::toImmutableState)
+            .orElseGet(() -> super.getBlock(position));
+    }
+
+    @Override
+    public BaseBlock getFullBlock(BlockVector3 position) {
+        return getBufferedBlock(position)
+            .orElseGet(() -> super.getFullBlock(position));
+    }
+
+    protected abstract Optional<BaseBlock> getBufferedBlock(BlockVector3 position);
+
+}

worldedit-core/src/main/java/com/sk89q/worldedit/extent/buffer/ExtentBuffer.java

@@ -21,16 +21,16 @@ package com.sk89q.worldedit.extent.buffer;
 
 import com.google.common.collect.Maps;
 import com.sk89q.worldedit.WorldEditException;
-import com.sk89q.worldedit.extent.AbstractDelegateExtent;
+import com.sk89q.worldedit.extent.AbstractBufferingExtent;
 import com.sk89q.worldedit.extent.Extent;
 import com.sk89q.worldedit.function.mask.Mask;
 import com.sk89q.worldedit.function.mask.Masks;
 import com.sk89q.worldedit.math.BlockVector3;
 import com.sk89q.worldedit.world.block.BaseBlock;
-import com.sk89q.worldedit.world.block.BlockState;
 import com.sk89q.worldedit.world.block.BlockStateHolder;
 
 import java.util.Map;
+import java.util.Optional;
 
 import static com.google.common.base.Preconditions.checkNotNull;
 
@@ -38,7 +38,7 @@ import static com.google.common.base.Preconditions.checkNotNull;
  * Buffers changes to an {@link Extent} and allows retrieval of the changed blocks,
  * without modifying the underlying extent.
  */
-public class ExtentBuffer extends AbstractDelegateExtent {
+public class ExtentBuffer extends AbstractBufferingExtent {
 
     private final Map<BlockVector3, BaseBlock> buffer = Maps.newHashMap();
     private final Mask mask;
@@ -67,23 +67,11 @@ public class ExtentBuffer extends AbstractDelegateExtent {
     }
 
     @Override
-    public BlockState getBlock(BlockVector3 position) {
+    protected Optional<BaseBlock> getBufferedBlock(BlockVector3 position) {
         if (mask.test(position)) {
-            return getOrDefault(position).toImmutableState();
+            return Optional.of(buffer.computeIfAbsent(position, (pos -> getExtent().getFullBlock(pos))));
         }
-        return super.getBlock(position);
+        return Optional.empty();
-    }
-
-    @Override
-    public BaseBlock getFullBlock(BlockVector3 position) {
-        if (mask.test(position)) {
-            return getOrDefault(position);
-        }
-        return super.getFullBlock(position);
-    }
-
-    private BaseBlock getOrDefault(BlockVector3 position) {
-        return buffer.computeIfAbsent(position, (pos -> getExtent().getFullBlock(pos)));
     }
 
     @Override

worldedit-core/src/main/java/com/sk89q/worldedit/extent/reorder/ChunkBatchingExtent.java

@@ -19,22 +19,25 @@
 
 package com.sk89q.worldedit.extent.reorder;
 
+import com.google.common.collect.Table;
+import com.google.common.collect.TreeBasedTable;
 import com.sk89q.worldedit.WorldEditException;
-import com.sk89q.worldedit.extent.AbstractDelegateExtent;
+import com.sk89q.worldedit.extent.AbstractBufferingExtent;
 import com.sk89q.worldedit.extent.Extent;
 import com.sk89q.worldedit.function.operation.Operation;
 import com.sk89q.worldedit.function.operation.RunContext;
-import com.sk89q.worldedit.function.operation.SetLocatedBlocks;
 import com.sk89q.worldedit.math.BlockVector2;
 import com.sk89q.worldedit.math.BlockVector3;
-import com.sk89q.worldedit.util.collection.LocatedBlockList;
+import com.sk89q.worldedit.world.block.BaseBlock;
 import com.sk89q.worldedit.world.block.BlockStateHolder;
 
 import java.util.Comparator;
+import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
-import java.util.SortedMap;
+import java.util.Map;
-import java.util.TreeMap;
+import java.util.Optional;
+import java.util.Set;
 
 /**
  * A special extent that batches changes into Minecraft chunks. This helps
@@ -42,17 +45,19 @@ import java.util.TreeMap;
  * loaded repeatedly, however it does take more memory due to caching the
  * blocks.
  */
-public class ChunkBatchingExtent extends AbstractDelegateExtent {
+public class ChunkBatchingExtent extends AbstractBufferingExtent {
 
     /**
      * Comparator optimized for sorting chunks by the region file they reside
      * in. This allows for file caches to be used while loading the chunk.
      */
     private static final Comparator<BlockVector2> REGION_OPTIMIZED_SORT =
-            Comparator.comparing((BlockVector2 vec) -> vec.divide(32), BlockVector2.COMPARING_GRID_ARRANGEMENT)
+            Comparator.comparing((BlockVector2 vec) -> vec.shr(5), BlockVector2.COMPARING_GRID_ARRANGEMENT)
                     .thenComparing(BlockVector2.COMPARING_GRID_ARRANGEMENT);
 
-    private final SortedMap<BlockVector2, LocatedBlockList> batches = new TreeMap<>(REGION_OPTIMIZED_SORT);
+    private final Table<BlockVector2, BlockVector3, BaseBlock> batches =
+        TreeBasedTable.create(REGION_OPTIMIZED_SORT, BlockVector3.sortByCoordsYzx());
+    private final Set<BlockVector3> containedBlocks = new HashSet<>();
     private boolean enabled;
 
     public ChunkBatchingExtent(Extent extent) {
@@ -76,16 +81,34 @@ public class ChunkBatchingExtent extends AbstractDelegateExtent {
         return enabled;
     }
 
+    private BlockVector2 getChunkPos(BlockVector3 location) {
+        return location.shr(4).toBlockVector2();
+    }
+
+    private BlockVector3 getInChunkPos(BlockVector3 location) {
+        return BlockVector3.at(location.getX() & 15, location.getY(), location.getZ() & 15);
+    }
+
     @Override
     public <B extends BlockStateHolder<B>> boolean setBlock(BlockVector3 location, B block) throws WorldEditException {
         if (!enabled) {
-            return getExtent().setBlock(location, block);
+            return setDelegateBlock(location, block);
         }
-        BlockVector2 chunkPos = BlockVector2.at(location.getBlockX() >> 4, location.getBlockZ() >> 4);
+        BlockVector2 chunkPos = getChunkPos(location);
-        batches.computeIfAbsent(chunkPos, k -> new LocatedBlockList()).add(location, block);
+        BlockVector3 inChunkPos = getInChunkPos(location);
+        batches.put(chunkPos, inChunkPos, block.toBaseBlock());
+        containedBlocks.add(location);
         return true;
     }
 
+    @Override
+    protected Optional<BaseBlock> getBufferedBlock(BlockVector3 position) {
+        if (!containedBlocks.contains(position)) {
+            return Optional.empty();
+        }
+        return Optional.of(batches.get(getChunkPos(position), getInChunkPos(position)));
+    }
+
     @Override
     protected Operation commitBefore() {
         if (!commitRequired()) {
@@ -94,17 +117,22 @@ public class ChunkBatchingExtent extends AbstractDelegateExtent {
         return new Operation() {
 
             // we get modified between create/resume -- only create this on resume to prevent CME
-            private Iterator<LocatedBlockList> batchIterator;
+            private Iterator<Map.Entry<BlockVector2, Map<BlockVector3, BaseBlock>>> batchIterator;
 
             @Override
             public Operation resume(RunContext run) throws WorldEditException {
                 if (batchIterator == null) {
-                    batchIterator = batches.values().iterator();
+                    batchIterator = batches.rowMap().entrySet().iterator();
                 }
                 if (!batchIterator.hasNext()) {
                     return null;
                 }
-                new SetLocatedBlocks(getExtent(), batchIterator.next()).resume(run);
+                Map.Entry<BlockVector2, Map<BlockVector3, BaseBlock>> next = batchIterator.next();
+                BlockVector3 chunkOffset = next.getKey().toBlockVector3().shl(4);
+                for (Map.Entry<BlockVector3, BaseBlock> block : next.getValue().entrySet()) {
+                    getExtent().setBlock(block.getKey().add(chunkOffset), block.getValue());
+                    containedBlocks.remove(block.getKey());
+                }
                 batchIterator.remove();
                 return this;
             }

worldedit-core/src/main/java/com/sk89q/worldedit/extent/reorder/MultiStageReorder.java

@@ -20,7 +20,7 @@
 package com.sk89q.worldedit.extent.reorder;
 
 import com.sk89q.worldedit.WorldEditException;
-import com.sk89q.worldedit.extent.AbstractDelegateExtent;
+import com.sk89q.worldedit.extent.AbstractBufferingExtent;
 import com.sk89q.worldedit.extent.Extent;
 import com.sk89q.worldedit.function.operation.Operation;
 import com.sk89q.worldedit.function.operation.OperationQueue;
@@ -36,13 +36,17 @@ import com.sk89q.worldedit.world.block.BlockTypes;
 
 import java.util.ArrayList;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.Set;
 
 /**
  * Re-orders blocks into several stages.
  */
-public class MultiStageReorder extends AbstractDelegateExtent implements ReorderingExtent {
+public class MultiStageReorder extends AbstractBufferingExtent implements ReorderingExtent {
 
     private static final Map<BlockType, PlacementPriority> priorityMap = new HashMap<>();
 
@@ -139,6 +143,7 @@ public class MultiStageReorder extends AbstractDelegateExtent implements Reorder
         priorityMap.put(BlockTypes.MOVING_PISTON, PlacementPriority.FINAL);
     }
 
+    private final Set<BlockVector3> containedBlocks = new HashSet<>();
     private Map<PlacementPriority, LocatedBlockList> stages = new HashMap<>();
 
     private boolean enabled;
@@ -212,7 +217,7 @@ public class MultiStageReorder extends AbstractDelegateExtent implements Reorder
     @Override
     public <B extends BlockStateHolder<B>> boolean setBlock(BlockVector3 location, B block) throws WorldEditException {
         if (!enabled) {
-            return super.setBlock(location, block);
+            return setDelegateBlock(location, block);
         }
 
         BlockState existing = getBlock(location);
@@ -240,9 +245,21 @@ public class MultiStageReorder extends AbstractDelegateExtent implements Reorder
         }
 
         stages.get(priority).add(location, block);
+        containedBlocks.add(location);
         return !existing.equalsFuzzy(block);
     }
 
+    @Override
+    protected Optional<BaseBlock> getBufferedBlock(BlockVector3 position) {
+        if (!containedBlocks.contains(position)) {
+            return Optional.empty();
+        }
+        return stages.values().stream()
+            .map(blocks -> blocks.get(position))
+            .filter(Objects::nonNull)
+            .findAny();
+    }
+
     @Override
     public Operation commitBefore() {
         if (!commitRequired()) {

worldedit-core/src/main/java/com/sk89q/worldedit/math/BlockVector2.java

@@ -19,7 +19,6 @@
 
 package com.sk89q.worldedit.math;
 
-import com.google.common.collect.ComparisonChain;
 import com.sk89q.worldedit.math.transform.AffineTransform;
 
 import java.util.Comparator;
@@ -48,12 +47,8 @@ public final class BlockVector2 {
      * cdef
      * </pre>
      */
-    public static final Comparator<BlockVector2> COMPARING_GRID_ARRANGEMENT = (a, b) -> {
+    public static final Comparator<BlockVector2> COMPARING_GRID_ARRANGEMENT =
-        return ComparisonChain.start()
+        Comparator.comparingInt(BlockVector2::getZ).thenComparingInt(BlockVector2::getX);
-                .compare(a.getBlockZ(), b.getBlockZ())
-                .compare(a.getBlockX(), b.getBlockX())
-                .result();
-    };
 
     public static BlockVector2 at(double x, double z) {
         return at((int) Math.floor(x), (int) Math.floor(z));
@@ -303,6 +298,27 @@ public final class BlockVector2 {
         return divide(n, n);
     }
 
+    /**
+     * Shift all components right.
+     *
+     * @param x the value to shift x by
+     * @param z the value to shift z by
+     * @return a new vector
+     */
+    public BlockVector2 shr(int x, int z) {
+        return at(this.x >> x, this.z >> z);
+    }
+
+    /**
+     * Shift all components right by {@code n}.
+     *
+     * @param n the value to shift by
+     * @return a new vector
+     */
+    public BlockVector2 shr(int n) {
+        return shr(n, n);
+    }
+
     /**
      * Get the length of the vector.
      *
@@ -532,5 +548,4 @@ public final class BlockVector2 {
     public String toString() {
         return "(" + x + ", " + z + ")";
     }
-
 }

worldedit-core/src/main/java/com/sk89q/worldedit/math/BlockVector3.java

@@ -19,13 +19,12 @@
 
 package com.sk89q.worldedit.math;
 
-import static com.google.common.base.Preconditions.checkArgument;
-
-import com.google.common.collect.ComparisonChain;
 import com.sk89q.worldedit.math.transform.AffineTransform;
 
 import java.util.Comparator;
 
+import static com.google.common.base.Preconditions.checkArgument;
+
 /**
  * An immutable 3-dimensional vector.
  */
@@ -64,13 +63,10 @@ public final class BlockVector3 {
 
     // thread-safe initialization idiom
     private static final class YzxOrderComparator {
-        private static final Comparator<BlockVector3> YZX_ORDER = (a, b) -> {
+        private static final Comparator<BlockVector3> YZX_ORDER =
-            return ComparisonChain.start()
+            Comparator.comparingInt(BlockVector3::getY)
-                    .compare(a.y, b.y)
+                .thenComparingInt(BlockVector3::getZ)
-                    .compare(a.z, b.z)
+                .thenComparingInt(BlockVector3::getX);
-                    .compare(a.x, b.x)
-                    .result();
-        };
     }
 
     /**
@@ -348,6 +344,50 @@ public final class BlockVector3 {
         return divide(n, n, n);
     }
 
+    /**
+     * Shift all components right.
+     *
+     * @param x the value to shift x by
+     * @param y the value to shift y by
+     * @param z the value to shift z by
+     * @return a new vector
+     */
+    public BlockVector3 shr(int x, int y, int z) {
+        return at(this.x >> x, this.y >> y, this.z >> z);
+    }
+
+    /**
+     * Shift all components right by {@code n}.
+     *
+     * @param n the value to shift by
+     * @return a new vector
+     */
+    public BlockVector3 shr(int n) {
+        return shr(n, n, n);
+    }
+
+    /**
+     * Shift all components left.
+     *
+     * @param x the value to shift x by
+     * @param y the value to shift y by
+     * @param z the value to shift z by
+     * @return a new vector
+     */
+    public BlockVector3 shl(int x, int y, int z) {
+        return at(this.x << x, this.y << y, this.z << z);
+    }
+
+    /**
+     * Shift all components left by {@code n}.
+     *
+     * @param n the value to shift by
+     * @return a new vector
+     */
+    public BlockVector3 shl(int n) {
+        return shl(n, n, n);
+    }
+
     /**
      * Get the length of the vector.
      *

worldedit-core/src/main/java/com/sk89q/worldedit/util/collection/LocatedBlockList.java

@@ -21,68 +21,72 @@ package com.sk89q.worldedit.util.collection;
 
 import static com.google.common.base.Preconditions.checkNotNull;
 
+import com.google.common.collect.ImmutableList;
 import com.sk89q.worldedit.math.BlockVector3;
 import com.sk89q.worldedit.util.LocatedBlock;
+import com.sk89q.worldedit.world.block.BaseBlock;
 import com.sk89q.worldedit.world.block.BlockStateHolder;
 
+import javax.annotation.Nullable;
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.Iterator;
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.ListIterator;
+import java.util.Map;
 
 /**
  * Wrapper around a list of blocks located in the world.
  */
 public class LocatedBlockList implements Iterable<LocatedBlock> {
 
-    private final List<LocatedBlock> list;
+    private final Map<BlockVector3, LocatedBlock> map = new LinkedHashMap<>();
 
     public LocatedBlockList() {
-        list = new ArrayList<>();
     }
 
     public LocatedBlockList(Collection<? extends LocatedBlock> collection) {
-        list = new ArrayList<>(collection);
+        for (LocatedBlock locatedBlock : collection) {
+            map.put(locatedBlock.getLocation(), locatedBlock);
+        }
     }
 
     public void add(LocatedBlock setBlockCall) {
         checkNotNull(setBlockCall);
-        list.add(setBlockCall);
+        map.put(setBlockCall.getLocation(), setBlockCall);
     }
 
     public <B extends BlockStateHolder<B>> void add(BlockVector3 location, B block) {
         add(new LocatedBlock(location, block.toBaseBlock()));
     }
 
+    public boolean containsLocation(BlockVector3 location) {
+        return map.containsKey(location);
+    }
+
+    public @Nullable BaseBlock get(BlockVector3 location) {
+        return map.get(location).getBlock();
+    }
+
     public int size() {
-        return list.size();
+        return map.size();
     }
 
     public void clear() {
-        list.clear();
+        map.clear();
     }
 
     @Override
     public Iterator<LocatedBlock> iterator() {
-        return list.iterator();
+        return map.values().iterator();
     }
 
     public Iterator<LocatedBlock> reverseIterator() {
-        return new Iterator<LocatedBlock>() {
+        List<LocatedBlock> data = new ArrayList<>(map.values());
-
+        Collections.reverse(data);
-            private final ListIterator<LocatedBlock> backingIterator = list.listIterator(list.size());
+        return data.iterator();
-
-            @Override
-            public boolean hasNext() {
-                return backingIterator.hasPrevious();
-            }
-
-            @Override
-            public LocatedBlock next() {
-                return backingIterator.previous();
-            }
-        };
     }
 
 }