Skip to content
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(StackSync): Miscellaneous fixes for stack image sync #3663

Merged
merged 31 commits into from
Oct 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
71b4827
fix: Miscellaneous fixes for stack image sync
wayfarer3130 Sep 19, 2023
c7ae0d5
PR comments
wayfarer3130 Sep 21, 2023
270c93f
Reducing number of changed lines
wayfarer3130 Sep 21, 2023
78022dd
Reducing number of changes
wayfarer3130 Sep 21, 2023
ff63b93
Merge remote-tracking branch 'ohif/master' into fix/stack-image-sync-for
wayfarer3130 Sep 21, 2023
479ca55
PR requested changes, plus some build warning removals
wayfarer3130 Sep 22, 2023
091034c
PR review comments
wayfarer3130 Sep 22, 2023
466545c
PR review comments
wayfarer3130 Sep 22, 2023
355670c
Merge remote-tracking branch 'ohif/master' into fix/stack-image-sync-for
wayfarer3130 Sep 22, 2023
c50a9c1
Initial changes for event naming
wayfarer3130 Sep 25, 2023
96edc5a
Merge remote-tracking branch 'ohif/master' into fix/stack-image-sync-for
wayfarer3130 Sep 26, 2023
c545b89
PR requested fixes
wayfarer3130 Sep 26, 2023
62bb7bf
Merge branch 'master' into fix/stack-image-sync-for
wayfarer3130 Sep 26, 2023
e1869f9
Add tracking to show performance difference
wayfarer3130 Sep 26, 2023
0d10b6d
Merge branch 'master' of github.com:OHIF/Viewers into pr/fix/stack-im…
sedghi Sep 26, 2023
2dce538
Fix a couple of minor issues
wayfarer3130 Sep 27, 2023
94a0887
Merge remote-tracking branch 'ohif/master' into fix/stack-image-sync-for
wayfarer3130 Sep 27, 2023
7666b93
Fix a couple of dependencies on newer CS3D modules
wayfarer3130 Sep 27, 2023
32ddcc2
Remove changes other than stack sync
wayfarer3130 Sep 27, 2023
c42aefa
Minor spelling issues
wayfarer3130 Sep 27, 2023
f4a5f01
Removing a couple more unused changes
wayfarer3130 Sep 27, 2023
6a0470a
Merge branch 'master' of github.com:OHIF/Viewers into pr/fix/stack-im…
sedghi Sep 29, 2023
2e8e899
Merge branch 'master' into fix/stack-image-sync-for
wayfarer3130 Sep 29, 2023
480a147
Merge branch 'fix/stack-image-sync-for' of github.com:OHIF/Viewers in…
sedghi Sep 29, 2023
15cc74e
Added a bit of documentation
wayfarer3130 Sep 29, 2023
29f799c
upgrade cs3d
sedghi Oct 3, 2023
1948368
Merge branch 'fix/stack-image-sync-for' of github.com:OHIF/Viewers in…
sedghi Oct 3, 2023
46dc64c
Merge branch 'master' of github.com:OHIF/Viewers into pr/fix/stack-im…
sedghi Oct 3, 2023
7ff1bba
putting back stack prefetch listener for now
sedghi Oct 3, 2023
78caf6b
fix displayset message bug
sedghi Oct 3, 2023
c88dc96
Merge branch 'master' of github.com:OHIF/Viewers into pr/fix/stack-im…
sedghi Oct 3, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 21 additions & 15 deletions .webpack/webpack.base.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,26 @@ const COMMIT_HASH = fs.readFileSync(path.join(__dirname, '../commit.txt'), 'utf8
//
dotenv.config();

const defineValues = {
wayfarer3130 marked this conversation as resolved.
Show resolved Hide resolved
/* Application */
'process.env.NODE_ENV': JSON.stringify(process.env.NODE_ENV),
'process.env.DEBUG': JSON.stringify(process.env.DEBUG),
'process.env.PUBLIC_URL': JSON.stringify(process.env.PUBLIC_URL || '/'),
'process.env.BUILD_NUM': JSON.stringify(BUILD_NUM),
'process.env.VERSION_NUMBER': JSON.stringify(VERSION_NUMBER),
'process.env.COMMIT_HASH': JSON.stringify(COMMIT_HASH),
/* i18n */
'process.env.USE_LOCIZE': JSON.stringify(process.env.USE_LOCIZE || ''),
'process.env.LOCIZE_PROJECTID': JSON.stringify(process.env.LOCIZE_PROJECTID || ''),
'process.env.LOCIZE_API_KEY': JSON.stringify(process.env.LOCIZE_API_KEY || ''),
'process.env.REACT_APP_I18N_DEBUG': JSON.stringify(process.env.REACT_APP_I18N_DEBUG || ''),
};

// Only redefine updated values. This avoids warning messages in the logs
if (!process.env.APP_CONFIG) {
defineValues['process.env.APP_CONFIG'] = '';
}

module.exports = (env, argv, { SRC_DIR, ENTRY }) => {
if (!process.env.NODE_ENV) {
throw new Error('process.env.NODE_ENV not set');
Expand Down Expand Up @@ -133,21 +153,7 @@ module.exports = (env, argv, { SRC_DIR, ENTRY }) => {
},
},
plugins: [
new webpack.DefinePlugin({
/* Application */
'process.env.NODE_ENV': JSON.stringify(process.env.NODE_ENV),
'process.env.DEBUG': JSON.stringify(process.env.DEBUG),
'process.env.APP_CONFIG': JSON.stringify(process.env.APP_CONFIG || ''),
'process.env.PUBLIC_URL': JSON.stringify(process.env.PUBLIC_URL || '/'),
'process.env.BUILD_NUM': JSON.stringify(BUILD_NUM),
'process.env.VERSION_NUMBER': JSON.stringify(VERSION_NUMBER),
'process.env.COMMIT_HASH': JSON.stringify(COMMIT_HASH),
/* i18n */
'process.env.USE_LOCIZE': JSON.stringify(process.env.USE_LOCIZE || ''),
'process.env.LOCIZE_PROJECTID': JSON.stringify(process.env.LOCIZE_PROJECTID || ''),
'process.env.LOCIZE_API_KEY': JSON.stringify(process.env.LOCIZE_API_KEY || ''),
'process.env.REACT_APP_I18N_DEBUG': JSON.stringify(process.env.REACT_APP_I18N_DEBUG || ''),
}),
new webpack.DefinePlugin(defineValues),
new webpack.ProvidePlugin({
Buffer: ['buffer', 'Buffer'],
}),
Expand Down
20 changes: 13 additions & 7 deletions extensions/cornerstone/src/commandsModule.ts
wayfarer3130 marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,11 @@ function commandsModule({
toolbarServiceRecordInteraction: props => {
toolbarService.recordInteraction(props);
},
// Enable or disable a toggleable command, without calling the activation
// Used to setup already active tools from hanging protocols
setToolbarToggled: props => {
toolbarService.setToggled(props.toolId, props.isActive ?? true);
},
setToolActive: ({ toolName, toolGroupId = null, toggledState }) => {
if (toolName === 'Crosshairs') {
const activeViewportToolGroup = toolGroupService.getToolGroup(null);
Expand Down Expand Up @@ -549,16 +554,16 @@ function commandsModule({

toggleStackImageSync: ({ toggledState }) => {
toggleStackImageSync({
getEnabledElement,
servicesManager,
toggledState,
});
},
setSourceViewportForReferenceLinesTool: ({ toggledState }) => {
const { activeViewportId } = viewportGridService.getState();
const viewportInfo = cornerstoneViewportService.getViewportInfo(activeViewportId);
setSourceViewportForReferenceLinesTool: ({ toggledState, viewportId }) => {
if (!viewportId) {
const { activeViewportId } = viewportGridService.getState();
viewportId = activeViewportId;
}

const viewportId = viewportInfo.getViewportId();
const toolGroup = toolGroupService.getToolGroupForViewport(viewportId);

toolGroup.setToolConfiguration(
Expand Down Expand Up @@ -699,8 +704,9 @@ function commandsModule({
},
storePresentation: {
commandFn: actions.storePresentation,
storeContexts: [],
options: {},
},
setToolbarToggled: {
commandFn: actions.setToolbarToggled,
},
};

Expand Down
78 changes: 47 additions & 31 deletions extensions/cornerstone/src/init.tsx
wayfarer3130 marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {
utilities as csUtilities,
Enums as csEnums,
} from '@cornerstonejs/core';
import { Enums, utilities, ReferenceLinesTool } from '@cornerstonejs/tools';
import { Enums } from '@cornerstonejs/tools';
import { cornerstoneStreamingImageVolumeLoader } from '@cornerstonejs/streaming-image-volume-loader';

import initWADOImageLoader from './initWADOImageLoader';
Expand Down Expand Up @@ -93,6 +93,7 @@ export default async function init({
cornerstoneViewportService,
hangingProtocolService,
toolGroupService,
toolbarService,
viewportGridService,
stateSyncService,
} = servicesManager.services as CornerstoneServices;
Expand Down Expand Up @@ -208,9 +209,45 @@ export default async function init({
commandsManager,
});

const newStackCallback = evt => {
/**
* When a viewport gets a new display set, this call will go through all the
* active tools in the toolbar, and call any commands registered in the
* toolbar service with a callback to re-enable on displaying the viewport.
*/
const toolbarEventListener = evt => {
const { element } = evt.detail;
utilities.stackPrefetch.enable(element);
const activeTools = toolbarService.getActiveTools();

activeTools.forEach(tool => {
const toolData = toolbarService.getNestedButton(tool);
const commands = toolData?.listeners?.[evt.type];
commandsManager.run(commands, { element, evt });
});
};

/** Listens for active viewport events and fires the toolbar listeners */
const activeViewportEventListener = evt => {
const { viewportId } = evt;
const toolGroup = toolGroupService.getToolGroupForViewport(viewportId);

const activeTools = toolbarService.getActiveTools();

activeTools.forEach(tool => {
if (!toolGroup?._toolInstances?.[tool]) {
return;
}

// check if tool is active on the new viewport
const toolEnabled = toolGroup._toolInstances[tool].mode === Enums.ToolModes.Enabled;

if (!toolEnabled) {
return;
}

const button = toolbarService.getNestedButton(tool);
const commands = button?.listeners?.[evt.type];
commandsManager.run(commands, { viewportId, evt });
});
};

const resetCrosshairs = evt => {
Expand All @@ -237,11 +274,16 @@ export default async function init({
}
};

eventTarget.addEventListener(EVENTS.STACK_VIEWPORT_NEW_STACK, evt => {
const { element } = evt.detail;
cornerstoneTools.utilities.stackContextPrefetch.enable(element);
});

function elementEnabledHandler(evt) {
const { element } = evt.detail;
element.addEventListener(EVENTS.CAMERA_RESET, resetCrosshairs);

eventTarget.addEventListener(EVENTS.STACK_VIEWPORT_NEW_STACK, newStackCallback);
eventTarget.addEventListener(EVENTS.STACK_VIEWPORT_NEW_STACK, toolbarEventListener);
}

function elementDisabledHandler(evt) {
Expand All @@ -262,33 +304,7 @@ export default async function init({

viewportGridService.subscribe(
viewportGridService.EVENTS.ACTIVE_VIEWPORT_ID_CHANGED,
({ viewportId }) => {
const toolGroup = toolGroupService.getToolGroupForViewport(viewportId);

if (!toolGroup || !toolGroup._toolInstances?.['ReferenceLines']) {
return;
}

// check if reference lines are active
const referenceLinesEnabled =
toolGroup._toolInstances['ReferenceLines'].mode === Enums.ToolModes.Enabled;

if (!referenceLinesEnabled) {
return;
}

toolGroup.setToolConfiguration(
ReferenceLinesTool.toolName,
{
sourceViewportId: viewportId,
},
true // overwrite
);

// make sure to set it to enabled again since we want to recalculate
// the source-target lines
toolGroup.setToolEnabled(ReferenceLinesTool.toolName);
}
activeViewportEventListener
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,12 +142,16 @@ const CornerstoneViewportDownloadForm = ({
const properties = viewport.getProperties();

downloadViewport.setStack([imageId]).then(() => {
downloadViewport.setProperties(properties);

const newWidth = Math.min(width || image.width, MAX_TEXTURE_SIZE);
const newHeight = Math.min(height || image.height, MAX_TEXTURE_SIZE);

resolve({ width: newWidth, height: newHeight });
try {
wayfarer3130 marked this conversation as resolved.
Show resolved Hide resolved
downloadViewport.setProperties(properties);
const newWidth = Math.min(width || image.width, MAX_TEXTURE_SIZE);
const newHeight = Math.min(height || image.height, MAX_TEXTURE_SIZE);

resolve({ width: newWidth, height: newHeight });
} catch (e) {
// Happens on clicking the cancel button
console.warn('Unable to set properties', e);
}
});
} else if (downloadViewport instanceof VolumeViewport) {
const actors = viewport.getActors();
Expand Down
136 changes: 63 additions & 73 deletions extensions/cornerstone/src/utils/stackSync/toggleStackImageSync.ts
Original file line number Diff line number Diff line change
@@ -1,90 +1,80 @@
import calculateViewportRegistrations from './calculateViewportRegistrations';
const STACK_SYNC_NAME = 'stackImageSync';

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

now that you removed calculateViewportRegistrations who is handling that?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Handled by the CS3D tool library - that way there is exactly one spot to do the calculations.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, I haven't removed calculateViewportRegistrations because it is needed for another use case which we don't yet support, allowing one to adjust hte registrations.

// [ {
// synchronizerId: string,
// viewports: [ { viewportId: string, renderingEngineId: string, index: number } , ...]
// ]}
let STACK_IMAGE_SYNC_GROUPS_INFO = [];
export default function toggleStackImageSync({
toggledState,
servicesManager,
viewports: providedViewports,
}) {
if (!toggledState) {
return disableSync(STACK_SYNC_NAME, servicesManager);
}

export default function toggleStackImageSync({ toggledState, servicesManager, getEnabledElement }) {
const { syncGroupService, viewportGridService, displaySetService, cornerstoneViewportService } =
servicesManager.services;

if (!toggledState) {
STACK_IMAGE_SYNC_GROUPS_INFO.forEach(syncGroupInfo => {
const { viewports, synchronizerId } = syncGroupInfo;
const viewports = providedViewports || getReconstructableStackViewports(viewportGridService, displaySetService);

viewports.forEach(({ viewportId, renderingEngineId }) => {
syncGroupService.removeViewportFromSyncGroup(viewportId, renderingEngineId, synchronizerId);
});
// create synchronization group and add the viewports to it.
viewports.forEach(gridViewport => {
const { viewportId } = gridViewport.viewportOptions;
const viewport = cornerstoneViewportService.getCornerstoneViewport(viewportId);
if (!viewport) {
return;
}
syncGroupService.addViewportToSyncGroup(viewportId, viewport.getRenderingEngine().id, {
type: 'stackimage',
id: STACK_SYNC_NAME,
source: true,
target: true,
});
});
}

return;
}

STACK_IMAGE_SYNC_GROUPS_INFO = [];
function disableSync(syncName, servicesManager) {
const { syncGroupService, viewportGridService, displaySetService, cornerstoneViewportService } =
servicesManager.services;
wayfarer3130 marked this conversation as resolved.
Show resolved Hide resolved
const viewports = getReconstructableStackViewports(viewportGridService, displaySetService);
viewports.forEach(gridViewport => {
const { viewportId } = gridViewport.viewportOptions;
const viewport = cornerstoneViewportService.getCornerstoneViewport(viewportId);
if (!viewport) {
return;
}
syncGroupService.removeViewportFromSyncGroup(
viewport.id,
viewport.getRenderingEngine().id,
syncName
);
});
};

// create synchronization groups and add viewports
const { viewports } = viewportGridService.getState();
/**
* Gets the consistent spacing stack viewport types, which are the ones which
* can be navigated using the stack image sync right now.
*/
function getReconstructableStackViewports(viewportGridService, displaySetService) {
let { viewports } = viewportGridService.getState();

viewports = [...viewports.values()];
// filter empty viewports
const viewportsArray = Array.from(viewports.values())
.filter(viewport => viewport.displaySetInstanceUIDs?.length)
// filter reconstructable viewports
.filter(viewport => {
const { displaySetInstanceUIDs } = viewport;
viewports = viewports.filter(
viewport => viewport.displaySetInstanceUIDs && viewport.displaySetInstanceUIDs.length
);

for (const displaySetInstanceUID of displaySetInstanceUIDs) {
const displaySet = displaySetService.getDisplaySetByUID(displaySetInstanceUID);

return !!displaySet?.isReconstructable;
}
});
// filter reconstructable viewports
viewports = viewports.filter(viewport => {
const { displaySetInstanceUIDs } = viewport;

const viewportsByOrientation = viewportsArray.reduce((acc, viewport) => {
const { viewportId, viewportType } = viewport.viewportOptions;
for (const displaySetInstanceUID of displaySetInstanceUIDs) {
const displaySet = displaySetService.getDisplaySetByUID(displaySetInstanceUID);

if (viewportType !== 'stack') {
console.warn('Viewport is not a stack, cannot sync images yet');
return acc;
}

const { element } = cornerstoneViewportService.getViewportInfo(viewportId);
const { viewport: csViewport, renderingEngineId } = getEnabledElement(element);
const { viewPlaneNormal } = csViewport.getCamera();

// Should we round here? I guess so, but not sure how much precision we need
const orientation = viewPlaneNormal.map(v => Math.round(v)).join(',');
// TODO - add a better test than isReconstructable
if (displaySet && displaySet.isReconstructable) {
return true;
}

if (!acc[orientation]) {
acc[orientation] = [];
return false;
}

acc[orientation].push({ viewportId, renderingEngineId });

return acc;
}, {});

// create synchronizer for each group
Object.values(viewportsByOrientation).map(viewports => {
let synchronizerId = viewports.map(({ viewportId }) => viewportId).join(',');

synchronizerId = `imageSync_${synchronizerId}`;

calculateViewportRegistrations(viewports);

viewports.forEach(({ viewportId, renderingEngineId }) => {
syncGroupService.addViewportToSyncGroup(viewportId, renderingEngineId, {
type: 'stackimage',
id: synchronizerId,
source: true,
target: true,
});
});

STACK_IMAGE_SYNC_GROUPS_INFO.push({
synchronizerId,
viewports,
});
});
}
return viewports;
};
2 changes: 1 addition & 1 deletion extensions/default/src/commandsModule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ const commandsModule = ({
(!protocolId || protocolId === protocol.id) &&
(stageIndex === undefined || stageIndex === toggleStageIndex) &&
(!stageId || stageId === stage.id);
toolbarService.setActive(button.id, isActive);
toolbarService.setToggled(button.id, isActive);
};
Object.values(toolbarService.getButtons()).forEach(enableListener);
},
Expand Down
Loading
Loading