Fixes to bitmask - #250
Conversation
There was a problem hiding this comment.
Pull request overview
Improves bitmask overlap handling, rendering performance, confidence filtering, and asynchronous loading behavior.
Changes:
- Adds cross-subtask, bounding-box-limited bitmask overlap resolution.
- Introduces cached bitmask rendering and throttled move overlays.
- Makes annotation/image replacement asynchronous and expands test coverage.
Reviewed changes
Copilot reviewed 15 out of 17 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/index.js |
Implements overlap, rendering, movement, and async API changes. |
src/mask_utils.ts |
Adds mask versions and box-limited operations. |
src/actions.ts |
Adds optimized bitmask move rendering. |
src/loader.ts |
Adds render-waiting support. |
src/version.js |
Bumps the library version. |
index.d.ts |
Updates public API types. |
package.json |
Bumps package version. |
package-lock.json |
Synchronizes package version. |
api_spec.md |
Documents filtering and async APIs. |
CHANGELOG.md |
Records the release changes. |
demo/bitmask-e2e.html |
Adds a bitmask test fixture. |
tests/mask_utils.test.js |
Tests mask operations and versioning. |
tests/bitmask_overlap.test.js |
Tests overlap semantics. |
tests/e2e/bitmask.spec.js |
Tests integrated bitmask behavior. |
tests/e2e/confidence-slider.spec.js |
Awaits asynchronous annotation replacement. |
tests/confidence_slider.test.js |
Tests bitmask confidence filtering. |
tests/annotation_operators.test.js |
Verifies bitmask filter eligibility. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 17 changed files in this pull request and generated 1 comment.
Suppressed comments (5)
demo/bitmask-e2e.html:23
- This newly introduced
console.logviolates the repository logging rule. The e2e fixture does not use submission output, so make the callback a no-op; if logging is needed, route it throughlog_messageinstead.
function on_submit(annotations) {
console.log("submit", annotations);
}
src/index.js:4856
frameis compared strictly here, butdraw_annotation_from_idtreats numeric frame strings as visible via loose equality. Imported annotations are assigned from JSON without coercingframe, so an annotation withframe: "0"is rendered on frame 0 but skipped by overlap resolution, allowing masks to overlap. Normalize the frame value while preserving the renderer's special"undefined"handling.
const frame = ann["frame"];
if (frame != null && frame !== current_frame) continue;
src/index.js:4898
- This re-encodes the entire existing mask before every carve, which is especially costly for the large masks this optimization targets. The annotation's current
spatial_payloadis already the pre-edit RLE, so reuse it for undo instead of performing this redundant full-image scan.
const before_rle = other_mask.to_rle();
src/index.js:6846
- Changing this method from returning the old source synchronously to returning
Promise<string>breaks callers that consume the return value directly, despite the PR declaring no breaking API changes. Either preserve the synchronous API (for example via a separate async loading method) or mark and version this as a breaking change with migration guidance.
async swap_frame_image(new_src, frame = 0) {
src/index.js:6947
- The new
awaitmakes annotation replacement asynchronous even for callers that ignore the return value: code immediately afterset_annotations()now observes the old annotations. That is a behavioral breaking change, contrary to the PR's “Breaking API Changes: No”. Preserve synchronous semantics via a separate async API, or explicitly classify/version/document this migration as breaking.
async set_annotations(new_annotations, subtask) {
// Show the loader while re-initializing annotations, since this is similar to a new init
const container = document.getElementById(this.config["container_id"]);
ULabelLoader.add_loader_div(container);
// Yield so the browser can paint the loader before the heavy synchronous work below
await ULabelLoader.wait_for_render();
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 17 changed files in this pull request and generated no new comments.
Suppressed comments (3)
demo/bitmask-e2e.html:22
- Avoid writing submitted annotation payloads to the browser console from this fixture. It creates noisy test output and exposes annotation data when the fixture is opened manually; this callback can be a no-op because submission is not exercised here.
console.log("submit", annotations);
src/index.js:4856
- This frame filter does not match
draw_annotation_from_id(): that render gate deliberately treats the legacy literal"undefined"and numerically equivalent frame values as visible. Those bitmasks can therefore be rendered while being skipped by overlap resolution, leaving visible masks overlapping. Mirror the render gate here (or normalize frame values during import).
if (frame != null && frame !== current_frame) continue;
package.json:4
- This patch version conflicts with the PR's “Breaking API Changes: No” claim:
swap_frame_image()now returns a Promise instead of the old source string, andset_annotations()now defers its mutation until after anawait. Existing callers that consume the return value or immediately read annotations will break. Either preserve synchronous compatibility or release/document these as breaking API changes with an appropriate synchronized version bump.
"version": "0.25.1",
Bitmask fixes
Description
ConfidenceSlidertoolbox item now also filtersbitmaskannotationsset_annotations()andswap_frame_image()public API methods now display the loading spinner while they run and return aPromiseexclude/overwrite) now resolve against bitmask annotations across all subtasks, not just the active one.exclude/overwrite) now uses a bounding-box pre-filter and box-limited pixel operations, so its cost scales with the size of the brush stroke rather than the number and size of existing masks.PR Checklist
package.jsonhas been bumped since last releasepackage.jsonandsrc/version.jsapi_spec.md)changelog.mdBreaking API Changes
No