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

add option to keep current agave settings when loading #160

Merged
merged 1 commit into from
Jan 30, 2024

Conversation

toloudis
Copy link
Contributor

If loading a new volume, offer a LoadDialog checkbox to keep or discard current volume appearance settings. Default is to keep settings. This is to make it efficient when loading consecutive images of same channels and approximate intensity ranges. Unchecking the box will present the previous behavior, where every setting goes back to a default.

Example: load a 9-channel variance dataset volume, enable a set of channels. Customize channel colors. Adjust thresholding. Then load another 9-channel variance dataset volume. Same channels should be enabled, with same colors etc.

@toloudis toloudis requested a review from a team as a code owner January 17, 2024 01:17
@toloudis toloudis requested review from meganrm, blairlyons, interim17, ShrimpCryptid, frasercl and ascibisz and removed request for a team January 17, 2024 01:17
m_appScene.initSceneFromImg(image);
m_glView->initCameraFromImage(&m_appScene);
m_appScene.initBoundsFromImg(image);
if (!keepCurrentUISettings || !wasVolumeLoaded) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is where we use the checkbox value to either reset settings or not

Comment on lines +178 to +179
layout->addRow("Keep current AGAVE settings", m_keepSettingsCheckbox);
layout->addItem(new QSpacerItem(0, spacing, QSizePolicy::Expanding, QSizePolicy::Expanding));
Copy link
Contributor

@ShrimpCryptid ShrimpCryptid Jan 19, 2024

Choose a reason for hiding this comment

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

Nit: AGAVE settings to me suggests the configuration/settings for the entire AGAVE application (almost like user preferences), rather than just the current viewer settings. Would "viewer settings" or "volume settings" be more clear?

@ShrimpCryptid
Copy link
Contributor

ShrimpCryptid commented Jan 19, 2024

It seems like any changes to the ROI on the LoadDialog are overridden when keeping the existing settings. Is this intended behavior?

UI has an existing ROI, and the LoadDialog has a different ROI set:
image

Once the image is loaded, it keeps the old ROI range, ignoring what was set in LoadDialog:
image

Copy link
Contributor

@ShrimpCryptid ShrimpCryptid left a comment

Choose a reason for hiding this comment

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

LGTM! Built and tested locally.

@toloudis
Copy link
Contributor Author

toloudis commented Jan 19, 2024

It seems like any changes to the ROI on the LoadDialog are overridden when keeping the existing settings. Is this intended behavior?

UI has an existing ROI, and the LoadDialog has a different ROI set...
Once the image is loaded, it keeps the old ROI range, ignoring what was set in LoadDialog...

I think this is actually the correct/intended behavior. The ROI in the loading dialog is letting you select a subset of what's in the original file to load. Once that is loaded, that volume becomes the ENTIRE volume that the app knows about. So if your loading ROI starts at x0, y0, z0 and spans a size of dx, dy, dz, then the LOADED volume (and the ROI you see in the appearance panel ROI section) will start at 0,0,0 and be of size dx,dy,dz.

The saved settings of ROI in the Appearance panel preserve the relative ROI selection relative to the volume that was loaded.

Copy link
Contributor

@ShrimpCryptid ShrimpCryptid left a comment

Choose a reason for hiding this comment

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

Oops, thought I hadn't approved yet and resubmitted

@toloudis toloudis merged commit 61284a8 into feature/light-manip-ui Jan 30, 2024
7 checks passed
@toloudis toloudis deleted the feature/keep-settings-on-load branch January 30, 2024 23:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants