diff --git a/core/src/main/java/com/seibel/distanthorizons/core/render/LodQuadTree.java b/core/src/main/java/com/seibel/distanthorizons/core/render/LodQuadTree.java index d223b6d8e..20733ea17 100644 --- a/core/src/main/java/com/seibel/distanthorizons/core/render/LodQuadTree.java +++ b/core/src/main/java/com/seibel/distanthorizons/core/render/LodQuadTree.java @@ -78,6 +78,11 @@ public class LodQuadTree extends QuadTree implements IDebugRen */ private final ConcurrentLinkedQueue sectionsToReload = new ConcurrentLinkedQueue<>(); private final IDhClientLevel level; + /** + * Note: this doesn't lock all operations as some other threads/operations + * that may traverse the tree while it's being modified. + * IE {@link RenderBufferHandler} will walk through the tree each frame. + */ private final ReentrantLock treeLock = new ReentrantLock(); private ArrayList debugRenderSections = new ArrayList<>(); @@ -166,7 +171,8 @@ public class LodQuadTree extends QuadTree implements IDebugRen - // don't traverse the tree if it is being modified + // don't tick the tree if a modification is still going + // TODO is this lock necessary for anything beyond this tick method? if (this.treeLock.tryLock()) { // this shouldn't be updated while the tree is being iterated through @@ -508,28 +514,23 @@ public class LodQuadTree extends QuadTree implements IDebugRen continue; } - try + // the section only needs to be updated if a buffer is currently present + LodRenderSection renderSection = this.tryGetValue(pos); + if (renderSection != null) { - // the section only needs to be updated if a buffer is currently present - LodRenderSection renderSection = this.getValue(pos); - if (renderSection != null) + if (renderSection.canRender()) { - if (renderSection.canRender()) + if (renderSection.gpuUploadInProgress() + || !renderSection.uploadRenderDataToGpuAsync()) { - if (renderSection.gpuUploadInProgress() - || !renderSection.uploadRenderDataToGpuAsync()) - { - // if a section is already loading or failed to start upload - // we need to wait to trigger it again - // if we don't trigger it again the LOD will be out of date - // and may be invisible/missing - positionsToRequeue.add(pos); - } + // if a section is already loading or failed to start upload + // we need to wait to trigger it again + // if we don't trigger it again the LOD will be out of date + // and may be invisible/missing + positionsToRequeue.add(pos); } } } - catch (IndexOutOfBoundsException e) - { /* the section is now out of bounds, it doesn't need to be reloaded */ } } this.sectionsToReload.addAll(positionsToRequeue); } diff --git a/core/src/main/java/com/seibel/distanthorizons/core/util/objects/quadTree/QuadTree.java b/core/src/main/java/com/seibel/distanthorizons/core/util/objects/quadTree/QuadTree.java index c61e3f868..bc020291c 100644 --- a/core/src/main/java/com/seibel/distanthorizons/core/util/objects/quadTree/QuadTree.java +++ b/core/src/main/java/com/seibel/distanthorizons/core/util/objects/quadTree/QuadTree.java @@ -31,12 +31,10 @@ import com.seibel.distanthorizons.coreapi.util.MathUtil; import com.seibel.distanthorizons.core.util.gridList.MovableGridRingList; import it.unimi.dsi.fastutil.longs.LongArrayFIFOQueue; import it.unimi.dsi.fastutil.longs.LongIterator; -import com.seibel.distanthorizons.core.logging.DhLogger; import org.jetbrains.annotations.Nullable; import java.util.*; import java.util.function.Consumer; -import java.util.function.Function; import java.util.function.LongConsumer; /** @@ -98,9 +96,27 @@ public class QuadTree // getters and setters // //=====================// + /** @return the value at the given section position. Null will be returned if the value is missing or the position is out of bounds. */ + @Nullable + public final T tryGetValue(long pos) + { + QuadNode node = this.tryGetNode(pos); + if (node != null) + { + return node.value; + } + return null; + } + + /** @return the node at the given section position, null if out of bounds */ + @Nullable + public final QuadNode tryGetNode(long pos) { return this.getOrSetNode(pos, false, null, false); } + + /** @return the node at the given section position */ @Nullable public final QuadNode getNode(long pos) throws IndexOutOfBoundsException { return this.getOrSetNode(pos, false, null, true); } + /** @return the value at the given section position */ @Nullable public final T getValue(long pos) throws IndexOutOfBoundsException @@ -122,16 +138,24 @@ public class QuadTree return previousValue; } - /** @param runBoundaryChecks should only ever be set to true internally for removing out of bound nodes */ + /** @param throwIfOutOfBounds if false returns null */ @Nullable - protected final QuadNode getOrSetNode(long pos, boolean setNewValue, T newValue, boolean runBoundaryChecks) throws IndexOutOfBoundsException + protected final QuadNode getOrSetNode(long pos, boolean setNewValue, T newValue, boolean throwIfOutOfBounds) throws IndexOutOfBoundsException { - if (runBoundaryChecks && !this.isSectionPosInBounds(pos)) + if (!this.isSectionPosInBounds(pos)) { - int radius = this.diameterInBlocks() / 2; - DhBlockPos2D minPos = this.getCenterBlockPos().add(new DhBlockPos2D(-radius, -radius)); - DhBlockPos2D maxPos = this.getCenterBlockPos().add(new DhBlockPos2D(radius, radius)); - throw new IndexOutOfBoundsException("QuadTree GetOrSet failed. Position out of bounds, min pos: " + minPos + ", max pos: " + maxPos + ", min detail level: " + this.treeLeafDetailLevel + ", max detail level: " + this.treeRootDetailLevel + ". Given Position: [" + DhSectionPos.toString(pos) + "] = block pos: " + DhSectionPos.convertToDetailLevel(pos, LodUtil.BLOCK_DETAIL_LEVEL)); + // how should out-of-bounds positions be handled? + if (throwIfOutOfBounds) + { + int radius = this.diameterInBlocks() / 2; + DhBlockPos2D minBlockPos = this.getCenterBlockPos().add(new DhBlockPos2D(-radius, -radius)); + DhBlockPos2D maxBlockPos = this.getCenterBlockPos().add(new DhBlockPos2D(radius, radius)); + throw new IndexOutOfBoundsException("QuadTree GetOrSet failed. Position out of bounds, min block pos: [" + minBlockPos + "], max block pos: [" + maxBlockPos + "], leaf detail level: [" + this.treeLeafDetailLevel + "], root detail level: [" + this.treeRootDetailLevel + "]. Requested section pos: [" + DhSectionPos.toString(pos) + "]."); + } + else + { + return null; + } } @@ -278,46 +302,6 @@ public class QuadTree removedItemConsumer.accept(quadNode.value); } }); - - -// // remove out of bound nodes and clean up empty nodes -// // Note: this will iterate over a lot of unnecessary nodes, hopefully speed won't be an issue -// Iterator rootNodePosIterator = this.rootNodePosIterator(); -// while (rootNodePosIterator.hasNext()) -// { -// // get the root node (regular nodeIterators won't return them if they are out of bounds) -// DhSectionPos rootPos = rootNodePosIterator.next(); -// QuadNode rootNode = this.getOrSetNode(rootPos, false, null, false); -// if (rootNode == null) -// { -// continue; -// } -// -// // remove any child nodes that are out of bounds -// Iterator> nodeIterator = this.nodeIterator(); -// while (nodeIterator.hasNext()) -// { -// QuadNode node = nodeIterator.next(); -// if(!this.isSectionPosInBounds(node.sectionPos)) -// { -// // node is out of bounds -// -// // FIXME(?) this appears to potentially return large nodes that are partially or entirely in bounds -// -// if (node.getNonNullChildCount() == 0) -// { -// // no child nodes, can be safely removed -// nodeIterator.remove(); -// } -// else -// { -// // node can't be removed, but its value can be set to null -// node.value = null; -// } -// } -// } -// } - } public final DhBlockPos2D getCenterBlockPos() { return this.centerBlockPos; } @@ -544,7 +528,9 @@ public class QuadTree && this.rootNodeIterator.hasNext()) { long sectionPos = this.rootNodeIterator.nextLong(); - QuadNode rootNode = QuadTree.this.getNode(sectionPos); + + // try-get to prevent concurrency errors if the tree is being moved while we walk through it + QuadNode rootNode = QuadTree.this.tryGetNode(sectionPos); if (rootNode != null) { nodeIterator = this.onlyReturnLeaves ? rootNode.getLeafNodeIterator() : rootNode.getNodeIterator(this.stopIteratingFunc); diff --git a/core/src/main/java/com/seibel/distanthorizons/core/util/objects/quadTree/iterators/QuadTreeNodeIterator.java b/core/src/main/java/com/seibel/distanthorizons/core/util/objects/quadTree/iterators/QuadTreeNodeIterator.java index 09b3d5691..68c3a4fbc 100644 --- a/core/src/main/java/com/seibel/distanthorizons/core/util/objects/quadTree/iterators/QuadTreeNodeIterator.java +++ b/core/src/main/java/com/seibel/distanthorizons/core/util/objects/quadTree/iterators/QuadTreeNodeIterator.java @@ -30,7 +30,7 @@ import java.util.function.Consumer; public class QuadTreeNodeIterator implements Iterator> { /** lowest numerical value, inclusive */ - private final byte highestDetailLevel; + private final byte leafDetailLevel; private final Queue> validNodesForDetailLevel = new ArrayDeque<>(); @@ -48,8 +48,7 @@ public class QuadTreeNodeIterator implements Iterator> { this.onlyReturnLeafValues = onlyReturnLeafValues; this.stopIteratingFunc = stopIteratingFunc; - // TODO the naming conversion for these are flipped in a lot of places - this.highestDetailLevel = rootNode.parentTreeLeafDetailLevel; + this.leafDetailLevel = rootNode.parentTreeLeafDetailLevel; this.iteratorDetailLevel = DhSectionPos.getDetailLevel(rootNode.sectionPos); @@ -110,9 +109,9 @@ public class QuadTreeNodeIterator implements Iterator> @Override public QuadNode next() { - if (this.iteratorDetailLevel < this.highestDetailLevel) + if (this.iteratorDetailLevel < this.leafDetailLevel) { - throw new NoSuchElementException("Highest detail level reached [" + this.highestDetailLevel + "]."); + throw new NoSuchElementException("Leaf detail level reached [" + this.leafDetailLevel + "]."); } if (this.iteratorNodeQueue.size() == 0) { @@ -133,7 +132,7 @@ public class QuadTreeNodeIterator implements Iterator> this.iteratorDetailLevel--; // only continue if we can go down farther - if (this.iteratorDetailLevel >= this.highestDetailLevel) + if (this.iteratorDetailLevel >= this.leafDetailLevel) { Queue> parentNodes = new LinkedList<>(this.validNodesForDetailLevel); this.validNodesForDetailLevel.clear();