Fix infinite hang on degenerate splats: O(N²) KD-tree build + decimate stall guard#272
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a pathological KD-tree build-time degradation triggered by highly degenerate point sets (many/all points sharing coordinates) by replacing the nthElement 2-way partition with a 3-way (Dutch National Flag) partition, preventing O(N²) behavior and the observed “Finding nearest neighbors” hang. Adds targeted regression tests to ensure KD-tree construction and KNN queries remain correct and terminate quickly under heavy duplication/ties.
Changes:
- Update
nthElementinsrc/lib/spatial/kd-tree.tsto use an in-place 3-way partition to collapse equal elements in one pass. - Add
test/kd-tree.test.mjscovering the prior hang case (large all-identical set) and correctness on duplicate/tied coordinate sets.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| test/kd-tree.test.mjs | Adds regression + correctness tests for KD-tree behavior on duplicate/tied coordinates, including the prior hang scenario. |
| src/lib/spatial/kd-tree.ts | Replaces 2-way partition with 3-way partition in nthElement to avoid O(N²) degeneration on all-equal ranges. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
splat-transform --decimatehangs indefinitely on certain inputs, wedging the production GPU pipeline until its 2h timeout. Reproduced on a 9.84M-gaussian PLY where nearly every gaussian is coincident at the origin (degenerate/uninitialized capture).There are two independent failures, both triggered by the same degenerate input:
1. O(N²) KD-tree build hang
nthElementinsrc/lib/spatial/kd-tree.tsused a 2-way Lomuto partition that only moves elements strictly less than the pivot. When a coordinate is all-equal, nothing moves and the selection range shrinks by one per pass — each pass O(N) — so the KD-tree build degrades from O(N log N) to O(N²). At ~10M coincident points that never completes. The stall is on the CPU before any GPU work dispatches, which is why it reproduced on both WebGPU/Vulkan and Metal.Fix: 3-way (Dutch National Flag) partition. Equal elements collapse in a single pass, restoring O(N log N). For distinct coordinates the equal block is one element, so the median index and resulting tree are identical to before — no behavioural change for normal splats, no memory impact (in-place).
2. Decimate makes no progress on degenerate input
With every point coincident, all KNN distances are 0 and the traversal returns the same ~K shared neighbour indices for every query, so the directed-edge graph funnels onto a handful of targets. The greedy disjoint-pair matching can then only merge ~K pairs per pass (17, in the repro) instead of ~N/2, so it crawls toward the target across thousands of passes. The existing degenerate guards only fire on zero progress (
edgeCount === 0,pairs.length === 0), so a tiny-but-nonzero matching slips through.Fix: a no-progress guard. When a pass's matching is exhausted before reaching the target (
pairs.length < mergesNeeded) yet removes less thanMIN_ITERATION_PROGRESS(5%) of the current splats, throw a clear error rather than grind. A well-formed scene removes ~50% per exhausted pass, so legitimate multi-pass decimation never trips it; it's gated on matching exhaustion, so single-pass decimation can't trip it either.Testing
test/kd-tree.test.mjs: builds a tree over 100k coincident points (would hang pre-fix) in milliseconds; brute-force KNN correctness on sets with frequent ties.test/decimate.test.mjscase: a fully-coincident scene now throws the stall error instead of grinding.