KdTree::queryNode(): rewrite it to avoid blowing up the stack

[rouault]

Fixes qgis/QGIS#45226

While we are it, also avoid recursive formulation for queryNodePoint()

Could be worth backporting to 3.9 branch

[rouault]

JTS could benefit from this fix: https://github.com/locationtech/jts/blob/539ef272f20507cfd666f5c219f9af150b593604/modules/core/src/main/java/org/locationtech/jts/index/kdtree/KdTree.java#L285

[dr-jts]

Thanks for the patch. It's always better to have an iterative algorithm rather than a recursive one, to avoid blowing out the stack.

The geometry causing the problem is pretty crazy - a polygonal coverage with hundreds of thousands of mis-aligned vertices! Kdtrees built on coherent sequences of coordinates (such as occur in polygons) are often highly unbalanced, and I suspect this is what is going on in this case. Switching to using an explicit stack avoids issues with stack overflow, but unfortunately doesn't do anything to improve performance. Elsewhere in OverlayNG randomization has been used to avoid this problem, but not sure if that is possible in this algorithm.

[rouault]

and what about rebalancing the tree ?

[dr-jts]

My research indicated that balancing KD-trees is complex. I guess it might be possible to monitor for an unbalanced condition and rebuild the tree when it occurs. But there's going to be some trade-off with performance, which would require careful turning.

This is likely a rare situation, so given the complexity of code required it's low on my priority list.

[dr-jts]

Fixed in PR JTS-779.

Also includes some refactoring to clarify code. It would be good to port this back to GEOS.