Skip to content

Commit 900ae6b

Browse files
fix geometric utils bug with polygon deletion
1 parent 41b4561 commit 900ae6b

4 files changed

Lines changed: 58 additions & 41 deletions

File tree

.github/tasks.md

Lines changed: 1 addition & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1,38 +1,3 @@
1-
## Tasks: Fix Type Declarations for npm Consumers
1+
## Tasks
22

3-
### Phase 1: Enable Declaration Generation
4-
- [x] Add `declaration: true` and `declarationMap: true` to `tsconfig.json`
5-
- [x] Add `"files"` field to `package.json` to include `dist/`, `build/`, `src/`, `index.d.ts`
6-
7-
### Phase 2: Fix Incorrect Types in `index.d.ts`
8-
- [x] Fix `get_allowed_toolbox_item_enum()` return type → `typeof AllowedToolboxItem`
9-
- [x] Fix `get_resize_toolbox_item()` return type → `typeof AnnotationResizeItem`
10-
- [x] Fix `get_annotations` / `set_annotations` parameter type → `string` (subtask key)
11-
- [x] Update constructor to accept kwargs object with deprecated overload
12-
13-
### Phase 3: Add Missing Declarations
14-
- [x] Add `static version(): string` and instance `version(): string`
15-
- [x] Re-export `AllowedToolboxItem`, `ULabelAnnotation`, `ULabelSubtask` from `index.d.ts`
16-
17-
### Phase 4: Verify
18-
- [x] Run `npm run build` successfully
19-
- [x] Run `npm run lint` successfully
20-
21-
### Phase 5: Fix Behavioral Changes from Strict TypeScript Migration
22-
- [x] Revert null→undefined changes in `suggest_edits`, `fly_to_annotation_id`, `show_global_edit_suggestion`
23-
- [x] Preserve `task_meta` null value in submit payload
24-
- [x] Preserve `class_def.keybind` null value in `original_class_keybinds`
25-
- [x] Widen `index.d.ts` types to accept null where runtime uses null
26-
- [x] Unit tests for keybind storage and configuration defaults
27-
- [x] E2E tests for API behavior contracts (api-behavior.spec.js)
28-
- [x] Assess `annotation_operators.ts` default value changes (id=0, distance=Infinity, distances={distance:0})
29-
- [x] Add annotation classification unit test
30-
- [x] Add distance_from property e2e test
31-
- [x] Revert `show_annotation_mode(undefined)``null` in html_builder.ts
32-
- [x] Revert `redraw_all_annotations(key, undefined, false)``null` in toolbox.ts (2 sites)
33-
- [x] Widen `index.d.ts` types for `redraw_all_annotations` offset and `show_annotation_mode` el
34-
- [x] Unit tests for `replaceLowerConcat` (split/join behavior)
35-
- [x] E2E tests for `show_annotation_mode(null)`, `redraw_all_annotations` with null offset/subtask
36-
- [x] Run full lint + build + test validation
37-
- [ ] Run full lint + build + test validation
383

src/geometric_utils.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -204,8 +204,10 @@ export class GeometricUtils {
204204
return turf.length(b) - turf.length(a);
205205
});
206206

207-
// Return the longest remaining split
208-
// TODO: split into multiple polylines?
207+
// Return the longest remaining split, or empty if all parts were inside the polygon
208+
if (remaining_splits.features.length === 0) {
209+
return [];
210+
}
209211
return <ULabelSpatialPayload2D>remaining_splits.features[0].geometry.coordinates;
210212
}
211213

src/utilities.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import { ULabel } from "../src/index";
77
import { ULabelSubtask } from "./subtask";
88
import { DELETE_CLASS_ID, DELETE_MODES } from "./annotation";
9+
import { log_message, LogLevel } from "./error_logging";
910

1011
/**
1112
* Checks if something is an object, not an array, and not null
@@ -61,12 +62,14 @@ export function get_active_class_id(ulabel: ULabel): number | undefined {
6162

6263
// If the payload is an object then return its id if its confidence is > 0
6364
if (payload.confidence > 0) {
64-
console.log(`payload: ${payload}`);
6565
return payload.class_id;
6666
}
6767
}
68-
console.error(`get_active_class_id was unable to determine an active class id.
69-
current_subtask: ${JSON.stringify(current_subtask)}`);
68+
log_message(
69+
`get_active_class_id was unable to determine an active class id. current_subtask: ${JSON.stringify(current_subtask)}`,
70+
LogLevel.WARNING,
71+
);
72+
return undefined;
7073
}
7174

7275
/**

tests/geometric_utils.test.js

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
// Tests for geometric utility functions
2+
const { GeometricUtils } = require("../build/geometric_utils");
3+
4+
describe("GeometricUtils", () => {
5+
describe("subtract_simple_polygon_from_polyline", () => {
6+
test("should return empty array when polyline is entirely inside polygon", () => {
7+
// A polyline completely inside a large polygon
8+
const polyline = [[2, 2], [3, 3], [4, 4]];
9+
const polygon = [[0, 0], [10, 0], [10, 10], [0, 10], [0, 0]];
10+
11+
const result = GeometricUtils.subtract_simple_polygon_from_polyline(polyline, polygon);
12+
expect(result).toEqual([]);
13+
});
14+
15+
test("should return the polyline unchanged when entirely outside polygon", () => {
16+
// A polyline completely outside the polygon
17+
const polyline = [[20, 20], [30, 30], [40, 40]];
18+
const polygon = [[0, 0], [10, 0], [10, 10], [0, 10], [0, 0]];
19+
20+
const result = GeometricUtils.subtract_simple_polygon_from_polyline(polyline, polygon);
21+
expect(result).toEqual(polyline);
22+
});
23+
24+
test("should return partial polyline when it crosses through the polygon", () => {
25+
// A polyline that crosses through the polygon
26+
const polyline = [[-5, 5], [5, 5], [15, 5]];
27+
const polygon = [[0, 0], [10, 0], [10, 10], [0, 10], [0, 0]];
28+
29+
const result = GeometricUtils.subtract_simple_polygon_from_polyline(polyline, polygon);
30+
// Should return a portion of the line (the longest part outside the polygon)
31+
expect(result.length).toBeGreaterThan(0);
32+
});
33+
34+
test("should not throw when all split segments are inside the polygon", () => {
35+
// A polyline that enters and exits but the remaining parts after split
36+
// are all inside — edge case that previously caused a crash
37+
const polyline = [[1, 5], [5, 5], [9, 5]];
38+
const polygon = [[0, 0], [10, 0], [10, 10], [0, 10], [0, 0]];
39+
40+
// Should not throw, should return empty array
41+
expect(() => {
42+
const result = GeometricUtils.subtract_simple_polygon_from_polyline(polyline, polygon);
43+
expect(result).toEqual([]);
44+
}).not.toThrow();
45+
});
46+
});
47+
});

0 commit comments

Comments
 (0)