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

fix(scrollto): Fix scroll to on rotated images #538

Merged
merged 2 commits into from
Jul 8, 2020
Merged
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
1 change: 1 addition & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ module.exports = {
rules: {
'@typescript-eslint/explicit-function-return-type': ['warn', { allowExpressions: true }],
'@typescript-eslint/no-explicit-any': ['error', { ignoreRestArgs: true }],
'@typescript-eslint/no-unused-vars': ['error', { varsIgnorePattern: '^_' }],
},
},
{
Expand Down
8 changes: 5 additions & 3 deletions src/image/ImageAnnotator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { Unsubscribe } from 'redux';
import BaseAnnotator, { Options } from '../common/BaseAnnotator';
import BaseManager from '../common/BaseManager';
import { getAnnotation, getRotation } from '../store';
import { centerRegion, isRegion, RegionManager } from '../region';
import { centerRegion, getTransformedShape, isRegion, RegionManager } from '../region';
import { CreatorStatus, getCreatorStatus } from '../store/creator';
import { scrollToLocation } from '../utils/scroll';
import './ImageAnnotator.scss';
Expand Down Expand Up @@ -99,15 +99,17 @@ export default class ImageAnnotator extends BaseAnnotator {
const annotation = getAnnotation(this.store.getState(), annotationId);
const annotationLocation = annotation?.target.location.value ?? 1;
const referenceEl = this.getReference();
const rotation = getRotation(this.store.getState()) || 0;

if (!annotation || !annotationLocation || !referenceEl || !this.annotatedEl) {
return;
}

if (isRegion(annotation)) {
// TODO: Add support for scroll when image is rotated
const transformedShape = getTransformedShape(annotation.target.shape, rotation);

scrollToLocation(this.annotatedEl, referenceEl, {
offsets: centerRegion(annotation.target.shape),
offsets: centerRegion(transformedShape),
jstoffan marked this conversation as resolved.
Show resolved Hide resolved
});
}
}
Expand Down
71 changes: 70 additions & 1 deletion src/region/__tests__/regionUtil-test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
import { Rect } from '../../@types';
import { annotation } from '../__mocks__/data';
import { centerRegion, centerShape, isRegion, styleShape } from '../regionUtil';
import {
centerRegion,
centerShape,
getPoints,
getTransformedShape,
isRegion,
styleShape,
selectTransformationPoint,
} from '../regionUtil';

describe('regionUtil', () => {
const getRect = (): Rect => ({
Expand All @@ -10,6 +18,12 @@ describe('regionUtil', () => {
x: 10,
y: 10,
});
const parseValue = (value: number): number => parseFloat(value.toFixed(3));

const regionShape = { height: 2, width: 3, x: 1, y: 1 };
const regionShapeRotated90 = { height: 3, width: 2, x: 1, y: 96 };
const regionShapeRotated180 = { height: 2, width: 3, x: 96, y: 97 };
const regionShapeRotated270 = { height: 3, width: 2, x: 97, y: 1 };

describe('centerShape()', () => {
test('should return the position of the center of the shape', () => {
Expand All @@ -27,6 +41,61 @@ describe('regionUtil', () => {
});
});

describe('getPoints()', () => {
test('should return the points based on a region shape', () => {
const [p1, p2, p3, p4] = getPoints(regionShape);
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(regionShape, rotation)).toEqual(expectedPoint);
},
);
});

describe('getTransformedShape()', () => {
test.each`
rotation | expectedShape
${0} | ${regionShape}
${-90} | ${regionShapeRotated90}
${-180} | ${regionShapeRotated180}
${-270} | ${regionShapeRotated270}
${-360} | ${regionShape}
`(
'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) || {};

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 } = getTransformedShape(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));
expect(parseValue(y)).toEqual(parseValue(expY));
});
});

describe('isRegion()', () => {
test.each`
type | result
Expand Down
52 changes: 52 additions & 0 deletions src/region/__tests__/transformUtils-test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import { invertYCoordinate, rotatePoint, translatePoint } from '../transformUtil';

describe('src/region/transformUtil', () => {
const parseValue = (value: number): number => parseFloat(value.toFixed(3));

describe('invertYCoordinate()', () => {
test.each`
point | height | expectedPoint
${{ x: 0, y: 0 }} | ${10} | ${{ x: 0, y: 10 }}
${{ x: 1, y: 5 }} | ${10} | ${{ x: 1, y: 5 }}
${{ x: 2, y: 7 }} | ${10} | ${{ x: 2, y: 3 }}
${{ x: 2, y: 7 }} | ${0} | ${{ x: 2, y: 7 }}
`(
'should return inverted y-coordinate given point=$point and height=$height',
({ point, height, expectedPoint }) => {
expect(invertYCoordinate(point, height)).toEqual(expectedPoint);
},
);
});

describe('rotatePoint()', () => {
test.each`
point | angle | expectedPoint
${{ x: 1, y: 9 }} | ${0} | ${{ x: 1, y: 9 }}
${{ x: 1, y: 9 }} | ${90} | ${{ x: -9, y: 1 }}
${{ x: 1, y: 9 }} | ${180} | ${{ x: -1, y: -9 }}
${{ x: 1, y: 9 }} | ${270} | ${{ x: 9, y: -1 }}
${{ x: 1, y: 9 }} | ${-90} | ${{ x: 9, y: -1 }}
`('should return rotated point given point=$point and angle=$angle', ({ point, angle, expectedPoint }) => {
const { x, y } = rotatePoint(point, angle);
const { x: expX, y: expY } = expectedPoint;

expect(parseValue(x)).toEqual(parseValue(expX));
expect(parseValue(y)).toEqual(parseValue(expY));
});
});

describe('translatePoint()', () => {
test.each`
point | translation | expectedPoint
${{ x: 1, y: 1 }} | ${{ dx: 3 }} | ${{ x: 4, y: 1 }}
${{ x: 1, y: 1 }} | ${{ dy: 3 }} | ${{ x: 1, y: 4 }}
${{ x: 1, y: 1 }} | ${{ dx: 3, dy: 5 }} | ${{ x: 4, y: 6 }}
${{ x: 1, y: 1 }} | ${{ dx: -3, dy: 5 }} | ${{ x: -2, y: 6 }}
`(
'should return translated point given point=$point and translation=$translation',
({ point, translation, expectedPoint }) => {
expect(translatePoint(point, translation)).toEqual(expectedPoint);
},
);
});
});
107 changes: 107 additions & 0 deletions src/region/regionUtil.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,19 @@
import * as React from 'react';
import { Annotation, AnnotationRegion, Position } from '../@types';
import { invertYCoordinate, Point, rotatePoint, translatePoint } from './transformUtil';

// Possible rotation values as supplied by box-content-preview
const ROTATION_ONCE_DEG = -90;
ConradJChan marked this conversation as resolved.
Show resolved Hide resolved
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 Dimensions = {
height: number;
width: number;
};

export type Shape = {
height: number;
Expand All @@ -8,6 +22,11 @@ export type Shape = {
y: number;
};

export type Translation = {
dx?: number;
dy?: number;
};

export const EMPTY_STYLE = { display: 'none' };

export const centerShape = (shape: Shape): Position => {
Expand All @@ -29,6 +48,94 @@ 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;
jstoffan marked this conversation as resolved.
Show resolved Hide resolved
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
ConradJChan marked this conversation as resolved.
Show resolved Hide resolved
// 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';
}
Expand Down
28 changes: 28 additions & 0 deletions src/region/transformUtil.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
export type Point = {
x: number;
y: number;
};

export const invertYCoordinate = ({ x, y }: Point, height: number): Point => ({
x,
y: height > 0 ? height - y : y,
});

export const rotatePoint = ({ x, y }: Point, rotationInDegrees: number): Point => {
const radians = (rotationInDegrees * Math.PI) / 180;
const cosine = Math.cos(radians);
const sine = Math.sin(radians);

// Formula to apply a rotation to a point is:
// x' = x * cos(θ) - y * sin(θ)
// y' = x * sin(θ) + y * cos(θ)
return {
x: x * cosine - y * sine,
y: x * sine + y * cosine,
};
};

export const translatePoint = ({ x, y }: Point, { dx = 0, dy = 0 }: { dx?: number; dy?: number }): Point => ({
x: x + dx,
y: y + dy,
});
Loading