diff --git a/.github/tasks.md b/.github/tasks.md index 1cd7fc41..2b7f9ad1 100644 --- a/.github/tasks.md +++ b/.github/tasks.md @@ -454,9 +454,12 @@ time than at save time. - [x] 6.1 Unterminated `/**` block in `annotation_operators.ts`, left behind by the 1.3 `mark_hidden` removal. -- [ ] 6.2 Mask barrier escape hatch. Read-only bitmasks still participate in - `resolve_bitmask_overlap`, so a `prediction` or `diff` mask invisibly clips - a GT brush stroke. Needed before segmentation editing ships. +- [x] 6.2 Brush strokes no longer interact with other subtasks' masks by + default: new `brush_overlap_across_subtasks` config flag (default false) + scopes overlap resolution to the active subtask. Supersedes the planned + read-only "barrier escape hatch": with the flag off a `prediction` or + `diff` mask cannot invisibly clip a GT stroke at all; with it on, + read-only masks still act as barriers under overwrite. - [ ] 6.3 (only if live GT editing during diff review is required) Apply an edit to a non-current subtask and record it in that subtask's undo stream. `set_subtask` clears hover and `fly_to_idx`, so a review queue cannot @@ -473,13 +476,60 @@ time than at save time. derived from (GT, run) and goes stale the moment GT is edited - the resolved FN keeps rendering as an FN. That is a repaint problem, which is what 7.9 solves without client-side rematching. -- [ ] 6.4 Two different `get_active_class_id` implementations disagree. The +- [x] 6.4 Two different `get_active_class_id` implementations disagree. The `ULabel` *method* (`index.js`) parses the selected toolbox anchor's id out of the DOM; the *utility* of the same name (`utilities.ts`) reads `state.id_payload`. The method throws outright before the toolbox has rendered, and the two can diverge whenever state changes without a DOM sync. Phase 5 uses the state-based one; the method's four remaining call sites should follow, and one of the two names should go. + RESOLVED in Phase 9: the method is now a thin delegate to the state-based + utility, so there is a single implementation (the DOM parse is gone). +- [x] 6.5 Switching subtasks left the Brush toolbox buttons lit: brush state + is per-subtask but the buttons are global, and `set_subtask` never tore the + outgoing brush down. Button display is now centralized in + `update_brush_toolbox_display()` (derived from current-subtask state) and + `set_subtask` disables the outgoing brush while it is still current. +- [x] 6.6 An opacity-0 subtask was still fully interactive (invisible + annotations could be created/edited). Vanish mode already encodes + "invisible implies non-interactive"; its gates (create, suggest_edits, + drag start, resize) now check a shared `is_subtask_hidden()` helper + (vanished OR opacity slider at 0). Draw gates deliberately still check + only `is_vanished`: opacity is CSS-only, so content must stay drawn for + the slider to reveal it without a redraw. +- [x] 6.7 Class keybinds edited live in the Keybinds toolbox item only wrote + the current subtask's class defs, while storage and the init restore are + id-keyed across all subtasks — shared-id classes (model-registry GT/pred) + showed the bind but didn't respond until a reload. Edits/resets now write + every subtask holding the id (`set_class_keybind_in_all_subtasks`). The + keypress listener's read-only gate also moved inside the reclass branch, + so selection keybinds work on read-only subtasks. +- [x] 6.8 Optional `show_loader = true` parameter on `set_annotations` / + `set_annotations_batch`: hosts pass false when every changed subtask is a + background layer, so the loading overlay doesn't flash over an unchanged + on-screen view. +- [x] 6.9 Host callback `on_focus_active_class_change(subtask_key, enabled)` + config option, fired from the single writer (`set_focus_active_class`, + which the focus keybind also funnels through) only on actual change — the + guard is what lets a host re-sync other subtasks from the callback + without recursing. +- [x] 6.10 Review fixes: `get_active_class_id_idx()` returned -1 in delete + modes (the active class resolves to DELETE_CLASS_ID, which has no index) + and three callers used it as an index — a class keybind press (through the + dead gate below) or an API `set_active_class` during a delete mode with an + annotation hovered zeroed its classification (class buttons are hidden in + delete modes, so no button path). Now falls back to the frozen real + selection, and `handle_id_dialog_click` rejects out-of-range indices. + Also: the class-keybind delete-mode gate read the never-assigned + `state.spatial_type` (now `annotation_mode`), `set_subtask` clears the + outgoing subtask's move/edit candidates, and `set_class_counter_options` / + `set_class_color(s)` gained `is_destroyed` guards. +- [x] 6.11 Layer opacity is cached in subtask state (`state.layer_opacity`) + instead of read from the slider DOM on every mousemove: + `readjust_subtask_opacities` is the DOM-to-state sync point (it already + runs on every slider input and at the end of `set_subtask`, covering all + slider writers) and `set_subtask_opacity` writes the cache directly. Also + removed the never-assigned `state.spatial_type` type field (the 6.10 trap). ### Phase 7 - model-registry (branch `three-fixed-subtasks` off `cropped-bitmasks-trevor`) @@ -572,6 +622,89 @@ time than at save time. the back-canvas removal) as a PR that can land immediately, architecture as another. Manual work, since `cropped-bitmasks`'s three commits mix both. +## Plan: fold class focus into class selection (edit-mode prep) + +Verified starting facts: `focused_class` is written only by `set_class_focus` +(host API; nothing in ULabel's UI touches it), toolbox class selection +(`id_payload`) is an independent axis, and model-registry never exercises the +null-focus state - `activeDiffLabel` falls back to `labels[0]` and +`activeOutcome` defaults `"tp"`, with no deselect gesture, so exactly one +class/outcome is focused at all times. Two axes that must always agree is +drift waiting for edit mode: the user could draw with a class that is +currently defocused. Fold focus into the selection under a per-subtask +opt-in, and give "set the active class" a real API instead of DOM clicks. + +### Phase 9 - ULabel: `set_active_class` + `focus_active_class` + +- [x] 9.1 Public `set_active_class(class_id, subtask_key?)`: extract the body + of `handle_soft_id_toolbox_button_click` (the `sel` swap, + `set_id_dialog_payload_nopin`, dialog display update, active-annotation + reclass, delete re-toggle, and the trailing + `sync_annotation_modes_to_active_class`) into the method; the DOM click + handler becomes a thin wrapper. Mode buttons stay correct because the sync + is the handler's last unconditional statement today. For a non-current + subtask, skip the button/DOM sync - `set_subtask` already runs the sync on + activation, so state is written now and the DOM reconciles on switch. + Subsumes 6.4: the state-based `get_active_class_id` (utilities) becomes + canonical and the DOM-parsing method's call sites migrate. +- [x] 9.2 Migrate the internal DOM-click workarounds to `set_active_class`: + `toggle_delete_class_id_in_toolbox` (3 trigger sites: delete-class on + entry; first class or hovered annotation's class on exit), + `update_id_toolbox_display` (state -> click -> handler -> state round + trip), the class keybind handler (`listeners.ts` class_button click), and + the soft-id handler's delete re-toggle. model-registry has no workarounds + to migrate (verified: zero `toolbox_sel`/`id_payload` references). +- [x] 9.3 Per-subtask `focus_active_class: boolean` (default false, so no + behavior change for vanilla consumers - the defocus gates restrict + hover/Tab/list, not just drawing, and must not engage unasked). When true, + focus derives from the *persistent* selection: a new + `get_selected_class_id(subtask)` resolves from `id_payload` and skips + `get_active_class_id`'s delete-mode `DELETE_CLASS_ID` short-circuit, so + focus freezes at the selected class during delete modes (no saved/restored + state, no dim flicker; precedent: the mode sync already bails on + `DELETE_CLASS_ID`). The delete-class button is excluded from the focus + path. Remove `set_class_focus` / `focused_class` as an independent axis; + migrate the `class_focus` tests to selection-driven focus. +- [x] 9.4 Focus-gate the bulk-delete collection loop + (`delete_polygon`/`delete_bbox`) on `is_annotation_defocused`. Restores + the protection the removed `hidden` machinery had, and is what makes + freeze-during-delete safe: focus scopes what is legible, interactive, + navigable - and deletable. Single-annotation delete is already gated via + hover (`get_edit_candidates` skips defocused). +- [x] 9.5 model-registry: sidebar class rows / outcome legend call + `set_active_class` instead of `set_class_focus`; all three subtasks set + `focus_active_class: true`. Matches the UI's actual invariant (always + exactly one focused class); the null-focus branch was only reachable with + an empty ontology. +- [x] 9.6 Enforce per-class modes on *reclassification*. The id-dialog pie, + the class keybind (already-selected branch), and `set_active_class`'s + reclass branch all funnel through `handle_id_dialog_click`, so one gate + there covers every gesture; `assign_annotation_id` stays ungated so + undo/redo replay history faithfully. New predicate + `can_annotation_be_class(annotation, class_id)` over + `get_class_allowed_modes`. Rejection: `shake_screen()` + quiet warning. + The gate checks only the *target* class, so an annotation whose current + class/type pairing is already invalid (bad import) can still be + reclassified to a valid class. +- [x] 9.7 The pie only offers classes compatible with the dialog's + annotation's spatial type (rebuilt per show when the compatible subset + changes; wedge hit-testing runs over the displayed subset and maps back to + the full class list). When fewer than two classes are compatible there is + nothing to choose, so no dialog appears at all. No shake on rejection - + the 9.6 gate stays as a silent backstop for keybind paths. +- [x] 9.8 Load-time validation: warn (never drop - the data is authoritative + and round-trips on export) when an imported annotation's class does not + allow its spatial type. NOTE: no subtask-level load check exists either + (`process_resume_from` only errors on *missing* type/payload); checking + against the class's effective modes covers both levels since class modes + are already a subset of the subtask's. +- [x] 9.9 Host callbacks `on_active_class_change(subtask_key, class_id)` and + `on_subtask_change(subtask_key, old_subtask_key)` config options, fired + from the single writers (`set_active_class` / `set_subtask`) only on + actual change. Needed because the Keybinds toolbox item lets users bind + class-select keys at runtime, so ULabel-side class changes are reachable + even when the host ships `keybind: null` and no id toolbox. + ### Verification - [ ] V3 ULabel: lint + jest + e2e green after each phase; 4.1-4.4 and @@ -584,3 +717,8 @@ time than at save time. GT annotation is corrected *in place* by the reclassify pie, keeping its id, `encord_object_hash` and undo coherence - no delete-and-recreate across subtasks. Must pass before the edit path ships. +- [ ] V6 Phase 9: with `focus_active_class` on, selecting a class (API, + toolbox button, or keybind) moves focus, dimming and mode buttons together; + entering a delete mode changes none of them; a delete polygon removes only + focused-class annotations. Vanilla configs (flag off) show zero behavior + change across the whole suite. diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 13a56693..f54c78a5 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -55,6 +55,12 @@ jobs: path: ~/.cache/ms-playwright key: playwright-${{ runner.os }}-${{ steps.playwright-version.outputs.version }} + # playwright's dep install runs apt-get update, which fails outright when any + # configured repo is mid-republish (hash mismatch). The runner image ships + # Google's and Microsoft's repos; this job needs neither, so drop them. + - name: Remove unneeded third-party apt repos + run: sudo rm -f /etc/apt/sources.list.d/google-chrome.list /etc/apt/sources.list.d/microsoft-prod.list + - name: Install Playwright browsers if: steps.playwright-cache.outputs.cache-hit != 'true' run: npx playwright install --with-deps diff --git a/CHANGELOG.md b/CHANGELOG.md index 5026acb6..8a624fc8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,9 +2,17 @@ All notable changes to this project will be documented here. -## [unreleased] - -## [0.28.0] - Sept 8th, 2026 +## [0.28.0] - Sept 10th, 2026 +- New `set_active_class(class_id, subtask_key?, redraw?)` and `get_selected_class_id(subtask_key?)` public API methods; all internal class-selection paths (toolbox clicks, keybinds, delete-mode toggles) route through `set_active_class`. +- New per-subtask `focus_active_class` option (default `false`): the selected class is the focused class — other classes dim to `defocused_opacity` and drop out of hover, Tab navigation, the annotation list, and bulk delete. Toggleable via `set_focus_active_class()` or `toggle_class_focus_keybind` (default `shift+f`). +- Per-class `allowed_modes` are now enforced on reclassification: the id-dialog pie only offers classes compatible with the annotation's spatial type; with fewer than two, no pie appears and the edit ring collapses. Importing an incompatible annotation logs a warning but still loads. +- New host callback config options `on_active_class_change`, `on_subtask_change`, and `on_focus_active_class_change`: fired from any writer (API, toolbox, keybind), and only when the value actually changes. +- New `brush_overlap_across_subtasks` config option (default `false`). **Behavior change**: brush overlap resolution now stays within the active subtask; reaching masks in other subtasks (including read-only barriers) is opt-in. +- `set_annotations()` and `set_annotations_batch()` gained an optional `show_loader` parameter (default `true`); pass `false` to swap background layers without flashing the loading overlay. +- A subtask with its layer opacity slider at 0 is now non-interactive, matching vanish mode (new `is_subtask_hidden()` helper). +- Fix the Brush/Erase toolbox buttons staying lit after a subtask switch. +- Fix class keybinds edited in the Keybinds toolbox item not applying to other subtasks sharing the class id until a reload; class-select keybinds now also work in read-only subtasks, and are correctly inert while a delete mode is active (the gate for this read a field that was never assigned). +- Fix stale containing boxes when `allow_annotations_outside_image = false` clamps loaded annotations at init. - Removed unused per-subtask back canvas. - `set_annotations()` gained a `skip_toolbox_update` parameter for batching several per-subtask swaps, plus a `refresh_toolbox()` method to run the deferred filter-distance + toolbox update once at the end. - `ClassCounter` toolbox item options via `class_counter_toolbox_item` config: `subtasks` (`string[] | "current"`) selects which subtasks to count, `layout` (`"current" | "grouped" | "flat"`) controls rendering (`grouped` adds a heading per subtask, `flat` merges shared class ids into one summed list). New `set_class_counter_options()` public API method changes them at runtime. diff --git a/api_spec.md b/api_spec.md index ac3fdda4..ab8f91b5 100644 --- a/api_spec.md +++ b/api_spec.md @@ -71,6 +71,7 @@ class ULabel({ decrease_brush_size_keybind: string, mask_annotation_opacity: number, default_brush_overlap_mode: BrushOverlapMode, + brush_overlap_across_subtasks: boolean, set_brush_overlap_none_keybind: string, set_brush_overlap_exclude_keybind: string, set_brush_overlap_overwrite_keybind: string, @@ -81,10 +82,14 @@ class ULabel({ annotation_size_plus_keybind: string, annotation_size_minus_keybind: string, annotation_vanish_keybind: string, + toggle_class_focus_keybind: string, fly_to_max_zoom: number, min_zoom_fit_ratio: number, n_annos_per_canvas: number, - auto_destroy_on_detach: boolean + auto_destroy_on_detach: boolean, + on_active_class_change: function, + on_subtask_change: function, + on_focus_active_class_change: function }) ``` @@ -310,7 +315,7 @@ The `"bitmask"` mode enables raster (per-pixel) segmentation. Each bitmask annot **Overlap modes** -When painting, the brush can enforce mutual exclusivity with *other* undeprecated bitmask annotations. The mode is a single **global** value, persisted to localStorage, and is chosen via the Brush toolbox item (shown in bitmask mode) or the overlap keybinds. Its initial value comes from [`default_brush_overlap_mode`](#default_brush_overlap_mode). +When painting, the brush can enforce mutual exclusivity with *other* undeprecated bitmask annotations. The mode is a single **global** value, persisted to localStorage, and is chosen via the Brush toolbox item (shown in bitmask mode) or the overlap keybinds. Its initial value comes from [`default_brush_overlap_mode`](#default_brush_overlap_mode). Resolution stays within the active subtask unless [`brush_overlap_across_subtasks`](#brush_overlap_across_subtasks) is set. - `"none"` (default): painting only adds to the active mask; other masks are untouched (pixels may be owned by multiple annotations). - `"exclude"`: newly-painted pixels never cover pixels owned by other bitmask annotations (existing masks win). @@ -608,6 +613,9 @@ The fill opacity (`0`-`1`) used when rendering `bitmask` (raster segmentation) a ### `default_brush_overlap_mode` The initial [brush overlap mode](#overlap-modes) for bitmask painting: `"none"` (default), `"exclude"`, or `"overwrite"`. The live value is global and persisted to localStorage, so a user's last choice takes precedence over this default on subsequent sessions. +### `brush_overlap_across_subtasks` +When `true`, [brush overlap resolution](#overlap-modes) also reaches undeprecated bitmask annotations in *other* subtasks: `"exclude"` clips the stroke against them, and `"overwrite"` carves them — except masks in `read_only` subtasks, which act as barriers (the stroke is clipped around them instead). Default is `false`: a stroke only interacts with masks in the active subtask. + ### `set_brush_overlap_none_keybind` Keybind to set the brush overlap mode to `none`. Default is `shift+n`. @@ -641,6 +649,9 @@ Keybind to toggle vanish mode for annotations in the current subtask. Default is ### `annotation_vanish_all_keybind` Keybind to toggle vanish mode for all subtasks. Default is `shift+v` +### `toggle_class_focus_keybind` +Keybind to toggle `focus_active_class` on the current subtask: with it on, classes other than the active one dim to `defocused_opacity` and drop out of hover, navigation, the annotation list, and bulk delete. Default is `shift+f`. + ### `fly_to_max_zoom` Maximum zoom factor used when flying-to an annotation. Default is `10`, value must be > `0`. @@ -661,6 +672,15 @@ When `true` (the default), ULabel installs a `MutationObserver` on the container > **Same-id replacement caveat.** With the default `true`, the one-frame grace period means a caller who removes the old container and mounts a new `