Pass item bbox to filterFn in search
#68
Merged
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.
Closes #63. Closes #45. Passes the item's bounding box values to the
filterFnin thesearchmethod, enabling the following use case:I chose this approach over adding a separate method because it's a minimal, unobtrusive change, doesn't affect performance in any way, doesn't introduce much new code, closures or additional allocations.
The main drawback is that I didn't extend this logic to the
neighborsmethod, because the algorithm there is different — at the time where we iterate over items in the search and have access to bbox values, they're only pushed to a distance queue rather than added to results, so thefilterFnis 1) called in a different order, 2) not guaranteed that the item it is called on will end up in results.This produces an API inconsistency between how
searchandneighborshandlefilterFn. So far I couldn't come up with a performant way to pass bbox values correctly inneighborsso thatfilterFncan be used the same way as above; another issue is that it's easy to shoot yourself in the foot here because while it's fine to not return anything in the above example, inneighbors, if you don't returntrue(adding tomaxResultscount), it will never stop the search until it goes through all items in the index. I tried documenting this discrepancy in the readme, and hope it's a net positive change overall. What do you think @muendlein @bryevdv @leeoniya?