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

[IDC-1672] Highlight SEG segment/ RT structure when click to jump. #2034

Merged
merged 5 commits into from
Sep 10, 2020

Conversation

JamesAPetts
Copy link
Member

@JamesAPetts JamesAPetts commented Sep 8, 2020

RE: #1672

I've added temporary crosshairs that stay until you scroll/move from the segment, as discussed on a previous call.

I added highlighting for the RT contours, which we might as well keep as well, but without the crosshairs it really wasn't clear sometimes what was being highlighted.

I'd like to add highlighting for the DICOM SEG segments, but this actually isn't straight forward with the way our labelmaps currently work. We currently have each segment set at 100% opacity, and a global opacity slider, so we can't highlight one segment for a frame without changing the way our current opacity slider works. I think we can do this in a hacky way, but won't do this unless it is priority @fedorov .

We already had good indication in the MPR view with the crosshairs, so that is left as is.

Examples:


@codecov
Copy link

codecov bot commented Sep 8, 2020

Codecov Report

Merging #2034 into master will decrease coverage by 5.64%.
The diff coverage is 6.38%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2034      +/-   ##
==========================================
- Coverage   19.24%   13.60%   -5.65%     
==========================================
  Files         188      285      +97     
  Lines        5097     7387    +2290     
  Branches     1012     1395     +383     
==========================================
+ Hits          981     1005      +24     
- Misses       3292     5181    +1889     
- Partials      824     1201     +377     
Impacted Files Coverage Δ
...atform/core/src/classes/OHIFStudyMetadataSource.js 0.00% <ø> (ø)
...latform/core/src/classes/metadata/StudyMetadata.js 1.69% <0.00%> (-0.03%) ⬇️
...atform/core/src/studies/retrieveStudiesMetadata.js 0.00% <0.00%> (ø)
...r/src/appExtensions/GenericViewerCommands/index.js 0.00% <0.00%> (ø)
...iewer/src/appExtensions/MeasurementsPanel/index.js 0.00% <0.00%> (ø)
platform/viewer/src/connectedComponents/Viewer.js 0.00% <ø> (ø)
...wer/src/connectedComponents/ViewerLocalFileData.js 0.00% <ø> (ø)
...src/connectedComponents/ViewerRetrieveStudyData.js 0.00% <0.00%> (ø)
platform/viewer/src/index.js 0.00% <0.00%> (ø)
platform/viewer/src/routes/StandaloneRouting.js 0.00% <ø> (ø)
... and 100 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 48eb4a9...65b5fe6. Read the comment docs.

@JamesAPetts JamesAPetts requested a review from dannyrb September 8, 2020 12:26
@fedorov
Copy link
Member

fedorov commented Sep 8, 2020

The demo looks great - thank you @JamesAPetts! 👍

@JamesAPetts
Copy link
Member Author

Awesome, glad you are happy, we'll get this after the technical review 👍.

@fedorov
Copy link
Member

fedorov commented Sep 8, 2020

I added highlighting for the RT contours, which we might as well keep as well, but without the crosshairs it really wasn't clear sometimes what was being highlighted.

Forgot to mention - maybe we should turn off this feature. I think it is best if we keep the user experience consistent between RT and SEG, and this will add to the current divergence which we already have. If you prefer to keep it, I am fine with that, but otherwise let's turn it off.

@JamesAPetts
Copy link
Member Author

Forgot to mention - maybe we should turn off this feature. I think it is best if we keep the user experience consistent between RT and SEG, and this will add to the current divergence which we already have. If you prefer to keep it, I am fine with that, but otherwise let's turn it off.

