Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ENH: Update Pre-Processing UI to improve BVR workflow #141

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

sbelsk
Copy link
Contributor

@sbelsk sbelsk commented Nov 10, 2024

Resolves #131, resolves #145.

These changes are aimed to improve the user workflow in the Pre-Processing module, specifically targeting the improvement of the Autoscoper configuration file generation. Further changes to improve error handling to make it more user-friendly are to be continued in a separate upcoming PR.

Changes in this PR include:

  • 'Volume Node' and 'Output Directory' at top of UI were swapped
  • 'Trial Name' moved to the config generation section as the ''.cfg field
  • The segmentation generation section was redesigned to better convey the two separate generation methods
  • The 'Adavnced Options' sections was renamed to 'Default Subdirectories', moved below the 'Segmentation Generation' section, and is now rendered as collapsed by default
  • The 'VRG Resolution' fields from 'Advanced Options' was moved to the config generation section
  • In the 'Generate Config' section, added file and path selectors to specify the camera calibration files, radiograph root directories, and volume files to be written to the generated config file. Also added 'Voxel Size' fields that are automatically populated based on the selected input volume node.
  • Elements in the .ui files were edited to be properly indexed and named

Remaining TODOs:

Existing Pre-Processing UI New Pre-Processing UI
image image

Copy link
Contributor

@amymmorton amymmorton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-voxel size should not be manually editable- but inherited from the volume node used to gen PV

  • as part of PV gen - assign the voxel size values
    *create a subdirectory in Models folder 'AUT' so that the Models level h used in Segmentation generation remains unchanged

  • remove 'virtual' radiograph subdirectory

@amymmorton
Copy link
Contributor

preproc_cfgeduts

@sbelsk
Copy link
Contributor Author

sbelsk commented Nov 18, 2024

@amymmorton In the last commit pushed to this branch, I moved a few of the UI widgets so that the pre-processing tab isn't so wide, made sure that the segmentation generation options are enabled/disabled based on which radio button is selected (automatic segmentation vs. seg from models), and also implemented the file selectors for the config generation group.

Current state of the UI:
image

@amymmorton
Copy link
Contributor

  • suggest adding color contrast to push buttons throughout
  • toggle on the STL Models directory label when 'Segmentation from Model' radio button active
  • Trial Names -> search for subfolders in RadiographImages subdirectory
  • rename
    image

'or enter:' label to 'AND enter:'

  • @jdholt04 ?? can assume order in radiograph subfolders is same order as CamerCalibration.txt

@sbelsk sbelsk force-pushed the improve-BVR-config-generation-UI branch 2 times, most recently from c83ec97 to a5201c9 Compare November 18, 2024 16:17
@sbelsk
Copy link
Contributor Author

sbelsk commented Nov 18, 2024

Remaining TODOs at this point:

  • toggle on the STL Models directory label when 'Segmentation from Model' radio button active
  • for Trial Names, search for subfolders instead of files in RadiographImages subdirectory
  • rename 'or enter:' label to 'AND enter:' at the top of the "Generate Config" section
  • decide and implement correct pairing of radiograph subfolders with their camera calibration files
  • populate the "Voxel Size" according to the input volume node selected (should we let users have the ability to manually override it too?)
  • update functionality of "Generate Config File" button using data from finalized UI design
  • update PR title & body to summarize all changes introduced here
  • update the documentation page

Optional/Future PR:

  • add color contrast to push buttons throughout?
  • reorder "Trial Names", "Partial Volumes" and "Camera Calibrations" file selectors to match the order they'll be written to the generated configuration file?
  • Improve error checking to be more user-friendly (error dialog as opposed to error in the hidden log window)

@sbelsk sbelsk force-pushed the improve-BVR-config-generation-UI branch 3 times, most recently from 1fefc3e to 8af2265 Compare November 20, 2024 18:32
@sbelsk
Copy link
Contributor Author

sbelsk commented Nov 20, 2024

Current status:

  • updated the list of remaining TODOs (see previous comment)
  • The "Generate Config File" button now works using the data from the different UI fields (with the exception of the voxel size fields what we currently grab from the input volume node, and also with the order of the the trial names and camera calibrations being assumed to match). It should successfully write the output file and then automatically populate the "Config File" field in the "Autoscoper Control" tab as before.
  • render resolution values are no longer being divided by two before writing to the generated config file
  • minor UI tweaks made since last screenshot:
    image

