add: mask drawing tools (brush/rectangle/line), undo/redo, and edit mode - #135
Open
Osayi-ANL wants to merge 10 commits into
Open
add: mask drawing tools (brush/rectangle/line), undo/redo, and edit mode#135Osayi-ANL wants to merge 10 commits into
Osayi-ANL wants to merge 10 commits into
Conversation
… editor add: qtawesome icons for undo/redo, a Clear button, and a two-row controls layout fix: scale mask overlay and initial view range to the detector image size
…k mask if none is loaded fix: set mask viewer checkboxes before show() so showEvent sees the correct state
… edge fills up to it instead of the shape vanishing
…y instead of just the overlay, so the mask overlay doesn't render stretched against the base image
pecomyint
requested changes
Aug 14, 2026
Collaborator
There was a problem hiding this comment.
Nice feature — the paint helpers are clean and detector-agnostic, the commits are well-split, and ruff passes. The separate red ImageItem overlay is the right call for cheap live drawing.
Blockers
- Close prompt's "Save" doesn't save (
mask_viewer.py:219-232).QMessageBox.Saveis the default button and falls through toevent.accept()—_save_mask()is never called. Harmless today only because_on_mask_editedsaves on every emit. - Save button then close still prompts "unsaved changes" —
_save_mask(:460-476) never refreshesself._original_mask. One line. _display_to_nativeis now in the mouse-move path (:330-350). The probe-array +np.argwherereverse-map costs 9 ms/call on a 2048² mask, and_event_nativenow calls it on every move during a drag — ~12 ms/move for the brush, ~28 ms for a long line preview, scaling to 50-110 ms on a 4096² detector (visible lag, skipped pixels). transpose+rot90 is a closed-form index permutation, so this can be pure arithmetic instead of building and scanning an array. Hoisting the three_get_display_mask()copies and the per-movenp.sumpicks up the rest.
Worth fixing here
- "Size" means different things per tool — at 3, brush paints 5 px but rectangle and line paint 3 px.
_paint_diskreads the value as a radius (diameter2R-1);_paint_linealready compensates with(thickness + 1) / 2, the brush should too. - Opening the editor writes to disk —
edit_mask_clicked(area_det_viewer.py:718-731) creates an all-Falsemask and immediately saves it, so just opening the window flipsmask_manager.maskoffNoneand the user needs "Clear Mask" to undo it. Can the create+save wait for the first real edit?
Minor: _get_parent_display_settings's colormap return is now dead (:410); the Redo tooltip says Ctrl+Y but QKeySequence.Redo is Cmd+Shift+Z on macOS; MASK_UNDO_MAX = 20 bounds snapshot count, not bytes (~360 MB on an Eiger 16M) despite the comment. The paint helpers fit the fake-self pattern in tests/unit/test_roi_visibility.py — cheap tests there would lock in 3 and 4.
Fixes save-on-close, stale mask, slow coordinate inverse, inconsistent tool sizing, blank mask on open.
Defer view fit until dialog has real geometry instead of fitting too early.
Stops the live-plot timer during edits so large detectors don't starve the editor's repaint.
Resamples the mask overlay to match the image's resolution and warns when they differ.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds interactive mask editing to the mask viewer: brush, rectangle, and line
tools with undo/redo, an edit mode toggle, and orientation-aware display so
the mask lines up with the diffraction image regardless of transpose/rotate
state.
Test Plan