Yeah I originally started with it but it wasn't clear enough, which is why I went for crosshairs. Long term I think its beneficial, so long as we can have it for segments too, as sometimes the crosshairs aren't super clear if there are overlapping segments in the RTSTRUCT, or in the case of SEG, where the segment is an arc, not a blob (the crosshairs just point to the "center of mass" within the plane. We could also improve the latter, but its probably fine in 99% of cases for the MVP.

I'll disable it for consistency for now 👍 .


const { clientWidth: width, clientHeight: height } = element;

const offset = 10;
Copy link
Member

Choose a reason for hiding this comment

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

what is the offset for?

Copy link
Member Author

Choose a reason for hiding this comment

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

That is for drawing the crosshairs, i.e. the bit of the crosshair that "isn't drawn". It could be configurable, but its hardcoded for now.

const drawLine = importInternal('drawing/drawLine');
const getNewContext = importInternal('drawing/getNewContext');

export default function _drawCanvasCrosshairs(eventData, center, options) {
Copy link
Member

Choose a reason for hiding this comment

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

I'm having deja vu here. I guess I see why this function is in both extensions though.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah it felt bad, but the extensions should be independent. We should keep this in mind when thinking about v3 and sharing generic code snippets.

@JamesAPetts JamesAPetts merged commit 5cb6e63 into master Sep 10, 2020
pieper pushed a commit to ImagingDataCommons/Viewers that referenced this pull request Sep 18, 2020
* feat: 🎸 Update react-vtkjs-viewport usage to use requestPool (OHIF#1984)

* feat: 🎸 Update react-vtkjs-viewport usage to use requestPool

* Fix import of react-vtkjs-viewport to cornerstone-tools path.

* Increase maximum load time of MPR test now we are throttling requests.

* Remove debugger

Co-authored-by: Erik Ziegler <[email protected]>

* chore(release): publish [skip ci]

 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]

* fix: Avoid lerna:restore unless we are on Netlify (closes OHIF#1926, OHIF#1996)

* wip

* fix: Fix incorrect command name in Percy test (OHIF#1999)

* perf(stackPrefetch): Added stackPrefetch config with 20 max concurrent requests (OHIF#2000)

Co-authored-by: Danny Brown <[email protected]>

* chore(release): publish [skip ci]

 - @ohif/[email protected]
 - @ohif/[email protected]

* feat: 🎸 Filter/promote multiple series instances (OHIF#1533)

improve filter/promote to be applied on multiple series instances

✅ Closes: 1532

Co-authored-by: Rodolfo Ladeira <[email protected]>

* chore(release): publish [skip ci]

 - @ohif/[email protected]

* fix: Updated react-cornerstone-viewport to version 4.0.2 (OHIF#2001)

Co-authored-by: Danny Brown <[email protected]>

* chore(release): publish [skip ci]

 - @ohif/[email protected]
 - @ohif/[email protected]

* fix: 🐛 Fail gracefully on an MPR load error (OHIF#1992)

* feat: 🎸 Update react-vtkjs-viewport usage to use requestPool

* Fix import of react-vtkjs-viewport to cornerstone-tools path.

* Increase maximum load time of MPR test now we are throttling requests.

* fix: 🐛 Fail gracefully on an MPR load error

* Respond to reviewer comments.

* chore(release): publish [skip ci]

 - @ohif/[email protected]
 - @ohif/[email protected]

* [IDC-1994] Sort series list by SeriesNumber, and sort by same SeriesNumber by date/time. (OHIF#2010)

* Sort based on SeriesNumber and SeriesDate/SeriesTime.

* Harden, and perform final sort in algorithm if last N entries have the same SeriesNumber.

* Switch to insertion rather than sorting as sorting is too slow. Reimplement low priority sorting into new insertion method.

* Fix local file viewing.

* chore(release): publish [skip ci]

 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]

* [IDC-2017] Harden segmentation import to support more SEGs (OHIF#2024)

* fix: 🐛 Upgrade dcmjs version to support more SEGs

* chore(release): publish [skip ci]

 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]

* [IDC-1939] Debug Dialog part 1 (OHIF#2011)

* WIP debug dialog

* Rename the p10 downloader extension to debugger extension, add button to toolbar. Deactivate it by default.

* Fix unit tests

* chore(release): publish [skip ci]

 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]

* IDC-1532 multiple series search google (OHIF#2026)

* Multiple series search for google cloud adapter.

* Revert IDC config.

* fix: 🐛 Series filtering on multiple series for google

* Revert changes to default config.

* Address reviewers comments.

* Fixed spelling mistakes

* chore(release): publish [skip ci]

 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]

* Split query parameter calls. (OHIF#2035)

* chore(release): publish [skip ci]

 - @ohif/[email protected]

* [IDC-1672] Highlight SEG segment/ RT structure when click to jump. (OHIF#2034)

* Highlight for RTSTRUCT.

* SEG temp crosshairs.

* Disable RT highlighting for now.

* Remove TODO

* chore(release): publish [skip ci]

 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]

* [IDC-1129] - DICOM Tag Viewer extension (OHIF#2022)

* WIP render top level tags.

* Add drop down to select series.

* Fix errors with type 2 sequences.

* WIP swap instance.

* Fix formatting and make fullscreen.

* Remove debuggers.

* Finish formatting.

* Fix error with double SeriesNumber deconstruction.

* Address reviewer comments.

* chore(release): publish [skip ci]

 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]

* [IDC-2006] - optional mailTo for debugging extension. (OHIF#2027)

* WIP debug dialog

* Rename the p10 downloader extension to debugger extension, add button to toolbar. Deactivate it by default.

* Fix unit tests

* WIP

* WIP

* Finish mailTo

* chore(release): publish [skip ci]

 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]

* [IDC-1905] Hover tooltips for Segmentation + RTSTRUCT extension (OHIF#2044)

* WIP debug dialog

* Rename the p10 downloader extension to debugger extension, add button to toolbar. Deactivate it by default.

* Fix unit tests

* WIP

* WIP

* Finish mailTo

* tooltop

Co-authored-by: Erik Ziegler <[email protected]>
Co-authored-by: ohif-bot <[email protected]>
Co-authored-by: Alex Broaddus <[email protected]>
Co-authored-by: Danny Brown <[email protected]>
Co-authored-by: ladeirarodolfo <[email protected]>
Co-authored-by: Rodolfo Ladeira <[email protected]>
pieper pushed a commit to ImagingDataCommons/Viewers that referenced this pull request Oct 7, 2020
* feat: 🎸 Update react-vtkjs-viewport usage to use requestPool (OHIF#1984)

* feat: 🎸 Update react-vtkjs-viewport usage to use requestPool

* Fix import of react-vtkjs-viewport to cornerstone-tools path.

* Increase maximum load time of MPR test now we are throttling requests.

* Remove debugger

Co-authored-by: Erik Ziegler <[email protected]>

* chore(release): publish [skip ci]

 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]

* fix: Avoid lerna:restore unless we are on Netlify (closes OHIF#1926, OHIF#1996)

* wip

* fix: Fix incorrect command name in Percy test (OHIF#1999)

* perf(stackPrefetch): Added stackPrefetch config with 20 max concurrent requests (OHIF#2000)

Co-authored-by: Danny Brown <[email protected]>

* chore(release): publish [skip ci]

 - @ohif/[email protected]
 - @ohif/[email protected]

* feat: 🎸 Filter/promote multiple series instances (OHIF#1533)

improve filter/promote to be applied on multiple series instances

✅ Closes: 1532

Co-authored-by: Rodolfo Ladeira <[email protected]>

* chore(release): publish [skip ci]

 - @ohif/[email protected]

* fix: Updated react-cornerstone-viewport to version 4.0.2 (OHIF#2001)

Co-authored-by: Danny Brown <[email protected]>

* chore(release): publish [skip ci]

 - @ohif/[email protected]
 - @ohif/[email protected]

* fix: 🐛 Fail gracefully on an MPR load error (OHIF#1992)

* feat: 🎸 Update react-vtkjs-viewport usage to use requestPool

* Fix import of react-vtkjs-viewport to cornerstone-tools path.

* Increase maximum load time of MPR test now we are throttling requests.

* fix: 🐛 Fail gracefully on an MPR load error

* Respond to reviewer comments.

* chore(release): publish [skip ci]

 - @ohif/[email protected]
 - @ohif/[email protected]

* [IDC-1994] Sort series list by SeriesNumber, and sort by same SeriesNumber by date/time. (OHIF#2010)

* Sort based on SeriesNumber and SeriesDate/SeriesTime.

* Harden, and perform final sort in algorithm if last N entries have the same SeriesNumber.

* Switch to insertion rather than sorting as sorting is too slow. Reimplement low priority sorting into new insertion method.

* Fix local file viewing.

* chore(release): publish [skip ci]

 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]

* [IDC-2017] Harden segmentation import to support more SEGs (OHIF#2024)

* fix: 🐛 Upgrade dcmjs version to support more SEGs

* chore(release): publish [skip ci]

 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]

* [IDC-1939] Debug Dialog part 1 (OHIF#2011)

* WIP debug dialog

* Rename the p10 downloader extension to debugger extension, add button to toolbar. Deactivate it by default.

* Fix unit tests

* chore(release): publish [skip ci]

 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]

* IDC-1532 multiple series search google (OHIF#2026)

* Multiple series search for google cloud adapter.

* Revert IDC config.

* fix: 🐛 Series filtering on multiple series for google

* Revert changes to default config.

* Address reviewers comments.

* Fixed spelling mistakes

* chore(release): publish [skip ci]

 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]

* Split query parameter calls. (OHIF#2035)

* chore(release): publish [skip ci]

 - @ohif/[email protected]

* [IDC-1672] Highlight SEG segment/ RT structure when click to jump. (OHIF#2034)

* Highlight for RTSTRUCT.

* SEG temp crosshairs.

* Disable RT highlighting for now.

* Remove TODO

* chore(release): publish [skip ci]

 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]

* [IDC-1129] - DICOM Tag Viewer extension (OHIF#2022)

* WIP render top level tags.

* Add drop down to select series.

* Fix errors with type 2 sequences.

* WIP swap instance.

* Fix formatting and make fullscreen.

* Remove debuggers.

* Finish formatting.

* Fix error with double SeriesNumber deconstruction.

* Address reviewer comments.

* chore(release): publish [skip ci]

 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]

* [IDC-2006] - optional mailTo for debugging extension. (OHIF#2027)

* WIP debug dialog

* Rename the p10 downloader extension to debugger extension, add button to toolbar. Deactivate it by default.

* Fix unit tests

* WIP

* WIP

* Finish mailTo

* chore(release): publish [skip ci]

 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]

* [IDC-1905] Hover tooltips for Segmentation + RTSTRUCT extension (OHIF#2044)

* WIP debug dialog

* Rename the p10 downloader extension to debugger extension, add button to toolbar. Deactivate it by default.

* Fix unit tests

* WIP

* WIP

* Finish mailTo

* tooltop

* chore(release): publish [skip ci]

 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]

* [IDC-2049] Sort Tags in Tag browser split items in sequences, add indent after space. (OHIF#2053)

* Sort tag browser, add items, add indent.

* Remove debugger.

* chore(release): publish [skip ci]

 - @ohif/[email protected]
 - @ohif/[email protected]

* Update react-cornerstone-viewport for performance improvements (OHIF#2062)

* Update react-cornerstone-viewport

* Bump react-cornerstone-viewport version.

* Comment out tests which always cause problems.

* chore(release): publish [skip ci]

 - @ohif/[email protected]
 - @ohif/[email protected]

* [IDC-2052] PWA start url (OHIF#2070)

Change start_url in manifest.json from "./index.html" to "./"

Co-authored-by: Woonchan Cho <[email protected]>

* chore(release): publish [skip ci]

 - @ohif/[email protected]

* fix: Enable port changes for Docker image using the PORT variable (OHIF#2016)

* fix: use SeriesMetadata method instead of property in findMostRecentStructuredReport (OHIF#2038)

fixes OHIF#1714

Call getInstanceCount() instead of manually checking length (which does not work)

Co-authored-by: Stefan Silviu <[email protected]>

* chore(release): publish [skip ci]

 - @ohif/[email protected]
 - @ohif/[email protected]
 - @ohif/[email protected]

Co-authored-by: Erik Ziegler <[email protected]>
Co-authored-by: ohif-bot <[email protected]>
Co-authored-by: Alex Broaddus <[email protected]>
Co-authored-by: Danny Brown <[email protected]>
Co-authored-by: ladeirarodolfo <[email protected]>
Co-authored-by: Rodolfo Ladeira <[email protected]>
Co-authored-by: Woonchan Cho <[email protected]>
Co-authored-by: Woonchan Cho <[email protected]>
Co-authored-by: Nicolas Byl <[email protected]>
Co-authored-by: Ștefan Silviu-Alexandru <[email protected]>
Co-authored-by: Stefan Silviu <[email protected]>
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.

3 participants