fix: NM multiframe with TimeSlotVector #5666
Conversation
…ad and optimize scalar data retrieval
…for cine playback
…tilizing metaData for instance retrieval
✅ Deploy Preview for ohif-dev canceled.
|
There was a problem hiding this comment.
Pull request overview
This PR addresses multiple issues related to 4D dynamic volume handling, including null safety improvements, histogram worker optimization, and multiframe volume support.
- Added null checks in ViewportWindowLevel opacity functions to prevent undefined reference errors
- Optimized histogram worker registration by moving it to module load time instead of per-function call
- Enhanced getSopClassHandlerModule to handle multiframe dynamic volumes using metadata retrieval
- Added null check for viewportId in Preclinical4D mode and refactored cine playback initialization
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| platform/ui-next/src/tailwind.css | Added a reusable ohif-disabled CSS class for disabled UI elements |
| modes/preclinical-4d/src/index.tsx | Added null check for viewportId and refactored cine service initialization to use setCine with explicit frame rate |
| extensions/default/src/getSopClassHandlerModule.js | Enhanced multiframe dynamic volume handling by using metaData for instance retrieval when dealing with multiframe volumes |
| extensions/cornerstone/src/components/ViewportWindowLevel/utils.ts | Added defensive null checks for volumeActor parameter in opacity-related functions |
| extensions/cornerstone/src/components/ViewportWindowLevel/getViewportVolumeHistogram.ts | Moved histogram worker registration to module load time and updated scalar data retrieval method for multi-timepoint volumes |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (volume.numTimePoints > 1) { | ||
| const targetTimePoint = volume.numTimePoints - 1; // or any other time point you need | ||
| scalarData = volume.voxelManager.getTimePointScalarData(targetTimePoint); | ||
| scalarData = volume.voxelManager.getDimensionGroupScalarData(volume.numTimePoints); |
There was a problem hiding this comment.
The argument passed to getDimensionGroupScalarData appears incorrect. The previous code used getTimePointScalarData(targetTimePoint) where targetTimePoint was set to volume.numTimePoints - 1 (a zero-based index). Now volume.numTimePoints (the total count) is being passed, which would be an out-of-bounds index if this method expects a zero-based time point index. This could result in undefined behavior or runtime errors when processing multi-timepoint volumes.
| scalarData = volume.voxelManager.getDimensionGroupScalarData(volume.numTimePoints); | |
| const targetTimePoint = volume.numTimePoints - 1; | |
| scalarData = volume.voxelManager.getDimensionGroupScalarData(targetTimePoint); |
Viewers
|
||||||||||||||||||||||||||||
| Project |
Viewers
|
| Branch Review |
4d-sssss
|
| Run status |
|
| Run duration | 02m 22s |
| Commit |
|
| Committer | Alireza |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
0
|
Pending
|
0
|
Skipped
|
0
|
Passing
|
37
|
| View all changes introduced in this branch ↗︎ | |
|
@sedghi - could you add test notes to this? I'd like to check out the branch and run the changes - I mostly can figure it out, but it helps to have a description containing test notes. |
wayfarer3130
left a comment
wayfarer3130
left a comment
There was a problem hiding this comment.
I'm not seeing the working when testing with the suggested sample data. I tried viewing the third example, which has 800 images, and it is clearly a time slot vector series, but it is just showing all 800 odd images in it.
|
As well as this change, the NM modality probably needs to get added to the available modalities for running the 4d data view mode. |
|
I updated the branch with latest cs3d, please check again against local orthanc or something CleanShot.2026-01-05.at.20.31.55.mp4 |
should be merged after cornerstonejs/cornerstone3D#2523
test with this data: #5554
CleanShot.2026-01-05.at.20.31.55.mp4
Note
Improves dynamic volume handling for multiframe series (e.g., NM with
TimeSlotVector).metaDatafrom@cornerstonejs/coreand, whenNumberOfFrames > 1with multipletimePoints, buildsfirstTimePointInstancesviametaData.get('instance', imageId)isDisplaySetReconstructableis evaluated on the correct instances for dynamic volumesWritten by Cursor Bugbot for commit 68f0739. This will update automatically on new commits. Configure here.