Skip to content

Fix AABB tree update/remove to prevent stale leaves, duplicate hits, and repeated-update crashes#15

Open
irondnb wants to merge 1 commit intocyberarm:masterfrom
irondnb:fix-AABB-tree-update
Open

Fix AABB tree update/remove to prevent stale leaves, duplicate hits, and repeated-update crashes#15
irondnb wants to merge 1 commit intocyberarm:masterfrom
irondnb:fix-AABB-tree-update

Conversation

@irondnb
Copy link
Copy Markdown

@irondnb irondnb commented Mar 27, 2026

While working on performance issues in my own Gosu games that use spatial partitioning, I used agentic AI to inspect the hot paths and bug-prone areas in collision structures. Since I’ve been happy with the results and have been following you, I wanted to check whether there were similar issues here as well.

This PR fixes two related problems in the AABB tree update path.

First, AABBNode#remove_subtree was not actually removing nodes in the normal case. Because of that, AABBTree#remove would leave stale leaves behind in the tree. When AABBTree#update removed and reinserted an object, the old leaf stayed in the structure and a new one was added. That caused duplicate search results and unnecessary tree growth over time.

Second, after AABBTree#update reinserted the leaf, it did not restore the object-to-leaf entry in @objects. That meant the first update could appear to work, but the second update for the same object failed because remove(object)
returned nil, which then caused a crash when trying to assign leaf.bounding_box.

What was happening before

  • Removing the last leaf did not properly clear the tree root.
  • Updating an object could leave the previous leaf in the tree.
  • Searches could return the same object multiple times after movement.
  • Repeated updates of the same object could raise NoMethodError.
  • Over time, moving objects could make the tree larger than it should be, which also hurts search performance.

What this PR changes

  • Fixes remove_subtree so deleting the current leaf returns nil when appropriate.
  • Preserves the existing sibling-promotion behavior when removing one child from a branch.
  • Updates AABBTree#update so the reinserted leaf is put back into @objects.
  • Duplicates the new bounding box on update to stay consistent with insert behavior.

Performance:

  • The tree stops accumulating duplicate leaves for moved objects.
  • Broad-phase queries do less unnecessary work.
  • Search cost stays closer to the intended structure size instead of growing because of leaked leaves.

In a small isolated reproduction, the broken version returned 2 hits for one moved object where the fixed version returned 1, and repeated search timing on that tiny case dropped from roughly 177.83 ms to 73.94 ms for 100k identical queries.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant