From eb8f3a63f2c3260996794888ab86c36fd0802fa0 Mon Sep 17 00:00:00 2001 From: James Petts Date: Tue, 30 Jun 2020 15:10:27 +0100 Subject: [PATCH 1/2] OHIF-197 (#1833) * Viewport letter only in multi-viewport layout. * If SliceThickness not available print N/A * Create displaySet in local session on SR generation. * remove empty line * remove console log * Give storeMeasurements an onSuccess handler instead of always creating dataSets. Co-authored-by: Danny Brown --- .../default/src/DicomWebDataSource/index.js | 6 ++- .../PanelMeasurementTableTracking/index.js | 19 +++++--- platform/core/src/DICOMSR/dataExchange.js | 47 +++---------------- .../core/src/DataSources/IWebApiDataSource.js | 2 + .../DicomMetadataStore/DicomMetadataStore.js | 18 +++---- 5 files changed, 34 insertions(+), 58 deletions(-) diff --git a/extensions/default/src/DicomWebDataSource/index.js b/extensions/default/src/DicomWebDataSource/index.js index 93a7b1b2f..e1b1a5f3a 100644 --- a/extensions/default/src/DicomWebDataSource/index.js +++ b/extensions/default/src/DicomWebDataSource/index.js @@ -10,7 +10,10 @@ import { DicomMetadataStore, IWebApiDataSource, utils } from '@ohif/core'; import getImageId from './utils/getImageId'; import * as dcmjs from 'dcmjs'; -import { retrieveStudyMetadata } from './retrieveStudyMetadata.js'; +import { + retrieveStudyMetadata, + deleteStudyMetadataPromise, +} from './retrieveStudyMetadata.js'; const { DicomMetaDictionary, DicomDict } = dcmjs.data; @@ -187,6 +190,7 @@ function createDicomWebApi(dicomWebConfig) { storeInstances(instances); }); }, + deleteStudyMetadataPromise, getImageIdsForDisplaySet(displaySet) { const images = displaySet.images; const imageIds = []; diff --git a/extensions/measurement-tracking/src/panels/PanelMeasurementTableTracking/index.js b/extensions/measurement-tracking/src/panels/PanelMeasurementTableTracking/index.js index 10184832a..b7cf5ae8f 100644 --- a/extensions/measurement-tracking/src/panels/PanelMeasurementTableTracking/index.js +++ b/extensions/measurement-tracking/src/panels/PanelMeasurementTableTracking/index.js @@ -24,7 +24,7 @@ function PanelMeasurementTableTracking({ servicesManager, extensionManager }) { measurementChangeTimestamp, 200 ); - const { MeasurementService } = servicesManager.services; + const { MeasurementService, DisplaySetService } = servicesManager.services; const [ trackedMeasurements, sendTrackedMeasurementsEvent, @@ -105,7 +105,7 @@ function PanelMeasurementTableTracking({ servicesManager, extensionManager }) { const activeMeasurementItem = 0; - const onExportClick = () => { + const exportReport = () => { const measurements = MeasurementService.getMeasurements(); const trackedMeasurements = measurements.filter( m => @@ -117,7 +117,7 @@ function PanelMeasurementTableTracking({ servicesManager, extensionManager }) { DICOMSR.downloadReport(trackedMeasurements, dataSource); }; - const onCreateReportClick = () => { + const createReport = () => { const measurements = MeasurementService.getMeasurements(); const trackedMeasurements = measurements.filter( m => @@ -125,13 +125,18 @@ function PanelMeasurementTableTracking({ servicesManager, extensionManager }) { trackedSeries.includes(m.referenceSeriesUID) ); - const dataSources = extensionManager.getDataSources(); // TODO -> Eventually deal with multiple dataSources. // Would need some way of saying which one is the "push" dataSource const dataSource = dataSources[0]; - DICOMSR.storeMeasurements(trackedMeasurements, dataSource); + DICOMSR.storeMeasurements( + trackedMeasurements, + dataSource, + naturalizedReport => { + DisplaySetService.makeDisplaySets([naturalizedReport]); + } + ); }; return ( @@ -154,8 +159,8 @@ function PanelMeasurementTableTracking({ servicesManager, extensionManager }) {
diff --git a/platform/core/src/DICOMSR/dataExchange.js b/platform/core/src/DICOMSR/dataExchange.js index 2b3cfe172..8f86f743c 100644 --- a/platform/core/src/DICOMSR/dataExchange.js +++ b/platform/core/src/DICOMSR/dataExchange.js @@ -43,45 +43,6 @@ const retrieveMeasurements = server => { return retrieveMeasurementFromSR(latestSeries, studies, serverUrl); }; -/** - * Function to be registered into MeasurementAPI to store measurements into DICOM Structured Reports - * - * @param {Object} measurementData - OHIF measurementData object - * @param {Object} filter - * @param {serverType} server - * @returns {Object} With message to be displayed on success - */ -const storeMeasurementsOld = async (measurementData, filter, server) => { - log.info('[DICOMSR] storeMeasurements'); - - if (!server || server.type !== 'dicomWeb') { - log.error('[DICOMSR] DicomWeb server is required!'); - return Promise.reject({}); - } - - const serverUrl = server.wadoRoot; - const firstMeasurementKey = Object.keys(measurementData)[0]; - const firstMeasurement = measurementData[firstMeasurementKey][0]; - const StudyInstanceUID = - firstMeasurement && firstMeasurement.StudyInstanceUID; - - try { - await stowSRFromMeasurements(measurementData, serverUrl); - if (StudyInstanceUID) { - studies.deleteStudyMetadataPromise(StudyInstanceUID); - } - - return { - message: 'Measurements saved successfully', - }; - } catch (error) { - log.error( - `[DICOMSR] Error while saving the measurements: ${error.message}` - ); - throw new Error('Error while saving the measurements.'); - } -}; - /** * * @param {object[]} measurementData An array of measurements from the measurements service @@ -119,7 +80,7 @@ const generateReport = measurementData => { * that you wish to serialize. * @param {object} dataSource The dataSource that you wish to use to persist the data. */ -const storeMeasurements = async (measurementData, dataSource) => { +const storeMeasurements = async (measurementData, dataSource, onSuccess) => { // TODO -> Eventually use the measurements directly and not the dcmjs adapter, // But it is good enough for now whilst we only have cornerstone as a datasource. log.info('[DICOMSR] storeMeasurements'); @@ -136,7 +97,11 @@ const storeMeasurements = async (measurementData, dataSource) => { await dataSource.store.dicom(naturalizedReport); if (StudyInstanceUID) { - studies.deleteStudyMetadataPromise(StudyInstanceUID); + dataSource.deleteStudyMetadataPromise(StudyInstanceUID); + } + + if (onSuccess) { + onSuccess(naturalizedReport); } return { diff --git a/platform/core/src/DataSources/IWebApiDataSource.js b/platform/core/src/DataSources/IWebApiDataSource.js index 10285d364..2ba208ea5 100644 --- a/platform/core/src/DataSources/IWebApiDataSource.js +++ b/platform/core/src/DataSources/IWebApiDataSource.js @@ -17,6 +17,7 @@ function create({ retrieve, store, retrieveSeriesMetadata, + deleteStudyMetadataPromise, getImageIdsForDisplaySet, }) { const defaultQuery = { @@ -59,6 +60,7 @@ function create({ store: store || defaultStore, getImageIdsForDisplaySet, retrieveSeriesMetadata, + deleteStudyMetadataPromise, }; } diff --git a/platform/core/src/services/DicomMetadataStore/DicomMetadataStore.js b/platform/core/src/services/DicomMetadataStore/DicomMetadataStore.js index 8fb176959..7768905fa 100644 --- a/platform/core/src/services/DicomMetadataStore/DicomMetadataStore.js +++ b/platform/core/src/services/DicomMetadataStore/DicomMetadataStore.js @@ -72,16 +72,16 @@ const BaseImplementation = { study = _model.studies[_model.studies.length - 1]; } - // TODO: Worth identifying why this is being called many times with series - // that are already "added"? - const didAddSeries = study.addSeries(instances); + study.addSeries(instances); - if (didAddSeries) { - this._broadcastEvent(EVENTS.INSTANCES_ADDED, { - StudyInstanceUID, - SeriesInstanceUID, - }); - } + // Broadcast an event even if we used cached data. + // This is because the mode needs to listen to instances that are added to build up its active displaySets. + // It will see there are cached displaySets and end early if this Series has already been fired in this + // Mode session for some reason. + this._broadcastEvent(EVENTS.INSTANCES_ADDED, { + StudyInstanceUID, + SeriesInstanceUID, + }); }, addStudy(study) { const { StudyInstanceUID } = study; From 2c11609c556bb7b5363eb2cd701a2b74cac38f2f Mon Sep 17 00:00:00 2001 From: James Petts Date: Tue, 30 Jun 2020 16:27:35 +0100 Subject: [PATCH 2/2] OHIF-198 - Jump to thumbnail when a new displaySet is added (#1834) * Viewport letter only in multi-viewport layout. * If SliceThickness not available print N/A * Create displaySet in local session on SR generation. * Jump to thumbnail when a new displaySet is added with the madeInClient property. * Check if scrollIntoView exists. --- .../default/src/Panels/PanelStudyBrowser.jsx | 7 +- .../dicom-sr/src/getSopClassHandlerModule.js | 5 +- .../PanelMeasurementTableTracking/index.js | 6 +- .../PanelStudyBrowserTracking.jsx | 83 ++++++++++++++++++- .../DisplaySetService/DisplaySetService.js | 13 ++- .../ui/src/components/Thumbnail/Thumbnail.jsx | 2 + .../ThumbnailList/ThumbnailList.jsx | 3 + .../ThumbnailNoImage/ThumbnailNoImage.jsx | 5 +- .../ThumbnailTracked/ThumbnailTracked.jsx | 2 + 9 files changed, 113 insertions(+), 13 deletions(-) diff --git a/extensions/default/src/Panels/PanelStudyBrowser.jsx b/extensions/default/src/Panels/PanelStudyBrowser.jsx index 8e813217f..2d66fc634 100644 --- a/extensions/default/src/Panels/PanelStudyBrowser.jsx +++ b/extensions/default/src/Panels/PanelStudyBrowser.jsx @@ -28,6 +28,8 @@ function PanelStudyBrowser({ const [displaySets, setDisplaySets] = useState([]); const [thumbnailImageSrcMap, setThumbnailImageSrcMap] = useState({}); + console.log(DisplaySetService); + // ~~ studyDisplayList useEffect(() => { // Fetch all studies for the patient in each primary study @@ -98,8 +100,9 @@ function PanelStudyBrowser({ // DISPLAY_SETS_ADDED returns an array of DisplaySets that were added const SubscriptionDisplaySetsAdded = DisplaySetService.subscribe( DisplaySetService.EVENTS.DISPLAY_SETS_ADDED, - newDisplaySets => { - newDisplaySets.forEach(async dSet => { + data => { + const { displaySetsAdded } = data; + displaySetsAdded.forEach(async dSet => { const newImageSrcEntry = {}; const displaySet = DisplaySetService.getDisplaySetByUID( dSet.displaySetInstanceUID diff --git a/extensions/dicom-sr/src/getSopClassHandlerModule.js b/extensions/dicom-sr/src/getSopClassHandlerModule.js index 9e57e3dcf..80a597a08 100644 --- a/extensions/dicom-sr/src/getSopClassHandlerModule.js +++ b/extensions/dicom-sr/src/getSopClassHandlerModule.js @@ -103,10 +103,11 @@ function _getDisplaySetsFromSeries( // Subscribe to new displaySets as the source may come in after. DisplaySetService.subscribe( DisplaySetService.EVENTS.DISPLAY_SETS_ADDED, - newDisplaySets => { + data => { + const { displaySetsAdded } = data; // If there are still some measurements that have not yet been loaded into cornerstone, // See if we can load them onto any of the new displaySets. - newDisplaySets.forEach(newDisplaySet => { + displaySetsAdded.forEach(newDisplaySet => { _checkIfCanAddMeasurementsToDisplaySet( displaySet, newDisplaySet, diff --git a/extensions/measurement-tracking/src/panels/PanelMeasurementTableTracking/index.js b/extensions/measurement-tracking/src/panels/PanelMeasurementTableTracking/index.js index b7cf5ae8f..968a51c1f 100644 --- a/extensions/measurement-tracking/src/panels/PanelMeasurementTableTracking/index.js +++ b/extensions/measurement-tracking/src/panels/PanelMeasurementTableTracking/index.js @@ -117,7 +117,7 @@ function PanelMeasurementTableTracking({ servicesManager, extensionManager }) { DICOMSR.downloadReport(trackedMeasurements, dataSource); }; - const createReport = () => { + const createReport = async () => { const measurements = MeasurementService.getMeasurements(); const trackedMeasurements = measurements.filter( m => @@ -134,7 +134,9 @@ function PanelMeasurementTableTracking({ servicesManager, extensionManager }) { trackedMeasurements, dataSource, naturalizedReport => { - DisplaySetService.makeDisplaySets([naturalizedReport]); + DisplaySetService.makeDisplaySets([naturalizedReport], { + madeInClient: true, + }); } ); }; diff --git a/extensions/measurement-tracking/src/panels/PanelStudyBrowserTracking/PanelStudyBrowserTracking.jsx b/extensions/measurement-tracking/src/panels/PanelStudyBrowserTracking/PanelStudyBrowserTracking.jsx index a3ef5f777..b45cd3691 100644 --- a/extensions/measurement-tracking/src/panels/PanelStudyBrowserTracking/PanelStudyBrowserTracking.jsx +++ b/extensions/measurement-tracking/src/panels/PanelStudyBrowserTracking/PanelStudyBrowserTracking.jsx @@ -34,6 +34,7 @@ function PanelStudyBrowserTracking({ const [studyDisplayList, setStudyDisplayList] = useState([]); const [displaySets, setDisplaySets] = useState([]); const [thumbnailImageSrcMap, setThumbnailImageSrcMap] = useState({}); + const [jumpToDisplaySet, setJumpToDisplaySet] = useState(null); // TODO: Should this be somewhere else? Feels more like a mode "lifecycle" setup/destroy? useEffect(() => { @@ -136,18 +137,26 @@ function PanelStudyBrowserTracking({ // DISPLAY_SETS_ADDED returns an array of DisplaySets that were added const SubscriptionDisplaySetsAdded = DisplaySetService.subscribe( DisplaySetService.EVENTS.DISPLAY_SETS_ADDED, - newDisplaySets => { - newDisplaySets.forEach(async dSet => { + data => { + const { displaySetsAdded, options } = data; + displaySetsAdded.forEach(async dSet => { + const displaySetInstanceUID = dSet.displaySetInstanceUID; + const newImageSrcEntry = {}; const displaySet = DisplaySetService.getDisplaySetByUID( - dSet.displaySetInstanceUID + displaySetInstanceUID ); + + if (options.madeInClient) { + setJumpToDisplaySet(displaySetInstanceUID); + } + const imageIds = dataSource.getImageIdsForDisplaySet(displaySet); const imageId = imageIds[Math.floor(imageIds.length / 2)]; // TODO: Is it okay that imageIds are not returned here for SR displaysets? if (imageId) { // When the image arrives, render it and store the result in the thumbnailImgSrcMap - newImageSrcEntry[dSet.displaySetInstanceUID] = await getImageSrc( + newImageSrcEntry[displaySetInstanceUID] = await getImageSrc( imageId ); setThumbnailImageSrcMap(prevState => { @@ -214,6 +223,51 @@ function PanelStudyBrowserTracking({ } } + useEffect(() => { + if (jumpToDisplaySet) { + // Get element by displaySetInstanceUID + const displaySetInstanceUID = jumpToDisplaySet; + const element = document.getElementById( + `thumbnail-${displaySetInstanceUID}` + ); + + if (element && typeof element.scrollIntoView === 'function') { + // TODO: Any way to support IE here? + element.scrollIntoView({ behavior: 'smooth' }); + + setJumpToDisplaySet(null); + } + } + }, [jumpToDisplaySet, expandedStudyInstanceUIDs, activeTabName]); + + useEffect(() => { + if (!jumpToDisplaySet) { + return; + } + + const displaySetInstanceUID = jumpToDisplaySet; + // Set the activeTabName and expand the study + const thumbnailLocation = _findTabAndStudyOfDisplaySet( + displaySetInstanceUID, + tabs + ); + if (!thumbnailLocation) { + console.warn('jumpToThumbnail: displaySet thumbnail not found.'); + + return; + } + const { tabName, StudyInstanceUID } = thumbnailLocation; + setActiveTabName(tabName); + const studyExpanded = expandedStudyInstanceUIDs.includes(StudyInstanceUID); + if (!studyExpanded) { + const updatedExpandedStudyInstanceUIDs = [ + ...expandedStudyInstanceUIDs, + StudyInstanceUID, + ]; + setExpandedStudyInstanceUIDs(updatedExpandedStudyInstanceUIDs); + } + }, [jumpToDisplaySet]); + return ( displaySet.displaySetInstanceUID === displaySetInstanceUid ); - makeDisplaySets = (input, batch = false) => { + makeDisplaySets = (input, { batch = false, madeInClient = false } = {}) => { if (!input || !input.length) { throw new Error('No instances were provided.'); } @@ -78,11 +78,20 @@ export default class DisplaySetService { displaySetsAdded = displaySets; } + const options = {}; + + if (madeInClient) { + options.madeInClient = true; + } + // TODO: This is tricky. How do we know we're not resetting to the same/existing DSs? // TODO: This is likely run anytime we touch DicomMetadataStore. How do we prevent uneccessary broadcasts? if (displaySetsAdded && displaySetsAdded.length) { - this._broadcastEvent(EVENTS.DISPLAY_SETS_ADDED, displaySetsAdded); this._broadcastEvent(EVENTS.DISPLAY_SETS_CHANGED, this.activeDisplaySets); + this._broadcastEvent(EVENTS.DISPLAY_SETS_ADDED, { + displaySetsAdded, + options, + }); } }; diff --git a/platform/ui/src/components/Thumbnail/Thumbnail.jsx b/platform/ui/src/components/Thumbnail/Thumbnail.jsx index 44460fa8a..7813928a0 100644 --- a/platform/ui/src/components/Thumbnail/Thumbnail.jsx +++ b/platform/ui/src/components/Thumbnail/Thumbnail.jsx @@ -9,6 +9,7 @@ import { Icon } from '@ohif/ui'; * */ const Thumbnail = ({ + displaySetInstanceUID, className, imageSrc, imageAltText, @@ -36,6 +37,7 @@ const Thumbnail = ({ className, 'flex flex-col flex-1 px-3 mb-8 cursor-pointer outline-none' )} + id={`thumbnail-${displaySetInstanceUID}`} onClick={onClick} onKeyDown={onClick} role="button" diff --git a/platform/ui/src/components/ThumbnailList/ThumbnailList.jsx b/platform/ui/src/components/ThumbnailList/ThumbnailList.jsx index 303159df8..0b1febd82 100644 --- a/platform/ui/src/components/ThumbnailList/ThumbnailList.jsx +++ b/platform/ui/src/components/ThumbnailList/ThumbnailList.jsx @@ -33,6 +33,7 @@ const ThumbnailList = ({ return (