Fast entity bounds queries #3403
Conversation
Switch to using more standard `IEntityBounds`. Add more tests. Add proper bounds for scatter plot and rect plot entities. Fix r-tree split strategy balancing and verify with test. Comments.
Verify entitiesInBoundsDemo: quicktests | fiddle |
Better naming for entitiesIn methodsDemo: quicktests | fiddle |
| * Returns the entites whose bounding boxes overlap the parameter on the | ||
| * x-axis. | ||
| */ | ||
| public entitiesInXRange(queryBounds: IEntityBounds): Plots.IPlotEntity[] { |
There was a problem hiding this comment.
I'd prefer not to have separate range/xRange/yRange endpoints. We can do XRange by passing in Infinity for the y/height on the generic Bounds object. Maybe it'd make more sense for Bounds to be two Range objects
There was a problem hiding this comment.
nah, that'd just add another semantically equiv interface.
I think different endpoints is really helpful. It allows the user of the API to know that it'll "do what it says on the tin" as opposed to having to think about how the same method will re-interpret their arguments at runtime.
src/plots/plot.ts
Outdated
| /** | ||
| * Returns the entites whose bounding boxes overlap the parameter. | ||
| */ | ||
| public entitiesInBounds(queryBounds: IEntityBounds): Plots.IPlotEntity[] { |
There was a problem hiding this comment.
Nit: to be clear - this ignores the y/height properties on IEntityBounds? This API feels inconsistent with the existing entitiesIn(xRangeOrBounds: Range | Bounds, yRange?: Range): IPlotEntity[] { that bar, scatter, line, and rectangle have.
I think part of the issue is that we have overlapping responsibilities in our interfaces:
export type Range = {
min: number;
max: number;
};
/**
* A location in pixel-space.
*/
export type Point = {
x: number;
y: number;
};
/**
* The corners of a box.
*/
export type Bounds = {
topLeft: Point;
bottomRight: Point;
};
/**
* Client bounds
*/
export interface IEntityBounds {
x: number;
y: number;
width: number;
height: number;
}An IEntityBounds and a Bounds are semantically equivalent to each other, as well as equivalent to two Ranges.
There was a problem hiding this comment.
very true. i'm not sure why the original bounds was structured as two points, but i've always found it awkward.
There was a problem hiding this comment.
I'll take a look at replacing the existing entitiesIn methods to defer to the relevant entitystore method
|
|
||
| constructor() { | ||
| this._entities = []; | ||
| this._rtree = new RTree<T>(); | ||
| this._tree = d3.quadtree<T>() |
There was a problem hiding this comment.
nit: we can probably implement "nearest entity to point" with the rtree and get rid of the quadtree completely
There was a problem hiding this comment.
it's not exactly the same. the quadtree structure allows us to guarantee that no other point is closer based on the bounding box of the tree node. r-trees don't make the same guarantees, but they are great for querying overlapping bounds!
|
|
||
| /** | ||
| * R-Tree is a multidimensional spatial region tree. It stores entries that have | ||
| * arbitrarily overlapping bounding boxes and supports efficient point and |
There was a problem hiding this comment.
nit: can we document runtime and space complexity to be more precise with what "efficient" means
|
Now that entitiesIn uses the entitystore methods, the dragbox quicktest should demonstrate this feature. this one |
Add panzoom update callbacksDemo: quicktests | fiddle |
entitiesInBoundsand as well as x- and y- only versions.IEntityBounds.