diff --git a/.github/tasks.md b/.github/tasks.md index b78a8a9e..8f2b1caf 100644 --- a/.github/tasks.md +++ b/.github/tasks.md @@ -1,3 +1,10 @@ ## Tasks -- [x] Remove the "line_size" property from ULabelAnnotation. Update the entire codebase to ensure no reference to it remains. Instead, use the line_size defined for the annotation's subtask when we need a line size for drawing the annotation. +- [x] Read the discussion in issue [#159](https://github.com/SenteraLLC/ulabel/issues/159) +- [x] Implement a vertex deletion keybind for polygon and polyline spatial types it should: + - [x] Delete the vertex when pressed when hovering over it such that the edit suggestion is showing + - [x] Delete the vertex when pressed when dragging/editing the vertex + - [x] For polylines, if only one point remains in the polyline, it should delete the polyline + - [x] For polygons, if fewer than 3 points remain in a polygon layer, the layer should be removed +- [x] Add a test for the keybind in keybind-functionality.spec.js +- [x] Update the api_spec and changelog diff --git a/CHANGELOG.md b/CHANGELOG.md index 435d8fbc..9e17ecf8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,14 @@ All notable changes to this project will be documented here. ## [unreleased] +## [0.23.0] - Jan 16th, 2026 +- Add vertex deletion keybind for polygon and polyline annotations + - New configurable `delete_vertex_keybind` (default: `x`) + - Delete individual vertices by hovering over them and pressing the keybind + - Automatically deletes entire polyline if only 1 point remains after deletion + - Automatically removes polygon layer if fewer than 3 points remain after deletion +- Fixed bug where deleting an annotation mid-edit would cause the ULabel state to be stuck in edit mode. + ## [0.22.1] - Jan 13th, 2026 - Don't draw annotations when a subtask is vanished - Add configurable `annotation_vanish_all_keybind` diff --git a/api_spec.md b/api_spec.md index e4fddceb..d6d4256e 100644 --- a/api_spec.md +++ b/api_spec.md @@ -56,6 +56,7 @@ class ULabel({ show_full_image_keybind: string, create_point_annotation_keybind: string, delete_annotation_keybind: string, + delete_vertex_keybind: string, keypoint_slider_default_value: number, filter_annotations_on_load: boolean, switch_subtask_keybind: string, @@ -433,6 +434,9 @@ Keybind to create a point annotation at the mouse location. Default is `c`. Requ ### `delete_annotation_keybind` Keybind to delete the annotation that the mouse is hovering over. Default is `d`. +### `delete_vertex_keybind` +Keybind to delete a vertex of a polygon or polyline annotation. The vertex must be the one currently being hovered (showing an edit suggestion) or actively being edited. For polylines, if only one point remains after deletion, the entire polyline is deleted. For polygons, if fewer than 3 points remain in a layer after deletion, that layer is removed. Default is `x`. + ### `keypoint_slider_default_value` Default value for the keypoint slider. Must be a number between 0 and 1. Default is `0`. diff --git a/index.d.ts b/index.d.ts index 7ec35d68..1474e655 100644 --- a/index.d.ts +++ b/index.d.ts @@ -194,6 +194,7 @@ export type ULabelActionType = "create_nonspatial_annotation" | "finish_move" | "cancel_annotation" | "delete_annotation" | + "delete_vertex" | "delete_annotations_in_polygon" | "start_complex_polygon" | "merge_polygon_complex_layer" | @@ -233,6 +234,7 @@ export type ULabelActionCandidate = { point: [number, number]; // Mouse location spatial_type: ULabelSpatialType; offset?: Offset; // Optional offset for move actions + is_vertex?: boolean; // True if hovering over an actual vertex, false if hovering over a segment }; export type ULabelSubtasks = { [key: string]: ULabelSubtask }; @@ -377,6 +379,12 @@ export class ULabel { redoing?: boolean, should_record_action?: boolean, ): void; + public delete_vertex( + annotation_id: string, + access_str: string | number | [number, number], + redoing?: boolean, + should_record_action?: boolean, + ): void; public cancel_annotation(annotation_id?: string): void; public assign_annotation_id(annotation_id?: string, redo_payload?: object): void; public create_point_annotation_at_mouse_location(): void; @@ -414,6 +422,7 @@ export class ULabel { public begin_edit__undo(annotation_id: string, undo_payload: object): void; public begin_move__undo(annotation_id: string, undo_payload: object): void; public delete_annotation__undo(annotation_id: string): void; + public delete_vertex__undo(annotation_id: string, undo_payload: object): void; public cancel_annotation__undo(annotation_id: string, undo_payload: object): void; public assign_annotation_id__undo(annotation_id: string, undo_payload: object): void; public create_annotation__undo(annotation_id: string): void; @@ -431,6 +440,7 @@ export class ULabel { public begin_edit__redo(annotation_id: string, redo_payload: object): void; public begin_move__redo(annotation_id: string, redo_payload: object): void; public delete_annotation__redo(annotation_id: string): void; + public delete_vertex__redo(annotation_id: string, redo_payload: object): void; public create_annotation__redo(annotation_id: string, redo_payload: object): void; public finish_modify_annotation__redo(annotation_id: string, redo_payload: object): void; diff --git a/package-lock.json b/package-lock.json index 4bb0c772..a4345ba5 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "ulabel", - "version": "0.22.1", + "version": "0.23.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "ulabel", - "version": "0.22.1", + "version": "0.23.0", "license": "MIT", "devDependencies": { "@eslint/config-inspector": "^1.3.0", @@ -6971,9 +6971,9 @@ "dev": true }, "node_modules/h3": { - "version": "1.15.4", - "resolved": "https://registry.npmjs.org/h3/-/h3-1.15.4.tgz", - "integrity": "sha512-z5cFQWDffyOe4vQ9xIqNfCZdV4p//vy6fBnr8Q1AWnVZ0teurKMG66rLj++TKwKPUP3u7iMUvrvKaEUiQw2QWQ==", + "version": "1.15.5", + "resolved": "https://registry.npmjs.org/h3/-/h3-1.15.5.tgz", + "integrity": "sha512-xEyq3rSl+dhGX2Lm0+eFQIAzlDN6Fs0EcC4f7BNUmzaRX/PTzeuM+Tr2lHB8FoXggsQIeXLj8EDVgs5ywxyxmg==", "dev": true, "license": "MIT", "dependencies": { @@ -6982,9 +6982,9 @@ "defu": "^6.1.4", "destr": "^2.0.5", "iron-webcrypto": "^1.2.1", - "node-mock-http": "^1.0.2", + "node-mock-http": "^1.0.4", "radix3": "^1.1.2", - "ufo": "^1.6.1", + "ufo": "^1.6.3", "uncrypto": "^0.1.3" } }, @@ -9039,9 +9039,9 @@ "license": "MIT" }, "node_modules/node-mock-http": { - "version": "1.0.3", - "resolved": "https://registry.npmjs.org/node-mock-http/-/node-mock-http-1.0.3.tgz", - "integrity": "sha512-jN8dK25fsfnMrVsEhluUTPkBFY+6ybu7jSB1n+ri/vOGjJxU8J9CZhpSGkHXSkFjtUhbmoncG/YG9ta5Ludqog==", + "version": "1.0.4", + "resolved": "https://registry.npmjs.org/node-mock-http/-/node-mock-http-1.0.4.tgz", + "integrity": "sha512-8DY+kFsDkNXy1sJglUfuODx1/opAGJGyrTuFqEoN90oRc2Vk0ZbD4K2qmKXBBEhZQzdKHIVfEJpDU8Ak2NJEvQ==", "dev": true, "license": "MIT" }, @@ -10825,9 +10825,9 @@ } }, "node_modules/ufo": { - "version": "1.6.1", - "resolved": "https://registry.npmjs.org/ufo/-/ufo-1.6.1.tgz", - "integrity": "sha512-9a4/uxlTWJ4+a5i0ooc1rU7C7YOw3wT+UGqdeNNHWnOF9qcMBgLRS+4IYUqbczewFx4mLEig6gawh7X6mFlEkA==", + "version": "1.6.3", + "resolved": "https://registry.npmjs.org/ufo/-/ufo-1.6.3.tgz", + "integrity": "sha512-yDJTmhydvl5lJzBmy/hyOAA0d+aqCBuwl818haVdYCRrWV84o7YyeVm4QlVHStqNrrJSTb6jKuFAVqAFsr+K3Q==", "dev": true, "license": "MIT" }, diff --git a/package.json b/package.json index 0e3b5dde..4dc68217 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "ulabel", "description": "An image annotation tool.", - "version": "0.22.1", + "version": "0.23.0", "main": "dist/ulabel.min.js", "module": "dist/ulabel.min.js", "exports": { diff --git a/src/actions.ts b/src/actions.ts index 3b5ebc26..1eb2a665 100644 --- a/src/actions.ts +++ b/src/actions.ts @@ -253,6 +253,10 @@ function trigger_action_listeners( action: on_annotation_deletion, undo: on_finish_annotation_spatial_modification, }, + delete_vertex: { + action: on_finish_annotation_spatial_modification, + undo: on_finish_annotation_spatial_modification, + }, assign_annotation_id: { action: on_annotation_id_change, undo: on_annotation_id_change, @@ -570,6 +574,9 @@ function undo_action(ulabel: ULabel, action: ULabelAction) { case "delete_annotation": ulabel.delete_annotation__undo(action.annotation_id); break; + case "delete_vertex": + ulabel.delete_vertex__undo(action.annotation_id, undo_payload); + break; case "cancel_annotation": ulabel.cancel_annotation__undo(action.annotation_id, undo_payload); break; @@ -644,6 +651,9 @@ export function redo_action(ulabel: ULabel, action: ULabelAction) { case "delete_annotation": ulabel.delete_annotation__redo(action.annotation_id); break; + case "delete_vertex": + ulabel.delete_vertex__redo(action.annotation_id, redo_payload); + break; case "cancel_annotation": ulabel.cancel_annotation(action.annotation_id); break; diff --git a/src/configuration.ts b/src/configuration.ts index 0e57f9b6..d475f914 100644 --- a/src/configuration.ts +++ b/src/configuration.ts @@ -198,6 +198,8 @@ export class Configuration { public delete_annotation_keybind: string = "d"; + public delete_vertex_keybind: string = "x"; + public keypoint_slider_default_value: number; public filter_annotations_on_load: boolean = true; diff --git a/src/index.js b/src/index.js index 4e64975f..f5e95e48 100644 --- a/src/index.js +++ b/src/index.js @@ -3087,6 +3087,15 @@ export class ULabel { current_subtask["state"]["starting_complex_polygon"] = false; } + // Clear drag state if we're in the middle of a drag + if (this.drag_state["active_key"] !== null) { + this.drag_state["active_key"] = null; + this.drag_state["release_button"] = null; + } + + // Clear edit candidate + current_subtask["state"]["edit_candidate"] = null; + let frame = this.state["current_frame"]; if (MODES_3D.includes(spatial_type)) { frame = null; @@ -3110,6 +3119,134 @@ export class ULabel { this.delete_annotation(annotation_id, true); } + /** + * Delete a vertex from a polygon or polyline annotation + * @param {string} annotation_id - The ID of the annotation + * @param {string|array} access_str - Access string identifying the vertex to delete + * @param {boolean} redoing - Whether this is a redo operation + * @param {boolean} should_record_action - Whether to record this action in the action stream + */ + delete_vertex(annotation_id, access_str, redoing = false, should_record_action = true) { + const current_subtask = this.get_current_subtask(); + const annotation = current_subtask["annotations"]["access"][annotation_id]; + const spatial_type = annotation["spatial_type"]; + + // Only allow vertex deletion for polygons and polylines + if (spatial_type !== "polygon" && spatial_type !== "polyline") { + return; + } + + let spatial_payload = annotation["spatial_payload"]; + let layer_index = 0; + let vertex_index; + let active_spatial_payload = spatial_payload; + let should_delete = false; + + // Parse access string based on spatial type + if (spatial_type === "polygon") { + // For polygons, access_str is [layer_index, vertex_index] + layer_index = parseInt(access_str[0], 10); + active_spatial_payload = spatial_payload[layer_index]; + vertex_index = parseInt(access_str[1], 10); + } else { + // For polylines, access_str is just the vertex_index + vertex_index = parseInt(access_str, 10); + } + + // Store the old state for undo + const old_spatial_payload = JSON.parse(JSON.stringify(spatial_payload)); + + // Delete the vertex + if (spatial_type === "polygon") { + // For polygons, handle the special case of first/last point + const n_points = active_spatial_payload.length; + if (vertex_index === 0 || vertex_index === n_points - 1) { + // First and last points are the same in a closed polygon + // Remove both + active_spatial_payload.splice(n_points - 1, 1); + active_spatial_payload.splice(0, 1); + // Make the new first point also the last point + if (active_spatial_payload.length > 0) { + active_spatial_payload.push([...active_spatial_payload[0]]); + } + } else { + // Remove the vertex + active_spatial_payload.splice(vertex_index, 1); + } + + // Check if the layer should be removed (fewer than 3 points means fewer than 4 including duplicate) + if (active_spatial_payload.length < 4) { + // Remove the entire layer + spatial_payload.splice(layer_index, 1); + + // If no layers remain, delete the entire annotation + if (spatial_payload.length === 0) { + should_delete = true; + } + } + } else { + // For polylines + active_spatial_payload.splice(vertex_index, 1); + + // If only one point remains, delete the entire polyline + if (active_spatial_payload.length <= 1) { + should_delete = true; + } + } + + // Clear any active edit state + if (current_subtask["state"]["active_id"] === annotation_id) { + current_subtask["state"]["active_id"] = null; + current_subtask["state"]["is_in_edit"] = false; + } + + // Clear drag state if we're in the middle of a drag + if (this.drag_state["active_key"] !== null) { + this.drag_state["active_key"] = null; + this.drag_state["release_button"] = null; + } + + // Clear edit candidate + current_subtask["state"]["edit_candidate"] = null; + + let frame = this.state["current_frame"]; + if (MODES_3D.includes(spatial_type)) { + frame = null; + } + + // Record the action + record_action(this, { + act_type: "delete_vertex", + annotation_id: annotation_id, + frame: frame, + undo_payload: { + old_spatial_payload: old_spatial_payload, + deleted: should_delete, + }, + redo_payload: { + access_str: access_str, + }, + }, redoing, should_record_action); + + // If the entire annotation should be deleted, do so + if (should_delete) { + this.delete_annotation(annotation_id, false, false); + } + } + + delete_vertex__undo(annotation_id, undo_payload) { + const annotation = this.get_current_subtask()["annotations"]["access"][annotation_id]; + annotation["spatial_payload"] = undo_payload.old_spatial_payload; + + if (undo_payload.deleted) { + this.delete_annotation__undo(annotation_id); + } + } + + delete_vertex__redo(annotation_id, redo_payload) { + this.delete_vertex(annotation_id, redo_payload.access_str, true); + } + /** * Get the annotation with nearest active keypoint (e.g. corners for a bbox, endpoints for polylines) to a point * @param {*} global_x @@ -3124,6 +3261,7 @@ export class ULabel { access: null, distance: max_dist / this.get_empirical_scale(), point: null, + is_vertex: true, }; if (candidates === null) { candidates = this.get_current_subtask()["annotations"]["ordering"]; @@ -3227,6 +3365,7 @@ export class ULabel { access: null, distance: max_dist / this.get_empirical_scale(), point: null, + is_vertex: false, }; if (candidates === null) { candidates = this.get_current_subtask()["annotations"]["ordering"]; diff --git a/src/listeners.ts b/src/listeners.ts index 1b0f50ae..bdc42af2 100644 --- a/src/listeners.ts +++ b/src/listeners.ts @@ -604,6 +604,16 @@ export function create_ulabel_listeners( ulabel.delete_annotation(edit_cand.annid); } } + // Check the key pressed against the delete vertex keybind in the config + if (event_matches_keybind(keypress_event, ulabel.config.delete_vertex_keybind)) { + const current_subtask = ulabel.get_current_subtask(); + const edit_cand = current_subtask.state.edit_candidate; + + // Only delete if we have an edit candidate that is an actual vertex (not a segment point) + if (edit_cand !== null && edit_cand.is_vertex === true) { + ulabel.delete_vertex(edit_cand.annid, edit_cand.access); + } + } }, ); diff --git a/src/toolbox_items/keybinds.ts b/src/toolbox_items/keybinds.ts index f55f9c2b..d4489e54 100644 --- a/src/toolbox_items/keybinds.ts +++ b/src/toolbox_items/keybinds.ts @@ -316,6 +316,14 @@ export class KeybindsToolboxItem extends ToolboxItem { config_key: "delete_annotation_keybind", }); + keybinds.push({ + key: config.delete_vertex_keybind, + label: "Delete Vertex", + description: "Delete a vertex from polygon/polyline (hover or drag)", + configurable: true, + config_key: "delete_vertex_keybind", + }); + keybinds.push({ key: config.switch_subtask_keybind, label: "Switch Subtask", diff --git a/src/version.js b/src/version.js index 4b5eb401..b9e6a99b 100644 --- a/src/version.js +++ b/src/version.js @@ -1 +1 @@ -export const ULABEL_VERSION = "0.22.1"; +export const ULABEL_VERSION = "0.23.0"; diff --git a/tests/e2e/keybind-functionality.spec.js b/tests/e2e/keybind-functionality.spec.js index 7a9c79e8..1f66ad77 100644 --- a/tests/e2e/keybind-functionality.spec.js +++ b/tests/e2e/keybind-functionality.spec.js @@ -2,7 +2,7 @@ import { test, expect } from "./fixtures"; import { wait_for_ulabel_init } from "../testing-utils/init_utils"; import { get_annotation_count, get_annotation_by_index, get_annotation_class_id } from "../testing-utils/annotation_utils"; -import { draw_bbox } from "../testing-utils/drawing_utils"; +import { draw_bbox, draw_polygon, draw_polyline } from "../testing-utils/drawing_utils"; import { get_current_subtask_key, get_current_subtask } from "../testing-utils/subtask_utils"; /** @@ -706,4 +706,106 @@ test.describe("Keybind Functionality Tests", () => { annotation = await get_annotation_by_index(page, 0); expect(get_annotation_class_id(annotation)).toBe(10); // Still Sedan }); + + test("delete_vertex_keybind should delete a vertex from a polygon when hovering", async ({ page }) => { + await wait_for_ulabel_init(page); + + // Get the delete vertex keybind from the toolbox + const keybind = await get_keybind_value(page, "Delete Vertex"); + + // Draw a polygon with 4 points (square) + await draw_polygon(page, [ + [200, 200], + [400, 200], + [400, 400], + [200, 400], + ]); + await page.waitForTimeout(200); + + // Get the annotation and verify it has 5 points (4 + duplicate first/last) + let annotation = await get_annotation_by_index(page, 0); + expect(annotation.spatial_payload[0].length).toBe(5); + + // Move mouse to hover over a vertex (point 4) + await page.mouse.move(200, 400); + await page.waitForTimeout(200); + + // Verify edit suggestion is showing and it's a vertex + let edit_candidate = await page.evaluate(() => { + return window.ulabel.get_current_subtask().state.edit_candidate; + }); + expect(edit_candidate).not.toBeNull(); + expect(edit_candidate.is_vertex).toBe(true); + + // Press the delete vertex keybind + await press_keybind(page, keybind); + await page.waitForTimeout(200); + + // Verify the polygon now has 4 points (3 + duplicate first/last) + annotation = await get_annotation_by_index(page, 0); + expect(annotation.spatial_payload[0].length).toBe(4); + + // Move mouse to hover over a segment (midpoint between two vertices) + await page.mouse.move(300, 200); // Midpoint of top edge + await page.waitForTimeout(200); + + // Verify edit suggestion is showing but it's NOT a vertex + edit_candidate = await page.evaluate(() => { + return window.ulabel.get_current_subtask().state.edit_candidate; + }); + expect(edit_candidate).not.toBeNull(); + expect(edit_candidate.is_vertex).toBe(false); + + // Press the delete vertex keybind + await press_keybind(page, keybind); + await page.waitForTimeout(200); + + // Verify the polygon still has the same number of points (not deleted) + annotation = await get_annotation_by_index(page, 0); + expect(annotation.spatial_payload[0].length).toBe(4); + + // Move mouse to hover over a vertex + await page.mouse.move(400, 200); + await page.waitForTimeout(200); + + // Press the delete vertex keybind + await press_keybind(page, keybind); + await page.waitForTimeout(200); + + // Verify the polygon is now deprecated + annotation = await get_annotation_by_index(page, 0); + expect(annotation.deprecated).toBe(true); + }); + + test("delete_vertex_keybind should delete entire polyline when only 1 point remains", async ({ page }) => { + await wait_for_ulabel_init(page); + + // Get the delete vertex keybind from the toolbox + const keybind = await get_keybind_value(page, "Delete Vertex"); + + // Draw a polyline with 2 points + await draw_polyline(page, [ + [200, 200], + [400, 400], + ]); + await page.waitForTimeout(200); + + // Verify polyline exists with 2 points + let annotation = await get_annotation_by_index(page, 0); + expect(annotation.spatial_type).toBe("polyline"); + expect(annotation.spatial_payload.length).toBe(2); + expect(annotation.deprecated).toBe(false); + + // Move mouse to hover over a vertex + await page.mouse.move(200, 200); + await page.waitForTimeout(300); + + // Press the delete vertex keybind + await press_keybind(page, keybind); + await page.waitForTimeout(200); + + // Verify the polyline is now deprecated (deleted) + annotation = await get_annotation_by_index(page, 0); + expect(annotation.deprecated).toBe(true); + }); }); diff --git a/tests/testing-utils/drawing_utils.js b/tests/testing-utils/drawing_utils.js index c5b8e47e..8ff3d458 100644 --- a/tests/testing-utils/drawing_utils.js +++ b/tests/testing-utils/drawing_utils.js @@ -52,3 +52,70 @@ export async function draw_point(page, position) { return [point]; }, [position]); } + +/** + * Draw a polygon and return its spatial payload. + * + * @param {Page} page + * @param {[number, number][]} points + * @returns {Promise<[ [number, number][] ]>} The polygon spatial payload in image coordinates. + */ +export async function draw_polygon(page, points) { + // Switch to polygon mode + await page.click("a#md-btn--polygon"); + + for (let i = 0; i < points.length; i++) { + const point = points[i]; + await page.mouse.move(point[0], point[1]); + await page.mouse.click(point[0], point[1]); + } + + // Close the polygon + await page.click(".ender_outer"); + await page.waitForTimeout(200); + + // Convert coordinates to image space + // Polygon spatial payload is an array of layers, where each layer has points with first/last duplicated + return await page.evaluate((pts) => { + const image_points = pts.map((pt) => + window.ulabel.get_image_aware_mouse_x_y( + { pageX: pt[0], pageY: pt[1] }, + ), + ); + // Duplicate the first point at the end to close the polygon (ulabel format) + image_points.push([...image_points[0]]); + return [image_points]; + }, points); +} + +/** + * Draw a polyline and return its spatial payload. + * + * @param {Page} page + * @param {[number, number][]} points + * @returns {Promise<[ [number, number][] ]>} The polyline spatial payload in image coordinates. + */ +export async function draw_polyline(page, points) { + // Switch to polyline mode + await page.click("a#md-btn--polyline"); + + for (let i = 0; i < points.length; i++) { + const point = points[i]; + await page.mouse.move(point[0], point[1]); + await page.mouse.click(point[0], point[1]); + } + + // Finish the polyline by right clicking in place + await page.mouse.click(points[points.length - 1][0], points[points.length - 1][1], { button: "right" }); + await page.waitForTimeout(200); + + // Convert coordinates to image space + return await page.evaluate((pts) => { + const image_points = pts.map((pt) => + window.ulabel.get_image_aware_mouse_x_y( + { pageX: pt[0], pageY: pt[1] }, + ), + ); + return [image_points]; + }, points); +}