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 14 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
30 changes: 23 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,18 @@ 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);
},

// Enable stack prefetch on the given element
stackPrefetch: props => {
const { element, prefetchType = 'stackContextPrefetch' } = props;
cstUtils[prefetchType].enable(element);
},

setToolActive: ({ toolName, toolGroupId = null, toggledState }) => {
if (toolName === 'Crosshairs') {
const activeViewportToolGroup = toolGroupService.getToolGroup(null);
Expand Down Expand Up @@ -549,16 +561,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 +711,12 @@ function commandsModule({
},
storePresentation: {
commandFn: actions.storePresentation,
storeContexts: [],
options: {},
},
stackPrefetch: {
commandFn: actions.stackPrefetch,
},
setToolbarToggled: {
commandFn: actions.setToolbarToggled,
},
};

Expand Down
78 changes: 46 additions & 32 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 All @@ -28,6 +28,7 @@ import interleaveTopToBottom from './utils/interleaveTopToBottom';
import initContextMenu from './initContextMenu';
import initDoubleClick from './initDoubleClick';
import { CornerstoneServices } from './types';
import initViewTiming from './utils/initViewTiming';

// TODO: Cypress tests are currently grabbing this from the window?
window.cornerstone = cornerstone;
Expand All @@ -42,7 +43,6 @@ export default async function init({
configuration,
appConfig,
}: Types.Extensions.ExtensionParams): Promise<void> {

await cs3DInit({
rendering: {
preferSizeOverAccuracy: Boolean(appConfig.use16BitDataType),
Expand Down Expand Up @@ -91,6 +91,7 @@ export default async function init({
cornerstoneViewportService,
hangingProtocolService,
toolGroupService,
toolbarService,
viewportGridService,
stateSyncService,
} = servicesManager.services as CornerstoneServices;
Expand Down Expand Up @@ -201,9 +202,46 @@ 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();

// TODO - get the event name from evt
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 Down Expand Up @@ -234,7 +272,9 @@ export default async function init({
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);

initViewTiming({ element, eventTarget });
}

function elementDisabledHandler(evt) {
Expand All @@ -255,33 +295,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
42 changes: 42 additions & 0 deletions extensions/cornerstone/src/utils/initViewTiming.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
import { log, Types } from '@ohif/core';
import { EVENTS } from '@cornerstonejs/core';

const { TimingEnum } = Types;

const IMAGE_TIMING_KEYS = [
TimingEnum.DISPLAY_SETS_TO_ALL_IMAGES,
TimingEnum.DISPLAY_SETS_TO_FIRST_IMAGE,
TimingEnum.STUDY_TO_FIRST_IMAGE,
];

const imageTiming = {
viewportsWaiting: 0,
};

/**
* Defines the initial view timing reporting.
* This allows knowing how many viewports are waiting for initial views and
* when the IMAGE_RENDERED gets sent out.
* The first image rendered will fire the FIRST_IMAGE timeEnd logs, while
* the last of the enabled viewport will fire the ALL_IMAGES timeEnd logs.
*
*/

export default function initViewTiming({ element }) {
if (!IMAGE_TIMING_KEYS.find(key => log.timingKeys[key])) {
return;
}
imageTiming.viewportsWaiting += 1;
element.addEventListener(EVENTS.IMAGE_RENDERED, imageRenderedListener);
}

function imageRenderedListener(evt) {
log.timeEnd(TimingEnum.DISPLAY_SETS_TO_FIRST_IMAGE);
log.timeEnd(TimingEnum.STUDY_TO_FIRST_IMAGE);
log.timeEnd(TimingEnum.SCRIPT_TO_VIEW);
imageTiming.viewportsWaiting -= 1;
evt.detail.element.removeEventListener(EVENTS.IMAGE_RENDERED, imageRenderedListener);
wayfarer3130 marked this conversation as resolved.
Show resolved Hide resolved
if (!imageTiming.viewportsWaiting) {
log.timeEnd(TimingEnum.DISPLAY_SETS_TO_ALL_IMAGES);
}
}
Loading
Loading