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

Max Merge option in Frame Selector #1306

Merged
merged 10 commits into from
Oct 4, 2023
Merged

Max Merge option in Frame Selector #1306

merged 10 commits into from
Oct 4, 2023

Conversation

annehaley
Copy link
Collaborator

@annehaley annehaley commented Sep 19, 2023

Addresses #1118

@manthey
Copy link
Member

manthey commented Sep 20, 2023

This looks like the right approach. In band compositing mode on the normmedia_8well_col2_livecellgfp.nd2 test file, I see nulls being send for the framedelta.

And, one further wrinkle: we really should be showing the histogram of the max-merged image, not the un-merged image. This means that we need to pass the appropriate style to the histogram endpoint when max-merge is checked.

@annehaley annehaley marked this pull request as ready for review September 26, 2023 14:03
@annehaley annehaley requested a review from manthey September 26, 2023 14:03
@manthey
Copy link
Member

manthey commented Sep 29, 2023

This is working, though there is an eccentricity in non-8-bit images. Specifically, when a style gets applied, the histogram ends up in 8 bits, which then breaks the sliders setting correct values for the min / max (the auto range works fine). I added a PR (#1326) that allows adding "dtype": "source" to the style at the top level (e.g., {"bands": [...], "dtype": "source"} that will return the histogram in the original data type.

Not necessarily for this PR:

Minor styling desires: Can we make "Max Merge" fit on the same line to reduce vertical height slightly?

The label for reference with max-merge quite do what you'd expect. Specifically, if for property of a label contains the id of another element, then clicking on the label acts on that element. Since this references the name property, it doesn't allow clicking on the words to activate the checkbox.

Otherwise, the only issue I see is one of performance -- we ask for the histograms faster than we can handle them resulting in having stale requests when scrubbing quickly. Can we debounce that in any way?

@annehaley annehaley merged commit a469851 into master Oct 4, 2023
7 checks passed
@annehaley annehaley deleted the max-merge branch October 4, 2023 14:30
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