From f791a4cafb67288a874c6dcb99c0475a6a0cf7aa Mon Sep 17 00:00:00 2001 From: Joe Boccanfuso <109477394+jbocce@users.noreply.github.com> Date: Tue, 14 Feb 2023 10:01:39 -0500 Subject: [PATCH] fix(ViewportGrid) fill blank viewports with display sets not yet in grid. (#3154) * fix(ViewportGrid): In ViewportGrid, fill blank viewports with display sets not yet in grid. ViewportGridService now allows off-screen viewports to remain so that so as to maintain continuity if they were filled by the UI/user. * PR feedback: moved getNumViewportPanes into the ViewportGridService API. * PR feedback: - renamed some variables - proper import/export of IDisplaySet - added some comments for clarification - fixed broken e2e tests * Some missed rename of Services. --- .../src/utils/mpr/toggleMPRHangingProtocol.ts | 25 +++++--- .../src/Toolbar/ToolbarLayoutSelector.tsx | 10 ++- .../PanelStudyBrowserTracking.tsx | 33 +++++----- .../ViewportGridService.ts | 8 +++ .../services/ui/viewport-grid-service.md | 3 +- .../services/ui/viewport-grid-service.md | 3 +- .../contextProviders/ViewportGridProvider.tsx | 28 ++++++++- .../viewer/src/components/ViewportGrid.tsx | 61 +++++++++++++++++-- 8 files changed, 138 insertions(+), 33 deletions(-) diff --git a/extensions/cornerstone/src/utils/mpr/toggleMPRHangingProtocol.ts b/extensions/cornerstone/src/utils/mpr/toggleMPRHangingProtocol.ts index 6515cea8d..663643ab0 100644 --- a/extensions/cornerstone/src/utils/mpr/toggleMPRHangingProtocol.ts +++ b/extensions/cornerstone/src/utils/mpr/toggleMPRHangingProtocol.ts @@ -59,6 +59,13 @@ export default function toggleMPRHangingProtocol({ viewports[activeViewportIndex].displaySetInstanceUIDs; const errorCallback = error => { + // Unable to create MPR, so be sure to return to the cached/original protocol. + hangingProtocolService.setProtocol( + cachedState.protocol.id, + viewportMatchDetails, + restoreErrorCallback + ); + uiNotificationService.show({ title: 'Multiplanar reconstruction (MPR) ', message: @@ -276,13 +283,17 @@ function _getViewportsInfo({ protocol, stage, viewports, servicesManager }) { .filter(Boolean); if (viewportIds.length) { - toolOptions = viewportIds.map(viewportId => { - const toolGroup = toolGroupService.getToolGroupForViewport(viewportId); - return { - toolGroupId: toolGroup.id, - toolOptions: toolGroup.toolOptions, - }; - }); + toolOptions = viewportIds + .map(viewportId => { + const toolGroup = toolGroupService.getToolGroupForViewport(viewportId); + return toolGroup + ? { + toolGroupId: toolGroup.id, + toolOptions: toolGroup.toolOptions, + } + : null; + }) + .filter(Boolean); } return { viewportMatchDetails, viewportStructure, toolOptions }; diff --git a/extensions/default/src/Toolbar/ToolbarLayoutSelector.tsx b/extensions/default/src/Toolbar/ToolbarLayoutSelector.tsx index a385c071d..6080de195 100644 --- a/extensions/default/src/Toolbar/ToolbarLayoutSelector.tsx +++ b/extensions/default/src/Toolbar/ToolbarLayoutSelector.tsx @@ -79,7 +79,15 @@ function LayoutSelector({ ], }); } - viewportGridService.setLayout({ numRows, numCols }); + + // When a new layout is selected, keep any extra/offscreen viewports + // so that if any of those viewports were populated via the UI then they + // will be maintained in case those viewports are redisplayed later. + viewportGridService.setLayout({ + numRows, + numCols, + keepExtraViewports: true, + }); }; return ( diff --git a/extensions/measurement-tracking/src/panels/PanelStudyBrowserTracking/PanelStudyBrowserTracking.tsx b/extensions/measurement-tracking/src/panels/PanelStudyBrowserTracking/PanelStudyBrowserTracking.tsx index 9983aadf5..a04989f0b 100644 --- a/extensions/measurement-tracking/src/panels/PanelStudyBrowserTracking/PanelStudyBrowserTracking.tsx +++ b/extensions/measurement-tracking/src/panels/PanelStudyBrowserTracking/PanelStudyBrowserTracking.tsx @@ -182,7 +182,7 @@ function PanelStudyBrowserTracking({ thumbnailImageSrcMap, trackedSeries, viewports, - isSingleViewport, + viewportGridService, dataSource, displaySetService, uiDialogService, @@ -245,7 +245,7 @@ function PanelStudyBrowserTracking({ thumbnailImageSrcMap, trackedSeries, viewports, - isSingleViewport, + viewportGridService, dataSource, displaySetService, uiDialogService, @@ -412,7 +412,7 @@ function _mapDisplaySets( thumbnailImageSrcMap, trackedSeriesInstanceUIDs, viewports, // TODO: make array of `displaySetInstanceUIDs`? - isSingleViewport, + viewportGridService, dataSource, displaySetService, uiDialogService, @@ -423,18 +423,21 @@ function _mapDisplaySets( displaySets.forEach(ds => { const imageSrc = thumbnailImageSrcMap[ds.displaySetInstanceUID]; const componentType = _getComponentType(ds.Modality); - const viewportIdentificator = isSingleViewport - ? [] - : viewports.reduce((acc, viewportData, index) => { - if ( - viewportData?.displaySetInstanceUIDs?.includes( - ds.displaySetInstanceUID - ) - ) { - acc.push(viewportData.viewportLabel); - } - return acc; - }, []); + const numPanes = viewportGridService.getNumViewportPanes(); + const viewportIdentificator = + numPanes === 1 + ? [] + : viewports.reduce((acc, viewportData, index) => { + if ( + index < numPanes && + viewportData?.displaySetInstanceUIDs?.includes( + ds.displaySetInstanceUID + ) + ) { + acc.push(viewportData.viewportLabel); + } + return acc; + }, []); const array = componentType === 'thumbnailTracked' diff --git a/platform/core/src/services/ViewportGridService/ViewportGridService.ts b/platform/core/src/services/ViewportGridService/ViewportGridService.ts index ae1a6a01e..c35cc34c3 100644 --- a/platform/core/src/services/ViewportGridService/ViewportGridService.ts +++ b/platform/core/src/services/ViewportGridService/ViewportGridService.ts @@ -31,6 +31,7 @@ class ViewportGridService extends PubSubService { reset: resetImplementation, onModeExit: onModeExitImplementation, set: setImplementation, + getNumViewportPanes: getNumViewportPanesImplementation, }): void { if (getStateImplementation) { this.serviceImplementation._getState = getStateImplementation; @@ -62,6 +63,9 @@ class ViewportGridService extends PubSubService { if (setImplementation) { this.serviceImplementation._set = setImplementation; } + if (getNumViewportPanesImplementation) { + this.serviceImplementation._getNumViewportPanes = getNumViewportPanesImplementation; + } } public setActiveViewportIndex(index) { @@ -122,6 +126,10 @@ class ViewportGridService extends PubSubService { public set(state) { this.serviceImplementation._set(state); } + + public getNumViewportPanes() { + return this.serviceImplementation._getNumViewportPanes(); + } } export default ViewportGridService; diff --git a/platform/docs/docs/platform/services/ui/viewport-grid-service.md b/platform/docs/docs/platform/services/ui/viewport-grid-service.md index d48ab4e00..b6806343e 100644 --- a/platform/docs/docs/platform/services/ui/viewport-grid-service.md +++ b/platform/docs/docs/platform/services/ui/viewport-grid-service.md @@ -19,8 +19,9 @@ is expected to support, [check out it's interface in `@ohif/core`][interface] | `setActiveViewportIndex(index)` | Sets the active viewport index in the app | | `getState()` | Gets the states of the viewport (see below) | | `setDisplaySetsForViewport({ viewportIndex, displaySetInstanceUID })` | Sets displaySet for viewport based on displaySet Id | -| `setLayout({numCols, numRows})` | Sets rows and columns | +| `setLayout({numCols, numRows, keepExtraViewports})` | Sets rows and columns. When the total number of viewports decreases, optionally keep the extra/offscreen viewports. | | `reset()` | Resets the default states | +| `getNumViewportPanes()` | Gets the number of visible viewport panes | ## Implementations diff --git a/platform/docs/versioned_docs/version-3.0/platform/services/ui/viewport-grid-service.md b/platform/docs/versioned_docs/version-3.0/platform/services/ui/viewport-grid-service.md index d48ab4e00..39e2378a3 100644 --- a/platform/docs/versioned_docs/version-3.0/platform/services/ui/viewport-grid-service.md +++ b/platform/docs/versioned_docs/version-3.0/platform/services/ui/viewport-grid-service.md @@ -19,8 +19,9 @@ is expected to support, [check out it's interface in `@ohif/core`][interface] | `setActiveViewportIndex(index)` | Sets the active viewport index in the app | | `getState()` | Gets the states of the viewport (see below) | | `setDisplaySetsForViewport({ viewportIndex, displaySetInstanceUID })` | Sets displaySet for viewport based on displaySet Id | -| `setLayout({numCols, numRows})` | Sets rows and columns | +| `setLayout({numCols, numRows, keepExtraViewports})` | Sets rows and columns. When the total number of viewports decreases, optionally keep the extra/offscreen viewports. | | `reset()` | Resets the default states | +| `getNumViewportPanes()` | Gets the number of visible viewport panes | ## Implementations diff --git a/platform/ui/src/contextProviders/ViewportGridProvider.tsx b/platform/ui/src/contextProviders/ViewportGridProvider.tsx index fa6eb03ee..80a1647d9 100644 --- a/platform/ui/src/contextProviders/ViewportGridProvider.tsx +++ b/platform/ui/src/contextProviders/ViewportGridProvider.tsx @@ -86,6 +86,7 @@ export function ViewportGridProvider({ children, service }) { numRows, layoutOptions, layoutType = 'grid', + keepExtraViewports = false, } = action.payload; // If empty viewportOptions, we use numRow and numCols to calculate number of viewports @@ -97,8 +98,14 @@ export function ViewportGridProvider({ children, service }) { while (viewports.length < numPanes) { viewports.push({}); } - while (viewports.length > numPanes) { - viewports.pop(); + + // Extra viewports are kept when the grid layout is changed in the UI + // because the user populated those viewports and if the viewports were to + // return on screen their contents should be maintained. + if (!keepExtraViewports) { + while (viewports.length > numPanes) { + viewports.pop(); + } } for (let i = 0; i < numPanes; i++) { @@ -238,7 +245,13 @@ export function ViewportGridProvider({ children, service }) { ); const setLayout = useCallback( - ({ layoutType, numRows, numCols, layoutOptions = [] }) => + ({ + layoutType, + numRows, + numCols, + layoutOptions = [], + keepExtraViewports = false, + }) => dispatch({ type: 'SET_LAYOUT', payload: { @@ -246,6 +259,7 @@ export function ViewportGridProvider({ children, service }) { numRows, numCols, layoutOptions, + keepExtraViewports, }, }), [dispatch] @@ -288,6 +302,11 @@ export function ViewportGridProvider({ children, service }) { [dispatch] ); + const getNumViewportPanes = useCallback(() => { + const { numCols, numRows, viewports } = viewportGridState; + return Math.min(viewports.length, numCols * numRows); + }, [viewportGridState]); + /** * Sets the implementation of ViewportGridService that can be used by extensions. * @@ -306,6 +325,7 @@ export function ViewportGridProvider({ children, service }) { setCachedLayout, restoreCachedLayout, set, + getNumViewportPanes, }); } }, [ @@ -319,6 +339,7 @@ export function ViewportGridProvider({ children, service }) { setCachedLayout, restoreCachedLayout, set, + getNumViewportPanes, ]); const api = { @@ -331,6 +352,7 @@ export function ViewportGridProvider({ children, service }) { restoreCachedLayout, reset, set, + getNumViewportPanes, }; return ( diff --git a/platform/viewer/src/components/ViewportGrid.tsx b/platform/viewer/src/components/ViewportGrid.tsx index 52a4eb62a..349c7e4b6 100644 --- a/platform/viewer/src/components/ViewportGrid.tsx +++ b/platform/viewer/src/components/ViewportGrid.tsx @@ -54,20 +54,35 @@ function ViewerViewportGrid(props) { return; } - // Match each viewport individually - const numViewports = viewportGrid.numRows * viewportGrid.numCols; + const gridDisplaySetUIDs = []; + const blankViewportIndices = []; + + // Match each viewport individually. + const numViewports = viewportGridService.getNumViewportPanes(); + for ( let viewportIndex = 0; viewportIndex < numViewports; viewportIndex++ ) { + const viewportDisplaySetUIDs = + viewports[viewportIndex]?.displaySetInstanceUIDs ?? []; + if (hpAlreadyApplied.get(viewportIndex)) { + gridDisplaySetUIDs.push(...viewportDisplaySetUIDs); continue; } // if current viewport doesn't have a match if (viewportMatchDetails.get(viewportIndex) === undefined) { - return; + // if the current viewport is empty/blank + if (viewportDisplaySetUIDs.length === 0) { + blankViewportIndices.push(viewportIndex); + } else { + gridDisplaySetUIDs.push(...viewportDisplaySetUIDs); + } + + continue; } const { displaySetsInfo, viewportOptions } = viewportMatchDetails.get( @@ -87,6 +102,8 @@ function ViewerViewportGrid(props) { } ); + gridDisplaySetUIDs.push(...displaySetUIDsToHang); + viewportGridService.setDisplaySetsForViewport({ viewportIndex: viewportIndex, displaySetInstanceUIDs: displaySetUIDsToHang, @@ -111,6 +128,25 @@ function ViewerViewportGrid(props) { ); } } + + blankViewportIndices.forEach((blankVPIndex: number) => { + // try to fill the empty viewport with a display set not already in the grid + const displaySetsNotInGrid = availableDisplaySets.filter( + displaySet => + gridDisplaySetUIDs.indexOf(displaySet.displaySetInstanceUID) === -1 + ); + + if (displaySetsNotInGrid.length > 0) { + const displaySetUIDToAdd = + displaySetsNotInGrid[0].displaySetInstanceUID; + gridDisplaySetUIDs.push(displaySetUIDToAdd); + + viewportGridService.setDisplaySetsForViewport({ + viewportIndex: blankVPIndex, + displaySetInstanceUIDs: [displaySetUIDToAdd], + }); + } + }); }, [viewportGrid, numRows, numCols] ); @@ -178,6 +214,20 @@ function ViewerViewportGrid(props) { }; }, [viewports]); + useEffect(() => { + const { unsubscribe } = hangingProtocolService.subscribe( + hangingProtocolService.EVENTS.STAGE_CHANGE, + () => { + const displaySets = DisplaySetService.getActiveDisplaySets(); + updateDisplaySetsForViewports(displaySets); + } + ); + + return () => { + unsubscribe(); + }; + }, [viewports]); + useEffect(() => { const { unsubscribe } = measurementService.subscribe( measurementService.EVENTS.JUMP_TO_MEASUREMENT, @@ -314,7 +364,8 @@ function ViewerViewportGrid(props) { const getViewportPanes = useCallback(() => { const viewportPanes = []; - for (let i = 0; i < viewports.length; i++) { + const numViewports = viewportGridService.getNumViewportPanes(); + for (let i = 0; i < numViewports; i++) { const viewportIndex = i; const isActive = activeViewportIndex === viewportIndex; const paneMetadata = viewports[i] || {}; @@ -388,7 +439,7 @@ function ViewerViewportGrid(props) { 1 ? viewportLabel : ''} + viewportLabel={numViewports > 1 ? viewportLabel : ''} dataSource={dataSource} viewportOptions={viewportOptions} displaySetOptions={displaySetOptions}