mourner / rbush

RBush — a high-performance JavaScript R-tree-based 2D spatial index for points and rectangles
MIT License
2.48k stars 237 forks source link

Returns all items when bounds provided to search contains center 0,0 #129

Closed gitcatrat closed 1 year ago

gitcatrat commented 1 year ago

I'm using a standard scene coordinate system where 0,0 is center and coordinates go from -50,000 to 50,000 in both x and y. That means that some of the items have negative coordinates, some positives.

But I discovered a curious issue: if bounds for tree.search(bounds) "wrap" 0,0, search returns all the items.

I'll add my extension in case it could affect it but I doubt it:

class RTree extends RBush {
  toBBox(item) {
    return {
      minX: item.x,
      minY: item.y,
      maxX: item.x + item.width,
      maxY: item.y + item.height,
    }:
  }

  compareMinX(a, b) { return a.x - b.x; }
  compareMinY(a, b) { return a.y - b.y; }
}

Item structure:

const item = {
  x: -1000,
  y: -500,
  width: 100,
  height: 50,
  // a lot of irrelevant members
}
IvanSanchez commented 1 year ago

Can you publish a fiddle/codepen/plunkr that displays this behaviour?

gitcatrat commented 1 year ago

@IvanSanchez Thanks for the push to replicate it in external environment. It's not the library's fault, I had few async race conditions which resulted in falsey spatial data on first iteration.

https://codesandbox.io/s/upbeat-lamarr-n3luuw?file=/src/index.js

I can eliminate the issue but this example still raises one question: if inserted item doesn't have valid spatial data, tree.remove will not remove them. Is this expected behaviour? I thought it's deleted by reference by I guess there's some spatial searching going on before we can actually compare the references?

Output will change if you change item spatial data from null to undefined.

IvanSanchez commented 1 year ago

if inserted item doesn't have valid spatial data, tree.remove will not remove them. Is this expected behaviour?

rbush assumes that the items have spatial data. If they don't, then that's a case of GIGO. IIRC, rbush needs spatial data to store an item in a rtree leaf and look for it through the branches.