Skip to content

Commit

Permalink
Fix: Image scaling/rotation is broken when an image has annotations (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
JustinHoldstock authored May 30, 2017
1 parent a9e1c52 commit c91e9f9
Show file tree
Hide file tree
Showing 6 changed files with 88 additions and 39 deletions.
2 changes: 2 additions & 0 deletions src/lib/annotations/Annotator.js
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,8 @@ import './Annotator.scss';
this.bindCustomListenersOnThread(thread);
}
});

this.emit('annotationsfetched');
});
}

Expand Down
7 changes: 7 additions & 0 deletions src/lib/annotations/__tests__/Annotator-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,13 @@ describe('lib/annotations/Annotator', () => {
expect(result).to.be.an.object;
});
});

it('should emit a message to indicate that all annotations have been fetched', () => {
annotator.fetchAnnotations();
return stubs.threadPromise.then(() => {
expect(annotator.emit).to.be.calledWith('annotationsfetched');
});
});
});

describe('bindCustomListenersOnService', () => {
Expand Down
25 changes: 6 additions & 19 deletions src/lib/annotations/image/imageAnnotatorUtil.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,20 @@ const ROTATION_THRICE_DEG = -270;
*/
export function getRotatedLocation(x, y, rotation, imageDimensions, scale) {
const { height, width } = imageDimensions;
const scaledWidth = width / scale;
const scaledHeight = height / scale;

switch (rotation) {
case ROTATION_ONCE_DEG:
return [y, height / scale - x];
return [y, scaledHeight - x];
case ROTATION_TWICE_DEG:
return [width / scale - x, height / scale - y];
return [scaledWidth - x, scaledHeight - y];
case ROTATION_THRICE_DEG:
return [width / scale - y, x];
return [scaledWidth - y, x];
default:
break;
}

return [x, y];
}

Expand Down Expand Up @@ -90,24 +93,8 @@ export function getBrowserCoordinatesFromLocation(location, annotatedElement) {

// Adjust annotation location if image is rotated
const rotation = Number(imageEl.getAttribute('data-rotation-angle'));
const isRotated = Math.abs(rotation) % 180 === 90;
let [x, y] = getRotatedLocation(location.x, location.y, rotation, imageDimensions, scale);

// Get scale coordinates for new image size
const topRotatedPadding = getRotatedPadding(imageEl, isRotated);
const rotatedDimensions = {
x: location.dimensions.y,
y: location.dimensions.x
};
const dimensions = isRotated ? rotatedDimensions : location.dimensions;
const dimensionScale = annotatorUtil.getDimensionScale(dimensions, imageDimensions, scale, topRotatedPadding);

// Scale coordinates to new image size
if (dimensionScale) {
x *= dimensionScale.x;
y *= dimensionScale.y;
}

x *= scale;
y *= scale;

Expand Down
56 changes: 41 additions & 15 deletions src/lib/viewers/BaseViewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,20 @@ const LOAD_TIMEOUT_MS = 180000; // 3m
const RESIZE_WAIT_TIME_IN_MILLIS = 300;

@autobind class BaseViewer extends EventEmitter {
/**
* Rotation value in degrees of the document, if rotated.
*
* @property {number}
*/
rotationAngle = 0;

/**
* Scale amount of the document, if zoomed.
*
* @property {number}
*/
scale = 1;

/**
* [constructor]
*
Expand Down Expand Up @@ -275,22 +289,9 @@ const RESIZE_WAIT_TIME_IN_MILLIS = 300;
this.resize();
});

// Add a custom listener for events related to scaling/orientation changes
this.addListener('scale', (data) => {
if (this.annotator) {
this.annotator.setScale(data.scale);
this.annotator.rotateAnnotations(data.rotationAngle);
}
});

// Add a resize handler for the window
document.defaultView.addEventListener('resize', this.debouncedResizeHandler);

/* istanbul ignore next */
this.addListener('togglepointannotationmode', () => {
this.annotator.togglePointModeHandler();
});

this.addListener('load', () => {
if (this.annotationsPromise) {
this.annotationsPromise.then(this.loadAnnotator);
Expand Down Expand Up @@ -562,6 +563,19 @@ const RESIZE_WAIT_TIME_IN_MILLIS = 300;
}
}

/**
* Orient anntations to the correct scale and orientatio of the annotated document.
* @TODO(jholdstock|spramod): Remove this once we are emitting the correct messaging.
*
* @protected
* @param {Object} data - Scale and orientation values needed to orient annotations.
* @return {void}
*/
scaleAnnotations(data) {
this.annotator.setScale(data.scale);
this.annotator.rotateAnnotations(data.rotationAngle);
}

/**
* Initializes annotations.
*
Expand All @@ -588,11 +602,23 @@ const RESIZE_WAIT_TIME_IN_MILLIS = 300;
this.annotator.init();

// Disables controls during point annotation mode
/* istanbul ignore next */
this.annotator.addListener('annotationmodeenter', this.disableViewerControls);

/* istanbul ignore next */
this.annotator.addListener('annotationmodeexit', this.enableViewerControls);

this.addListener('togglepointannotationmode', () => {
this.annotator.togglePointModeHandler();
});

// Add a custom listener for events related to scaling/orientation changes
this.addListener('scale', this.scaleAnnotations.bind(this));

this.annotator.addListener('annotationsfetched', () => {
this.scaleAnnotations({
scale: this.scale,
rotationAngle: this.rotationAngle
});
});
}

/**
Expand Down
33 changes: 30 additions & 3 deletions src/lib/viewers/__tests__/BaseViewer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -246,9 +246,7 @@ describe('lib/viewers/BaseViewer', () => {
expect(fullscreen.addListener).to.be.calledWith('enter', sinon.match.func);
expect(fullscreen.addListener).to.be.calledWith('exit', sinon.match.func);
expect(document.defaultView.addEventListener).to.be.calledWith('resize', base.debouncedResizeHandler);
expect(base.addListener).to.be.calledWith('togglepointannotationmode', sinon.match.func);
expect(base.addListener).to.be.calledWith('load', sinon.match.func);
expect(base.addListener).to.be.calledWith('scale', sinon.match.func);
});
});

Expand Down Expand Up @@ -666,8 +664,31 @@ describe('lib/viewers/BaseViewer', () => {
});
});

describe('scaleAnnotations()', () => {
const scaleData = {
scale: 0.4321,
rotationAngle: 90
};
beforeEach(() => {
base.annotator = {
setScale: sandbox.stub(),
rotateAnnotations: sandbox.stub()
};

base.scaleAnnotations(scaleData);
});

it('should invoke setScale() on annotator to scale annotations', () => {
expect(base.annotator.setScale).to.be.calledWith(scaleData.scale);
});

it('should invoke rotateAnnotations() on annotator to orient annotations', () => {
expect(base.annotator.rotateAnnotations).to.be.calledWith(scaleData.rotationAngle);
});
});

describe('initAnnotations()', () => {
it('should initialize the annotator', () => {
beforeEach(() => {
base.options = {
container: document,
file: {
Expand All @@ -679,6 +700,7 @@ describe('lib/viewers/BaseViewer', () => {
locale: 'en-US'
}
};
base.addListener = sandbox.stub();
base.annotator = {
init: sandbox.stub(),
addListener: sandbox.stub()
Expand All @@ -687,9 +709,14 @@ describe('lib/viewers/BaseViewer', () => {
CONSTRUCTOR: sandbox.stub().returns(base.annotator)
};
base.initAnnotations();
});
it('should initialize the annotator', () => {
expect(base.annotator.init).to.be.called;
expect(base.annotator.addListener).to.be.calledWith('annotationmodeenter', sinon.match.func);
expect(base.annotator.addListener).to.be.calledWith('annotationmodeexit', sinon.match.func);
expect(base.annotator.addListener).to.be.calledWith('annotationsfetched', sinon.match.func);
expect(base.addListener).to.be.calledWith('togglepointannotationmode', sinon.match.func);
expect(base.addListener).to.be.calledWith('scale', sinon.match.func);
});
});

Expand Down
4 changes: 2 additions & 2 deletions src/lib/viewers/image/MultiImageViewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -169,8 +169,8 @@ const ZOOM_UPDATE_PAN_DELAY = 50;
setScale(width, height) {
// Grab the first page image dimensions
const imageEl = this.singleImageEls[0];
const scale = width ? width / imageEl.naturalWidth : height / imageEl.naturalHeight;
this.emit('scale', { scale });
this.scale = width ? width / imageEl.naturalWidth : height / imageEl.naturalHeight;
this.emit('scale', { scale: this.scale });
}

/**
Expand Down

0 comments on commit c91e9f9

Please sign in to comment.