[WIP] Feat/decimated volume loading #5800
Conversation
…mands and components
…ds for sample distance multiplier and volume reload with decimation
…on logic for improved performance and memory management
…h parsing from volumeId for improved clarity and maintainability
✅ Deploy Preview for ohif-dev canceled.
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| /** Parses IJK decimation from volumeId only when format is decimatedVolumeLoader:baseVolumeId:i_j_k; otherwise [1,1,1]. */ | ||
| function getIjkDecimationFromVolumeId(volumeId: string): [number, number, number] { | ||
| const parts = volumeId.split(':'); | ||
| if (parts.length < 3 || parts[0] !== 'decimatedVolumeLoader') { |
There was a problem hiding this comment.
Decimation parsing always wrong
getIjkDecimationFromVolumeId assumes decimatedVolumeLoader:baseVolumeId:i_j_k is 3 parts and uses parts[2] as the i_j_k suffix, but reloadVolumeWithDecimation generates IDs like decimatedVolumeLoader:${baseVolumeId}:${i}_${j}_${k} where baseVolumeId itself is schema:uid (contains :). That makes parts[2] the displaySet UID, not the i_j_k suffix, so this function will always fall back to [1,1,1] and the UI will misreport current decimation / dimensions.
| const idParts = currentVolumeId.split(':'); | ||
| const baseVolumeId = idParts.length > 1 ? idParts[1] : displaySetInstanceUID; | ||
| const decimationSuffix = `${i}_${j}_${k}`; | ||
| const newVolumeId = `decimatedVolumeLoader:${baseVolumeId}:${decimationSuffix}`; | ||
|
|
There was a problem hiding this comment.
New volumeId is malformed
reloadVolumeWithDecimation builds baseVolumeId via currentVolumeId.split(':')[1], which breaks as soon as the volumeId has more than one : segment (e.g. decimatedVolumeLoader:<displaySetUID>:volume3d or any schema:uid:suffix). This can drop the loader scheme / suffix and produce an invalid newVolumeId, leading to cache misses or loading the wrong volume when reloading with decimation.
| this.volumeImageIds.delete(invalidatedDisplaySetInstanceUID); | ||
|
|
There was a problem hiding this comment.
Cache invalidation leaves stale map entry
invalidateViewportData stores volumeImageIds in this.volumeImageIds keyed by displaySet.displaySetInstanceUID (see _getVolumeViewportData), but deletes with this.volumeImageIds.delete(invalidatedDisplaySetInstanceUID) only after switching to candidateVolumeIds deletion. This is fine only if the key is always the displaySet UID; however earlier code deleted by volumeId, and this method still treats invalidatedDisplaySetInstanceUID as a volume identifier in other spots. If volumeImageIds was previously written using volumeId keys (from older cached state), this deletion won’t clear it and the reload path will incorrectly think the volume is already cached.
extensions/cornerstone/src/init.tsx
Outdated
| displaySetService._broadcastEvent( | ||
| displaySetService.EVENTS.DISPLAY_SETS_CHANGED, | ||
| displaySetService.getActiveDisplaySets() | ||
| ); | ||
|
|
There was a problem hiding this comment.
Uses private displaySetService API
volumeLoadedHandler calls displaySetService._broadcastEvent(...). This is a private method (underscore-prefixed) and not part of the public service contract, so it’s likely to break on service refactors and can’t be relied on for extensions. If consumers depend on DISPLAY_SETS_CHANGED, this should be emitted via the public API (or the displaySet should be updated through the normal service pathways) rather than reaching into a private method.
| volumeLoader.registerVolumeLoader( | ||
| 'cornerstoneStreamingImageVolume', | ||
| cornerstoneStreamingImageVolumeLoader | ||
| (volumeId: string, options: any) => { | ||
| return decimatedVolumeLoader(volumeId, options); | ||
| } | ||
| ); |
There was a problem hiding this comment.
Overrides standard volume loader
volumeLoader.registerVolumeLoader('cornerstoneStreamingImageVolume', ...) now returns decimatedVolumeLoader instead of cornerstoneStreamingImageVolumeLoader. Any callsites expecting the streaming loader’s semantics (progressive streaming, options, or volumeId format) will now get a different loader implementation, which can cause functional regressions outside the decimation feature. If this is intentional, it needs a narrower opt-in (e.g., only for specific schemes) rather than replacing a core loader by name.
…nfig threshold and refactoring related functions
|
please sync with @dan-rukas i'm pretty sure he can help you with UI |
…s and display sets
…nt to use AllInOneMenu for improved layout and organization
…nality and update related components for improved clarity
… in display sets and enhance event broadcasting
2 participants
Context
This PR adds a 'Volume Options' menu in the volume3d viewport to control volume decimation and downsampling:
Changes & Results
Automatic decimation is applied to volume and volume3d viewports when the number of voxel is higher than a threshold set in the configuration file with name volumeAutoDecimationThreshold .
Testing
To enable auto decimation
Set volumeAutoDecimationThreshold to a number (e.g. 500 million) in configuration file:
volumeAutoDecimationThreshold: 500_000_000,
Any volume whose estimated voxel count is above this value will be decimated when loaded (e.g. downsized so the resulting voxel count is at or below the threshold). The exact decimation (in-plane and slice) is chosen automatically.
Disable auto decimation
Omit volumeAutoDecimationThreshold from the config, or set it to null / undefined. No automatic decimation will be applied; volumes load at full resolution (subject to other limits in your app).
Manually changing the decimation can be done in the 3D viewport.
Checklist
PR
semantic-release format and guidelines.
Code
etc.)
Public Documentation Updates
additions or removals.
Tested Environment