Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support adding fewer than numItems via auto-trim() #44

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 45 additions & 4 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,18 @@ export default class Flatbush {
* @param {ArrayBuffer | SharedArrayBuffer} [data] (Only used internally)
*/
constructor(numItems, nodeSize = 16, ArrayType = Float64Array, ArrayBufferType = ArrayBuffer, data) {
this.init(numItems, nodeSize, ArrayType, ArrayBufferType, data);
Copy link
Author

@leeoniya leeoniya Apr 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this causes TS to be confused now about initializing various this.* things it expects to see in the constructor, not sure how to fix it.

honestly, i think having Flatbush and FlatQueue be ES6 classes does not help here. there's really no reason for either to be a class instead of a plain closure (which can still be used just fine with new or without. having constructor be special is problematic because it cannot be invoked without new like you can do with a plain init function. i dont think the polymorphism afforded by classes adds anything to this lib.

related discussion about "resetting" already-allocated class instances: https://stackoverflow.com/questions/47917721/pooling-with-javascript-classes

if you're interested, i can port Flatbush to TS and a closure fn instead of a class.

}

/**
* Create a Flatbush index that will hold a given number of items.
* @param {number} numItems
* @param {number} [nodeSize=16] Size of the tree node (16 by default).
* @param {TypedArrayConstructor} [ArrayType=Float64Array] The array type used for coordinates storage (`Float64Array` by default).
* @param {ArrayBufferConstructor | SharedArrayBufferConstructor} [ArrayBufferType=ArrayBuffer] The array buffer type used to store data (`ArrayBuffer` by default).
* @param {ArrayBuffer | SharedArrayBuffer} [data] (Only used internally)
*/
init(numItems, nodeSize = 16, ArrayType = Float64Array, ArrayBufferType = ArrayBuffer, data) {
if (numItems === undefined) throw new Error('Missing required argument: numItems.');
if (isNaN(numItems) || numItems <= 0) throw new Error(`Unexpected numItems value: ${numItems}.`);

Expand Down Expand Up @@ -103,6 +115,27 @@ export default class Flatbush {
this._queue = new FlatQueue();
}

/**
* Trim index to number of added rectangles.
*/
trim() {
const {_boxes, _indices, _pos, minX, minY, maxX, maxY, nodeSize, ArrayType} = this;
const numAdded = _pos >> 2;

if (numAdded < this.numItems) {
this.init(numAdded, nodeSize, ArrayType, this.data.constructor);

this._pos = _pos;
this.minX = minX;
this.minY = minY;
this.maxX = maxX;
this.maxY = maxY;

this._boxes.set(_boxes.slice(0, this._boxes.length));
this._indices.set(_indices.slice(0, this._indices.length));
}
}

/**
* Add a given rectangle to the index.
* @param {number} minX
Expand All @@ -128,11 +161,19 @@ export default class Flatbush {
return index;
}

/** Perform indexing of the added rectangles. */
finish() {
if (this._pos >> 2 !== this.numItems) {
throw new Error(`Added ${this._pos >> 2} items when expected ${this.numItems}.`);
/**
* Perform indexing of the added rectangles.
* @param {boolean} trim Whether to auto-trim index when number of added rectangles is less than numItems (`false` by default).
*/
finish(trim = false) {
const numAdded = this._pos >> 2;

if (numAdded < this.numItems && trim) {
this.trim();
} else if (numAdded !== this.numItems) {
throw new Error(`Added ${numAdded} items when expected ${this.numItems}.`);
}

const boxes = this._boxes;

if (this.numItems <= this.nodeSize) {
Expand Down
8 changes: 8 additions & 0 deletions test.js
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,14 @@ test('throws an error if added less items than the index size', () => {
});
});

test('does not throw an error if added less items than the index size and trim = true', () => {
assert.doesNotThrow(() => {
const index = new Flatbush(data.length / 4);
index.add(data[0], data[1], data[2], data[3]);
index.finish(true);
});
});

test('throws an error if searching before indexing', () => {
assert.throws(() => {
const index = new Flatbush(data.length / 4);
Expand Down