diff --git a/extensions/default/src/getSopClassHandlerModule.js b/extensions/default/src/getSopClassHandlerModule.js index 5ba32f989..902e679a9 100644 --- a/extensions/default/src/getSopClassHandlerModule.js +++ b/extensions/default/src/getSopClassHandlerModule.js @@ -67,7 +67,7 @@ function getDisplaySetInfo(instances) { }; } -const makeDisplaySet = instances => { +const makeDisplaySet = (instances, index) => { // Need to sort the instances in order to get a consistent instance/thumbnail sortStudyInstances(instances); const instance = instances[0]; @@ -190,7 +190,7 @@ function getDisplaySetsFromSeries(instances) { // into their own specific display sets. Place the rest of each // series into another display set. const stackableInstances = []; - instances.forEach(instance => { + instances.forEach((instance, instanceIndex) => { // All imaging modalities must have a valid value for sopClassUid (x00080016) or rows (x00280010) if (!isImage(instance.SOPClassUID) && !instance.Rows) { return; @@ -198,7 +198,7 @@ function getDisplaySetsFromSeries(instances) { let displaySet; if (isMultiFrame(instance)) { - displaySet = makeDisplaySet([instance]); + displaySet = makeDisplaySet([instance], instanceIndex); displaySet.setAttributes({ sopClassUids, numImageFrames: instance.NumberOfFrames, @@ -207,7 +207,7 @@ function getDisplaySetsFromSeries(instances) { }); displaySets.push(displaySet); } else if (isSingleImageModality(instance.Modality)) { - displaySet = makeDisplaySet([instance]); + displaySet = makeDisplaySet([instance], instanceIndex); displaySet.setAttributes({ sopClassUids, instanceNumber: instance.InstanceNumber, @@ -220,7 +220,7 @@ function getDisplaySetsFromSeries(instances) { }); if (stackableInstances.length) { - const displaySet = makeDisplaySet(stackableInstances); + const displaySet = makeDisplaySet(stackableInstances, displaySets.length); displaySet.setAttribute('studyInstanceUid', instances[0].StudyInstanceUID); displaySet.setAttributes({ sopClassUids, diff --git a/platform/app/src/routes/Mode/defaultRouteInit.ts b/platform/app/src/routes/Mode/defaultRouteInit.ts index fceb7573c..b7aeb844f 100644 --- a/platform/app/src/routes/Mode/defaultRouteInit.ts +++ b/platform/app/src/routes/Mode/defaultRouteInit.ts @@ -1,8 +1,8 @@ -import getStudies from './studiesList'; import { DicomMetadataStore, log, utils, Enums } from '@ohif/core'; +import getStudies from './studiesList'; import isSeriesFilterUsed from '../../utils/isSeriesFilterUsed'; -const { getSplitParam } = utils; +const { seriesSortCriteria, getSplitParam } = utils; /** * Initialize the route. @@ -14,7 +14,12 @@ const { getSplitParam } = utils; * @returns array of subscriptions to cancel */ export async function defaultRouteInit( - { servicesManager, studyInstanceUIDs, dataSource, filters, appConfig }: withAppTypes, + { + servicesManager, + studyInstanceUIDs, + dataSource, + filters, + }: withAppTypes & { studyInstanceUIDs?: string[] }, hangingProtocolId, stageIndex ) { @@ -27,13 +32,17 @@ export async function defaultRouteInit( */ function applyHangingProtocol() { const displaySets = displaySetService.getActiveDisplaySets(); + // The display sets are not necessarily in load order, even though the + // series got started in load order, so re-sort them before hanging + const sortCriteria = seriesSortCriteria.default; if (!displaySets || !displaySets.length) { return; } + const sortedDisplaySets = [...displaySets].sort(sortCriteria); // Gets the studies list to use - const studies = getStudies(studyInstanceUIDs, displaySets); + const studies = getStudies(studyInstanceUIDs, sortedDisplaySets); // study being displayed, and is thus the "active" study. const activeStudy = studies[0]; diff --git a/platform/core/src/services/DisplaySetService/DisplaySetService.ts b/platform/core/src/services/DisplaySetService/DisplaySetService.ts index 2bef0070f..194eb0011 100644 --- a/platform/core/src/services/DisplaySetService/DisplaySetService.ts +++ b/platform/core/src/services/DisplaySetService/DisplaySetService.ts @@ -464,12 +464,17 @@ export default class DisplaySetService extends PubSubService { /** * * @param sortFn function to sort the display sets - * @param direction direction to sort the display sets + * @param direction direction to sort the display sets. Ascending means + * increasing in value, which will typically put the lowest series numbers + * first, with low priority display sets last with newest first. + * The meaning of this flag may change to leave the image/non-image display + * set sorting alone and only affect sorting within groups, or have additional + * values for specific changes to the sort. * @returns void */ public sortDisplaySets( sortFn: (a: DisplaySet, b: DisplaySet) => number, - direction: string, + direction: 'ascending' | 'descending' = 'ascending', suppressEvent = false ): void { this.activeDisplaySets.sort(sortFn); diff --git a/platform/core/src/types/DisplaySet.ts b/platform/core/src/types/DisplaySet.ts index 9e41f2a1c..3e39e9aea 100644 --- a/platform/core/src/types/DisplaySet.ts +++ b/platform/core/src/types/DisplaySet.ts @@ -78,6 +78,12 @@ export type DisplaySet = { isLoaded?: boolean; isHydrated?: boolean; isRehydratable?: boolean; + + /** + * The name of the comparison function (for sort) to use when comparing display + * sets that are coming from same series instanceUID. + */ + compareSameSeries?: string; }; export type DisplaySetSeriesMetadataInvalidatedEvent = { diff --git a/platform/core/src/utils/createStudyBrowserTabs.ts b/platform/core/src/utils/createStudyBrowserTabs.ts index 7d2782ebf..2fa21fc4e 100644 --- a/platform/core/src/utils/createStudyBrowserTabs.ts +++ b/platform/core/src/utils/createStudyBrowserTabs.ts @@ -1,13 +1,14 @@ import { useSystem } from '../contextProviders/SystemProvider'; +import { seriesSortCriteria } from './sortStudy'; /** * Tab properties that drive which tab group is used for thumbnail display. */ export type TabProp = { - name: string, - label: string, - studies: any[], -} + name: string; + label: string; + studies: any[]; +}; /** * Collection of tab properties with studies presorted depending on tab mod. @@ -36,7 +37,7 @@ export function createStudyBrowserTabs( recentTimeframeMS = 31536000000 ): TabsProps { const { servicesManager } = useSystem(); - const { displaySetService } = servicesManager.services; + const { displaySetService, customizationService } = servicesManager.services; const shouldSortBySeriesUID = process.env.TEST_ENV === 'true'; const primaryStudies = []; @@ -48,17 +49,16 @@ export function createStudyBrowserTabs( ); // sort them by seriesInstanceUID - let sortedDisplaySets; - if (shouldSortBySeriesUID) { - sortedDisplaySets = displaySetsForStudy.sort((a, b) => { - const displaySetA = displaySetService.getDisplaySetByUID(a.displaySetInstanceUID); - const displaySetB = displaySetService.getDisplaySetByUID(b.displaySetInstanceUID); + const sortCriteria = shouldSortBySeriesUID + ? seriesSortCriteria.compareSeriesUID + : (customizationService.getCustomization('sortingCriteria') as (a, b) => number); + const sortedDisplaySets = displaySetsForStudy.sort((a, b) => { + const displaySetA = displaySetService.getDisplaySetByUID(a.displaySetInstanceUID); + const displaySetB = displaySetService.getDisplaySetByUID(b.displaySetInstanceUID); + return sortCriteria(displaySetA, displaySetB); + }); - return displaySetA.SeriesInstanceUID.localeCompare(displaySetB.SeriesInstanceUID); - }); - } else { - sortedDisplaySets = displaySetsForStudy; - } + // return displaySetA.SeriesInstanceUID.localeCompare(displaySetB.SeriesInstanceUID); const tabStudy = Object.assign({}, study, { displaySets: sortedDisplaySets, diff --git a/platform/core/src/utils/sortStudy.test.js b/platform/core/src/utils/sortStudy.test.js new file mode 100644 index 000000000..b59934290 --- /dev/null +++ b/platform/core/src/utils/sortStudy.test.js @@ -0,0 +1,44 @@ +import { compareSeriesUID, addSameSeriesCompare, compare } from './sortStudy'; + +addSameSeriesCompare('default', (a,b) => compare(a.default,b.default), 5); +const altCompare = 'altCompare' +addSameSeriesCompare(altCompare, (a,b) => compare(a.altCompare,b.altCompare), 3); + +const ds1 = { + SeriesInstanceUID: '1', + default: 'ds1', +} + +const ds2 = { + ...ds1, + default: 'ds2', +} + +const ds3 = { + ...ds2, + altCompare: 3, + compareSameSeries: altCompare, +} + +const ds4 = { + ...ds1, + altCompare:4, + compareSameSeries: altCompare, +} + +const ds5 = { + ...ds1, + SeriesInstanceUID: '3', +} + +describe('sortStudy', () => { + test('compareSameSeries',() => { + const initial = [ds5, ds4,ds3,ds2,ds1]; + initial.sort(compareSeriesUID); + expect(initial[0]).toBe(ds1); + expect(initial[1]).toBe(ds2); + expect(initial[2]).toBe(ds3); + expect(initial[3]).toBe(ds4); + expect(initial[4]).toBe(ds5); + }) +}) diff --git a/platform/core/src/utils/sortStudy.ts b/platform/core/src/utils/sortStudy.ts index 404a4853d..f3c32694b 100644 --- a/platform/core/src/utils/sortStudy.ts +++ b/platform/core/src/utils/sortStudy.ts @@ -3,19 +3,77 @@ import isLowPriorityModality from './isLowPriorityModality'; import calculateScanAxisNormal from './calculateScanAxisNormal'; import areAllImageOrientationsEqual from './areAllImageOrientationsEqual'; -const compareSeriesDateTime = (a, b) => { - const seriesDateA = Date.parse(`${a.seriesDate ?? a.SeriesDate} ${a.seriesTime ?? a.SeriesTime}`); - const seriesDateB = Date.parse(`${b.seriesDate ?? b.SeriesDate} ${b.seriesTime ?? b.SeriesTime}`); - return seriesDateA - seriesDateB; +export const compare = (a, b) => { + if (a == b) return 0; + if (!a && b) return -1; + if (!b && a) return 1; + if (a < b) return -1; + return 1; }; -const defaultSeriesSort = (a, b) => { +type CompareSameSeries = { + priority: number; + compare: (a, b) => number; +}; + +const mapCompareSameSeries = new Map(); + +/** + * Adds a comparison for same series display sets. + * Supply null for compareF to delete the function. + */ +export function addSameSeriesCompare(name: string, compareF: (a, b) => number, priority: number) { + if (!compareF) { + mapCompareSameSeries.delete(name); + } else { + mapCompareSameSeries.set(name, { compare: compareF, priority }); + } +} + +/** + * When the "series" sort is used on display sets, it is possible to get the + * same series twice. This method compares two display sets from the same series + * + * If both display sets have the same compareSameSeries name, then the + * function registered for that name will be used. + * + * If they differ, then the priority between the two functions will be used. + * + * Otherwise, the instance compare will be used on the default instance. + * + * This provides a configurable well defined sorting order. + */ +export const compareSameSeriesDisplaySet = (a, b) => { + const { compareSameSeries: compareAName = 'default' } = a; + const { compareSameSeries: compareBName = 'default' } = b; + const compareA = mapCompareSameSeries.get(compareAName); + const compareB = mapCompareSameSeries.get(compareBName); + if (compareA && compareB) { + const compareValue = + compareA === compareB + ? compareA.compare(a, b) + : compare(compareA.priority, compareB.priority); + if (!compareValue) { + return compareValue; + } + } + return sortByInstanceNumber(a.instance, b.instance); +}; + +export const compareSeriesUID = (a, b) => + compare(a.SeriesInstanceUID, b.SeriesInstanceUID) || compareSameSeriesDisplaySet(a, b); + +export const compareSeriesDateTime = (a, b) => { + // Natural order of string is good enough here + const seriesDateA = `${a.seriesDate ?? a.SeriesDate} ${a.seriesTime ?? a.SeriesTime}`; + const seriesDateB = `${b.seriesDate ?? b.SeriesDate} ${b.seriesTime ?? b.SeriesTime}`; + return compare(seriesDateA, seriesDateB) || compareSeriesUID(a, b); +}; + +export const defaultSeriesSort = (a, b) => { const seriesNumberA = a.SeriesNumber ?? a.seriesNumber; const seriesNumberB = b.SeriesNumber ?? b.seriesNumber; - if (seriesNumberA === seriesNumberB) { - return compareSeriesDateTime(a, b); - } - return seriesNumberA - seriesNumberB; + return compare(seriesNumberA, seriesNumberB) || compareSeriesDateTime(a, b); }; /** @@ -24,14 +82,14 @@ const defaultSeriesSort = (a, b) => { * @param {Object} firstSeries * @param {Object} secondSeries */ -function seriesInfoSortingCriteria(firstSeries, secondSeries) { +export function seriesInfoSortingCriteria(firstSeries, secondSeries) { const aLowPriority = isLowPriorityModality(firstSeries.Modality ?? firstSeries.modality); const bLowPriority = isLowPriorityModality(secondSeries.Modality ?? secondSeries.modality); if (aLowPriority) { // Use the reverse sort order for low priority modalities so that the // most recent one comes up first as usually that is the one of interest. - return bLowPriority ? defaultSeriesSort(secondSeries, firstSeries) : 1; + return bLowPriority ? compareSeriesDateTime(secondSeries, firstSeries) : 1; } else if (bLowPriority) { return -1; } @@ -39,44 +97,50 @@ function seriesInfoSortingCriteria(firstSeries, secondSeries) { return defaultSeriesSort(firstSeries, secondSeries); } -const seriesSortCriteria = { +export const seriesSortCriteria = { default: seriesInfoSortingCriteria, seriesInfoSortingCriteria, + compareSameSeries: compareSameSeriesDisplaySet, + compareSeriesDateTime, + compareSeriesUID, }; -const sortByInstanceNumber = (a, b) => { - // Sort by InstanceNumber (0020,0013) +/** + * Compares two instances first by instance number, and then by + * sop and frame numbers. + * Handles undefined values for use with display set comparison. + */ +export const sortByInstanceNumber = (a, b) => { + if (!a || !b) { + return (!a && !b && 0) || (!a && -1) || 1; + } const aInstance = parseInt(a.InstanceNumber) || 0; const bInstance = parseInt(b.InstanceNumber) || 0; if (aInstance !== bInstance) { return (parseInt(a.InstanceNumber) || 0) - (parseInt(b.InstanceNumber) || 0); } - // Fallback rule to enable consistent sorting - if (a.SOPInstanceUID === b.SOPInstanceUID) { - return 0; - } - return a.SOPInstanceUID < b.SOPInstanceUID ? -1 : 1; + return compare(a.SOPInstanceUID, b.SOPInstanceUID) || compare(a.frameNumber, b.frameNumber); }; -const instancesSortCriteria = { +export const instancesSortCriteria = { default: sortByInstanceNumber, sortByInstanceNumber, }; -const sortingCriteria = { +export const sortingCriteria = { seriesSortCriteria, instancesSortCriteria, }; /** - * Sorts given series (given param is modified) + * Sorts given series or display sets * The default criteria is based on series number in ascending order. * - * @param {Array} series List of series - * @param {function} seriesSortingCriteria method for sorting - * @returns {Array} sorted series object + * @param series - List of series (modified in place) + * @param seriesSortingCriteria - method for sorting + * @returns sorted series object */ -const sortStudySeries = ( +export const sortStudySeries = ( series, seriesSortingCriteria = seriesSortCriteria.default, sortFunction = null @@ -96,7 +160,7 @@ const sortStudySeries = ( * @param {function} instancesSortingCriteria method for sorting * @returns {Array} sorted instancesList object */ -const sortStudyInstances = ( +export const sortStudyInstances = ( instancesList, instancesSortingCriteria = instancesSortCriteria.default ) => { @@ -113,7 +177,7 @@ const sortStudyInstances = ( * @param {function} [instancesSortingCriteria = instancesSortCriteria.default] method for sorting instances * @returns {Object} sorted study object */ -export default function sortStudy( +export function sortStudy( study, deepSort = true, seriesSortingCriteria = seriesSortCriteria.default, @@ -134,7 +198,7 @@ export default function sortStudy( return study; } -function isValidForPositionSort(images): boolean { +export function isValidForPositionSort(images): boolean { if (images.length <= 1) { return false; // No need to sort if there's only one image } @@ -161,7 +225,7 @@ function isValidForPositionSort(images): boolean { * * @returns images - reference to images after sorting */ -const sortImagesByPatientPosition = images => { +export const sortImagesByPatientPosition = images => { const referenceImagePositionPatient = images[0].ImagePositionPatient; const imageOrientationPatient = images[0].ImageOrientationPatient; @@ -188,13 +252,4 @@ const sortImagesByPatientPosition = images => { return images; }; -export { - sortStudy, - sortStudySeries, - sortStudyInstances, - sortingCriteria, - seriesSortCriteria, - instancesSortCriteria, - isValidForPositionSort, - sortImagesByPatientPosition, -}; +export default sortStudy; diff --git a/platform/docs/docs/development/notes-requirements.md b/platform/docs/docs/development/notes-requirements.md new file mode 100644 index 000000000..ee32a2329 --- /dev/null +++ b/platform/docs/docs/development/notes-requirements.md @@ -0,0 +1,68 @@ + +--- +sidebar_position: 14 +sidebar_label: Notes and Requirements +title: Notes and Requirements for general OHIF behaviour +summary: Specifies some of the expected behavior of OHIF generally +--- + +# Notes and Requirements + +This document just lists general notes and requirements for how OHIF behaves. +The plan is to break this document down into a new sub-category once there +are sufficient notes/requirements. + + +## Series and Display Set Sorting `sortStudy.ts` + +Often a user will want to see a sorted list of series, or more generally +a sorted list of display sets. Series are the original data and can be split +up into several display sets, although they are the same general sort of concept + +For example, an MR series might contain both T1 and T2 echos, and the T2 echo +should occur after the T1. Or, a single series might contain 4 mammography views: +`LCC`, `RCC`, `LMLO`, `RMLO` with all the `CC` views shown first, and within +that all the left views first for a given sub-type of CC view. + +To allow controlling that, the `sortStudy` can register sort functions +that user used when two display sets come from the same series. Between +those display sets, the registration also registers a default ordering +for that compare function. Thus, the registration might look like: + +```javascript + addSameSeriesCompare('mammographyCompare', mammographyCompare, 5) + addSameSeriesCompare('mrT1T2Compare', mrT1T2Compare, 7); +``` + +Then, the display set for mr and mammography need to set the field `compareSameSeries` +to the value `mammographyCompare`. + +```javascript + makeDisplaySet + ... + displaySet = { + ..., + compareSameSeries: 'mammographyCompare', +``` + +### Specifying Sort Order from Series Split + +The series split rules (`getSopClassHandlerModule`) can specify a custom order +of display sets for the same series by adding a `sortVector` +to the display set created. Display sets which match on series instance uid +are then compared using the sort vector. The first element is the general sort +order for this type of value among all other sort types, and must be numeric. +The remainder of the values in the vector should be consistent for all +sort vectors whose first value is the same value. + +For example, the mammography sort vector might have a primary value of '25', +and then use the next three values for `view type`, `sub type` and `side`. +It might also be true that "both" side views sort before everything and would be assigned +a value less than `25` here. + +``` + // LCC view + [25, 'CC', 'L', 'XO'] + // BCC view + [24, 'CC', 'B'] +```