New Frame selector Vue component

[annehaley]

Resolves #498, resolves #1108, resolves #1111

Improvements for future PRs:

• Histograms
• Save configurations?
• Reset button

[manthey]

@annehaley See the changes I made and the comments on the last commit. If you think this would be clearer rebased onto master, please do so.

[annehaley]

@manthey thank you for fixing geojs.js! It works smoothly now. I'll rebase and get tests passing, then I'll throw it back to you for review

[annehaley]

@manthey could you help me figure out why the following failure is happening?

FAILED girder/test_girder/test_web_client.py::testWebClientNoStream[imageViewerSpec.js]

[manthey]

@manthey could you help me figure out why the following failure is happening?

FAILED girder/test_girder/test_web_client.py::testWebClientNoStream[imageViewerSpec.js]

I used URLSearchParams to extract a value to help with cache control. Our client tests run on an older javascript framework which apparently doesn't define this but since there is a timing factor, it only sometimes fails. Let me switch how this is done or add a polyfill.

[manthey]

I don't want to merge anything that shows unfinished functionality. So, we either need to handle the unfinished parts (e.g., bands) or finish them. Once we've done that and addressed my color comments, we could merge this and leave everything else to future PRs.

Ultimately, I haven't seen real-world data sets that have both more than three bands AND channels, so I think that we could have our modes be "Sliders / Composite Frames / Composite Bands". Composite Bands could be an option even on single band (greyscale) images with only one frame, since that would be a way of exposing the min/max controls.

For bands, the colors should default based on the band interpretation (red/green/blue/gray) and then use unused colors for other interpretations (including alpha). The default initial checked bands should just be the first three bands (as opposed to channels where we default to all channels being on and composited).

When we use this in the HistomicsUI context, we'll want these controls to fit in one of the side panels -- we'll be able to use CSS to adjust this. We just need to be aware that controls will be tight (more relevant as we add histograms and options for min/max besides numbers).

The use of "0" as undefined for min/max isn't appropriate. Technically, the unset value can be different than 0 for min (and is determined based on datatype for max), and with float-based samples, 0 might actually be a sensible max in some odd case. We eventually want to have a mode for min and max that would be "default/value/min(or max)" where "default" wouldn't use a number, "value" would be the current mode (but there can be data where this is floating point on a scale of [0,1] or [-1,1] or other values, so the controls can't restrict to integers unless we check that makes sense), and "min" or "max" would let you specify a numerical value from [0,1] (or maybe from [0%, 100%]) which would be the amount of the histogram to exclude from the ends (e.g., "min:0.002" sets the minimum such that 0.2% of the samples are below the picked value).

And, for non-composited axes, we'll eventually want a "max-merge" option (which would disable the slider). This might conceptually have other choices (so maybe it is a drop-down control of slider/max-merge rather than a checkbox).

When we do show this histogram, it is relatively cheap to get the histogram for all the bands of the current frame. For multiple channels, we end up needing to currently ask for the histogram for all channels at the current frame position. Since each frame has its own histogram, we might want to overlay two histograms -- one of the current frame/channel (possibly applying max-merge) and one of the combined histograms of all frames with that channel. Getting that combined histogram is initially expensive in time (and when we have it, we might want an additional min/max mode of combined min/combined max). We'll want to show the min/max values on the histogram as vertical bars.

And, in my list of wants, we may eventually want a checkbox to enable/disable all channels (or bands) so that you can quickly re-enable them or go to a single channel/band.

[manthey]

When we construct the style, it is somewhat more efficient to specify the base frame as the frame parameter and the style with framedelta rather than frame per band (see https://girder.github.io/large_image/tilesource_options.html#style).

[annehaley]

When we construct the style, it is somewhat more efficient to specify the base frame as the frame parameter and the style with framedelta rather than frame per band (see https://girder.github.io/large_image/tilesource_options.html#style).

Could you provide an example? I don't understand what this would look like from the documentation.

[manthey]

I see some linting errors in the latest commit. Running tox -e format,formatclient should resolve most such issues.

[annehaley]

If you have enough channels that you need to scroll, the color pickers appear in the wrong place instead of scrolling with the channels (and, for an item with 40 channels, a select all/none would be really handy)

Changed in 21e920a and 464146c

[manthey]

When I have the axis mode and slide the channel slide to a non-zero value and then switch to composite channels, it still adds frame = so it ends up with the wrong frames being composited or fails if the last channel is picked.

[manthey]

Ideally, when we switch between frame and axis or vice versa we would preserve the view (that is frame is directly equivalent to a set of axis values and vice versa).

This could be fixed in a follow on PR.

[annehaley]

@manthey is 1b15871 what you mean?

[manthey]

This is undesirable when we don't have a base image (such as a map). It makes panning much less attractive as low res tiles aren't shown while high res tiles are loaded. Probably when we are using keepLower false, we need to explicitly hide the lower of our two styled layers. Can you recall what the symptoms were that you were correcting?

[annehaley]

I had included this because it was in the params object on line 98 previously, but now I understand that this was only intended for the geospatial cases. I can move this to the geospatial if block instead.

[manthey]

Let me look into how to refactor what you've done -- we really don't want to use the setFrameQuad on images that aren't multiframe or are geospatial.

[manthey]

If you think my latest changes are good and CI passes, go ahead and merge.