@amymmorton
Copy link
Contributor

"populate the "Voxel Size" according to the input volume node selected (should we let users have the ability to manually override it too?)"

I say No. The only options are voxel size form volume node / volume.tfm (which must both be the same)

@amymmorton
Copy link
Contributor

amymmorton commented Nov 21, 2024

Nice.

image

Suggest

  • update voxel size on select vol node
  • correct: error when parsing path + trial name (python slash issues) (see bottom right)
    image

Copy link
Contributor

@amymmorton amymmorton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

image

Copy link
Contributor

@amymmorton amymmorton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@sbelsk sbelsk force-pushed the improve-BVR-config-generation-UI branch from ffe64d1 to cadfda4 Compare November 24, 2024 20:04
@sbelsk
Copy link
Contributor Author

sbelsk commented Nov 24, 2024

Cleaned up the commit history on this branch for ease of reviewing

@sbelsk sbelsk marked this pull request as ready for review November 24, 2024 20:16
@sbelsk sbelsk changed the title WIP: Improve BVR config generation UI ENH: Update Pre-Processing UI to improve BVR workflow Nov 24, 2024
@sbelsk sbelsk force-pushed the improve-BVR-config-generation-UI branch 3 times, most recently from 04eadf0 to 15940b2 Compare November 24, 2024 20:28
sbelsk and others added 3 commits November 24, 2024 15:29
* 'Volume Node' and 'Output Directory' at top of UI were swapped
* 'Trial Name' moved to the config generation section as the
  '<blank>'.cfg field
* The segmentation generation section was redesigned to better convey
  the two separate generation methods
* The 'Adavnced Options' sections was renamed to 'Default
  Subdirectories', moved below the 'Segmentation Generation' section,
  and is now rendered as collapsed by default
* The 'VRG Resolution' fields from 'Advanced Options' was moved to the
  config generation section
* In the 'Generate Config' section, added file and path selectors to
  specify the camera calibration files, radiograph root directories, and
  volume files to be written to the generated config file. Also added
  'Voxel Size' fields that are automatically populated based on the
  selected input volume node.
* Elements in the .ui files were edited to be properly indexed and named

Co-authored-by: Amy M Morton <[email protected]>
TODO: decide and implement correct pairing of radiograph subfolders with
their camera calibration files
@sbelsk sbelsk force-pushed the improve-BVR-config-generation-UI branch 2 times, most recently from 0f9c9e1 to a594fac Compare December 1, 2024 20:04
@sbelsk
Copy link
Contributor Author

sbelsk commented Dec 1, 2024

Updated the config paths selection based on @amymmorton's draft. I ended up implementing a design that supports any number of corresponding camera file & radiograph subdir pairs (see updated screenshot in the PR summary). Feel free to review the update, particularly any UI text/labels and tooltips on the "add" buttons to see if we can better clarify their purpose and emphasize the importance of the order.

Copy link
Contributor

@amymmorton amymmorton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

@jdholt04 any test updates from you from last week?...

@jdholt04
Copy link

jdholt04 commented Dec 9, 2024

@amymmorton

I think the ui looks great. When testing I am unable to select files/subdirectories after populating the selection boxes. This could be an error on my part...

I replaced AutoscoperM.py, IO.py and AutoscoperM.ui with the changed files. Are there any additional files that I need to replace?

Capture

EDIT: Simply could not see boxes to select files in dark mode. Successfully generated config file.

Copy link

@jdholt04 jdholt04 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was able to successfully generate a config file using the changes made to the pre-processing module (tested in Slicer 5.6.2). Config file was generated and was able to be loaded into Autoscoper.

image

image

One change to the UI that would be helpful: In dark mode the boxes to select files to be added to the config file are not visible. Could changes be made to the UI to make it more intuitive for users in dark mode?

image
image

Changes include some reordering of the UI to better match the order of
the config v1.1 file format.

Co-authored-by: Amy M Morton <[email protected]>
@sbelsk sbelsk force-pushed the improve-BVR-config-generation-UI branch from dc97ba1 to 0b16371 Compare December 10, 2024 21:36
@sbelsk
Copy link
Contributor Author

sbelsk commented Dec 10, 2024

Thanks for the feedback! I updated the checkbox background color so it stands out more in dark mode, following this fix in a similar issue in Slicer.

image
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants