(extensions/cornerstone) more reusable cornerstone-extension and minor fixes, increased vtk 25.11.1 #3016

Ouwen · 2022-11-11T19:32:55Z

Exposes some internals of extensions/cornerstone to be reusable.

netlify · 2022-11-11T19:33:02Z

✅ Deploy Preview for ohif-platform-viewer ready!

Name	Link
🔨 Latest commit	`84afd86`
🔍 Latest deploy log	https://app.netlify.com/sites/ohif-platform-viewer/deploys/63de031fa4e9b600093e1db0
😎 Deploy Preview	https://deploy-preview-3016--ohif-platform-viewer.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 settings.

netlify · 2022-11-11T19:33:02Z

✅ Deploy Preview for ohif-platform-docs canceled.

Name	Link
🔨 Latest commit	`84afd86`
🔍 Latest deploy log	https://app.netlify.com/sites/ohif-platform-docs/deploys/63de031fb5ebfa00090473cd

codecov · 2022-11-11T19:36:27Z

Codecov Report

Merging #3016 (4bf3528) into v3-stable (9f1e813) will increase coverage by 13.11%.
The diff coverage is n/a.

❗ Current head 4bf3528 differs from pull request most recent head 84afd86. Consider uploading reports for the commit 84afd86 to get more accurate results

@@              Coverage Diff               @@
##           v3-stable    #3016       +/-   ##
==============================================
+ Coverage      25.15%   38.27%   +13.11%     
==============================================
  Files            119       98       -21     
  Lines           2862     2098      -764     
  Branches         555      432      -123     
==============================================
+ Hits             720      803       +83     
+ Misses          1856     1060      -796     
+ Partials         286      235       -51

Impacted Files	Coverage Δ
...tform/core/src/services/CineService/CineService.js	`12.50% <0.00%> (-6.25%)`	⬇️
.../services/MeasurementService/MeasurementService.js	`47.23% <0.00%> (-1.96%)`	⬇️
platform/core/src/classes/CommandsManager.js	`90.69% <0.00%> (-1.14%)`	⬇️
platform/core/src/classes/MetadataProvider.js	`4.54% <0.00%> (-0.72%)`	⬇️
...core/src/services/ToolBarService/ToolBarService.js	`1.21% <0.00%> (-0.28%)`	⬇️
platform/viewer/src/index.js	`0.00% <0.00%> (ø)`
platform/viewer/src/appInit.js	`0.00% <0.00%> (ø)`
platform/core/src/utils/index.js	`100.00% <0.00%> (ø)`
platform/core/src/utils/isImage.js	`100.00% <0.00%> (ø)`
platform/core/src/services/ServicesManager.js	`100.00% <0.00%> (ø)`
... and 48 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 980b44f...84afd86. Read the comment docs.

Ouwen · 2022-11-21T07:05:08Z

feat: increased vtk 25.11.1, more reusable cornerstone-extension and minor fixes

Reusability
 - Use React.forwardRef on cornerstone viewport to allow more reusability
 - disableHeader and viewerLayoutHeight optional props added to ViewerLayout
 - Disable border option added to viewpane and viewgridcomp of mode
 - Expose Expose getActiveViewportEnabledElement command
 - Expose setEnabledElement on common utils
 - Optional Overlay Disable Props

Fixes
 - Fix(Overlay was not grabbing instanceNumber)
 - Add resetCamera in commandsManager for VolumeViewport
 - Move CornerstoneViewportService.EVENTS.VIEWPORT_DATA_CHANGED so CornerstoneViewportService can update the active element before event fires
 - Fix CornerstoneCacheService reference
 - Filter requestImageIds to prevent null values
 - Reference Line error log

@sedghi awesome segmentation release.
I made some minor fixes to extensions/cornerstone based on some of behavior which I think are bugs.
I also exposed some of the extensions/cornerstone APIs so other modes/extensions could reuse services/utils/components.

sedghi · 2022-11-22T14:23:41Z

extensions/cornerstone/src/Viewport/Overlays/CornerstoneOverlays.tsx

+  const { 
+    disableViewportImageScrollbar,
+    disableViewportOverlay,
+    disableViewportImageSliceLoadingIndicator,
+    disableViewportOrientationMarkers,
+  } = props;


can we group these? maybe into viewportProperties or something? we are using viewportOptions in hanging protocol but I don't believe these belong there, maybe they do, what do you think?

sedghi · 2022-11-22T14:26:49Z

extensions/cornerstone/src/Viewport/OHIFCornerstoneViewport.tsx

  } = props;

  const [scrollbarHeight, setScrollbarHeight] = useState('100px');

  const elementRef = useRef();

+  React.useImperativeHandle(ref, () => elementRef.current);


why you are adding a useImperativeHandle?

If a parent passes ref into the DOM which is forwarded, it should refer to the internal elementRef.current

However, if a parent is not passing any ref, the internal of the component should still just rely on elementRef.

sedghi · 2022-11-22T14:28:36Z

extensions/cornerstone/src/services/ViewportService/CornerstoneViewportService.ts

+  VIEWPORT_STACK_SET: 'event::cornerstone::viewportservice:viewportstackset',
+  VIEWPORT_VOLUME_SET: 'event::cornerstone::viewportservice:viewportvolumeset',


Why do you need these two events?
We already have STACK_VIEWPORT_NEW_STACK and VOLUME_VIEWPORT_NEW_VOLUME in cs3d

sedghi · 2022-11-22T14:30:24Z

extensions/default/src/ViewerLayout/index.tsx

@@ -32,6 +32,8 @@ function ViewerLayout({
  rightPanels = [],
  leftPanelDefaultClosed = false,
  rightPanelDefaultClosed = false,
+  disableHeader = false,


Can you add a comment that this can be used for mobile use case for instance (since people might think why one wants to remove the toolbar)

sedghi · 2022-11-22T14:38:39Z

platform/viewer/src/routes/Mode/Mode.tsx

@@ -352,7 +353,9 @@ export default function ModeRoute({
            studyInstanceUIDs[0] !== undefined &&
            renderLayoutData({
              ...layoutTemplateData.current.props,
-              ViewportGridComp: ViewportGridWithDataSource,
+              ViewportGridComp: ViewportGridWithDataSourceFactory(
+                layoutTemplateData.current?.props?.viewportGridProps


I don't think this file should be touched. proprs that are returned from modes are layout specific, and all of them are passed to the layout specified (currently we have only one layout that accepts leftPanels, rightPanels). You can just use this viewportGridProps there

sedghi · 2022-11-22T14:39:25Z

platform/viewer/src/components/ViewportGrid.tsx

+    servicesManager,
+    viewportComponents,
+    dataSource,
+    modeViewportPaneStyles = {},


What is this, there are too many things happening. There is viewoprtGridProps, modeViewportPaneStyles and there is customStyle what is their relationship with each other

Hmmm the challenge is in trying to customize the viewportPane. The desired feature is to turn on and off the border. To toggle this the Mode injects viewportGridProps = { modeViewportPaneStyles = {} }. I figured if there were future viewport grid props they could easily be added via the mode viewportGridProps config.

…minor fixes Reusability - Use React.forwardRef on cornerstone viewport to allow more reusability - disableHeader and viewerLayoutHeight optional props added to ViewerLayout - Disable border option added to viewpane and viewgridcomp of mode - Expose Expose getActiveViewportEnabledElement command - Expose setEnabledElement on common utils - Optional Overlay Disable Props Fixes - Fix(Overlay was not grabbing instanceNumber) - Add resetCamera in commandsManager for VolumeViewport - Move CornerstoneViewportService.EVENTS.VIEWPORT_DATA_CHANGED so CornerstoneViewportService can update the active element before event fires - Fix CornerstoneCacheService reference - Filter requestImageIds to prevent null values - Reference Line error log

sedghi · 2023-06-19T13:36:07Z

@Ouwen Also for this one, any one free you have to work on this?

Ouwen force-pushed the gradienthealth/update_cornerstone_extension branch 2 times, most recently from 4bf3528 to f0229d0 Compare November 21, 2022 06:58

Ouwen changed the title ~~(extensions/cornerstone) expose setVolume, setStack events, and CornerstoneCacheService~~ (extensions/cornerstone) more reusable cornerstone-extension and minor fixes, increased vtk 25.11.1 Nov 21, 2022

Ouwen force-pushed the gradienthealth/update_cornerstone_extension branch 2 times, most recently from 1b122c3 to 54a0704 Compare November 21, 2022 07:17

sedghi reviewed Nov 22, 2022

View reviewed changes

Ouwen force-pushed the gradienthealth/update_cornerstone_extension branch from 54a0704 to b73eee9 Compare February 1, 2023 03:44

Ouwen force-pushed the gradienthealth/update_cornerstone_extension branch from b73eee9 to 84afd86 Compare February 4, 2023 07:02

sedghi changed the base branch from v3-stable to master June 19, 2023 13:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

(extensions/cornerstone) more reusable cornerstone-extension and minor fixes, increased vtk 25.11.1 #3016

(extensions/cornerstone) more reusable cornerstone-extension and minor fixes, increased vtk 25.11.1 #3016

Ouwen commented Nov 11, 2022

netlify bot commented Nov 11, 2022 •

edited

Loading

netlify bot commented Nov 11, 2022 •

edited

Loading

codecov bot commented Nov 11, 2022 •

edited

Loading

Ouwen commented Nov 21, 2022

sedghi Nov 22, 2022

sedghi Nov 22, 2022

Ouwen Nov 23, 2022

sedghi Nov 22, 2022

sedghi Nov 22, 2022

sedghi Nov 22, 2022

sedghi Nov 22, 2022

Ouwen Nov 23, 2022

sedghi commented Jun 19, 2023

		VIEWPORT_STACK_SET: 'event::cornerstone::viewportservice:viewportstackset',
		VIEWPORT_VOLUME_SET: 'event::cornerstone::viewportservice:viewportvolumeset',

(extensions/cornerstone) more reusable cornerstone-extension and minor fixes, increased vtk 25.11.1 #3016

Are you sure you want to change the base?

(extensions/cornerstone) more reusable cornerstone-extension and minor fixes, increased vtk 25.11.1 #3016

Conversation

Ouwen commented Nov 11, 2022

netlify bot commented Nov 11, 2022 • edited Loading

✅ Deploy Preview for ohif-platform-viewer ready!

netlify bot commented Nov 11, 2022 • edited Loading

✅ Deploy Preview for ohif-platform-docs canceled.

codecov bot commented Nov 11, 2022 • edited Loading

Codecov Report

Ouwen commented Nov 21, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sedghi commented Jun 19, 2023

netlify bot commented Nov 11, 2022 •

edited

Loading

netlify bot commented Nov 11, 2022 •

edited

Loading

codecov bot commented Nov 11, 2022 •

edited

Loading