diff --git a/modules/core/src/main/java/org/locationtech/jts/index/kdtree/KdNode.java b/modules/core/src/main/java/org/locationtech/jts/index/kdtree/KdNode.java index 92a0790d29..d260c74adf 100644 --- a/modules/core/src/main/java/org/locationtech/jts/index/kdtree/KdNode.java +++ b/modules/core/src/main/java/org/locationtech/jts/index/kdtree/KdNode.java @@ -13,6 +13,7 @@ package org.locationtech.jts.index.kdtree; import org.locationtech.jts.geom.Coordinate; +import org.locationtech.jts.geom.Envelope; /** * A node of a {@link KdTree}, which represents one or more points in the same location. @@ -74,6 +75,23 @@ public double getY() { return p.y; } + /** + * Gets the split value at a node, depending on + * whether the node splits on X or Y. + * The X (or Y) ordinates of all points in the left subtree + * are less than the split value, and those + * in the right subtree are greater than or equal to the split value. + * + * @param isSplitOnX whether the node splits on X or Y + * @return the splitting value + */ + public double splitValue(boolean isSplitOnX) { + if (isSplitOnX) { + return p.getX(); + } + return p.getY(); + } + /** * Returns the location of this node * @@ -141,4 +159,72 @@ void setLeft(KdNode _left) { void setRight(KdNode _right) { right = _right; } + + /** + * Tests whether the node's left subtree may contain values + * in a given range envelope. + * + * @param isSplitOnX whether the node splits on X or Y + * @param env the range envelope + * @return true if the left subtree is in range + */ + boolean isRangeOverLeft(boolean isSplitOnX, Envelope env) { + double envMin; + if ( isSplitOnX ) { + envMin = env.getMinX(); + } else { + envMin = env.getMinY(); + } + double splitValue = splitValue(isSplitOnX); + boolean isInRange = envMin < splitValue; + return isInRange; + } + + /** + * Tests whether the node's right subtree may contain values + * in a given range envelope. + * + * @param isSplitOnX whether the node splits on X or Y + * @param env the range envelope + * @return true if the right subtree is in range + */ + boolean isRangeOverRight(boolean isSplitOnX, Envelope env) { + double envMax; + if ( isSplitOnX ) { + envMax = env.getMaxX(); + } else { + envMax = env.getMaxY(); + } + double splitValue = splitValue(isSplitOnX); + boolean isInRange = splitValue <= envMax; + return isInRange; + } + + /** + * Tests whether a point is strictly to the left + * of the splitting plane for this node. + * If so it may be in the left subtree of this node, + * Otherwise, the point may be in the right subtree. + * The point is to the left if its X (or Y) ordinate + * is less than the split value. + * + * @param isSplitOnX whether the node splits on X or Y + * @param pt the query point + * @return true if the point is strictly to the left. + * + * @see #splitValue(boolean) + */ + boolean isPointOnLeft(boolean isSplitOnX, Coordinate pt) { + double ptOrdinate; + if (isSplitOnX) { + ptOrdinate = pt.x; + } + else { + ptOrdinate = pt.y; + } + double splitValue = splitValue(isSplitOnX); + boolean isInRange = (ptOrdinate < splitValue); + return isInRange; + } + } diff --git a/modules/core/src/main/java/org/locationtech/jts/index/kdtree/KdTree.java b/modules/core/src/main/java/org/locationtech/jts/index/kdtree/KdTree.java index f268e6bfca..1c9e54ffe9 100644 --- a/modules/core/src/main/java/org/locationtech/jts/index/kdtree/KdTree.java +++ b/modules/core/src/main/java/org/locationtech/jts/index/kdtree/KdTree.java @@ -16,35 +16,39 @@ import java.util.Collection; import java.util.Iterator; import java.util.List; +import java.util.Stack; import org.locationtech.jts.geom.Coordinate; import org.locationtech.jts.geom.CoordinateList; import org.locationtech.jts.geom.Envelope; - /** - * An implementation of a 2-D KD-Tree. KD-trees provide fast range searching - * and fast lookup for point data. + * An implementation of a + * KD-Tree + * over two dimensions (X and Y). + * KD-trees provide fast range searching and fast lookup for point data. + * The tree is built dynamically by inserting points. + * The tree supports queries by range and for point equality. + * For querying an internal stack is used instead of recursion to avoid overflow. *

* This implementation supports detecting and snapping points which are closer * than a given distance tolerance. * If the same point (up to tolerance) is inserted * more than once, it is snapped to the existing node. - * In other words, if a point is inserted which lies within the tolerance of a node already in the index, + * In other words, if a point is inserted which lies + * within the tolerance of a node already in the index, * it is snapped to that node. - * When a point is snapped to a node then a new node is not created but the count of the existing node - * is incremented. + * When an inserted point is snapped to a node then a new node is not created + * but the count of the existing node is incremented. * If more than one node in the tree is within tolerance of an inserted point, * the closest and then lowest node is snapped to. *

- * Note that the structure of a KD-Tree depends on the order of insertion of the points. - * A tree may become imbalanced if the inserted points are coherent + * The structure of a KD-Tree depends on the order of insertion of the points. + * A tree may become unbalanced if the inserted points are coherent * (e.g. monotonic in one or both dimensions). * A perfectly balanced tree has depth of only log2(N), - * but an imbalanced tree may be much deeper. + * but an unbalanced tree may be much deeper. * This has a serious impact on query efficiency. - * Even worse, since recursion is used for querying the tree - * an extremely deep tree may cause a {@link StackOverflowError}. * One solution to this is to randomize the order of points before insertion * (e.g. by using Fisher-Yates shuffling). * @@ -237,7 +241,7 @@ public void visit(KdNode node) { private KdNode insertExact(Coordinate p, Object data) { KdNode currentNode = root; KdNode leafNode = root; - boolean isOddLevel = true; + boolean isXLevel = true; boolean isLessThan = true; /** @@ -254,10 +258,11 @@ private KdNode insertExact(Coordinate p, Object data) { return currentNode; } - if (isOddLevel) { - isLessThan = p.x < currentNode.getX(); + double splitValue = currentNode.splitValue(isXLevel); + if (isXLevel) { + isLessThan = p.x < splitValue; } else { - isLessThan = p.y < currentNode.getY(); + isLessThan = p.y < splitValue; } leafNode = currentNode; if (isLessThan) { @@ -268,7 +273,7 @@ private KdNode insertExact(Coordinate p, Object data) { currentNode = currentNode.getRight(); } - isOddLevel = ! isOddLevel; + isXLevel = ! isXLevel; } //System.out.println("<<"); // no node found, add new leaf node to tree @@ -282,81 +287,67 @@ private KdNode insertExact(Coordinate p, Object data) { return node; } - private void queryNode(KdNode currentNode, - Envelope queryEnv, boolean odd, KdNodeVisitor visitor) { - if (currentNode == null) - return; - - double min; - double max; - double discriminant; - if (odd) { - min = queryEnv.getMinX(); - max = queryEnv.getMaxX(); - discriminant = currentNode.getX(); - } else { - min = queryEnv.getMinY(); - max = queryEnv.getMaxY(); - discriminant = currentNode.getY(); - } - boolean searchLeft = min < discriminant; - boolean searchRight = discriminant <= max; - - // search is computed via in-order traversal - if (searchLeft) { - queryNode(currentNode.getLeft(), queryEnv, !odd, visitor); - } - if (queryEnv.contains(currentNode.getCoordinate())) { - visitor.visit(currentNode); - } - if (searchRight) { - queryNode(currentNode.getRight(), queryEnv, !odd, visitor); - } - - } - - private KdNode queryNodePoint(KdNode currentNode, - Coordinate queryPt, boolean odd) { - if (currentNode == null) - return null; - if (currentNode.getCoordinate().equals2D(queryPt)) - return currentNode; - - double ord; - double discriminant; - if (odd) { - ord = queryPt.getX(); - discriminant = currentNode.getX(); - } else { - ord = queryPt.getY(); - discriminant = currentNode.getY(); - } - boolean searchLeft = ord < discriminant; - - if (searchLeft) { - return queryNodePoint(currentNode.getLeft(), queryPt, !odd); - } - else { - return queryNodePoint(currentNode.getRight(), queryPt, !odd); - } - } - /** * Performs a range search of the points in the index and visits all nodes found. * - * @param queryEnv - * the range rectangle to query + * @param queryEnv the range rectangle to query * @param visitor a visitor to visit all nodes found by the search */ public void query(Envelope queryEnv, KdNodeVisitor visitor) { - queryNode(root, queryEnv, true, visitor); + + Stack queryStack = new Stack(); + KdNode currentNode = root; + boolean isXLevel = true; + + // search is computed via in-order traversal + while (true) { + if ( currentNode != null ) { + queryStack.push(new QueryStackFrame(currentNode, isXLevel)); + + boolean searchLeft = currentNode.isRangeOverLeft(isXLevel, queryEnv); + if ( searchLeft ) { + currentNode = currentNode.getLeft(); + if ( currentNode != null ) { + isXLevel = ! isXLevel; + } + } + else { + currentNode = null; + } + } + else if ( ! queryStack.isEmpty() ) { + // currentNode is empty, so pop stack + QueryStackFrame frame = queryStack.pop(); + currentNode = frame.getNode(); + isXLevel = frame.isXLevel(); + + //-- check if search matches current node + if ( queryEnv.contains(currentNode.getCoordinate()) ) { + visitor.visit(currentNode); + } + + boolean searchRight = currentNode.isRangeOverRight(isXLevel, queryEnv); + if ( searchRight ) { + currentNode = currentNode.getRight(); + if ( currentNode != null ) { + isXLevel = ! isXLevel; + } + } + else { + currentNode = null; + } + } else { + //-- stack is empty and no current node + return; + } + } + } - + /** * Performs a range search of the points in the index. * - * @param queryEnv - * the range rectangle to query + * @param queryEnv the range rectangle to query * @return a list of the KdNodes found */ public List query(Envelope queryEnv) { @@ -374,7 +365,7 @@ public List query(Envelope queryEnv) { * a list to accumulate the result nodes into */ public void query(Envelope queryEnv, final List result) { - queryNode(root, queryEnv, true, new KdNodeVisitor() { + query(queryEnv, new KdNodeVisitor() { public void visit(KdNode node) { result.add(node); @@ -390,7 +381,23 @@ public void visit(KdNode node) { * @return the point node, if it is found in the index, or null if not */ public KdNode query(Coordinate queryPt) { - return queryNodePoint(root, queryPt, true); + KdNode currentNode = root; + boolean isXLevel = true; + + while (currentNode != null) { + if ( currentNode.getCoordinate().equals2D(queryPt) ) + return currentNode; + + boolean searchLeft = currentNode.isPointOnLeft(isXLevel, queryPt); + if ( searchLeft ) { + currentNode = currentNode.getLeft(); + } else { + currentNode = currentNode.getRight(); + } + isXLevel = ! isXLevel; + } + //-- point not found + return null; } /** @@ -428,4 +435,23 @@ private int sizeNode(KdNode currentNode) { int sizeR = sizeNode(currentNode.getRight()); return 1 + sizeL + sizeR; } + + static class QueryStackFrame { + private KdNode node; + private boolean isXLevel = false; + + public QueryStackFrame(KdNode node, boolean isXLevel) { + this.node = node; + this.isXLevel = isXLevel; + } + + public KdNode getNode() { + return node; + } + + public boolean isXLevel() { + return isXLevel; + } + + } } diff --git a/modules/core/src/test/java/test/jts/perf/index/KdtreeStressTest.java b/modules/core/src/test/java/test/jts/perf/index/KdtreeStressTest.java new file mode 100644 index 0000000000..ed1dbf940b --- /dev/null +++ b/modules/core/src/test/java/test/jts/perf/index/KdtreeStressTest.java @@ -0,0 +1,55 @@ +package test.jts.perf.index; + +import org.locationtech.jts.geom.Coordinate; +import org.locationtech.jts.geom.Envelope; +import org.locationtech.jts.index.kdtree.KdTree; + +/** + * Tests an issue where deep KdTrees caused a {@link StackOverflowError} + * when using a recursive query implementation. + * + * See a fix for this issue in GEOS + * at https://github.com/libgeos/geos/pull/481. + * + * @author mdavis + * + */ +public class KdtreeStressTest { + + // In code with recursive query 50,000 points causes StackOverflowError + int NUM_PTS = 50000; + + public static void main(String[] args) throws Exception + { + KdtreeStressTest test = new KdtreeStressTest(); + test.run(); + } + + private void run() { + System.out.format("Loading iIndex with %d points\n", NUM_PTS); + KdTree index = createUnbalancedTree(NUM_PTS); + + System.out.format("Querying Index loaded with %d points\n", NUM_PTS); + for (int i = 0; i < NUM_PTS; i++) { + Envelope env = new Envelope(i, i + 10, 0, 1); + index.query(env); + } + System.out.format("Queries complete\n"); + } + + /** + * Create an unbalanced tree by loading a + * series of monotonically increasing points + * + * @param numPts number of points to load + * @return a new index + */ + private KdTree createUnbalancedTree(int numPts) { + KdTree index = new KdTree(); + for (int i = 0; i < numPts; i++) { + Coordinate pt = new Coordinate(i, 0); + index.insert(pt); + } + return index; + } +}