From 2a6722e6d363b8def9cb84611c205df401ae08db Mon Sep 17 00:00:00 2001 From: Bill Wallace Date: Fri, 22 Nov 2024 09:15:53 -0500 Subject: [PATCH] PR review comments --- .../src/commandsModule.ts | 88 ++++--------------- extensions/cornerstone/src/commandsModule.ts | 54 ++++++++++++ .../src/panels/PanelMeasurement.tsx | 10 --- extensions/default/src/commandsModule.ts | 38 +++++++- .../core/src/extensions/ExtensionManager.ts | 5 +- platform/core/src/utils/measurementFilters.ts | 66 ++++++++++++-- 6 files changed, 169 insertions(+), 92 deletions(-) diff --git a/extensions/cornerstone-dicom-sr/src/commandsModule.ts b/extensions/cornerstone-dicom-sr/src/commandsModule.ts index 78704fb35a..c572ca00a5 100644 --- a/extensions/cornerstone-dicom-sr/src/commandsModule.ts +++ b/extensions/cornerstone-dicom-sr/src/commandsModule.ts @@ -11,7 +11,6 @@ const { MeasurementReport } = adaptersSR.Cornerstone3D; const { log } = OHIF; /** - * * @param measurementData An array of measurements from the measurements service * that you wish to serialize. * @param additionalFindingTypes toolTypes that should be stored with labels as Findings @@ -46,52 +45,24 @@ const commandsModule = (props: withAppTypes) => { const { customizationService, measurementService, viewportGridService, uiDialogService } = servicesManager.services; const actions = { - jumpToMeasurement: ({ uid }) => { - measurementService.jumpToMeasurement(viewportGridService.getActiveViewportId(), uid); - }, - - removeMeasurement: ({ uid }) => { - measurementService.remove(uid); - }, - - renameMeasurement: ({ uid }) => { - const labelConfig = customizationService.get('measurementLabels'); - const measurement = measurementService.getMeasurement(uid); - showLabelAnnotationPopup(measurement, uiDialogService, labelConfig).then(val => { - measurementService.update( - uid, - { - ...val, - }, - true - ); - }); - }, - changeColorMeasurement: ({ uid }) => { - const { color } = measurementService.getMeasurement(uid); - const rgbaColor = { - r: color[0], - g: color[1], - b: color[2], - a: color[3] / 255.0, - }; - colorPickerDialog(uiDialogService, rgbaColor, (newRgbaColor, actionId) => { - if (actionId === 'cancel') { - return; - } - - const color = [newRgbaColor.r, newRgbaColor.g, newRgbaColor.b, newRgbaColor.a * 255.0]; - // segmentationService.setSegmentColor(viewportId, segmentationId, segmentIndex, color); - }); - }, - - toggleLockMeasurement: ({ uid }) => { - measurementService.toggleLockMeasurement(uid); - }, - - toggleVisibilityMeasurement: ({ uid }) => { - measurementService.toggleVisibilityMeasurement(uid); + // When this gets supported, it probably belongs in cornerstone, not sr + throw new Error('Unsupported operation: changeColorMeasurement'); + // const { color } = measurementService.getMeasurement(uid); + // const rgbaColor = { + // r: color[0], + // g: color[1], + // b: color[2], + // a: color[3] / 255.0, + // }; + // colorPickerDialog(uiDialogService, rgbaColor, (newRgbaColor, actionId) => { + // if (actionId === 'cancel') { + // return; + // } + + // const color = [newRgbaColor.r, newRgbaColor.g, newRgbaColor.b, newRgbaColor.a * 255.0]; + // segmentationService.setSegmentColor(viewportId, segmentationId, segmentIndex, color); + // }); }, /** @@ -111,13 +82,6 @@ const commandsModule = (props: withAppTypes) => { window.location.assign(objectUrl); }, - /** - * Clear the measurements - */ - clearMeasurements: ({ measurementFilter }) => { - measurementService.clearMeasurements(measurementFilter); - }, - /** * Download the CSV report for the measurements. */ @@ -196,27 +160,9 @@ const commandsModule = (props: withAppTypes) => { storeMeasurements: { commandFn: actions.storeMeasurements, }, - clearMeasurements: { - commandFn: actions.clearMeasurements, - }, downloadCSVMeasurementsReport: { commandFn: actions.downloadCSVMeasurementsReport, }, - jumpToMeasurement: { - commandFn: actions.jumpToMeasurement, - }, - removeMeasurement: { - commandFn: actions.removeMeasurement, - }, - renameMeasurement: { - commandFn: actions.renameMeasurement, - }, - toggleLockMeasurement: { - commandFn: actions.toggleLockMeasurement, - }, - toggleVisibilityMeasurement: { - commandFn: actions.toggleVisibilityMeasurement, - }, }; return { diff --git a/extensions/cornerstone/src/commandsModule.ts b/extensions/cornerstone/src/commandsModule.ts index 857997dee1..b5adc54f33 100644 --- a/extensions/cornerstone/src/commandsModule.ts +++ b/extensions/cornerstone/src/commandsModule.ts @@ -266,6 +266,42 @@ function commandsModule({ measurementService.update(updatedMeasurement.uid, updatedMeasurement, true); }, + jumpToMeasurement: ({ uid }) => { + measurementService.jumpToMeasurement(viewportGridService.getActiveViewportId(), uid); + }, + + removeMeasurement: ({ uid }) => { + measurementService.remove(uid); + }, + + renameMeasurement: ({ uid }) => { + const labelConfig = customizationService.get('measurementLabels'); + const measurement = measurementService.getMeasurement(uid); + showLabelAnnotationPopup(measurement, uiDialogService, labelConfig).then(val => { + measurementService.update( + uid, + { + ...val, + }, + true + ); + }); + }, + + toggleLockMeasurement: ({ uid }) => { + measurementService.toggleLockMeasurement(uid); + }, + + toggleVisibilityMeasurement: ({ uid }) => { + measurementService.toggleVisibilityMeasurement(uid); + }, + /** + * Clear the measurements + */ + clearMeasurements: ({ measurementFilter }) => { + measurementService.clearMeasurements(measurementFilter); + }, + // Retrieve value commands getActiveViewportEnabledElement: _getActiveViewportEnabledElement, @@ -1252,6 +1288,24 @@ function commandsModule({ updateMeasurement: { commandFn: actions.updateMeasurement, }, + clearMeasurements: { + commandFn: actions.clearMeasurements, + }, + jumpToMeasurement: { + commandFn: actions.jumpToMeasurement, + }, + removeMeasurement: { + commandFn: actions.removeMeasurement, + }, + renameMeasurement: { + commandFn: actions.renameMeasurement, + }, + toggleLockMeasurement: { + commandFn: actions.toggleLockMeasurement, + }, + toggleVisibilityMeasurement: { + commandFn: actions.toggleVisibilityMeasurement, + }, setViewportWindowLevel: { commandFn: actions.setViewportWindowLevel, }, diff --git a/extensions/cornerstone/src/panels/PanelMeasurement.tsx b/extensions/cornerstone/src/panels/PanelMeasurement.tsx index 6fdc98e38b..5a1d5fb021 100644 --- a/extensions/cornerstone/src/panels/PanelMeasurement.tsx +++ b/extensions/cornerstone/src/panels/PanelMeasurement.tsx @@ -122,11 +122,6 @@ export default function PanelMeasurementTable({ title="Additional Findings" {...onArgs} > - - <> -
Hello World
- - )} @@ -137,11 +132,6 @@ export default function PanelMeasurementTable({ title="Untracked Findings" {...onArgs} > - - <> - Untracked - - )} diff --git a/extensions/default/src/commandsModule.ts b/extensions/default/src/commandsModule.ts index 6e128e726e..a6875dbb43 100644 --- a/extensions/default/src/commandsModule.ts +++ b/extensions/default/src/commandsModule.ts @@ -47,6 +47,39 @@ const commandsModule = ({ const contextMenuController = new ContextMenuController(servicesManager, commandsManager); const actions = { + /** + * Runs commands specified in the options. + * This allows running multiple commands when only one is specified. It + * also allows an async instantiation of a command so that the return is + * immediate without waiting for the command to complete. + * + * @param options.options - shared options for the child command + * @param options.commands - a list of commands to run. This is an array, and can + * be either a full command, or just a name. + * @param options.async - set to true to run async + * @param options.parallel - set to false, WITH async true to run non-parallel + */ + run: options => { + const { options: sharedOptions, commands = [], parallel = true, async = false } = options; + const childOptions = { + ...options, + options: undefined, + commands: undefined, + parallel: undefined, + async: undefined, + ...sharedOptions, + }; + if (async) { + if (parallel) { + return Promise.all( + commands.map(async command => commandsManager.run(command, childOptions)) + ); + } + return (async () => await commandsManager.run(commands, childOptions))(); + } + return commandsManager.run(commands, childOptions); + }, + /** * Show the context menu. * @param options.menuId defines the menu name to lookup, from customizationService @@ -554,9 +587,8 @@ const commandsModule = ({ }; const definitions = { - showContextMenu: { - commandFn: actions.showContextMenu, - }, + run: actions.run, + showContextMenu: actions.showContextMenu, closeContextMenu: { commandFn: actions.closeContextMenu, }, diff --git a/platform/core/src/extensions/ExtensionManager.ts b/platform/core/src/extensions/ExtensionManager.ts index 6c7d7707ee..c9d1987e1b 100644 --- a/platform/core/src/extensions/ExtensionManager.ts +++ b/platform/core/src/extensions/ExtensionManager.ts @@ -596,7 +596,10 @@ export default class ExtensionManager extends PubSubService { } Object.keys(definitions).forEach(commandName => { - const commandDefinition = definitions[commandName]; + let commandDefinition = definitions[commandName]; + if (typeof commandDefinition === 'function') { + commandDefinition = { commandFn: commandDefinition }; + } const commandHasContextThatDoesNotExist = commandDefinition.context && !this._commandsManager.getContext(commandDefinition.context); diff --git a/platform/core/src/utils/measurementFilters.ts b/platform/core/src/utils/measurementFilters.ts index 6ddde11304..9f60820469 100644 --- a/platform/core/src/utils/measurementFilters.ts +++ b/platform/core/src/utils/measurementFilters.ts @@ -1,3 +1,7 @@ +/** + * Returns a filter function which filters for measurements belonging to both + * the study and series. + */ export function filterTracked(trackedStudy: string, trackedSeries) { return measurement => { const result = @@ -7,25 +11,28 @@ export function filterTracked(trackedStudy: string, trackedSeries) { }; } +/** A filter that always returns true */ export function filterAny(_measurement) { return true; } +/** A filter that excludes everything */ export function filterNone(_measurement) { return false; } -export function filterOthers(...filters) { - return item => !filters.find(filter => filter(item)); -} - +/** + * Filters the measurements which are found in any of the filters in the provided + * object. This can be used to query for any matching of a set. + * This passes the this argument to the child function(s) + */ export function filterOr(measurementFilters) { - return item => { + return function (item) { for (const filter of Object.values(measurementFilters)) { if (typeof filter !== 'function') { continue; } - if (filter(item)) { + if (filter.call(this, item)) { return true; } } @@ -33,18 +40,63 @@ export function filterOr(measurementFilters) { }; } +/** + * Filters for additional findings, that is, measurements with + * a value of type point, and having a referenced image + */ export function filterAdditionalFinding(measurementService) { const { POINT } = measurementService.VALUE_TYPES; return dm => dm.type === POINT && dm.referencedImageId; } +/** + * Returns a filter that applies the second filter unless the first filter would + * include the given measurement. + * That is, (!filterUnless) && filterThen + */ export function filterUnless(filterUnless, filterThen) { return item => (filterUnless(item) ? false : filterThen(item)); } const isString = s => typeof s === 'string' || s instanceof String; -export function filterNot(filter) { +/** + * Returns true if all the filters return true. + * Any filter can be a string name of a filter on the "this" object + * called on the final filter call. + */ +export function filterAnd(...filters) { + return function (item) { + for (const filter of filters) { + if (isString(filter)) { + if (!this[filter](item)) { + return false; + } + } else if (!filter.call(this, item)) { + return false; + } + } + return true; + }; +} + +/** + * Returns a filter that returns true if none of the filters supplied return true. + * Any filter supplied can be a name, in which case hte filter will be retrieved + * from "this" object on the call. + * + * For example, for filterNot("otherFilterName"), if that is called on + * `{ otherFilterName: filterNone }` + * then otherFilterName will be called, returning false in this case and + * filterNot will return true. + * + * + */ +export function filterNot(...filters) { + if (filters.length !== 1) { + return filterAnd.apply(null, filters.map(filterNot)); + } + const [filter] = filters; if (isString(filter)) { return function (item) { return !this[filter](item);