From 4f753ccef0ade1dbd28c5ed99fd8858db85a1098 Mon Sep 17 00:00:00 2001 From: Mingze Xiao Date: Mon, 23 Nov 2020 14:53:42 -0800 Subject: [PATCH 1/3] feat(drawing): Add DrawingManager for ImageAnnotator --- src/image/ImageAnnotator.ts | 14 +++++++++++++- src/image/__tests__/ImageAnnotator-test.ts | 22 ++++++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/src/image/ImageAnnotator.ts b/src/image/ImageAnnotator.ts index 29cb19f0f..0b63c41e7 100644 --- a/src/image/ImageAnnotator.ts +++ b/src/image/ImageAnnotator.ts @@ -4,6 +4,7 @@ import PopupManager from '../popup/PopupManager'; import { getAnnotation, getRotation } from '../store'; import { centerRegion, getTransformedShape, isRegion, RegionCreationManager, RegionManager } from '../region'; import { CreatorStatus, getCreatorStatus } from '../store/creator'; +import { DrawingManager, getShape, isDrawing } from '../drawing'; import { Manager } from '../common/BaseManager'; import { scrollToLocation } from '../utils/scroll'; import './ImageAnnotator.scss'; @@ -49,6 +50,9 @@ export default class ImageAnnotator extends BaseAnnotator { if (this.managers.size === 0) { this.managers.add(new PopupManager({ referenceEl })); + if (this.isFeatureEnabled('drawing')) { + this.managers.add(new DrawingManager({ referenceEl })); + } this.managers.add(new RegionManager({ referenceEl })); this.managers.add(new RegionCreationManager({ referenceEl })); } @@ -112,8 +116,16 @@ export default class ImageAnnotator extends BaseAnnotator { return; } + let shape = null; if (isRegion(annotation)) { - const transformedShape = getTransformedShape(annotation.target.shape, rotation); + // eslint-disable-next-line prefer-destructuring + shape = annotation.target.shape; + } else if (isDrawing(annotation)) { + shape = getShape(annotation.target.path_groups); + } + + if (shape) { + const transformedShape = getTransformedShape(shape, rotation); scrollToLocation(this.annotatedEl, referenceEl, { offsets: centerRegion(transformedShape), diff --git a/src/image/__tests__/ImageAnnotator-test.ts b/src/image/__tests__/ImageAnnotator-test.ts index cdb74c95a..a6d64d000 100644 --- a/src/image/__tests__/ImageAnnotator-test.ts +++ b/src/image/__tests__/ImageAnnotator-test.ts @@ -1,9 +1,11 @@ +import DrawingManager from '../../drawing/DrawingManager'; import ImageAnnotator, { CSS_IS_DRAWING_CLASS } from '../ImageAnnotator'; import PopupManager from '../../popup/PopupManager'; import RegionCreationManager from '../../region/RegionCreationManager'; import RegionManager from '../../region/RegionManager'; import { Annotation } from '../../@types'; import { CreatorStatus, fetchAnnotationsAction, setStatusAction } from '../../store'; +import { annotations as drawings } from '../../drawing/__mocks__/drawingData'; import { annotations as regions } from '../../region/__mocks__/data'; import { scrollToLocation } from '../../utils/scroll'; @@ -98,6 +100,18 @@ describe('ImageAnnotator', () => { expect(managerIterator.next().value).toBeInstanceOf(RegionCreationManager); }); + test('should create DrawingManager is feature is enabled', () => { + annotator.destroy(); + annotator = getAnnotator({ features: { drawing: true } }); + const managers = annotator.getManagers(getParent(), getImage()); + const managerIterator = managers.values(); + + expect(managerIterator.next().value).toBeInstanceOf(PopupManager); + expect(managerIterator.next().value).toBeInstanceOf(DrawingManager); + expect(managerIterator.next().value).toBeInstanceOf(RegionManager); + expect(managerIterator.next().value).toBeInstanceOf(RegionCreationManager); + }); + test('should destroy any existing managers if they are not present in the annotated element', () => { mockManager.exists.mockReturnValue(false); annotator.managers = new Set([mockManager]); @@ -220,6 +234,9 @@ describe('ImageAnnotator', () => { annotator.annotatedEl = getParent(); annotator.store.dispatch(fetchAnnotationsAction.fulfilled(payload, 'test', undefined)); + annotator.store.dispatch( + fetchAnnotationsAction.fulfilled({ ...payload, entries: drawings as Annotation[] }, 'test', undefined), + ); }); test('should call scrollToLocation for region annotations', () => { @@ -227,6 +244,11 @@ describe('ImageAnnotator', () => { expect(scrollToLocation).toHaveBeenCalledWith(getParent(), getImage(), { offsets: { x: 15, y: 15 } }); }); + test('should call scrollToLocation for drawing anntotations', () => { + annotator.scrollToAnnotation('drawing_anno_1'); + expect(scrollToLocation).toHaveBeenCalledWith(getParent(), getImage(), { offsets: { x: 16, y: 16 } }); + }); + test('should do nothing if the annotation id is undefined or not available in the store', () => { annotator.scrollToAnnotation('nonsense'); expect(scrollToLocation).not.toHaveBeenCalled(); From b085727a6e626f7d2d35380e789b77e45e0ee863 Mon Sep 17 00:00:00 2001 From: Mingze Xiao Date: Mon, 23 Nov 2020 16:09:27 -0800 Subject: [PATCH 2/3] feat(drawing): Move transformation function to utils/rotate --- src/image/ImageAnnotator.ts | 20 ++-- src/region/__tests__/regionUtil-test.ts | 15 +-- src/region/regionUtil.ts | 104 +------------------ src/utils/__tests__/rotate-test.ts | 65 ++++++++++++ src/utils/rotate.ts | 131 ++++++++++++++++++++++++ 5 files changed, 210 insertions(+), 125 deletions(-) create mode 100644 src/utils/__tests__/rotate-test.ts create mode 100644 src/utils/rotate.ts diff --git a/src/image/ImageAnnotator.ts b/src/image/ImageAnnotator.ts index 0b63c41e7..6f0f982eb 100644 --- a/src/image/ImageAnnotator.ts +++ b/src/image/ImageAnnotator.ts @@ -1,10 +1,11 @@ import { Unsubscribe } from 'redux'; import BaseAnnotator, { Options } from '../common/BaseAnnotator'; import PopupManager from '../popup/PopupManager'; -import { getAnnotation, getRotation } from '../store'; -import { centerRegion, getTransformedShape, isRegion, RegionCreationManager, RegionManager } from '../region'; +import { centerDrawing, DrawingManager, isDrawing } from '../drawing'; +import { centerRegion, isRegion, RegionCreationManager, RegionManager } from '../region'; import { CreatorStatus, getCreatorStatus } from '../store/creator'; -import { DrawingManager, getShape, isDrawing } from '../drawing'; +import { getAnnotation, getRotation } from '../store'; +import { getRotatedPosition } from '../utils/rotate'; import { Manager } from '../common/BaseManager'; import { scrollToLocation } from '../utils/scroll'; import './ImageAnnotator.scss'; @@ -116,19 +117,16 @@ export default class ImageAnnotator extends BaseAnnotator { return; } - let shape = null; + let offsets = null; if (isRegion(annotation)) { - // eslint-disable-next-line prefer-destructuring - shape = annotation.target.shape; + offsets = centerRegion(annotation.target.shape); } else if (isDrawing(annotation)) { - shape = getShape(annotation.target.path_groups); + offsets = centerDrawing(annotation.target.path_groups); } - if (shape) { - const transformedShape = getTransformedShape(shape, rotation); - + if (offsets) { scrollToLocation(this.annotatedEl, referenceEl, { - offsets: centerRegion(transformedShape), + offsets: getRotatedPosition(offsets, rotation), }); } } diff --git a/src/region/__tests__/regionUtil-test.ts b/src/region/__tests__/regionUtil-test.ts index 04895dd76..f78d4cefe 100644 --- a/src/region/__tests__/regionUtil-test.ts +++ b/src/region/__tests__/regionUtil-test.ts @@ -1,14 +1,7 @@ import { Rect } from '../../@types'; import { annotation } from '../__mocks__/data'; -import { - centerRegion, - centerShape, - getPoints, - getTransformedShape, - isRegion, - styleShape, - selectTransformationPoint, -} from '../regionUtil'; +import { centerRegion, centerShape, isRegion, styleShape } from '../regionUtil'; +import { getPoints, getRotatedShape, selectTransformationPoint } from '../../utils/rotate'; describe('regionUtil', () => { const getRect = (): Rect => ({ @@ -79,7 +72,7 @@ describe('regionUtil', () => { 'should return the transformed shape based on rotation=$rotation and reference element', ({ rotation, expectedShape }) => { const { height: expHeight, width: expWidth, x: expX, y: expY } = expectedShape; - const { height, width, x = NaN, y = NaN } = getTransformedShape(regionShape, rotation) || {}; + const { height, width, x = NaN, y = NaN } = getRotatedShape(regionShape, rotation) || {}; expect({ height, width }).toEqual({ height: expHeight, width: expWidth }); expect(parseValue(x)).toEqual(parseValue(expX)); @@ -88,7 +81,7 @@ describe('regionUtil', () => { ); test('should transform -90deg shape back to an unrotated state', () => { - const { height, width, x, y } = getTransformedShape(regionShapeRotated90, -270); + const { height, width, x, y } = getRotatedShape(regionShapeRotated90, -270); const { height: expHeight, width: expWidth, x: expX, y: expY } = regionShape; expect({ height, width }).toEqual({ height: expHeight, width: expWidth }); expect(parseValue(x)).toEqual(parseValue(expX)); diff --git a/src/region/regionUtil.ts b/src/region/regionUtil.ts index 28cdae35d..1e69e1166 100644 --- a/src/region/regionUtil.ts +++ b/src/region/regionUtil.ts @@ -1,19 +1,5 @@ import * as React from 'react'; -import { Annotation, AnnotationRegion, Dimensions, Position, Shape } from '../@types'; -import { invertYCoordinate, Point, rotatePoint, translatePoint } from './transformUtil'; - -// Possible rotation values as supplied by box-content-preview -const ROTATION_ONCE_DEG = -90; -const ROTATION_TWICE_DEG = -180; -const ROTATION_THRICE_DEG = -270; - -// Region annotation shapes are, by default, percentages, so a 100x100 space -const DEFAULT_DIMENSIONS = { height: 100, width: 100 }; - -export type Translation = { - dx?: number; - dy?: number; -}; +import { Annotation, AnnotationRegion, Position, Shape } from '../@types'; export const EMPTY_STYLE = { display: 'none' }; @@ -36,94 +22,6 @@ export const centerRegion = (shape: Shape): Position => { }; }; -export const getPoints = (shape: Shape): [Point, Point, Point, Point] => { - const { height, width, x, y } = shape; - - // Define the points from x,y and then in a clockwise fashion - // p1 p2 - // +-------+ - // | | - // +-------+ - // p4 p3 - const p1 = { x, y }; - const p2 = { x: x + width, y }; - const p3 = { x: x + width, y: y + height }; - const p4 = { x, y: y + height }; - - return [p1, p2, p3, p4]; -}; - -export function selectTransformationPoint(shape: Shape, rotation: number): Point { - const [p1, p2, p3, p4] = getPoints(shape); - - // Determine which point will be the new x,y (as defined as the top left point) after rotation - // If -90deg: use p2 - // If -180deg: use p3 - // If -270deg: use p4 - // Otherwise: use p1 - switch (rotation) { - case ROTATION_ONCE_DEG: - return p2; - case ROTATION_TWICE_DEG: - return p3; - case ROTATION_THRICE_DEG: - return p4; - default: - return p1; - } -} - -// Determines the translation needed to anchor the bottom left corner of the -// coordinate space at the (0, 0) origin after rotation. -export function selectTranslation(dimensions: Dimensions, rotation: number): Translation { - const { height, width } = dimensions; - - switch (rotation) { - case ROTATION_ONCE_DEG: - return { dx: height }; - case ROTATION_TWICE_DEG: - return { dx: width, dy: height }; - case ROTATION_THRICE_DEG: - return { dy: width }; - default: - } - - return { dx: 0, dy: 0 }; -} - -export function getTransformedShape(shape: Shape, rotation: number): Shape { - const { height: shapeHeight, width: shapeWidth } = shape; - const { height: spaceHeight, width: spaceWidth } = DEFAULT_DIMENSIONS; - const isInverted = rotation % 180 === 0; - const isNoRotation = rotation % 360 === 0; - const translation = selectTranslation(DEFAULT_DIMENSIONS, rotation); - const point = selectTransformationPoint(shape, rotation); - - if (isNoRotation) { - return shape; - } - - // To transform from shape with 0 rotation to provided rotation: - // 1. Invert y-axis to convert from web to mathematical coordinate system - // 2. Apply rotation transformation (with inverted rotation -- again mathematical coordinate system) - // 3. Translate to align coordinate space with mathematical origin - // 4. Invert y-axis to convert back to web coordinate system - const invertedPoint = invertYCoordinate(point, spaceHeight); - const rotatedPoint = rotatePoint(invertedPoint, -rotation); - const translatedPoint = translatePoint(rotatedPoint, translation); - const { x: transformedX, y: transformedY } = invertYCoordinate( - translatedPoint, - isInverted ? spaceHeight : spaceWidth, - ); - - return { - height: isInverted ? shapeHeight : shapeWidth, - width: isInverted ? shapeWidth : shapeHeight, - x: transformedX, - y: transformedY, - }; -} - export function isRegion(annotation: Annotation): annotation is AnnotationRegion { return annotation?.target?.type === 'region'; } diff --git a/src/utils/__tests__/rotate-test.ts b/src/utils/__tests__/rotate-test.ts new file mode 100644 index 000000000..ff956a95f --- /dev/null +++ b/src/utils/__tests__/rotate-test.ts @@ -0,0 +1,65 @@ +import { getPoints, getRotatedShape, selectTransformationPoint } from '../../utils/rotate'; + +describe('rotate', () => { + const parseValue = (value: number): number => parseFloat(value.toFixed(3)); + + const shape = { height: 2, width: 3, x: 1, y: 1 }; + const shapeRotated90 = { height: 3, width: 2, x: 1, y: 96 }; + const shapeRotated180 = { height: 2, width: 3, x: 96, y: 97 }; + const shapeRotated270 = { height: 3, width: 2, x: 97, y: 1 }; + + describe('getPoints()', () => { + test('should return the points based on a shape', () => { + const [p1, p2, p3, p4] = getPoints(shape); + expect(p1).toEqual({ x: 1, y: 1 }); + expect(p2).toEqual({ x: 4, y: 1 }); + expect(p3).toEqual({ x: 4, y: 3 }); + expect(p4).toEqual({ x: 1, y: 3 }); + }); + }); + + describe('selectTransformationPoint()', () => { + test.each` + rotation | expectedPoint + ${0} | ${{ x: 1, y: 1 }} + ${-90} | ${{ x: 4, y: 1 }} + ${-180} | ${{ x: 4, y: 3 }} + ${-270} | ${{ x: 1, y: 3 }} + ${-360} | ${{ x: 1, y: 1 }} + `( + 'should return the appropriate point if shape=$shape and rotation=$rotation', + ({ rotation, expectedPoint }) => { + expect(selectTransformationPoint(shape, rotation)).toEqual(expectedPoint); + }, + ); + }); + + describe('getRotatedShape()', () => { + test.each` + rotation | expectedShape + ${0} | ${shape} + ${-90} | ${shapeRotated90} + ${-180} | ${shapeRotated180} + ${-270} | ${shapeRotated270} + ${-360} | ${shape} + `( + 'should return the transformed shape based on rotation=$rotation and reference element', + ({ rotation, expectedShape }) => { + const { height: expHeight, width: expWidth, x: expX, y: expY } = expectedShape; + const { height, width, x = NaN, y = NaN } = getRotatedShape(shape, rotation) || {}; + + expect({ height, width }).toEqual({ height: expHeight, width: expWidth }); + expect(parseValue(x)).toEqual(parseValue(expX)); + expect(parseValue(y)).toEqual(parseValue(expY)); + }, + ); + + test('should transform -90deg shape back to an unrotated state', () => { + const { height, width, x, y } = getRotatedShape(shapeRotated90, -270); + const { height: expHeight, width: expWidth, x: expX, y: expY } = shape; + expect({ height, width }).toEqual({ height: expHeight, width: expWidth }); + expect(parseValue(x)).toEqual(parseValue(expX)); + expect(parseValue(y)).toEqual(parseValue(expY)); + }); + }); +}); diff --git a/src/utils/rotate.ts b/src/utils/rotate.ts new file mode 100644 index 000000000..f36a0fa22 --- /dev/null +++ b/src/utils/rotate.ts @@ -0,0 +1,131 @@ +import { Dimensions, Position, Shape } from '../@types'; + +export type Translation = { + dx?: number; + dy?: number; +}; + +// Possible rotation values as supplied by box-content-preview +const ROTATION_ONCE_DEG = -90; +const ROTATION_TWICE_DEG = -180; +const ROTATION_THRICE_DEG = -270; + +// Annotation shapes are, by default, percentages, so a 100x100 space +const DEFAULT_DIMENSIONS = { height: 100, width: 100 }; + +export const invertYCoordinate = ({ x, y }: Position, height: number): Position => ({ + x, + y: height > 0 ? height - y : y, +}); + +export const rotatePoint = ({ x, y }: Position, rotationInDegrees: number): Position => { + const radians = (rotationInDegrees * Math.PI) / 180; + const cosine = Math.cos(radians); + const sine = Math.sin(radians); + + // Formula to apply a rotation to a position is: + // x' = x * cos(θ) - y * sin(θ) + // y' = x * sin(θ) + y * cos(θ) + return { + x: x * cosine - y * sine, + y: x * sine + y * cosine, + }; +}; + +export const getPoints = (shape: Shape): [Position, Position, Position, Position] => { + const { height, width, x, y } = shape; + + // Define the points from x,y and then in a clockwise fashion + // p1 p2 + // +-------+ + // | | + // +-------+ + // p4 p3 + const p1 = { x, y }; + const p2 = { x: x + width, y }; + const p3 = { x: x + width, y: y + height }; + const p4 = { x, y: y + height }; + + return [p1, p2, p3, p4]; +}; + +export const translatePoint = ({ x, y }: Position, { dx = 0, dy = 0 }: { dx?: number; dy?: number }): Position => ({ + x: x + dx, + y: y + dy, +}); + +export function selectTransformationPoint(shape: Shape, rotation: number): Position { + const [p1, p2, p3, p4] = getPoints(shape); + + // Determine which position will be the new x,y (as defined as the top left position) after rotation + // If -90deg: use p2 + // If -180deg: use p3 + // If -270deg: use p4 + // Otherwise: use p1 + switch (rotation) { + case ROTATION_ONCE_DEG: + return p2; + case ROTATION_TWICE_DEG: + return p3; + case ROTATION_THRICE_DEG: + return p4; + default: + return p1; + } +} + +// Determines the translation needed to anchor the bottom left corner of the +// coordinate space at the (0, 0) origin after rotation. +export function selectTranslation(dimensions: Dimensions, rotation: number): Translation { + const { height, width } = dimensions; + + switch (rotation) { + case ROTATION_ONCE_DEG: + return { dx: height }; + case ROTATION_TWICE_DEG: + return { dx: width, dy: height }; + case ROTATION_THRICE_DEG: + return { dy: width }; + default: + } + + return { dx: 0, dy: 0 }; +} + +export function getRotatedShape(shape: Shape, rotation: number): Shape { + const { height: shapeHeight, width: shapeWidth } = shape; + const { height: spaceHeight, width: spaceWidth } = DEFAULT_DIMENSIONS; + const isInverted = rotation % 180 === 0; + const isNoRotation = rotation % 360 === 0; + const translation = selectTranslation(DEFAULT_DIMENSIONS, rotation); + const position = selectTransformationPoint(shape, rotation); + + if (isNoRotation) { + return shape; + } + + // To transform from shape with 0 rotation to provided rotation: + // 1. Invert y-axis to convert from web to mathematical coordinate system + // 2. Apply rotation transformation (with inverted rotation -- again mathematical coordinate system) + // 3. Translate to align coordinate space with mathematical origin + // 4. Invert y-axis to convert back to web coordinate system + const invertedPoint = invertYCoordinate(position, spaceHeight); + const rotatedPoint = rotatePoint(invertedPoint, -rotation); + const translatedPoint = translatePoint(rotatedPoint, translation); + const { x: transformedX, y: transformedY } = invertYCoordinate( + translatedPoint, + isInverted ? spaceHeight : spaceWidth, + ); + + return { + height: isInverted ? shapeHeight : shapeWidth, + width: isInverted ? shapeWidth : shapeHeight, + x: transformedX, + y: transformedY, + }; +} + +export function getRotatedPosition({ x, y }: Position, rotation: number): Position { + const { x: rotatedX, y: rotatedY } = getRotatedShape({ height: 0, width: 0, x, y }, rotation); + return { x: rotatedX, y: rotatedY }; +} From 01929a216b327943170e4f46376d7d4593f97383 Mon Sep 17 00:00:00 2001 From: Mingze Xiao Date: Mon, 23 Nov 2020 16:31:01 -0800 Subject: [PATCH 3/3] feat(drawing): Address comments --- src/image/__tests__/ImageAnnotator-test.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/image/__tests__/ImageAnnotator-test.ts b/src/image/__tests__/ImageAnnotator-test.ts index a6d64d000..623132125 100644 --- a/src/image/__tests__/ImageAnnotator-test.ts +++ b/src/image/__tests__/ImageAnnotator-test.ts @@ -226,7 +226,7 @@ describe('ImageAnnotator', () => { describe('scrollToAnnotation()', () => { beforeEach(() => { const payload = { - entries: regions as Annotation[], + entries: [...regions, ...drawings] as Annotation[], limit: 1000, next_marker: null, previous_marker: null, @@ -234,9 +234,6 @@ describe('ImageAnnotator', () => { annotator.annotatedEl = getParent(); annotator.store.dispatch(fetchAnnotationsAction.fulfilled(payload, 'test', undefined)); - annotator.store.dispatch( - fetchAnnotationsAction.fulfilled({ ...payload, entries: drawings as Annotation[] }, 'test', undefined), - ); }); test('should call scrollToLocation for region annotations', () => {