Skip to content

feat(ecg): add DICOM ECG waveform extension#5856

Open
TFRadicalImaging wants to merge 3 commits intoOHIF:masterfrom
TFRadicalImaging:feat/dicom-ecg-extension
Open

feat(ecg): add DICOM ECG waveform extension#5856
TFRadicalImaging wants to merge 3 commits intoOHIF:masterfrom
TFRadicalImaging:feat/dicom-ecg-extension

Conversation

@TFRadicalImaging
Copy link
Contributor

@TFRadicalImaging TFRadicalImaging commented Feb 26, 2026

Context

This PR adds DICOM ECG/waveform rendering support to the OHIF viewer. Previously, ECG studies were listed in NON_IMAGE_MODALITIES and not rendered. ECG support is now integrated directly into the cornerstone extension rather than as a separate package.

Changes & Results

  • Add DicomEcgSopClassHandler to extensions/cornerstone/src/getSopClassHandlerModule.js alongside the existing microscopy handler
  • Add ECG waveform helpers (buildEcgModule, decodeInt16Multiplex, base64ToArrayBuffer) in extensions/cornerstone/src/utils/ecgMetadata.ts; ECG metadata registered via genericMetadataProvider at display set creation time
  • Handle ECGViewport in CornerstoneViewportService._setDisplaySets — detects the viewport type and calls setEcg(imageId) so the standard OHIFCornerstoneViewport works without a custom component
  • Add ECG support to getCornerstoneViewportType
  • Update basic mode to use the cornerstone extension's ECG handler and base cornerstone viewport
  • Remove ECG from NON_IMAGE_MODALITIES

Before: ECG studies were skipped/not rendered.
After: ECG studies are routed through the cornerstone extension and displayed as waveforms using the standard viewport infrastructure.

Testing

  1. Open a DICOM study containing ECG waveform data (e.g., https://viewer-dev.ohif.org/viewer?StudyInstanceUIDs=2.25.209974489360710696739324151261716440238)
  2. Verify the ECG waveform renders correctly in the viewport
  3. Open a non-ECG study and confirm no regression in existing viewport behaviour

Checklist

PR

  • My Pull Request title is descriptive, accurate and follows the semantic-release format and guidelines.

Code

  • My code has been well-documented (function documentation, inline comments, etc.)

Tested Environment

  • OS: macOS 25.3.0
  • Node version: v22.17.1
  • Browser: Chrome

@netlify
Copy link

netlify bot commented Feb 26, 2026

Deploy Preview for ohif-dev canceled.

Name Link
🔨 Latest commit 8445b83
🔍 Latest deploy log https://app.netlify.com/projects/ohif-dev/deploys/69a97b09e5bd930008ff7a83

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

19 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

renderingEngineRef.current = null;
}
};
}, [imageId, viewportId]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing instance in dependency array — effect uses it but won't re-run if it changes

Suggested change
}, [imageId, viewportId]);
}, [imageId, viewportId, instance]);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed — the custom OHIFCornerstoneEcgViewport component has been removed entirely, so this is no longer applicable.

Introduce @ohif/extension-dicom-ecg for rendering DICOM waveform (ECG)
data. Register the extension in the basic mode and pluginConfig.json,
and remove ECG from NON_IMAGE_MODALITIES so waveform display sets are
handled by the new viewport.
Copy link
Contributor

Choose a reason for hiding this comment

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

You shouldn't need a new OHIFCornerstoneEcgViewport type - you can use the existing OHIFCornerstoneViewport base instance, and just treat it like video or other types that just render with a base viewport.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed OHIFCornerstoneEcgViewport. ECG now uses OHIFCornerstoneViewport — CornerstoneViewportService detects ECGViewport instances and calls setEcg(imageId) directly.

Copy link
Contributor

Choose a reason for hiding this comment

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

This should go into the existing metadata provider that OHIF is using - eventually it is going into @cornerstonejs/metadata providers, but right now it should go in the OHIF provider.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved to extensions/cornerstone/src/utils/ecgMetadata.ts. The ECG module is now registered via genericMetadataProvider.addRaw() in the SOP class handler when the display set is created, so Cornerstone can find it through the standard metaData.get() lookup.

Copy link
Contributor

Choose a reason for hiding this comment

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

This can go alongside the cornerstone extension so you don't need a custom extension - again, once @cornerstonejs/metadata is released, this will get added as a customization module with configuration values, but right now it can just live alongside the cornerstone viewport types - probably the video should move there too.

Copy link
Contributor

Choose a reason for hiding this comment

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

Not asking or saying you to move the video - that is a longer term change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. The dicom-ecg extension has been removed. The ECG SOP class handler (DicomEcgSopClassHandler) now lives in extensions/cornerstone/src/getSopClassHandlerModule.js alongside the microscopy handler.

… feedback

- Remove standalone dicom-ecg extension; fold all ECG functionality into
  the cornerstone extension as requested by reviewer
- Add ECG SOP class handler (DicomEcgSopClassHandler) to the cornerstone
  extension getSopClassHandlerModule, registering ECG waveform metadata
  via genericMetadataProvider on display set creation
- Move ECG helpers (buildEcgModule, decodeInt16Multiplex, base64ToArrayBuffer)
  into extensions/cornerstone/src/utils/ecgMetadata.ts
- Handle ECGViewport in CornerstoneViewportService._setDisplaySets by
  detecting ECGViewport instanceof and calling setEcg(imageId) directly,
  so OHIFCornerstoneViewport can be used without a custom ECG viewport component
- Add ECG support to getCornerstoneViewportType utility
- Update basic mode to reference the cornerstone extension's ECG SOP handler
  and use the base cornerstone viewport for ECG display sets
- Migrate ecgMetadata and getCornerstoneViewportType tests
@TFRadicalImaging TFRadicalImaging force-pushed the feat/dicom-ecg-extension branch from 4cf4602 to 997fd2b Compare March 5, 2026 02:29
Copy link
Contributor

Choose a reason for hiding this comment

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

Revert this in favour of upstream origin/master version - you should not commit lock files unless you have specific updates.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, reverted to upstream origin/master version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants