Skip to content

Commit

Permalink
Update the _preventCoincident functions, use 'rbush' consistently
Browse files Browse the repository at this point in the history 
(closes #1516)

- This adds `_preventCoincident` to the Mapillary detections to avoid
  markers appearing on top of one another.
- Also simplifies the `_preventCoincident` function that we use in
  various places (Mapillary, Osmose, KeepRight) - it just moves down now.
- Also consistently use the word "rbush" for these spatial caches.
  Previously, our code had a mix of "rbush" and "rtree" in different places.
  But the library we use is RBush, so we will call it that.
  • Loading branch information
@bhousel
bhousel committed Aug 18, 2024
1 parent 7357a58 commit 4af8069
Show file tree
Hide file tree
Showing 12 changed files with 165 additions and 130 deletions.
6 changes: 3 additions & 3 deletions modules/services/GeoScribbleService.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ export class GeoScribbleService extends AbstractSystem {
shapes: {},
loadedTile: {},
inflightTile: {},
rtree: new RBush()
rbush: new RBush()
};

return Promise.resolve();
Expand All @@ -88,7 +88,7 @@ export class GeoScribbleService extends AbstractSystem {
*/
getData() {
const extent = this.context.viewport.visibleExtent();
return this._cache.rtree.search(extent.bbox()).map(d => d.data);
return this._cache.rbush.search(extent.bbox()).map(d => d.data);
}


Expand Down Expand Up @@ -143,7 +143,7 @@ export class GeoScribbleService extends AbstractSystem {

const box = geojsonExtent(shape).bbox();
box.data = shape;
cache.rtree.insert(box);
cache.rbush.insert(box);
}

this.context.deferredRedraw();
Expand Down
18 changes: 9 additions & 9 deletions modules/services/KartaviewService.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ export class KartaviewService extends AbstractSystem {
nextPage: new Map(), // Map(tileID, Number)
images: new Map(), // Map(imageID, image data)
sequences: new Map(), // Map(sequenceID, sequence data)
rtree: new RBush()
rbush: new RBush()
};

this._selectedImage = null;
Expand All @@ -164,7 +164,7 @@ export class KartaviewService extends AbstractSystem {
*/
getImages() {
const extent = this.context.viewport.visibleExtent();
return this._cache.rtree.search(extent.bbox()).map(d => d.data);
return this._cache.rbush.search(extent.bbox()).map(d => d.data);
}


Expand All @@ -178,7 +178,7 @@ export class KartaviewService extends AbstractSystem {
const sequenceIDs = new Set();

// Gather sequences for images in viewport
for (const box of this._cache.rtree.search(extent.bbox())) {
for (const box of this._cache.rbush.search(extent.bbox())) {
if (box.data.sequenceID) {
sequenceIDs.add(box.data.sequenceID);
}
Expand Down Expand Up @@ -477,13 +477,13 @@ export class KartaviewService extends AbstractSystem {
sequence.images[image.sequenceIndex] = image;
sequence.v++;

// Add to rtree, replacing existing if needed.
// Add to rbush, replacing existing if needed.
const box = { minX: loc[0], minY: loc[1], maxX: loc[0], maxY: loc[1], data: image };
cache.rtree.remove(box, (a, b) => a.data.id === b.data.id);
cache.rbush.remove(box, (a, b) => a.data.id === b.data.id);
boxes.push(box);
}

cache.rtree.load(boxes); // bulk load
cache.rbush.load(boxes); // bulk load

context.deferredRedraw();
this.emit('loadedData');
Expand Down Expand Up @@ -574,10 +574,10 @@ export class KartaviewService extends AbstractSystem {
sequence.images[image.sequenceIndex] = image;
sequence.v++;

// Add to rtree, replacing existing if needed.
// Add to rbush, replacing existing if needed.
const box = { minX: loc[0], minY: loc[1], maxX: loc[0], maxY: loc[1], data: image };
cache.rtree.remove(box, (a, b) => a.data.id === b.data.id);
cache.rtree.insert(box);
cache.rbush.remove(box, (a, b) => a.data.id === b.data.id);
cache.rbush.insert(box);

context.deferredRedraw();
this.emit('loadedData');
Expand Down
51 changes: 30 additions & 21 deletions modules/services/KeepRightService.js
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Extent, Tiler, vecAdd} from '@rapid-sdk/math';
import { Tiler, vecSubtract } from '@rapid-sdk/math';
import { utilQsString } from '@rapid-sdk/util';
import RBush from 'rbush';

Expand Down Expand Up @@ -108,7 +108,7 @@ export class KeepRightService extends AbstractSystem {
inflightTile: {},
inflightPost: {},
closed: {},
rtree: new RBush()
rbush: new RBush()
};

this._lastv = null;
Expand All @@ -124,7 +124,7 @@ export class KeepRightService extends AbstractSystem {
*/
getData() {
const extent = this.context.viewport.visibleExtent();
return this._cache.rtree.search(extent.bbox()).map(d => d.data);
return this._cache.rbush.search(extent.bbox()).map(d => d.data);
}


Expand Down Expand Up @@ -223,16 +223,7 @@ export class KeepRightService extends AbstractSystem {
break;
}

// move markers slightly so it doesn't obscure the geometry,
// then move markers away from other coincident markers
let coincident = false;
do {
// first time, move marker up. after that, move marker right.
let delta = coincident ? [0.00001, 0] : [0, 0.00001];
loc = vecAdd(loc, delta);
let bbox = new Extent(loc).bbox();
coincident = this._cache.rtree.search(bbox).length;
} while (coincident);
loc = this._preventCoincident(this._cache.rbush, loc);

const d = new QAItem(this, itemType, id, {
loc: loc,
Expand All @@ -250,7 +241,7 @@ export class KeepRightService extends AbstractSystem {
d.replacements = this._tokenReplacements(d);

this._cache.data[id] = d;
this._cache.rtree.insert(this._encodeIssueRtree(d));
this._cache.rbush.insert(this._encodeIssueRBush(d));
}

this.context.deferredRedraw();
Expand Down Expand Up @@ -348,7 +339,7 @@ export class KeepRightService extends AbstractSystem {
if (!(item instanceof QAItem) || !item.id) return null;

this._cache.data[item.id] = item;
this._updateRtree(this._encodeIssueRtree(item), true); // true = replace
this._updateRBush(this._encodeIssueRBush(item), true); // true = replace
return item;
}

Expand All @@ -362,7 +353,7 @@ export class KeepRightService extends AbstractSystem {
if (!(item instanceof QAItem) || !item.id) return;

delete this._cache.data[item.id];
this._updateRtree(this._encodeIssueRtree(item), false); // false = remove
this._updateRBush(this._encodeIssueRBush(item), false); // false = remove
}


Expand Down Expand Up @@ -404,17 +395,35 @@ export class KeepRightService extends AbstractSystem {
});
}

_encodeIssueRtree(d) {
_encodeIssueRBush(d) {
return { minX: d.loc[0], minY: d.loc[1], maxX: d.loc[0], maxY: d.loc[1], data: d };
}


// Replace or remove QAItem from rtree
_updateRtree(item, replace) {
this._cache.rtree.remove(item, (a, b) => a.data.id === b.data.id);
// Replace or remove QAItem from rbush
_updateRBush(item, replace) {
this._cache.rbush.remove(item, (a, b) => a.data.id === b.data.id);

if (replace) {
this._cache.rtree.insert(item);
this._cache.rbush.insert(item);
}
}


/**
* _preventCoincident
* This checks if the cache already has something at that location, and if so, moves down slightly.
* @param {RBush} rbush - the spatial cache to check
* @param {Array<number>} loc - original [longitude,latitude] coordinate
* @return {Array<number>} Adjusted [longitude,latitude] coordinate
*/
_preventCoincident(rbush, loc) {
for (let dy = 0; ; dy++) {
loc = vecSubtract(loc, [0, dy * 0.00001]);
const box = { minX: loc[0], minY: loc[1], maxX: loc[0], maxY: loc[1] };
if (!rbush.collides(box)) {
return loc;
}
}
}

Expand Down
41 changes: 22 additions & 19 deletions modules/services/MapRouletteService.js
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Extent, Tiler, vecAdd } from '@rapid-sdk/math';
import { Tiler, vecSubtract } from '@rapid-sdk/math';
import RBush from 'rbush';

import { AbstractSystem } from '../core/AbstractSystem';
Expand Down Expand Up @@ -188,12 +188,12 @@ export class MapRouletteService extends AbstractSystem {

task.id = taskID; // force to string
task.parentId = challengeID; // force to string
task.loc = this._preventCoincident([task.point.lng, task.point.lat]);
task.loc = this._preventCoincident(cache.rbush, [task.point.lng, task.point.lat]);

// save the task
const d = new QAItem(this, null, taskID, task);
cache.tasks.set(taskID, d);
cache.rbush.insert(this._encodeIssueRbush(d));
cache.rbush.insert(this._encodeIssueRBush(d));
}

this.loadChallenges(); // call this sometimes
Expand Down Expand Up @@ -407,7 +407,7 @@ export class MapRouletteService extends AbstractSystem {
if (!(task instanceof QAItem) || !task.id) return;

this._cache.tasks.set(task.id, task);
this._updateRbush(this._encodeIssueRbush(task), true); // true = replace
this._updateRBush(this._encodeIssueRBush(task), true); // true = replace
return task;
}

Expand All @@ -420,7 +420,7 @@ export class MapRouletteService extends AbstractSystem {
removeTask(task) {
if (!(task instanceof QAItem) || !task.id) return;
this._cache.tasks.delete(task.id);
this._updateRbush(this._encodeIssueRbush(task), false);
this._updateRBush(this._encodeIssueRBush(task), false);
}


Expand Down Expand Up @@ -458,31 +458,34 @@ export class MapRouletteService extends AbstractSystem {
}


_encodeIssueRbush(d) {
_encodeIssueRBush(d) {
return { minX: d.loc[0], minY: d.loc[1], maxX: d.loc[0], maxY: d.loc[1], data: d };
}


// Replace or remove Task from rbush
_updateRbush(task, replace) {
_updateRBush(task, replace) {
this._cache.rbush.remove(task, (a, b) => a.data.id === b.data.id);
if (replace) {
this._cache.rbush.insert(task);
}
}


// Markers shouldn't obscure each other
_preventCoincident(loc) {
let coincident = false;
do {
// first time, move marker up. after that, move marker right.
let delta = coincident ? [0.00001, 0] : [0, 0.00001];
loc = vecAdd(loc, delta);
const bbox = new Extent(loc).bbox();
coincident = this._cache.rbush.search(bbox).length;
} while (coincident);

return loc;
/**
* _preventCoincident
* This checks if the cache already has something at that location, and if so, moves down slightly.
* @param {RBush} rbush - the spatial cache to check
* @param {Array<number>} loc - original [longitude,latitude] coordinate
* @return {Array<number>} Adjusted [longitude,latitude] coordinate
*/
_preventCoincident(rbush, loc) {
for (let dy = 0; ; dy++) {
loc = vecSubtract(loc, [0, dy * 0.00001]);
const box = { minX: loc[0], minY: loc[1], maxX: loc[0], maxY: loc[1] };
if (!rbush.collides(box)) {
return loc;
}
}
}
}
41 changes: 30 additions & 11 deletions modules/services/MapillaryService.js
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { select as d3_select } from 'd3-selection';
import { Tiler, geoSphericalDistance } from '@rapid-sdk/math';
import { Tiler, geoSphericalDistance, vecSubtract } from '@rapid-sdk/math';
import { VectorTile } from '@mapbox/vector-tile';
import Protobuf from 'pbf';
import RBush from 'rbush';
Expand Down Expand Up @@ -137,9 +137,9 @@ export class MapillaryService extends AbstractSystem {
}

this._cache = {
images: { lastv: null, data: new Map(), rtree: new RBush() },
signs: { lastv: null, data: new Map(), rtree: new RBush() },
detections: { lastv: null, data: new Map(), rtree: new RBush() },
images: { lastv: null, data: new Map(), rbush: new RBush() },
signs: { lastv: null, data: new Map(), rbush: new RBush() },
detections: { lastv: null, data: new Map(), rbush: new RBush() },
sequences: { data: new Map() } , // Map<sequenceID, Array of LineStrings>
image_detections: { forImageID: {} },
inflight: new Map(), // Map<url, {tileID, promise, controller}>
Expand Down Expand Up @@ -197,7 +197,7 @@ export class MapillaryService extends AbstractSystem {

const extent = this.context.viewport.visibleExtent();
const cache = this._cache[datasetID];
return cache.rtree.search(extent.bbox()).map(d => d.data);
return cache.rbush.search(extent.bbox()).map(d => d.data);
}


Expand All @@ -210,7 +210,7 @@ export class MapillaryService extends AbstractSystem {
const extent = this.context.viewport.visibleExtent();
let result = new Map(); // Map(sequenceID -> Array of LineStrings)

for (const box of this._cache.images.rtree.search(extent.bbox())) {
for (const box of this._cache.images.rbush.search(extent.bbox())) {
const sequenceID = box.data.sequenceID;
if (!sequenceID) continue; // no sequence for this image
const sequence = this._cache.sequences.data.get(sequenceID);
Expand Down Expand Up @@ -889,7 +889,7 @@ export class MapillaryService extends AbstractSystem {
cache.data.set(image.id, image);

const [x, y] = props.loc;
cache.rtree.insert({ minX: x, minY: y, maxX: x, maxY: y, data: image });
cache.rbush.insert({ minX: x, minY: y, maxX: x, maxY: y, data: image });
}

// Update whatever additional props we were passed..
Expand All @@ -913,18 +913,19 @@ export class MapillaryService extends AbstractSystem {
_cacheDetection(cache, props) {
let detection = cache.data.get(props.id);
if (!detection) {
const loc = this._preventCoincident(cache.rbush, props.loc);
detection = {
type: 'detection',
service: 'mapillary',
id: props.id,
loc: props.loc,
object_type: props.object_type // 'point' or 'traffic_sign'
object_type: props.object_type, // 'point' or 'traffic_sign'
loc: loc
};

cache.data.set(detection.id, detection);

const [x, y] = props.loc;
cache.rtree.insert({ minX: x, minY: y, maxX: x, maxY: y, data: detection });
const [x, y] = loc;
cache.rbush.insert({ minX: x, minY: y, maxX: x, maxY: y, data: detection });
}

// Update whatever additional props we were passed..
Expand All @@ -938,4 +939,22 @@ export class MapillaryService extends AbstractSystem {
return detection;
}


/**
* _preventCoincident
* This checks if the cache already has something at that location, and if so, moves down slightly.
* @param {RBush} rbush - the spatial cache to check
* @param {Array<number>} loc - original [longitude,latitude] coordinate
* @return {Array<number>} Adjusted [longitude,latitude] coordinate
*/
_preventCoincident(rbush, loc) {
for (let dy = 0; ; dy++) {
loc = vecSubtract(loc, [0, dy * 0.00001]);
const box = { minX: loc[0], minY: loc[1], maxX: loc[0], maxY: loc[1] };
if (!rbush.collides(box)) {
return loc;
}
}
}

}
Loading

0 comments on commit 4af8069

Please sign in to comment.