From e42c26d4897201d0e1b131982043252953e981ee Mon Sep 17 00:00:00 2001 From: klakhov Date: Thu, 3 Mar 2022 15:44:40 +0300 Subject: [PATCH 1/5] fixed incorrect deletion --- cvat-canvas/src/typescript/canvasView.ts | 28 +++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/cvat-canvas/src/typescript/canvasView.ts b/cvat-canvas/src/typescript/canvasView.ts index 6a4348a34725..51bd2690d5a5 100644 --- a/cvat-canvas/src/typescript/canvasView.ts +++ b/cvat-canvas/src/typescript/canvasView.ts @@ -793,6 +793,15 @@ export class CanvasViewImpl implements CanvasView, Listener { if (['polygon', 'polyline', 'points'].includes(state.shapeType)) { if (e.altKey) { + if (state.shapeType === 'points') { + const selectedClientID = parseInt( + ((e.target as HTMLElement).parentElement as HTMLElement).getAttribute('clientID'), 10, + ); + + if (state.clientID !== selectedClientID) { + return; + } + } const { points } = state; this.onEditDone(state, points.slice(0, pointID * 2).concat(points.slice(pointID * 2 + 2))); } else if (e.shiftKey) { @@ -860,6 +869,8 @@ export class CanvasViewImpl implements CanvasView, Listener { if (value) { const getGeometry = (): Geometry => this.geometry; + const getController = (): CanvasController => this.controller; + const getActiveElement = (): ActiveElement => this.activeElement; (shape as any).selectize(value, { deepSelect: true, pointSize: (2 * consts.BASE_POINT_SIZE) / this.geometry.scale, @@ -875,7 +886,22 @@ export class CanvasViewImpl implements CanvasView, Listener { 'stroke-width': consts.POINTS_STROKE_WIDTH / getGeometry().scale, }); - circle.on('mouseenter', (): void => { + circle.on('mouseenter', (e: MouseEvent): void => { + const activeElement = getActiveElement(); + if (activeElement !== null && e.altKey) { + const [state] = getController().objects.filter( + (_state: any): boolean => _state.clientID === activeElement.clientID, + ); + if (state.shapeType === 'points') { + const selectedClientID = parseInt( + ((e.target as HTMLElement).parentElement as HTMLElement).getAttribute('clientID'), 10, + ); + if (state.clientID !== selectedClientID) { + return; + } + } + } + circle.attr({ 'stroke-width': consts.POINTS_SELECTED_STROKE_WIDTH / getGeometry().scale, }); From ac2431589c25a2e2949fc53ba432a4961df03300 Mon Sep 17 00:00:00 2001 From: klakhov Date: Sat, 5 Mar 2022 10:13:01 +0300 Subject: [PATCH 2/5] update changelog --- CHANGELOG.md | 2 +- cvat-canvas/package-lock.json | 4 ++-- cvat-canvas/package.json | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 289f0210bd24..8ed080d7bf65 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,7 +19,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - TDB ### Fixed -- TDB +- Bug: Incorrect point deletion with keyboard shortcut () ### Security - TDB diff --git a/cvat-canvas/package-lock.json b/cvat-canvas/package-lock.json index 0ab129552269..4c915d045a79 100644 --- a/cvat-canvas/package-lock.json +++ b/cvat-canvas/package-lock.json @@ -1,12 +1,12 @@ { "name": "cvat-canvas", - "version": "2.13.1", + "version": "2.13.2", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "cvat-canvas", - "version": "2.13.1", + "version": "2.13.2", "license": "MIT", "dependencies": { "@types/polylabel": "^1.0.5", diff --git a/cvat-canvas/package.json b/cvat-canvas/package.json index 776dd7ca4c11..4c5d7da82d56 100644 --- a/cvat-canvas/package.json +++ b/cvat-canvas/package.json @@ -1,6 +1,6 @@ { "name": "cvat-canvas", - "version": "2.13.1", + "version": "2.13.2", "description": "Part of Computer Vision Annotation Tool which presents its canvas library", "main": "src/canvas.ts", "scripts": { From 68cb6385676a669ef225b7cc451218eae9b99cef Mon Sep 17 00:00:00 2001 From: klakhov Date: Thu, 10 Mar 2022 14:28:38 +0300 Subject: [PATCH 3/5] added test --- .../issues_prs2/issue_3821_delete_point.js | 73 +++++++++++++++++++ 1 file changed, 73 insertions(+) create mode 100644 tests/cypress/integration/issues_prs2/issue_3821_delete_point.js diff --git a/tests/cypress/integration/issues_prs2/issue_3821_delete_point.js b/tests/cypress/integration/issues_prs2/issue_3821_delete_point.js new file mode 100644 index 000000000000..16f43236312f --- /dev/null +++ b/tests/cypress/integration/issues_prs2/issue_3821_delete_point.js @@ -0,0 +1,73 @@ +// Copyright (C) 2022 Intel Corporation +// +// SPDX-License-Identifier: MIT + +/// + +import { taskName, labelName } from '../../support/const'; + +context('When delete a point, the required point is deleted.', () => { + const issueId = '3821'; + + const firstPointsShape = { + labelName, + type: 'Shape', + pointsMap: [ + { x: 300, y: 250 }, + { x: 300, y: 350 }, + { x: 300, y: 450 }, + ], + }; + const secondPointsShape = { + labelName, + type: 'Shape', + pointsMap: [ + { x: 330, y: 250 }, + { x: 330, y: 350 }, + { x: 330, y: 450 }, + ], + }; + + before(() => { + cy.openTaskJob(taskName); + }); + + describe(`Testing issue "${issueId}"`, () => { + it('Crearte points shapes', () => { + cy.createPoint(firstPointsShape); + cy.get('#cvat-objects-sidebar-state-item-1').should('contain', '1').and('contain', 'POINTS SHAPE'); + cy.createPoint(secondPointsShape); + cy.get('#cvat-objects-sidebar-state-item-2').should('contain', '2').and('contain', 'POINTS SHAPE'); + }); + it('Remove point holding Alt key from each shape', () => { + cy.get('#cvat_canvas_shape_1').trigger('mousemove', { force: true }).trigger('mouseover', { force: true }); + cy.get('body').type('{alt}', { release: false }); + cy.get('#cvat_canvas_shape_1') + .children() + .then((children) => { + cy.get(children) + .eq(1) + .then((elem) => { + cy.get(elem).click(); + }); + }); + cy.get('#cvat_canvas_shape_2') + .children() + .then((children) => { + cy.get(children) + .eq(1) + .then((elem) => { + cy.get(elem).click(); + }); + }); + }); + it('Point must be removed from first shape, second one should stay the same', () => { + cy.get('#cvat_canvas_shape_1') + .children() + .should('have.length', 2); + cy.get('#cvat_canvas_shape_2') + .children() + .should('have.length', 3); + }); + }); +}); From 34e2b70f89236907f671f651aaa51b7f2af3fed6 Mon Sep 17 00:00:00 2001 From: klakhov Date: Fri, 11 Mar 2022 09:44:33 +0300 Subject: [PATCH 4/5] applied comments --- cvat-canvas/src/typescript/canvasView.ts | 20 ++--- .../issues_prs2/issue_3821_delete_point.js | 82 ++++++++++++------- 2 files changed, 59 insertions(+), 43 deletions(-) diff --git a/cvat-canvas/src/typescript/canvasView.ts b/cvat-canvas/src/typescript/canvasView.ts index 51bd2690d5a5..ec396afb3baa 100644 --- a/cvat-canvas/src/typescript/canvasView.ts +++ b/cvat-canvas/src/typescript/canvasView.ts @@ -792,16 +792,14 @@ export class CanvasViewImpl implements CanvasView, Listener { ); if (['polygon', 'polyline', 'points'].includes(state.shapeType)) { - if (e.altKey) { - if (state.shapeType === 'points') { - const selectedClientID = parseInt( - ((e.target as HTMLElement).parentElement as HTMLElement).getAttribute('clientID'), 10, - ); + if (state.shapeType === 'points' && (e.altKey || e.ctrlKey)) { + const selectedClientID = +((e.target as HTMLElement).parentElement as HTMLElement).getAttribute('clientID'); - if (state.clientID !== selectedClientID) { - return; - } + if (state.clientID !== selectedClientID) { + return; } + } + if (e.altKey) { const { points } = state; this.onEditDone(state, points.slice(0, pointID * 2).concat(points.slice(pointID * 2 + 2))); } else if (e.shiftKey) { @@ -888,14 +886,12 @@ export class CanvasViewImpl implements CanvasView, Listener { circle.on('mouseenter', (e: MouseEvent): void => { const activeElement = getActiveElement(); - if (activeElement !== null && e.altKey) { + if (activeElement !== null && (e.altKey || e.ctrlKey)) { const [state] = getController().objects.filter( (_state: any): boolean => _state.clientID === activeElement.clientID, ); if (state.shapeType === 'points') { - const selectedClientID = parseInt( - ((e.target as HTMLElement).parentElement as HTMLElement).getAttribute('clientID'), 10, - ); + const selectedClientID = +((e.target as HTMLElement).parentElement as HTMLElement).getAttribute('clientID'); if (state.clientID !== selectedClientID) { return; } diff --git a/tests/cypress/integration/issues_prs2/issue_3821_delete_point.js b/tests/cypress/integration/issues_prs2/issue_3821_delete_point.js index 16f43236312f..e1fbd5b023e9 100644 --- a/tests/cypress/integration/issues_prs2/issue_3821_delete_point.js +++ b/tests/cypress/integration/issues_prs2/issue_3821_delete_point.js @@ -9,37 +9,28 @@ import { taskName, labelName } from '../../support/const'; context('When delete a point, the required point is deleted.', () => { const issueId = '3821'; - const firstPointsShape = { - labelName, - type: 'Shape', - pointsMap: [ - { x: 300, y: 250 }, - { x: 300, y: 350 }, - { x: 300, y: 450 }, - ], - }; - const secondPointsShape = { - labelName, - type: 'Shape', - pointsMap: [ - { x: 330, y: 250 }, - { x: 330, y: 350 }, - { x: 330, y: 450 }, - ], - }; + const pointsShapes = []; + for (let i = 0; i < 4; i++) { + pointsShapes.push({ + labelName, + type: 'Shape', + pointsMap: [ + { x: 300 + i * 50, y: 250 }, + { x: 300 + i * 50, y: 350 }, + { x: 300 + i * 50, y: 450 }, + ], + }); + } before(() => { cy.openTaskJob(taskName); + pointsShapes.forEach((shape) => { + cy.createPoint(shape); + }); }); describe(`Testing issue "${issueId}"`, () => { - it('Crearte points shapes', () => { - cy.createPoint(firstPointsShape); - cy.get('#cvat-objects-sidebar-state-item-1').should('contain', '1').and('contain', 'POINTS SHAPE'); - cy.createPoint(secondPointsShape); - cy.get('#cvat-objects-sidebar-state-item-2').should('contain', '2').and('contain', 'POINTS SHAPE'); - }); - it('Remove point holding Alt key from each shape', () => { + it('Remove point holding Alt key from each shape. Point must be removed from first shape, second one should stay the same', () => { cy.get('#cvat_canvas_shape_1').trigger('mousemove', { force: true }).trigger('mouseover', { force: true }); cy.get('body').type('{alt}', { release: false }); cy.get('#cvat_canvas_shape_1') @@ -47,8 +38,8 @@ context('When delete a point, the required point is deleted.', () => { .then((children) => { cy.get(children) .eq(1) - .then((elem) => { - cy.get(elem).click(); + .then((point) => { + cy.get(point).click(); }); }); cy.get('#cvat_canvas_shape_2') @@ -56,12 +47,10 @@ context('When delete a point, the required point is deleted.', () => { .then((children) => { cy.get(children) .eq(1) - .then((elem) => { - cy.get(elem).click(); + .then((point) => { + cy.get(point).click(); }); }); - }); - it('Point must be removed from first shape, second one should stay the same', () => { cy.get('#cvat_canvas_shape_1') .children() .should('have.length', 2); @@ -69,5 +58,36 @@ context('When delete a point, the required point is deleted.', () => { .children() .should('have.length', 3); }); + + it('Remove point holding Ctrl key from each shape. Point must be removed from first shape, second one should stay the same', () => { + cy.get('#cvat_canvas_shape_3').trigger('mousemove', { force: true }).trigger('mouseover', { force: true }); + cy.get('body').type('{ctrl}', { release: false }); + cy.get('#cvat_canvas_shape_3') + .children() + .then((children) => { + cy.get(children) + .eq(1) + .then((point) => { + cy.get(point).rightclick(); + }); + }); + cy.contains('Delete point').click(); + cy.get('#cvat_canvas_shape_4') + .children() + .then((children) => { + cy.get(children) + .eq(1) + .then((point) => { + cy.get(point).rightclick(); + }); + }); + cy.contains('Delete point').should('not.exist'); + cy.get('#cvat_canvas_shape_3') + .children() + .should('have.length', 2); + cy.get('#cvat_canvas_shape_4') + .children() + .should('have.length', 3); + }); }); }); From 8091e248b67ade4c3baa1ae1a32684d16c41a5bf Mon Sep 17 00:00:00 2001 From: klakhov Date: Fri, 11 Mar 2022 09:51:16 +0300 Subject: [PATCH 5/5] added small check --- cvat-canvas/src/typescript/canvasView.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cvat-canvas/src/typescript/canvasView.ts b/cvat-canvas/src/typescript/canvasView.ts index ec396afb3baa..4cc1377f54b0 100644 --- a/cvat-canvas/src/typescript/canvasView.ts +++ b/cvat-canvas/src/typescript/canvasView.ts @@ -890,7 +890,7 @@ export class CanvasViewImpl implements CanvasView, Listener { const [state] = getController().objects.filter( (_state: any): boolean => _state.clientID === activeElement.clientID, ); - if (state.shapeType === 'points') { + if (state?.shapeType === 'points') { const selectedClientID = +((e.target as HTMLElement).parentElement as HTMLElement).getAttribute('clientID'); if (state.clientID !== selectedClientID) { return;