-
Notifications
You must be signed in to change notification settings - Fork 453
fix(Annotation Tools): Fix active volume to show correct cachedStats (especially on fusion viewport) #2418
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(Annotation Tools): Fix active volume to show correct cachedStats (especially on fusion viewport) #2418
Changes from 3 commits
54f2d7e
8a9b07a
dd27a70
22244b6
55050e8
a6e794f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,7 +2,12 @@ import { utilities } from '@cornerstonejs/core'; | |
| import type { Types } from '@cornerstonejs/core'; | ||
| import ToolModes from '../../enums/ToolModes'; | ||
| import type StrategyCallbacks from '../../enums/StrategyCallbacks'; | ||
| import type { InteractionTypes, ToolProps, PublicToolProps } from '../../types'; | ||
| import type { | ||
| InteractionTypes, | ||
| ToolProps, | ||
| PublicToolProps, | ||
| ToolConfiguration, | ||
| } from '../../types'; | ||
|
|
||
| const { DefaultHistoryMemo } = utilities.HistoryMemo; | ||
|
|
||
|
|
@@ -15,8 +20,17 @@ abstract class BaseTool { | |
| static toolName; | ||
| /** Supported Interaction Types - currently only Mouse */ | ||
| public supportedInteractionTypes: InteractionTypes[]; | ||
| /** | ||
| * The configuration for this tool. | ||
| * IBaseTool contains some default configuration values, and you can use | ||
| * configurationTyped to get the typed version of this. | ||
| */ | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| public configuration: Record<string, any>; | ||
| public get configurationTyped() { | ||
| return <ToolConfiguration>this.configuration; | ||
| } | ||
|
|
||
| /** ToolGroup ID the tool instance belongs to */ | ||
| public toolGroupId: string; | ||
| /** Tool Mode - Active/Passive/Enabled/Disabled/ */ | ||
|
|
@@ -76,6 +90,15 @@ abstract class BaseTool { | |
| return utilities.deepMerge(defaultProps, additionalProps); | ||
| } | ||
|
|
||
| /** | ||
| * A function generator to test if the target id is the desired one. | ||
| * Used for deciding which set of cached stats is appropriate to display | ||
| * for a given viewport. | ||
| */ | ||
| public static isSpecifiedTargetId(desiredTargetId: string) { | ||
|
||
| return (_viewport, { targetId }) => targetId.includes(desiredTargetId); | ||
|
||
| } | ||
|
|
||
| /** | ||
| * Newer method for getting the tool name as a property | ||
| */ | ||
|
|
@@ -228,18 +251,36 @@ abstract class BaseTool { | |
| /** | ||
| * Get the target Id for the viewport which will be used to store the cached | ||
| * statistics scoped to that target in the annotations. | ||
| * For StackViewport, targetId is the viewportId, but for the volume viewport, | ||
| * the targetId will be grabbed from the volumeId if particularly specified | ||
| * in the tool configuration, or if not, the first actorUID in the viewport. | ||
| * For StackViewport, targetId is usually derived from the imageId. | ||
| * For VolumeViewport, it's derived from the volumeId. | ||
| * This method allows prioritizing a specific volumeId from the tool's | ||
| * configuration if available in the cachedStats. | ||
| * | ||
| * @param viewport - viewport to get the targetId for | ||
| * @param data - Optional: The annotation's data object, containing cachedStats. | ||
| * @returns targetId | ||
| */ | ||
| protected getTargetId(viewport: Types.IViewport): string | undefined { | ||
| const targetId = viewport.getViewReferenceId?.(); | ||
| if (targetId) { | ||
| return targetId; | ||
| protected getTargetId( | ||
| viewport: Types.IViewport, | ||
| data?: unknown & { cachedStats?: Record<string, unknown> } | ||
| ): string | undefined { | ||
| const { isPreferredTargetId } = this.configurationTyped; // Get preferred ID from config | ||
|
|
||
| // Check if cachedStats is available and contains the preferredVolumeId | ||
| if (isPreferredTargetId && data?.cachedStats) { | ||
| for (const [targetId, cachedStat] of Object.entries(data.cachedStats)) { | ||
| if (isPreferredTargetId(viewport, { targetId, cachedStat })) { | ||
| return targetId; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // If not found or not applicable, use the viewport's default method | ||
| const defaultTargetId = viewport.getViewReferenceId?.(); | ||
| if (defaultTargetId) { | ||
| return defaultTargetId; | ||
| } | ||
|
|
||
| throw new Error( | ||
| 'getTargetId: viewport must have a getViewReferenceId method' | ||
| ); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,28 @@ | ||
| import type BaseTool from '../tools/base/BaseTool'; | ||
|
|
||
| /** | ||
| * General tool configuration. This is intended to be extended | ||
| * by various tools to add the different configuration options. | ||
| */ | ||
| export interface ToolConfiguration { | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| strategies: any; | ||
| defaultStrategy?: string; | ||
| activeStrategy?: string; | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| strategyOptions: any; | ||
|
|
||
| /** | ||
| * @returns true if the given targetId is preferred. | ||
| */ | ||
| isPreferredTargetId?: ( | ||
| viewport, | ||
| targetInfo: { | ||
| targetId: string; | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| cachedStat?: any; | ||
| } | ||
| ) => boolean; | ||
| } | ||
|
|
||
| export type IBaseTool = BaseTool; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is confusing api, what are we achieving here?
isPreferredTargetId: RectangleROITool.isSpecifiedTargetId(ptVolumeId)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is saying that the preferred target for statistics is one belonging to the specified volume id. If you have better naming on that, please let me know.