Windowing from dicom tags

[bnmajor]

Adds support for automatically reducing the range of data, setting width/level from presets, and setting width/level from tags.

Fixes #181, #270, #6

[netlify]

✅ Deploy Preview for volview-dev ready!

Built without sensitive environment variables

Name	Link
🔨 Latest commit	6ca7d71
🔍 Latest deploy log	https://app.netlify.com/sites/volview-dev/deploys/65369615067e360007d9151d
😎 Deploy Preview	https://deploy-preview-442--volview-dev.netlify.app/
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[bnmajor]

This draft needs a little more testing but t This should be ready for review now. The core logic has been added for providing w/l presets for CT data (pulled from this) as well as automatic ranges (available for all modalities). If windowing tags are provided they are also used.

Some additional pieces I would love feedback on:

• The current UI choices
  • Right now everything is in a menu available by selecting the W/L tool. I have used expansion panels in an attempt to simplify things but there is still the potential for this menu to get quite large. Should this approach be changed? I'm open to ideas (either for the menu or moving the options somewhere else entirely).
  • NOTE: If the modality is not CT or CTPROTOCOL the presets will not be shown. If there are no presets or tags that panel is disabled.
• Some of the provided preset values are ranges and there very well may be there are better selections than what I've made. When an equivalent was offered in another application I tried to use those values (for example: OHIF values).
• What reset should mean
  • At the moment selecting "reset" recomputes the width and level values for the currently selected "auto range". If the user has selected a preset or tag the width and level are reset to those values. I am not sure if this is the behavior that users would expect or if it should behave diffrently (or do we need a tooltip or something?).
• The option of using the full range of data no longer exists and instead the new default is to use data in the [0.1%, 99.9%] range. Should the "full range" be an additional option?
• I'm always happy to rename anything that doesn't make sense

@floryst @aylward

Demo:

[bnmajor]

Old vs New preset:

[floryst]

Functionality looks good so far! We can discuss styling the menu after I finish going through your code.

[aylward]

Great start!

A few minor tweaks

1. Don't list the window and level values in the menu - it looks too cluttered.
2. The selection circle is too large. Make is slightly smaller than the capital letters.
3. Space is missing between "SoftTissue"
4. What are "Tags" - I'm not familiar with them. Are they from the dicom file?
5. Auto Compute Range should instead be called "Auto Window / Level"
6. No need to repeat "Auto" under "Auto Window / Level"
7. There is no default under Auto - just pick one.
8. The auto values should be percentiles of the data, not percentages of the range - you were correct. Instead of sorting, perhaps assign to a historgram first - give the histogram 512 bins and then determine clippings from there.

Also, Default and Default Width/Level (if it is read from the dicom) shouldn't be under CT Presets. It seems more appropriate under Auto Window / Level. It could replace the Default (currently 0.1 and 99.9) in the list. For data without a default, have the default (the value of this selection) be the full range.

I don't think we need a reset buttong.

Right now it is possible to select something in Auto and in CT Presets. Both circles remain selected - so I don't know which W/L is actually being shown. All circles (under Auto and CT Presets) should be exclusive - only one should ever be lit.

If the user changes W/L manually, then no preset should be lit (no circle filled-in).

[bnmajor]

1. Don't list the window and level values in the menu - it looks too cluttered.
2. The selection circle is too large. Make is slightly smaller than the capital letters.
3. Space is missing between "SoftTissue"
4. Auto Compute Range should instead be called "Auto Window / Level"
5. No need to repeat "Auto" under "Auto Window / Level"

All cleaned up 👍

4. What are "Tags" - I'm not familiar with them. Are they from the dicom file?

Yes, these are the DICOM tags (if any)

7. There is no default under Auto - just pick one.

Fixed

8. The auto values should be percentiles of the data, not percentages of the range - you were correct. Instead of sorting, perhaps assign to a historgram first - give the histogram 512 bins and then determine clippings from there.

Thanks! Done.

Also, Default and Default Width/Level (if it is read from the dicom) shouldn't be under CT Presets. It seems more appropriate under Auto Window / Level. It could replace the Default (currently 0.1 and 99.9) in the list. For data without a default, have the default (the value of this selection) be the full range.

Fixed

I don't think we need a reset buttong.

This reset button lets users get back to the starting point (full data range). Requested in #401, added in #437.

Right now it is possible to select something in Auto and in CT Presets. Both circles remain selected - so I don't know which W/L is actually being shown. All circles (under Auto and CT Presets) should be exclusive - only one should ever be lit.

Fixed

If the user changes W/L manually, then no preset should be lit (no circle filled-in).

This is now the behavior

[floryst]

UI tweak: if we don't have any presets, I'd suggest hiding the "Presets" expansion panel.

[aylward]

There should always be presets...the ones based on percentiles. S

…

On Thu, Oct 12, 2023, 8:31 AM Forrest Li ***@***.***> wrote: UI tweak: if we don't have any presets, I'd suggest hiding the "Presets" expansion panel. — Reply to this email directly, view it on GitHub <#442 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACEJLZQKJSEGR2UJ4M3GRLX7AENLAVCNFSM6AAAAAA5RPP63OVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTONJZHA2TONRQGI> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[bnmajor]

There should always be presets...the ones based on percentiles. S

I think @floryst's comment meant just the section with the CT Preset values. So for non-CT images we'd change from disabled to hidden.

[floryst]

Yes, thanks for the clarification @bnmajor. I did intend to convey what your graphics indicate.

[bnmajor]

UI tweak: if we don't have any presets, I'd suggest hiding the "Presets" expansion panel.

Fixed and rebased

[aylward]

Looks great!

A few suggestions:

1. When CT data is loaded, the CT Presets should be opened by default and the auto W/L can be closed
2. Thanks for the explanation, the Tags should go into their own section at the top. Call the category "Data-Specific Presets". Also, we should have the default W/L settings to be Tag 1 if it exists.
3. The "Percent" should be "Percentiles" right? If so, let's call them "0.1 Percentile", "1.0 Percentile", "2.0 Percentile" and "5.0 Percentile". There is no need to list the values. You could add "+/-" in front if you want to be even more precise, but it isn't necessary and might make the labels too long.

Thanks!!

[aylward]

Please include the tooltip - it is a nice convenience for the (inquisitive) user!

…

On Wed, Oct 18, 2023 at 4:54 PM Brianna Major ***@***.***> wrote: I also like the Low, Medium, High contrast settings. Let's use them! Awesome! Let me know if you want the tooltip or not and I'll push that change. Compatibility with Slicer is not critical by any means - in general it should not be our guide. Instead, compatibility with traditional radiological viewers should be our guide, e.g., Osirix is one of the best examples available, with outstanding radiological/clinical adoption. Great, thanks for clarifying! 🙂 — Reply to this email directly, view it on GitHub <#442 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACEJL5UQK2BFMQ3YLAIDR3YAA6YRAVCNFSM6AAAAAA5RPP63OVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTONRZGMYDKMBSGQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

-- Stephen R. Aylward, Ph.D. Chair, MONAI Advisory Board Senior Director, Strategic Initiatives, Kitware

[bnmajor]

Looks like e2e tests should pass after #458 (update to 117 and pin version) is merged

[bnmajor]

@floryst @aylward I don't have permissions to merge but this has been tested with non-DICOM data and should be ready to go

[floryst]

LGTM once rebased on main!

[bnmajor]

LGTM once rebased on main!

Rebased!