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.

Sign up for GitHub

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

Jump to bottom

Fixed cropping polygon in some corner cases #3184

Merged
merged 2 commits into from
May 13, 2021
Merged

Fixed cropping polygon in some corner cases #3184

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
08ecf26
Fixed issue #3178
May 12, 2021
46840b9
Updated version, fixed changelog
May 12, 2021
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 CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Make sure frame unzip web worker correctly terminates after unzipping all images in a requested chunk (<https://github.com/openvinotoolkit/cvat/pull/3096>)
- Reset password link was unavailable before login (<https://github.com/openvinotoolkit/cvat/pull/3140>)
- Manifest: migration (<https://github.com/openvinotoolkit/cvat/pull/3146>)
- Fixed cropping polygon in some corner cases (<https://github.com/openvinotoolkit/cvat/pull/3184>)

### Security

Expand Down
2 changes: 1 addition & 1 deletion cvat-canvas/package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion cvat-canvas/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "cvat-canvas",
"version": "2.4.2",
"version": "2.4.3",
"description": "Part of Computer Vision Annotation Tool which presents its canvas library",
"main": "src/canvas.ts",
"scripts": {
Expand Down
156 changes: 75 additions & 81 deletions cvat-canvas/src/typescript/drawHandler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,11 @@ export interface DrawHandler {
cancel(): void;
}

interface FinalCoordinates {
points: number[];
box: Box;
}

export class DrawHandlerImpl implements DrawHandler {
// callback is used to notify about creating new shape
private onDrawDone: (data: object | null, duration?: number, continueDraw?: boolean) => void;
Expand Down Expand Up @@ -73,19 +78,14 @@ export class DrawHandlerImpl implements DrawHandler {
return [xtl, ytl, xbr, ybr];
}

private getFinalPolyshapeCoordinates(
targetPoints: number[],
): {
points: number[];
box: Box;
} {
private getFinalPolyshapeCoordinates(targetPoints: number[]): FinalCoordinates {
const { offset } = this.geometry;
let points = targetPoints.map((coord: number): number => coord - offset);
const box = {
xtl: Number.MAX_SAFE_INTEGER,
ytl: Number.MAX_SAFE_INTEGER,
xbr: Number.MAX_SAFE_INTEGER,
ybr: Number.MAX_SAFE_INTEGER,
xbr: Number.MIN_SAFE_INTEGER,
ybr: Number.MIN_SAFE_INTEGER,
};

const frameWidth = this.geometry.image.width;
Expand All @@ -96,9 +96,9 @@ export class DrawHandlerImpl implements DrawHandler {
Vertical,
}

const isBetween = (x1: number, x2: number, c: number): boolean => (
c >= Math.min(x1, x2) && c <= Math.max(x1, x2)
);
function isBetween(x1: number, x2: number, c: number): boolean {
return c >= Math.min(x1, x2) && c <= Math.max(x1, x2);
}
nmanovic marked this conversation as resolved.
Show resolved Hide resolved

const isInsideFrame = (p: Point, direction: Direction): boolean => {
if (direction === Direction.Horizontal) {
Expand All @@ -121,22 +121,35 @@ export class DrawHandlerImpl implements DrawHandler {

const findIntersectionsWithFrameBorders = (p1: Point, p2: Point, direction: Direction): number[] => {
const resultPoints = [];
const leftLine = [
{ x: 0, y: 0 },
{ x: 0, y: frameHeight },
];
const topLine = [
{ x: frameWidth, y: 0 },
{ x: 0, y: 0 },
];
const rightLine = [
{ x: frameWidth, y: frameHeight },
{ x: frameWidth, y: 0 },
];
const bottomLine = [
{ x: 0, y: frameHeight },
{ x: frameWidth, y: frameHeight },
];

if (direction === Direction.Horizontal) {
resultPoints.push(...findInersection(p1, p2, { x: 0, y: 0 }, { x: 0, y: frameHeight }));
resultPoints.push(
...findInersection(p1, p2, { x: frameWidth, y: frameHeight }, { x: frameWidth, y: 0 }),
);
resultPoints.push(...findInersection(p1, p2, leftLine[0], leftLine[1]));
resultPoints.push(...findInersection(p1, p2, rightLine[0], rightLine[1]));
} else {
resultPoints.push(
...findInersection(p1, p2, { x: 0, y: frameHeight }, { x: frameWidth, y: frameHeight }),
);
resultPoints.push(...findInersection(p1, p2, { x: frameWidth, y: 0 }, { x: 0, y: 0 }));
resultPoints.push(...findInersection(p1, p2, bottomLine[0], bottomLine[1]));
resultPoints.push(...findInersection(p1, p2, topLine[0], topLine[1]));
}

if (resultPoints.length === 4) {
if (
Math.sign(resultPoints[0] - resultPoints[2]) !== Math.sign(p1.x - p2.x)
&& Math.sign(resultPoints[1] - resultPoints[3]) !== Math.sign(p1.y - p2.y)
(p1.x === p2.x || Math.sign(resultPoints[0] - resultPoints[2]) !== Math.sign(p1.x - p2.x))
&& (p1.y === p2.y || Math.sign(resultPoints[1] - resultPoints[3]) !== Math.sign(p1.y - p2.y))
Comment on lines +151 to +152
Copy link
Member Author

@bsekachev bsekachev May 12, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@azhavoro @ActiveChooN

The core change to fix (others are to satisfy eslint, minor refactoring, and some minor bug fixes).

As I understand the issue was following:
After the first (Horizontal) crop p1.x in a corner case could be equal to p2.x).
The same if we reverse crop order (first is Vertical), then p1.y in a corner case could be equal to p2.y.

In these corner cases the condition is false and we do not reverse points when it is actually necessary.

) {
[resultPoints[0], resultPoints[2]] = [resultPoints[2], resultPoints[0]];
[resultPoints[1], resultPoints[3]] = [resultPoints[3], resultPoints[1]];
Expand All @@ -145,24 +158,23 @@ export class DrawHandlerImpl implements DrawHandler {
return resultPoints;
};

const crop = (polygonPoints: number[], direction: Direction): number[] => {
const crop = (shapePoints: number[], direction: Direction): number[] => {
const resultPoints = [];
for (let i = 0; i < polygonPoints.length - 1; i += 2) {
const curPoint = { x: polygonPoints[i], y: polygonPoints[i + 1] };
const isPolyline = this.drawData.shapeType === 'polyline';
const isPolygon = this.drawData.shapeType === 'polygon';

for (let i = 0; i < shapePoints.length - 1; i += 2) {
const curPoint = { x: shapePoints[i], y: shapePoints[i + 1] };
if (isInsideFrame(curPoint, direction)) {
resultPoints.push(polygonPoints[i], polygonPoints[i + 1]);
resultPoints.push(shapePoints[i], shapePoints[i + 1]);
}
const isLastPoint = i === polygonPoints.length - 2;
if (
isLastPoint
&& (this.drawData.shapeType === 'polyline'
|| (this.drawData.shapeType === 'polygon' && polygonPoints.length === 4))
) {
const isLastPoint = i === shapePoints.length - 2;
if (isLastPoint && (isPolyline || (isPolygon && shapePoints.length === 4))) {
break;
}
const nextPoint = isLastPoint
? { x: polygonPoints[0], y: polygonPoints[1] }
: { x: polygonPoints[i + 2], y: polygonPoints[i + 3] };
? { x: shapePoints[0], y: shapePoints[1] }
: { x: shapePoints[i + 2], y: shapePoints[i + 3] };
const intersectionPoints = findIntersectionsWithFrameBorders(curPoint, nextPoint, direction);
if (intersectionPoints.length !== 0) {
resultPoints.push(...intersectionPoints);
Expand All @@ -187,20 +199,15 @@ export class DrawHandlerImpl implements DrawHandler {
};
}

private getFinalCuboidCoordinates(
targetPoints: number[],
): {
points: number[];
box: Box;
} {
private getFinalCuboidCoordinates(targetPoints: number[]): FinalCoordinates {
const { offset } = this.geometry;
let points = targetPoints;

const box = {
xtl: 0,
ytl: 0,
xbr: Number.MAX_SAFE_INTEGER,
ybr: Number.MAX_SAFE_INTEGER,
xtl: Number.MAX_SAFE_INTEGER,
ytl: Number.MAX_SAFE_INTEGER,
xbr: Number.MIN_SAFE_INTEGER,
ybr: Number.MIN_SAFE_INTEGER,
};

const frameWidth = this.geometry.image.width;
Expand Down Expand Up @@ -238,27 +245,34 @@ export class DrawHandlerImpl implements DrawHandler {

if (cuboidOffsets.length === points.length / 2) {
cuboidOffsets.forEach((offsetCoords: number[]): void => {
if (Math.sqrt((offsetCoords[0] ** 2) + (offsetCoords[1] ** 2)) < minCuboidOffset.d) {
minCuboidOffset.d = Math.sqrt((offsetCoords[0] ** 2) + (offsetCoords[1] ** 2));
const dx = offsetCoords[0] ** 2;
const dy = offsetCoords[1] ** 2;
Comment on lines +248 to +249
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Conflict between prettier and eslint.
There were suggestion to split to several lines:
https://github.com/prettier/eslint-config-prettier#no-mixed-operators

if (Math.sqrt(dx + dy) < minCuboidOffset.d) {
minCuboidOffset.d = Math.sqrt(dx + dy);
[minCuboidOffset.dx, minCuboidOffset.dy] = offsetCoords;
}
});

points = points.map((coord: number, i: number): number => {
const finalCoord = coord + (i % 2 === 0 ? minCuboidOffset.dx : minCuboidOffset.dy);

if (i % 2 === 0) {
box.xtl = Math.max(box.xtl, finalCoord);
box.xbr = Math.min(box.xbr, finalCoord);
} else {
box.ytl = Math.max(box.ytl, finalCoord);
box.ybr = Math.min(box.ybr, finalCoord);
if (i % 2) {
return coord + minCuboidOffset.dy;
}

return finalCoord;
return coord + minCuboidOffset.dx;
});
}

points.forEach((coord: number, i: number): number => {
if (i % 2 === 0) {
box.xtl = Math.min(box.xtl, coord);
box.xbr = Math.max(box.xbr, coord);
} else {
box.ytl = Math.min(box.ytl, coord);
box.ybr = Math.max(box.ybr, coord);
}

return coord;
});

return {
points: points.map((coord: number): number => coord - offset),
box,
Expand Down Expand Up @@ -449,8 +463,9 @@ export class DrawHandlerImpl implements DrawHandler {
} else {
this.drawInstance.draw('update', e);
const deltaTreshold = 15;
const delta = Math.sqrt(((e.clientX - lastDrawnPoint.x) ** 2)
+ ((e.clientY - lastDrawnPoint.y) ** 2));
const dx = (e.clientX - lastDrawnPoint.x) ** 2;
const dy = (e.clientY - lastDrawnPoint.y) ** 2;
const delta = Math.sqrt(dx + dy);
if (delta > deltaTreshold) {
this.drawInstance.draw('point', e);
}
Expand Down Expand Up @@ -482,36 +497,15 @@ export class DrawHandlerImpl implements DrawHandler {
&& (box.xbr - box.xtl) * (box.ybr - box.ytl) >= consts.AREA_THRESHOLD
&& points.length >= 3 * 2
) {
this.onDrawDone(
{
clientID,
shapeType,
points,
},
Date.now() - this.startTimestamp,
);
this.onDrawDone({ clientID, shapeType, points }, Date.now() - this.startTimestamp);
} else if (
shapeType === 'polyline'
&& (box.xbr - box.xtl >= consts.SIZE_THRESHOLD || box.ybr - box.ytl >= consts.SIZE_THRESHOLD)
&& points.length >= 2 * 2
) {
this.onDrawDone(
{
clientID,
shapeType,
points,
},
Date.now() - this.startTimestamp,
);
this.onDrawDone({ clientID, shapeType, points }, Date.now() - this.startTimestamp);
} else if (shapeType === 'points' && (e.target as any).getAttribute('points') !== '0,0') {
this.onDrawDone(
{
clientID,
shapeType,
points,
},
Date.now() - this.startTimestamp,
);
this.onDrawDone({ clientID, shapeType, points }, Date.now() - this.startTimestamp);
// TODO: think about correct constraign for cuboids
} else if (shapeType === 'cuboid' && points.length === 4 * 2) {
this.onDrawDone(
Expand Down
2 changes: 1 addition & 1 deletion cvat-canvas/tsconfig.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"baseUrl": ".",
"emitDeclarationOnly": true,
"module": "es6",
"target": "es6",
"target": "es2016",
"noImplicitAny": true,
"preserveConstEnums": true,
"declaration": true,
Expand Down