feat(ImageOverlayViewerTool) - add ImageOverlayViewer tool that can render image overlay (pixel overlay) of the DICOM images #3163
Conversation
✅ Deploy Preview for ohif-platform-docs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for ohif-platform-viewer canceled.
|
|
@sedghi As I left into the code comment, we would need a new icon to enable the toggle button for image overlays on the toolbar. |
Codecov Report
@@ Coverage Diff @@
## master #3163 +/- ##
=======================================
Coverage 41.94% 41.94%
=======================================
Files 79 79
Lines 1421 1421
Branches 341 341
=======================================
Hits 596 596
Misses 658 658
Partials 167 167 Continue to review full report in Codecov by Sentry.
|
There was a problem hiding this comment.
Great work.
See my minor comments. The major comment is to clean up all the cachedStats, canvases, svg nodes etc. that you create on destroy. Maybe this is something we need to add at cs3d level that when a toolGroup is destroyed each tool should run their own destroy as we see here it is storing a lot of state that will later cause memory leak IMO
md-prog
force-pushed
the
feat/image-overlay-coloring
branch
from
c827fa7 to
6b1c305
Compare
…isabled to toggle
| this._renderingViewport = viewport; | ||
|
|
||
| const imageId = this._getReferencedImageId(viewport); | ||
| if (!imageId) return; |
There was a problem hiding this comment.
please follow the new eslint rule -> no same line if statement
| const imageId = this._getReferencedImageId(viewport); | ||
| if (!imageId) return; | ||
|
|
||
| const { overlays } = metaData.get('overlayPlaneModule', imageId); |
There was a problem hiding this comment.
we should cache this, as we hit this A LOT! so this.overlayCache.get(imageId) if not found grab from metadata and add it to the cache
| const getCachedStat = async ( | ||
| imageId: string, | ||
| overlayMetadata: any[], | ||
| color: number[] = [127, 127, 127, 255] |
There was a problem hiding this comment.
there is no need for default color here, as the configuration has a default
| if ( | ||
| !cachedStats[imageId] || | ||
| !isSameColor(cachedStats[imageId].color, color) | ||
| ) { |
There was a problem hiding this comment.
I prefer less indentation. So can we early return on the opposite condition and make indentation one less?
| private _getReferencedImageId( | ||
| viewport: Types.IStackViewport | Types.IVolumeViewport | ||
| ): string { | ||
| const targetId = this.getTargetId(viewport); | ||
|
|
||
| let referencedImageId; | ||
|
|
||
| if (viewport instanceof StackViewport) { | ||
| referencedImageId = targetId.split('imageId:')[1]; | ||
| } | ||
|
|
||
| return referencedImageId; | ||
| } | ||
| } |
There was a problem hiding this comment.
I think this is already implemented in either BaseAnnotationTool or AnnotationTool
- added new icon for toggle DICOM overlay - use new exported classes and functions from CS3D - allow ToolbarService to initialize toggles - setToolActive command now handles toggles - reference lines tool and DICOM overlay tool use the setToolActive command
…nder image overlay (pixel overlay) of the DICOM images (OHIF#3163) Co-authored-by: Joe Boccanfuso <[email protected]>
Context
DICOM images can have Overlay Plane and this overlay can be one of two types: "G" for graphics, and "R" for "ROI".
We want to be able to render "G" type overlays: image overlays or pixel overlays, over the DICOM image.
Changes & Results
@ohif/extension-cornerstoneextensionTesting
Checklist
PR
semantic-release format and guidelines.
Code
etc.)
Public Documentation Updates
additions or removals.
Tested Environment