Skip to content

Commit

Permalink
New: Store all page highlight threads as an rbush tree (#100)
Browse files Browse the repository at this point in the history 
* More efficient method of determining which highlight thread is being hovered over
* Chore: Determine hovered/clicked highlight by tree search
* Chore: Ensure destroyed thread isn't shown
  • Loading branch information
@pramodsum
pramodsum committed Jan 19, 2018
1 parent aab067e commit 86d0468
Show file tree
Hide file tree
Showing 17 changed files with 485 additions and 412 deletions.
4 changes: 4 additions & 0 deletions src/AnnotationDialog.js
View file
Expand Up @@ -127,6 +127,10 @@ class AnnotationDialog extends EventEmitter {
* @return {void}
*/
scrollToLastComment() {
if (!this.element) {
return;
}

const annotationsEl = this.element.querySelector(constants.SELECTOR_ANNOTATION_CONTAINER);
if (annotationsEl) {
const isDialogFlipped = this.dialogEl.classList.contains(CLASS_FLIPPED_DIALOG);
Expand Down
19 changes: 19 additions & 0 deletions src/AnnotationThread.js
View file
Expand Up @@ -59,6 +59,8 @@ class AnnotationThread extends EventEmitter {
this.localized = data.localized;
this.state = STATES.inactive;

this.regenerateBoundary();

// Explicitly bind listeners
this.showDialog = this.showDialog.bind(this);

Expand Down Expand Up @@ -616,6 +618,23 @@ class AnnotationThread extends EventEmitter {
this.deleteAnnotation(data.annotationID);
}

/**
* Regenerate the coordinates of the rectangular boundary on the saved thread for inserting into the rtree
*
* @private
* @return {void}
*/
regenerateBoundary() {
if (!this.location || !this.location.x || !this.location.y) {
return;
}

this.minX = this.location.x;
this.maxX = this.location.x;
this.minY = this.location.y;
this.maxY = this.location.y;
}

/**
* Deletes the temporary annotation if the annotation failed to save on the server
*
Expand Down
28 changes: 28 additions & 0 deletions src/__tests__/AnnotationThread-test.js
View file
Expand Up @@ -873,6 +873,34 @@ describe('AnnotationThread', () => {
});
});

describe('regenerateBoundary()', () => {
it('should do nothing if a valid location does not exist', () => {
thread.location = undefined;
thread.regenerateBoundary();

thread.location = {};
thread.regenerateBoundary();

thread.location = { x: 'something' };
thread.regenerateBoundary();

thread.location = { y: 'something' };
thread.regenerateBoundary();

expect(thread.minX).to.be.undefined;
expect(thread.minY).to.be.undefined;
});

it('should set the min/max x/y values to the thread location', () => {
thread.location = { x: 1, y: 2 };
thread.regenerateBoundary();
expect(thread.minX).equals(1);
expect(thread.minY).equals(2);
expect(thread.maxX).equals(1);
expect(thread.maxY).equals(2);
});
});

describe('handleThreadSaveError()', () => {
it('should delete temp annotation and emit event', () => {
sandbox.stub(thread, 'deleteAnnotation');
Expand Down
2 changes: 1 addition & 1 deletion src/constants.js
View file
Expand Up @@ -180,5 +180,5 @@ export const ID_MOBILE_ANNOTATION_DIALOG = 'mobile-annotation-dialog';

export const DRAW_RENDER_THRESHOLD = 16.67; // 60 FPS target using 16.667ms/frame
export const DRAW_BASE_LINE_WIDTH = 3;
export const DRAW_BORDER_OFFSET = 5;
export const BORDER_OFFSET = 5;
export const DRAW_DASHED_SPACING = 5;
128 changes: 83 additions & 45 deletions src/controllers/AnnotationModeController.js
View file
@@ -1,3 +1,4 @@
import rbush from 'rbush';
import EventEmitter from 'events';
import { insertTemplate, isPending } from '../util';
import {
Expand All @@ -6,7 +7,9 @@ import {
CLASS_ANNOTATION_MODE,
ANNOTATOR_EVENT,
THREAD_EVENT,
CONTROLLER_EVENT
CONTROLLER_EVENT,
TYPES,
BORDER_OFFSET
} from '../constants';

class AnnotationModeController extends EventEmitter {
Expand Down Expand Up @@ -54,13 +57,9 @@ class AnnotationModeController extends EventEmitter {
* @return {void}
*/
destroy() {
Object.keys(this.threads).forEach((page) => {
const pageThreads = this.threads[page] || {};

Object.keys(pageThreads).forEach((threadID) => {
const thread = pageThreads[threadID];
this.unregisterThread(thread);
});
Object.keys(this.threads).forEach((pageNum) => {
const pageThreads = this.threads[pageNum].all() || [];
pageThreads.forEach(this.unregisterThread.bind(this));
});

if (this.buttonEl) {
Expand Down Expand Up @@ -122,6 +121,10 @@ class AnnotationModeController extends EventEmitter {
* @return {void}
*/
exit() {
if (this.createDialog) {
this.createDialog.hide();
}

this.destroyPendingThreads();
this.annotatedElement.classList.remove(CLASS_ANNOTATION_MODE);
if (this.buttonEl) {
Expand Down Expand Up @@ -200,12 +203,17 @@ class AnnotationModeController extends EventEmitter {
* @return {void}
*/
registerThread(thread) {
// Add thread to in-memory map
const page = thread.location.page || 1; // Defaults to page 1 if thread has no page'
const pageThreads = this.threads[page] || {};
if (!thread || !thread.location) {
return;
}

pageThreads[thread.threadID] = thread;
this.threads[page] = pageThreads;
const page = thread.location.page || 1; // Defaults to page 1 if thread has no page'
if (!(page in this.threads)) {
/* eslint-disable new-cap */
this.threads[page] = new rbush();
/* eslint-enable new-cap */
}
this.threads[page].insert(thread);

this.emit(CONTROLLER_EVENT.register, thread);
thread.addListener('threadevent', (data) => this.handleThreadEvents(thread, data));
Expand All @@ -219,36 +227,41 @@ class AnnotationModeController extends EventEmitter {
* @return {void}
*/
unregisterThread(thread) {
const page = thread.location.page || 1; // Defaults to page 1 if thread has no page'
const pageThreads = this.threads[page] || {};

delete pageThreads[thread.threadID];
this.threads[page] = pageThreads;
if (!thread || !thread.location || !thread.location.page) {
return;
}

this.threads[thread.location.page].remove(thread);
this.emit(CONTROLLER_EVENT.unregister, thread);
thread.removeListener('threadevent', this.handleThreadEvents);
}

/**
* Apply predicate method to every thread on either the specified page or
* the entire file
* Apply predicate method to every thread on the specified page
*
* @private
* @param {Function} func Predicate method to apply on threads
* @param {number} [pageNum] Optional page number
* @param {number} pageNum Page number
* @return {void}
*/
applyActionToThreads(func, pageNum) {
if (pageNum) {
const pageThreads = this.threads[pageNum] || {};
Object.keys(pageThreads).forEach((threadID) => func(pageThreads[threadID]));
applyActionToPageThreads(func, pageNum) {
if (!this.threads[pageNum]) {
return;
}

Object.keys(this.threads).forEach((page) => {
const pageThreads = this.threads[page] || {};
Object.keys(pageThreads).forEach((threadID) => func(pageThreads[threadID]));
});
const pageThreads = this.threads[pageNum].all() || [];
pageThreads.forEach(func);
}

/**
* Apply predicate method to every thread on the entire file
*
* @private
* @param {Function} func Predicate method to apply on threads
* @return {void}
*/
applyActionToThreads(func) {
Object.keys(this.threads).forEach((page) => this.applyActionToPageThreads(func, page));
}

/**
Expand All @@ -260,11 +273,12 @@ class AnnotationModeController extends EventEmitter {
*/
getThreadByID(threadID) {
let thread = null;
Object.keys(this.threads).some((page) => {
const pageThreads = this.threads[page] || {};
if (threadID in pageThreads) {
thread = pageThreads[threadID];
}
Object.keys(this.threads).some((pageNum) => {
const pageThreads = this.threads[pageNum].all() || [];
thread = pageThreads.filter((t) => {
return t.threadID === threadID;
})[0];

return thread !== null;
});

Expand All @@ -286,7 +300,6 @@ class AnnotationModeController extends EventEmitter {
* the type of events to listen for, and the callback
*/
setupHandlers() {}
/* eslint-enable no-unused-vars */

/**
* Handles annotation thread events and emits them to the viewer
Expand Down Expand Up @@ -388,17 +401,16 @@ class AnnotationModeController extends EventEmitter {
* @return {void}
*/
renderPage(pageNum) {
if (!this.threads) {
if (!this.threads || !this.threads[pageNum]) {
return;
}

const pageThreads = this.threads[pageNum] || {};
Object.keys(pageThreads).forEach((threadID) => {
const pageThreads = this.threads[pageNum].all() || [];
pageThreads.forEach((thread, index) => {
// Sets the annotatedElement if the thread was fetched before the
// dependent document/viewer finished loading
const thread = pageThreads[threadID];
if (!thread.annotatedElement) {
thread.annotatedElement = this.annotatedElement;
pageThreads[index].annotatedElement = this.annotatedElement;
}

thread.show();
Expand All @@ -415,11 +427,9 @@ class AnnotationModeController extends EventEmitter {
destroyPendingThreads() {
let hadPendingThreads = false;

Object.keys(this.threads).forEach((page) => {
const pageThreads = this.threads[page] || {};

Object.keys(pageThreads).forEach((threadID) => {
const thread = pageThreads[threadID];
Object.keys(this.threads).forEach((pageNum) => {
const pageThreads = this.threads[pageNum].all() || [];
pageThreads.forEach((thread) => {
if (isPending(thread.state)) {
hadPendingThreads = true;

Expand All @@ -434,6 +444,34 @@ class AnnotationModeController extends EventEmitter {
return hadPendingThreads;
}

/**
* Find the intersecting threads given a pointer event
*
* @protected
* @param {Event} event The event object containing the pointer information
* @return {AnnotationThread[]} Array of intersecting annotation threads
*/
getIntersectingThreads(event) {
if (!event || !this.threads || !this.annotator) {
return [];
}

const location = this.annotator.getLocationFromEvent(event, TYPES.point);
if (!location || Object.keys(this.threads).length === 0 || !this.threads[location.page]) {
return [];
}

const eventBoundary = {
minX: +location.x - BORDER_OFFSET,
minY: +location.y - BORDER_OFFSET,
maxX: +location.x + BORDER_OFFSET,
maxY: +location.y + BORDER_OFFSET
};

// Get the threads that correspond to the point that was clicked on
return this.threads[location.page].search(eventBoundary);
}

/**
* Emits a generic annotator event
*
Expand Down

0 comments on commit 86d0468

Please sign in to comment.