Skip to content

feat(next): native Generic Viewport migration (useNextViewports)#6101

Open
sedghi wants to merge 64 commits into
masterfrom
ohifohifnextapi
Open

feat(next): native Generic Viewport migration (useNextViewports)#6101
sedghi wants to merge 64 commits into
masterfrom
ohifohifnextapi

Conversation

@sedghi

@sedghi sedghi commented Jun 23, 2026

Copy link
Copy Markdown
Member

Linked integration test: builds this OHIF branch against the cornerstone3D PR branch cornerstoneohifnextapi (cornerstonejs/cornerstone3D#2766) and runs the Playwright suite. Add/keep the ohif-integration label. The CS3D Branch Merge Guard intentionally fails while CS3D_REF points to a branch; switch to a published version once the CS3D PR merges.

Summary

Work-in-progress migration to render OHIF viewports on cornerstone3D's native Generic ("next") viewport API, gated behind the useNextViewports opt-in flag. Legacy rendering is unchanged when the flag is off.

This is a draft for ongoing review; it contains WIP commits and depends on parallel cornerstone3D work (branch cornerstoneohifnextapi).

Highlights so far

  • Render native Generic (next) stack, volume/MPR, and 3D viewports behind useNextViewports.
  • Presentation bridge (getViewportProperties/setViewportProperties) so VOI/colormap/invert work across legacy and next backends.
  • TMTV fusion parity: colorbar shows the correct (PT) colormap and value range, linear opacity slider (100% = full PT), and a lower native default fusion opacity (0.4).
  • Dual-backend segmentation handling.

Recent next-viewport parity fixes

  • Window-level overlay (W:/L:) now refreshes when the display set changes in a next viewport.
  • Data overlay (e.g. PET on CT) comes in at the legacy-equivalent initial opacity (~40%) instead of ~80–90% on the native (flat-blend) path.
  • Changing orientation on a CT+PET fusion preserves the fusion (CT background + PET colormap/opacity) instead of collapsing to PET-only.
  • Hydrating a DICOM SEG preserves the base image window/level (no brightness shift) instead of restoring a stale computed VOI.
  • Stack→MPR keeps the slice scrollbar in sync with the rendered slice, and the acquisition (axial) MPR view centers the volume like legacy instead of pinning to the first slice.

Synced with latest master.

Status

Draft — not ready to merge. Open for incremental review feedback.

Summary by CodeRabbit

  • New Features

    • Added support for the newer native viewport experience, including updated segmentation, rendering, and overlay handling.
    • Improved viewport controls so orientation, colorbars, window/level, opacity, and threshold tools work more consistently across viewport types.
  • Bug Fixes

    • Fixed segmentation display for overlapping and multi-layer SEG data.
    • Improved slice navigation, progress bars, and orientation markers for better behavior during viewport changes.
    • Prevented missing or incorrect overlay/image references during viewport load and measurement navigation.

Greptile Summary

This draft PR introduces the useNextViewports opt-in path for rendering OHIF viewports on cornerstone3D's native Generic ("next") viewport API, adding a clean backend-split architecture (Legacy/Next pairs for both viewport service and segmentation service) behind a single flag seam with an automated allowlist guard.

  • Backend split: LegacyViewportBackend/NextViewportBackend and LegacyViewportOperations/nextViewportOperations extract per-lane mount dispatch, position/LUT presentation, and viewport operations into typed, testable classes — the legacy off-path is byte-identical to the original code.
  • Native mount paths: native stack, volume/MPR, and 3D viewports are mounted via setDisplaySets + setDisplaySetPresentation + setViewState; the DataIdRegistry ref-counts global provider registrations across shared MPR panes.
  • Presentation bridge: getViewportPresentation.ts bridges legacy getProperties/setCamera to native getDisplaySetPresentation/setViewState so colorbar, W-L toolbar, and the opacity slider work across both backends.

Confidence Score: 4/5

Not ready to merge — fusion viewport appearance (PT colormap/VOI) is silently dropped on every navigation cycle in the native path, and the download capture misses overlay bindings for fusion viewports.

The native fusion LUT save/restore only captures the source (CT) binding: _getLutPresentation calls getViewportProperties without a dataId on PLANAR_NEXT, reading only the source binding. For a PT/CT fusion, the PT overlay colormap, VOI range, and threshold are never saved and never restored on nav-away-and-back. The download capture has the same gap. Both are isolated to the native fusion path and the overall backend architecture is sound.

extensions/cornerstone/src/services/ViewportService/CornerstoneViewportService.ts (_getLutPresentation native branch) and extensions/cornerstone/src/utils/CornerstoneViewportDownloadForm.tsx (native fusion capture path)

Important Files Changed

Filename Overview
extensions/cornerstone/src/services/ViewportService/CornerstoneViewportService.ts Large refactor: backend split for mount dispatch, LUT/position presentation, and dataId lifecycle. Native fusion LUT save/restore only captures the source (CT) binding — PT appearance is dropped on every nav-away-and-back cycle.
extensions/cornerstone/src/services/ViewportService/backends/NextViewportBackend.ts New native viewport backend: owns position/LUT presentation for PLANAR_NEXT viewports and ref-counted dataId lifecycle. The setLutPresentation only reads/writes the source binding for fusion viewports.
extensions/cornerstone/src/services/ViewportService/backends/NextViewportOperations.ts Native operations lane: scaleBy calls vp.getZoom()/setZoom() without optional-chain guards; would throw on non-PLANAR native types if those receive a scale command.
extensions/cornerstone/src/services/ViewportService/backends/dataIdRegistry.ts Ref-counted lifecycle for the global GenericViewport metadata provider. Correct across shared MPR dataIds; byViewport list grows unboundedly on repeated remounts without release.
extensions/cornerstone/src/services/ViewportService/backends/viewportOperations.ts Thin per-viewport dispatcher routing to legacy or native operations lane. Clean design, no issues.
extensions/cornerstone/src/services/SegmentationService/backends/NextSegmentationBackend.ts Never promotes PLANAR_NEXT to ORTHOGRAPHIC; builds multi-layer overlapping SEG labelmaps correctly.
extensions/cornerstone/src/utils/CornerstoneViewportDownloadForm.tsx Native capture only transfers source-binding presentation; fusion overlay appearance is not captured.
extensions/cornerstone/src/utils/getViewportPresentation.ts Clean bridge: correctly dispatches to getDisplaySetPresentation/setViewState (native) or getProperties/setCamera (legacy) for all four helpers.
extensions/cornerstone/src/hooks/useViewportRendering.tsx Adds native-aware opacity gamma and per-binding property reads. resolveOpacityScalar fix for array-valued opacity is correct.
.scripts/check-next-viewports-flag-reads.mjs CI guard for the flag-read allowlist. Logic is sound.

Comments Outside Diff (1)

  1. extensions/cornerstone/src/services/ViewportService/CornerstoneViewportService.ts, line 450-460 (link)

    P1 LUT presentation only captures the source binding for native fusion viewports

    For a native PT/CT fusion viewport, isVolumeViewportType(csViewport) is false (PLANAR_NEXT is not ORTHOGRAPHIC), so the code takes the non-Map branch and calls getViewportProperties(csViewport) without a dataId. That reads vp.getDisplaySetPresentation(vp.getSourceDataId()) — the CT (source) binding only. The PT overlay colormap, VOI range, and threshold are never saved. When setLutPresentation later calls nativeViewport.setDisplaySetPresentation(presentationProps) (also without a dataId) it again targets the source binding, so the PT appearance is silently dropped on every navigation-away-and-back cycle.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: extensions/cornerstone/src/services/ViewportService/CornerstoneViewportService.ts
    Line: 450-460
    
    Comment:
    **LUT presentation only captures the source binding for native fusion viewports**
    
    For a native PT/CT fusion viewport, `isVolumeViewportType(csViewport)` is `false` (PLANAR_NEXT is not ORTHOGRAPHIC), so the code takes the non-Map branch and calls `getViewportProperties(csViewport)` without a `dataId`. That reads `vp.getDisplaySetPresentation(vp.getSourceDataId())` — the CT (source) binding only. The PT overlay colormap, VOI range, and threshold are never saved. When `setLutPresentation` later calls `nativeViewport.setDisplaySetPresentation(presentationProps)` (also without a `dataId`) it again targets the source binding, so the PT appearance is silently dropped on every navigation-away-and-back cycle.
    
    How can I resolve this? If you propose a fix, please make it concise.

Reviews (4): Last reviewed commit: "fix(next): address review findings (guar..." | Re-trigger Greptile

sedghi added 30 commits June 17, 2026 09:52
…step 1)

Opt-in flag to drive viewports through the DIRECT native cornerstone3D
GenericViewport ("next") API surface (PLANAR_NEXT / VOLUME_3D_NEXT, setDisplaySets,
setDisplaySetPresentation, setViewState, view references) instead of the legacy
stack/volume methods.

Distinct from (and overrides) useGenericViewport, which only routes legacy
viewport types through cornerstone compatibility adapters. This flag does NOT set
cornerstone rendering.useGenericViewport; it is read by getCornerstoneViewportType
and the CornerstoneViewportService backend split (subsequent M0 steps). Defaults
false; the legacy path stays byte-identical. Opt-in only.
…iewports (M0 step 2)

getCornerstoneViewportType gains an optional useNextViewports param. When set,
stack/volume/orthographic collapse to PLANAR_NEXT (render path inferred from data
shape), and volume3d/video/wholeslide/ecg map to their VOLUME_3D_NEXT / VIDEO_NEXT /
WHOLE_SLIDE_NEXT / ECG_NEXT types. Defaults false → legacy mapping byte-identical;
no caller passes true yet (wired via appConfig in the service split, step 3).

Tests: +6 cases (21 total) covering the *_NEXT mapping, displaySet override, the
invalid-type throw, and that the legacy mapping is unchanged when the flag is off.
…0 step 3 foundation)

Wires the useNextViewports flag through and mounts stack viewports natively:
- nextViewports.ts: module accessor; init.tsx captures appConfig.useNextViewports.
- getCornerstoneViewportType: defaults the flag from the accessor, and now passes
  native (*_NEXT) types through idempotently (a viewport's stored cs type is
  re-fed into the mapper; legacy types were already idempotent, native were not).
- CornerstoneViewportService._setDisplaySets: route native generic viewports by
  data shape (StackData vs VolumeData), since PLANAR_NEXT is one type for both.
- _setStackViewport: native branch mounts via genericViewportDataSetMetadataProvider.add
  + setDisplaySets, applies VOI/colormap via setDisplaySetPresentation and
  displayArea/rotation/flip via setViewState (no legacy setStack/setProperties/setCamera).

Validated in a running OHIF (linked cornerstone 5.0.8): with the flag on, the
viewer creates a native PlanarViewport (window.cornerstone...getViewports()[0] ->
'PlanarViewport :: type=planarNext'); the prior 'Invalid viewport type: planarNext'
is resolved. Flag OFF is byte-identical (native branches gated by isGenericViewport).

KNOWN WIP (next): flag-ON full render is blocked by the presentation-read seam —
peripheral consumers (useViewportRendering, overlays, colorbar, resize) still call
legacy getProperties/getViewPresentation/getCamera on the viewport. Stack render
completes once those are routed through getDisplaySetPresentation/viewportProjection.
… useNextViewports

Makes the flag-on native PLANAR_NEXT stack render end-to-end in OHIF:

- CornerstoneCacheService: resolve the stack-vs-volume data-builder from the
  legacy mapping, since native types collapse that distinction into PLANAR_NEXT.
  Without this the stack fell through to the 'other' builder and imageIds were
  never populated (PlanarViewport threw 'No registered planar dataset metadata').
- ImageOverlayViewerTool: skip overlay rendering when the viewport has no
  resolvable view reference yet (native returns falsy until data is bound),
  instead of letting getTargetId() throw and kill the route during enable.
- Add getViewportPresentation helpers (getViewportProperties / getViewportCameraState)
  bridging legacy getProperties/getCamera and native getDisplaySetPresentation/
  getViewState; apply at the toolbar property evaluator, VOI-range init, and the
  position/LUT presentation snapshots so reads no longer throw on native viewports.

Validated in a running OHIF (flag on): PlanarViewport(planarNext), 295 slices,
image renders, zero console errors, scroll + setImageIdIndex navigate correctly.
Legacy (flag off) behavior is unchanged. Native volume/MPR is a later increment.
…NextViewports

Mounts volumes on a direct PLANAR_NEXT viewport for volume/MPR rendering:

- CornerstoneViewportService.setVolumesForViewport: native branch -> new
  _setNativeVolumeDisplaySets. Each base volume is registered with its
  already-cached volumeId and bound via setDisplaySets at the viewport's
  orientation (first = source, others = overlay); VOI/colormap/invert applied
  per-binding via setDisplaySetPresentation(dataId, props). Skips the legacy
  setVolumes/setProperties/setPresentations surface a PLANAR_NEXT viewport does
  not expose. Cornerstone reuses the OHIF-cached volume (getVolumeId returns the
  passed volumeId) and selects the image vs reformatted-volume render path from
  the requested orientation.
- useViewportRendering colormap resolver: read via getViewportProperties for
  native viewports (getDisplaySetPresentation) instead of getProperties/getActors,
  which threw 'Error getting viewport colormap' on native volume.

Validated in a running OHIF (flag on): axial/sagittal/coronal all render in
volume mode, scroll navigates the volume, round-trip volume<->stack switches
getCurrentMode cleanly, zero console errors. Legacy (flag off) unchanged.
…ports

The setViewportOrientation command guarded on isOrthographicViewportType, which
is false for a native PLANAR_NEXT viewport (it reports type=planarNext even when
rendering MPR). Add the content-mode capability guard (csUtils.viewportIsInVolumeMode)
so the MPR orientation toolbar works on native viewports; PlanarViewport.setOrientation
already exists. Legacy behavior is unchanged (viewportIsInVolumeMode is false for
legacy viewports, so the existing isOrthographicViewportType branch still gates them).

Also flips useNextViewports on in the dev default config for local testing of the
native path (NOT for merge; see TODO_BEFORE_MERGE).
…tion

- Introduced a dev-only configuration flag to toggle the native viewport backend.
- Added a toolbar button to switch between legacy and native viewports for debugging.
- Implemented logic to handle image slice data and viewport type detection for native viewports.
- Enhanced viewport service to derive default VOI window/level from DICOM metadata.
- Added utility functions for managing localStorage overrides for viewport settings.
- Marked all temporary changes with comments for easy identification and removal before merging.
…kend (§4.3)

Extends the legacyBackend/nextBackend seam so presentation read/write is forked,
not inline-guarded in the service:

- IViewportBackend gains getPositionPresentation / setPositionPresentation /
  setLutPresentation. LegacyViewportBackend keeps the exact legacy logic
  (getViewPresentation / setProperties / setViewPresentation) byte-identical;
  NextViewportBackend uses the native surface (getViewReference + setViewReference;
  setDisplaySetPresentation for VOI/colormap/invert). WITH_ORIENTATION is inlined in
  the backends to avoid a backend->service value-import cycle.
- CornerstoneViewportService._getPositionPresentation / _setLutPresentation /
  _setPositionPresentation now delegate to this.backend. _getLutPresentation stays
  shared (already native-aware via the getViewportProperties bridge).

Effect: setPresentations is now native-safe — it previously threw on a PLANAR_NEXT
viewport (setProperties / setViewPresentation), so the native mount skipped it; the
native path now round-trips presentation cleanly. Native pan/zoom persistence
(viewPresentation is still undefined on native) is a later increment.

Validated both lanes: native PlanarViewport renders + storePresentation/getPresentations/
setPresentations round-trip with no crash; legacy StackViewport byte-identical
round-trip; zero console errors each.
… the backend

A PLANAR_NEXT viewport has no getViewPresentation/setViewPresentation, so pan/zoom
previously did not survive navigation/resize/layout on the native path. NextViewportBackend
now snapshots the pan/zoom subset of the semantic view state and restores it:

- getPositionPresentation: snapshots the PlanarViewState pan/zoom fields (displayArea,
  anchorWorld, anchorCanvas, scale, scaleMode, rotation, flipHorizontal, flipVertical) via
  getViewState() (already deep-cloned + serializable), stored in viewPresentation. Slice and
  orientation are intentionally excluded (they ride on the view reference).
- setPositionPresentation: applies the view reference first (slice/orientation), then a partial
  setViewState patch with only the pan/zoom subset — the merge preserves slice/orientation, so
  the view reference is never clobbered. Stale displayArea is cleared when live anchor/scale
  pan/zoom is restored. anchorCanvas is canvas-fractional, so it survives resize without drift.
- _setStackViewport native branch now restores the persisted positionPresentation on mount
  (position-only; LUT already applied inline), so a returning stack recovers its camera.

Validated on native: zoom -> snapshot (scale captured) -> reset -> restore (scale back),
slice unchanged, zero errors. Legacy path unchanged. Volume/MPR mount pan/zoom restore is a
follow-up (its native mount helper does not yet thread presentations).
Extends native pan/zoom persistence to the volume/MPR mount: the native branch of
setVolumesForViewport now restores the persisted positionPresentation (view reference
+ pan/zoom via the backend) after _setNativeVolumeDisplaySets, mirroring the stack
mount. Position-only (LUT applied per-binding above), native-safe.

Validated on a native sagittal MPR: zoom -> snapshot (scale 1.7) -> reset -> restore
(scale back to 1.7) with orientation (sagittal) and slice (256) preserved; zero errors.
Add setViewportProperties/setViewportCameraState write bridges alongside the
existing read bridges, and route invertViewport, flipViewportHorizontal,
flipViewportVertical and setViewportWindowLevel through them.

Direct PLANAR_NEXT viewports have no getCamera/setCamera/getProperties/
setProperties, so these four commands threw on native. The bridges dispatch on
isGenericViewport: native reads/writes via getViewState/setViewState (flip) and
getDisplaySetPresentation/setDisplaySetPresentation (invert/voiRange) on the
active binding; legacy falls through to the identical getCamera/setCamera/
getProperties/setProperties calls, so flag-off stays byte-identical.

Verified live on a native stack viewport: all four now apply (invert
undefined->true, flipH/flipV false->true, voiRange retargets) instead of
throwing.
setViewportColormap was fully guarded by isStackViewportType/
isOrthographicViewportType, both of which report false for native PLANAR_NEXT
viewports, so the command was a silent no-op on native (returned ok but applied
nothing). Add a native branch that applies the colormap via the
setViewportProperties bridge (setDisplaySetPresentation) on the active binding,
honoring the immediate render flag, before the legacy guards.

Verified live: HSV colormap now applies and renders on a native stack viewport.
ColorbarService called getActors/getProperties/setProperties directly, all of
which throw on direct PLANAR_NEXT (next) viewports, so toggling a colorbar threw
on native. Replace the getActors content gate with a viewportHasContent helper
(getCurrentMode for native, getActors for legacy), and route the property
read/write through the getViewportProperties/setViewportProperties bridges.
Legacy keeps the identical getActors/getProperties/setProperties calls.

Verified live on native: addColorbar no longer throws, hasColorbar becomes true,
and the colormap applies via setDisplaySetPresentation.
getWindowLevelsData drives the per-volume histogram WL panel via
getAllVolumeIds/getProperties, which direct PLANAR_NEXT (next) viewports do not
expose (they throw). Add an early guard that returns no rows when getAllVolumeIds
is absent, so the panel degrades to 'No window level data available' instead of
erroring on the interval/event refresh. Legacy stack viewports also lack
getAllVolumeIds and were never passed here, so this is a no-op for them; the
native stack/volume WL path is driven by setViewportWindowLevel.
These three command paths assumed legacy camera APIs that direct PLANAR_NEXT
(next) viewports do not expose:
- resetViewport called viewport.resetCamera() (absent on native -> threw). Native
  branch uses resetViewState() (resets pan/zoom/rotation/orientation/flip;
  navigation/slice preserved). resetProperties stays optional-chained.
- scaleViewport (scaleUpViewport/scaleDownViewport) was guarded by
  isStackViewportType, which is false for native, so the zoom buttons were a
  silent no-op. Native branch uses getZoom/setZoom; parallelScale and zoom are
  inversely related so it divides by scaleFactor to match legacy direction.
- _rotateViewport (rotateViewportCW/CCW/CWSet) used getViewPresentation/
  setViewPresentation (absent on native). Native branch reads rotation/flip via
  the getViewportCameraState bridge (getViewState) and writes the new rotation
  via setViewportCameraState (setViewState), preserving the flip-parity logic of
  the 'set' mode.

Verified live on native stack: reset no longer throws and resets zoom; zoom in
1->1.111; CW/CCW/Set rotation applies (90/180/90/90). Legacy unchanged.
jumpToMeasurement re-centers the camera when a measurement is off-screen via
isMeasurementWithinViewport (calls viewport.getCamera()) + getCamera/setCamera.
Native PLANAR_NEXT viewports have neither, so jumping to a measurement threw
'viewport.getCamera is not a function' at the gate before the centering block.

Short-circuit the centering on native (isGenericViewport) so it is skipped;
setViewReference above already navigated to the measurement's slice, so the
measurement is still reached - only in-plane re-centering is deferred.
TODO(next): port in-plane centering via the camera bridge + setViewState pan.

Verified live: native viewport has no getCamera (getCamera() throws) and
isGenericViewport is true, so the throwing branch is now skipped; setViewReference
remains available for slice navigation. Legacy unchanged.
Two CornerstoneViewportService methods called getCamera()/setCamera(), which
direct PLANAR_NEXT (next) viewports lack:
- getViewportAlignmentData looped every viewport reading getCamera().viewPlaneNormal
  (reached from findNavigationCompatibleViewportId on a cross-orientation
  jumpToMeasurement). Native reads viewPlaneNormal from getViewReference() instead
  (both backends populate it); legacy keeps getCamera (byte-identical).
- updateViewport (metadata-invalidation re-mount, keepCamera) read getCamera()
  unconditionally and had no native branch in its stack/volume if/else, so it threw
  and would not re-mount native data. Add a native branch that snapshots/restores the
  camera via the view-state bridges and routes the re-mount through _setDisplaySets
  (backend.dispatchMount, which dispatches by data shape). Legacy path unchanged.

Verified live on native: getViewportAlignmentData returns data (no throw);
updateViewport(keepCamera) re-mounts, restores zoom (1.4/1.5), keeps the image actor
and renders, with zero errors.
…ations backend

The commandsModule carried inline native-vs-legacy branches for every viewport
interaction/appearance command. Extract them into a dedicated operations backend
that mirrors the existing IViewportBackend twin pattern:
- IViewportOperations: the interface (flip/invert/rotate/reset/scaleBy/
  setWindowLevel/setColormap/getViewPlaneNormal/centerOnMeasurement + 3D VR ops)
- LegacyViewportOperations: legacy lane via direct legacy APIs (getCamera/
  setProperties/getViewPresentation/resetCamera/actors), lifted verbatim
- NextViewportOperations: native lane via the presentation/camera-state bridges +
  native semantic API (getViewState/resetViewState/getViewReference/setZoom); the
  3D VR ops warn-once and no-op behind a CS-14 gate (native VR not supported yet)
- viewportOperations: per-viewport dispatcher (isGenericViewport ? next : legacy)

commandsModule's 13 interaction commands become one-line delegations (the file
shrinks ~239 lines) and CornerstoneViewportService.getViewportAlignmentData uses
viewportOperations.getViewPlaneNormal.

Dispatch is per-viewport (not flag-selected like the lifecycle IViewportBackend)
because operations run on already-created, self-describing viewports and a session
can mix lanes; this preserves the previous inline isGenericViewport branching
exactly. Render() stays in the command (per-command render timing preserved).

Validated live both lanes: native (flag on) applies all ops (invert/flip/rotate/
zoom/WL/colormap; reset is camera-only) and legacy (flag off) is byte-identical
(parallelScale*0.9 zoom, getViewPresentation rotation, resetProperties+resetCamera),
both with a clean console. Adversarial review found no byte-identity/runtime defects.
Segmentation untouched.
Make native VOLUME_3D_NEXT viewports actually render volume rendering and wire up
the 3D VR operations:

- CornerstoneViewportService._setNativeVolumeDisplaySets: a 3D viewport (cornerstone
  type VOLUME_3D_NEXT) now mounts with setDisplaySets({ options: { renderMode:
  'vtkVolume3d' } }) instead of the planar { orientation, role }, and applies the
  display-set's volume-rendering preset to the volume actor via csUtils.applyPreset
  (the bare native VolumeViewport3D has no setProperties). colormap (a planar LUT
  concept) is skipped for 3D. The dataId registration is unchanged (the volume3d data
  provider reads imageIds/volumeId and ignores the stored kind).

- NextViewportOperations: the four VR ops are no longer CS-14 no-ops. setPreset
  applies the preset to the volume actor via applyPreset; setVolumeRenderingQuality/
  shiftVolumeOpacityPoints/setVolumeLighting operate on the vtk volume actor through
  getActors (which native VolumeViewport3D exposes), so they reuse the legacy
  actor-based implementations. Pairs with the cornerstone fix that keeps the 3D
  viewport's canvas visible.

Verified live: native 3D VR renders (CT-Bone/CT-Cardiac presets) and all four VR
commands apply on a native VOLUME_3D_NEXT viewport; legacy VOLUME_3D unchanged.
NextViewportOperations.setColormap dropped params.displaySetInstanceUID and
always wrote to the active source binding via getSourceDataId(), so a PT/CT
fusion colormap landed on the CT source instead of the PT overlay. Thread the
displaySetInstanceUID through to setViewportProperties so it targets the right
native binding (OHIF maps each display set 1:1 onto its bare dataId); falls back
to the source when no id is given (single-volume / plain stack colormap).

Also document DataIdRegistry.dataIdFor's 'overlay' suffix as reserved for the
same-UID source/overlay case (derived labelmap overlays, M4) rather than fusion,
whose distinct-UID overlays are already collision-free under the bare id.
Sweeps the OHIF-side sites a native PLANAR_NEXT viewport reaches that still
called legacy-only APIs (getProperties/getCamera) or branched on the collapsed
viewportType:

- CornerstoneCacheService: persist the legacy stack/volume decision as
  viewportData.dataShapeType (createViewportData) and branch on it in
  invalidateViewportData instead of viewportType. Native collapses stack+volume
  onto PLANAR_NEXT, so a native stack previously fell through to the VOLUME
  rebuild and re-mounted as volume data on metadata invalidation. Falls back to
  viewportType for legacy/older data (byte-identical off-path).
- ViewportOrientationMarkers: gate the synthetic-IOP default-cosine check on
  dataShapeType, not viewportType==='stack' (dead on native -> guard was skipped).
- CornerstoneViewportDownloadForm: the capture viewport is the source's type, so a
  native source threw on getProperties/setStack/setProperties. Add a native capture
  path that re-mounts the source's already-registered dataId via setDisplaySets and
  copies presentation + view state through the bridges (legacy path byte-identical).
- tmtv ROI-threshold: read the slice focal point via a new getViewportFocalPoint
  bridge (native getViewReference().cameraFocalPoint vs legacy getCamera().focalPoint)
  instead of getCamera(), which is absent on native.

New bridge getViewportFocalPoint added to getViewportPresentation.ts and exported
from @ohif/extension-cornerstone. Validated live: native stack renders, console
clean, viewportData.dataShapeType='stack' while viewportType='planarNext',
orientation markers render.
Under useNextViewports, NextViewportBackend.dispatchMount routed ALL viewports by
data shape (volume vs stack), so VIDEO_NEXT/WHOLE_SLIDE_NEXT/ECG_NEXT constructed
as native classes but mis-ran _setStackViewport's stack-specific prefetch/VOI/
kind:'planar' logic and never reached their dedicated mounts; ECG additionally
called the absent setEcg.

- NextViewportBackend.dispatchMount: route the non-planar families by viewport
  type to _setEcgViewport / _setOtherViewport (mirrors the legacy backend's type
  dispatch); planar stack/volume still routes by data shape.
- _setEcgViewport: native branch registers {kind:'ecg', sourceDataId} and mounts
  via the generic setDisplaySets API (native ECG has no setEcg).
- _setOtherViewport: native branch registers {kind:'video', sourceDataId} or, for
  WSI, {kind:'wsi', imageIds, options:{webClient}} with the client resolved from
  WADO_WEB_CLIENT metadata exactly as the legacy WSI adapter does, then mounts via
  setDisplaySets + setViewReference.
- DataIdPayload widened to a family-specific union (planar/video/ecg/wsi).

All registration goes through the ref-counted DataIdRegistry (§4.7). OHIF-only —
the cornerstone native classes already support setDisplaySets (per the
genericVideo/genericEcg/genericWsi examples). Validated: native planar render
unaffected by the dispatch change (console clean); video/WSI/ECG mounts follow the
canonical cornerstone examples but are not yet live-validated (no such study on the
dev dicomweb).
Adds .scripts/check-next-viewports-flag-reads.mjs (wired as `yarn
next:check-flag-reads`) enforcing migration plan §4.2: the flag may be read only
in the sanctioned seam — getCornerstoneViewportType (type selection),
CornerstoneViewportService (backend selection), nextViewports.ts (the accessor),
init.tsx (the one appConfig.useNextViewports read), and the TEMP dev toggle in
getToolbarModule.tsx. Any other isNextViewportsEnabled()/appConfig.useNextViewports
read under extensions/cornerstone/src fails the check, so the legacy off-path
cannot drift. (Comment-only mentions are ignored; tests are exempt.)

The earlier audit framed the '2 sanctioned reads' contract as already violated by a
'backend trio', but those files only MENTION the flag in doc comments — the actual
runtime read surface is the sanctioned set above, so the rule is enforceable as
written. TODO_BEFORE_MERGE.md updated: the guard is permanent (not a dev revert),
and removing the dev toggle must also drop its allowlist entry.

Does NOT perform the destructive M7 reverts (config default flip, toggle button):
those are premature while segmentation/M4 is unmigrated and would disable the
in-browser test loop.
Re-audited every milestone (M0-M7) and CS blocker against HEAD source via a
multi-agent audit + adversarial verification pass. Five commits landed after the
last full prose refresh (05e0df0) and only d5d03d8 touched the doc, so the
Implementation status section was materially stale. Corrections:

- M2: fusion colormap keying is FIXED (7b61e08), not 'unsound'
- M3: four 'native-unsafe throws' are FIXED (a19bd78); the per-volume WL panel
  is guarded (d282026), not throwing - feature port, not a crash
- M6: video/WSI/ECG ARE mounted natively (ca746e2)
- M7: flag-read allowlist IS built (b5784ca), just not wired into CI
- CS-21: stated trigger is unreachable; narrowed to single-point SCOORD3D, and
  the open code is PlanarViewReferenceController.ts (not planarViewReference.ts)

Adds a consolidated, verified remaining-work punch-list and a corrections
subsection. The one real native crash that remains is the M4 segmentation OHIF
half (convertStackToVolumeViewport throws AND promotes to legacy ORTHOGRAPHIC).
…n handling

- Introduced ISegmentationBackend interface to define methods for segmentation backends.
- Implemented LegacySegmentationBackend for existing behavior with stack/volume promotion.
- Implemented NextSegmentationBackend for native GenericViewport handling without promotion.
- Updated SegmentationService to utilize the appropriate backend based on viewport type.
- Removed legacy viewport handling logic from SegmentationService and delegated to backends.
- Enhanced segmentation data assembly to support overlapping segments in the Next backend.
- Updated CornerstoneViewportService to ensure proper restoration of segmentation presentations.
- Added support for preserving additional query parameters in the application.
sedghi added 24 commits June 23, 2026 18:24
The overlay's WW/WL comes from useViewportRendering, whose init effect reads the
VOI via getViewportProperties. On series change the effect re-runs, but native
(next) viewports expose only explicit VOI overrides through
getDisplaySetPresentation; a freshly shown series has none, so properties.voiRange
was undefined, setVoiRange was skipped, and the overlay kept showing the previous
series' window level. Legacy getProperties always returns the applied VOI, so only
native viewports were affected.

Fall back to the viewport's computed default VOI (getDefaultVOIRange) for generic
viewports when no override is stored, matching the LivewireContourTool and
WindowLevelTool bridges. Legacy behavior is unchanged.
…orts

The progress scrollbar seeded its slice state (imageIndex/numberOfSlices) only
when viewportData changed. Native (next) viewports keep the same viewportData
across a stack->volume transition or an orientation change, and the slice-
navigation event does not fire until the first scroll, so the scrollbar was
missing on the initial slice (or stale with a wrong slice count) until the user
scrolled once.

Re-seed the slice state from the live viewport on CAMERA_MODIFIED (which native
viewports emit on orientation/geometry changes, as the sibling full-mode hook
already relies on). A guard skips redundant state updates so pure pan/zoom does
not churn React state.
Follow the cs3D rename of the native viewport's presentation-reset method from
resetProperties to resetDisplaySetPresentation (the next viewport API uses
get/set DisplaySetPresentation, not get/set Properties). Behavior unchanged.
The Border (outline width) slider for labelmap and RTSTRUCT used step=0.1,
allowing fractional outline widths. Outline thickness is a pixel width and
should be a whole number, so use step=1 and round the committed value. The
fill/opacity sliders keep their fractional step.
…s tool

resetCrosshairs (run by Reset Viewport) called toolGroup.getToolInstance('Crosshairs')
for every tool group; getToolInstance logs 'Crosshairs is not registered with this
toolGroup' when the tool is absent, and a next viewport's default tool group does not
include Crosshairs, so Reset Viewport logged a spurious warning. Guard the lookup with
toolGroup.hasTool('Crosshairs') and skip tool groups that lack it; also guard against a
missing tool group for the viewport.
…is added

A data overlay (fusion) on a next (PLANAR_NEXT) viewport rendered the source as a
vtkImage stack while the overlay was a vtkVolumeSlice, producing a broken/unstable
fusion (geometry mismatch, intermittent across slices/scroll). Two causes:

1. CornerstoneCacheService.createViewportData built stack data when the fusion's
   primary display set resolved to a stack shape, so the source never got a
   volumeId. Force a volume (orthographic) shape when there are 2+ reconstructable
   image display sets (a data fusion must be volume; legacy already did this, and
   non-reconstructable SEG/RT overlays are excluded).

2. dataIdRegistry.register used first-writer-wins, so re-registering the source
   (originally a vtkImage stack, no volumeId) with its fusion volumeId was dropped,
   leaving its dataset volumeId-less -> the render-path decision kept it vtkImage.
   Update the provider when a payload promotes a dataId to volume-backed.

Validated via agent-browser: adding a PT overlay onto a CT source now mounts both
as vtkVolumeSlice (mode=volume), the fusion is anatomically coherent and stable
across scroll, with no console errors.
RTSTRUCT (contour) hydration on a native PLANAR_NEXT viewport re-mounted the
referenced CT as a volume slice, which is the slow path the perf AC forbids.

Spike proved cs3d already renders contour segmentations on a stack/vtkImage
PLANAR_NEXT viewport (via the annotation + isReferenceViewable path), and that
stack-mode contour scroll is fast (~0.15ms/scroll, no metadata storm) once the
canvas-dimension layout-thrash fix is in place. The only remaining issue was the
hydrate re-mount promoting the CT to volume.

Pin the referenced viewport to 'stack' for RTSTRUCT hydration on next viewports:
hydrateSecondaryDisplaySet passes viewportType:'stack' (scoped to RTSTRUCT +
isNextViewportsEnabled), and loadSegmentationDisplaySetsForViewport applies it as
a per-mount viewportOptions override. SEG and legacy keep their current behavior.

Verified at runtime: after hydrate the viewport stays vtkImage/stack, contours
render across slices, scroll is 0.16ms with 0 metaData.get calls, CT VOI intact.
… next viewports

The data-overlay add path passes a nominal colormap opacity of 0.9. Legacy
volume rendering attenuates that to ~40% effective via ray-cast opacity-unit-
distance correction, but native PLANAR_NEXT viewports composite the overlay as
a flat 2D image-slice blend with no such attenuation, so the same 0.9 rendered
at ~80-90%. Override the initial overlay opacity to 0.4 for next viewports
(mirrors the TMTV fusion NEXT_FUSION_PT_OPACITY), gated on isGenericViewport and
a numeric opacity so SEG/RTSTRUCT overlays and the legacy path are unaffected.
…orts

The orientation corner menu branched on viewportType === ORTHOGRAPHIC to decide
in-place reorient vs viewport recreation. Native next viewports always report
planarNext, so a fusion already in volume mode wrongly took the recreation path,
which passes empty displaySetOptions and drops the PET overlay colormap/opacity
(rendering PET only). Branch instead on whether the live viewport is already in
volume mode (isOrthographicViewportType || utilities.viewportIsInVolumeMode):
volume-mode viewports reorient in place (setViewportOrientation, which preserves
all bindings and their presentation); only a genuine stack->volume conversion
recreates. Legacy ortho/stack behavior is unchanged.
Hydrating a SEG re-mounted the referenced image and restored a stale, computed
VOI from the LUT presentation store, brightening the base image (e.g. an MR went
from its DICOM WC/WW default to a volume min/max default). During the SEG-load
intermediate mount the base image briefly carries a computed default VOI; the
native read bridge returned it with no isComputedVOI marker, so cleanProperties
never stripped it and it was persisted then replayed over the correct default.
Legacy StackViewport tags computed VOIs (isComputedVOI) so they are stripped.
Mirror that: in getViewportProperties, stamp isComputedVOI on a native binding's
VOI when it matches the binding's getDefaultVOIRange, so the LUT capture strips
it. Harmless when a genuine user VOI equals the default (stripping falls back to
the same value).
On a stack->volume/MPR transition the slice carry (e.g. layout-selector MPR HP
restoring the prior slice) moves the camera and fires its slice events
synchronously during the mount, before the scrollbar effect attaches its
listeners and around its initial seed -- so the scrollbar latched the mount-time
index. Re-seed once on the next frame after mount+carry settle; the pushSliceData
guard makes it a no-op when nothing changed (no churn).
…port

The next viewport backend imported genericViewportDataSetMetadataProvider
from @cornerstonejs/core, a symbol that only existed in the local custom
cornerstone worktree (linked via symlink). Published cornerstone exports it
as genericViewportDisplaySetMetadataProvider (same add/remove/get/clear API).

CI always builds against published packages, so the production rspack build
failed (ESModulesLinkingError -> Netlify red) and the cypress dev-server
build showed a full-screen error overlay that blocked all clicks (PR_CHECKS
red). Renaming to the published symbol fixes both.
The jump-to-measurement screenshot was the only failing assertion (pre/post
hydration pass). It uses waitVolumeLoad:false, so on slower CI the dynamic
tfl_dyn_fast_tra series can still be progressively loading when the shot is
taken, giving a partial probe value (52.0 vs baseline 78.0) and shifted VOI.
Raise checkForScreenshot attempts 10->20 and delay 1250->2000ms (~11s -> ~40s)
to let the volume settle before failing.
…et, off-path gate, seg export perf)

- NextViewportOperations.scaleBy: guard getZoom/setZoom so the zoom hotkey
  no-ops on a native VolumeViewport3D instead of throwing (matches legacy).
- Native setWindowLevel: forward displaySetInstanceUID so PT/CT fusion W/L
  targets the intended binding (mirrors setColormap) instead of always source.
- CornerstoneCacheService: scope the reconstructable-fusion STACK->ORTHOGRAPHIC
  promotion to PLANAR_NEXT so the legacy (flag-off) path stays byte-identical.
- dicom-seg buildLabelmap3D: precompute a referencedImageId->index Map to drop
  the multi-layer export from O(slices^2) to O(slices).
…urn contract, error msg)

- NextViewportBackend.dispatchMount: route stack/volume on the persisted
  dataShapeType contract instead of the lazily-populated 'volume' field probe.
- SegmentationService.attemptStackToVolumeConversion: return false explicitly on
  the frame-of-reference-mismatch path to honor the Promise<boolean> contract.
- getCornerstoneViewportType: list orthographic/volume3d in the legacy-path
  invalid-type error (both are valid and handled above the throw).
Match the legacy-path error message now listing orthographic/volume3d (b16129e).
- Remove the .scripts/check-next-viewports-flag-reads.mjs CI guard and its
  package.json script entry.
- Drop the TEMP ToggleNextViewport toolbar button across all modes plus the
  toggleNextViewports command and getToolbarModule evaluator (revert mode files
  to their master state; keep the real native-path toolbar fixes).
- Remove the localStorage override / toggleNextViewportsAndReload from
  nextViewports.ts; resolveNextViewportsEnabled now honors a ?useNextViewports
  URL query param (true/1/empty enable) over appConfig.
- Stop forcing useNextViewports:true in default.js (opt-in via URL/appConfig).
- Preserve useNextViewports across navigation (preserveQueryParameters).
Flattening the fusion PT opacity to a scalar 0.9 in hpViewports changed the
legacy TMTV fusion rendering (the ramp keeps low PT values transparent so the CT
shows through). Restore the legacy ramp in hpViewports and instead replace it
with the flat native scalar (0.4) inside getHangingProtocolModule only when
useNextViewports is on, so legacy is unchanged and native still gets a flat blend.
# Conflicts:
#	extensions/cornerstone-dicom-pmap/package.json
#	extensions/cornerstone-dicom-seg/package.json
#	extensions/cornerstone-dicom-sr/package.json
#	extensions/cornerstone-dynamic-volume/package.json
#	extensions/cornerstone/package.json
#	extensions/cornerstone/src/services/ViewportService/CornerstoneViewportService.ts
#	extensions/measurement-tracking/package.json
#	extensions/usAnnotation/package.json
#	modes/usAnnotation/package.json
#	platform/app/package.json
#	platform/core/package.json
#	pnpm-lock.yaml
@sedghi

sedghi commented Jul 2, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 2, 2026

Copy link
Copy Markdown

@sedghi I'll review the changes now.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

getSopClassHandlerModule destructured transferDenaturalizedDataset /
fixMultiValueKeys from dicomWebUtils at module-eval time. This module and
@ohif/extension-default form a circular import, so dicomWebUtils can be
undefined at eval time depending on bundler module order; the top-level
destructure then throws (Cannot destructure property ... of dicomWebUtils
as it is undefined) and crashes app boot before the Layout renders, which
surfaced as the Playwright globalSetup warmup timing out on [data-cy=Layout].

Access the utils lazily at call time inside getDICOMwebMetadata instead.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 6

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
extensions/cornerstone/src/services/ViewportService/backends/NextViewportBackend.ts (1)

604-610: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Restore Map-backed LUT presentations for fusion bindings.

mountVolumes replays lutPresentation, but this branch drops Map properties entirely. Native fusion presentations are per display-set/dataId, so overlay PT VOI/colormap/threshold state will not restore on remount/navigation.

Proposed fix
-    const nativeViewport = viewport as unknown as {
-      setDisplaySetPresentation: (props: Record<string, unknown>) => void;
-    };
-    const presentationProps: Record<string, unknown> = {};
-    if (properties.voiRange) {
-      presentationProps.voiRange = properties.voiRange;
-    }
-    if (properties.invert !== undefined) {
-      presentationProps.invert = properties.invert;
-    }
-    if (properties.colormap) {
-      presentationProps.colormap = properties.colormap;
-    }
-    if (Object.keys(presentationProps).length > 0) {
-      nativeViewport.setDisplaySetPresentation(presentationProps);
-    }
+    const nativeViewport = viewport as unknown as {
+      setDisplaySetPresentation: (
+        dataIdOrProps: string | Record<string, unknown>,
+        props?: Record<string, unknown>
+      ) => void;
+    };
+    const toNativePresentationProps = (entry: Record<string, unknown>) => {
+      const presentationProps: Record<string, unknown> = {};
+      for (const key of ['voiRange', 'invert', 'colormap', 'opacity', 'threshold']) {
+        if (entry[key] !== undefined) {
+          presentationProps[key] = entry[key];
+        }
+      }
+      return presentationProps;
+    };
+
+    if (properties instanceof Map) {
+      properties.forEach((entry, dataId) => {
+        const presentationProps = toNativePresentationProps(entry);
+        if (Object.keys(presentationProps).length > 0) {
+          nativeViewport.setDisplaySetPresentation(dataId, presentationProps);
+        }
+      });
+      return;
+    }
+
+    const presentationProps = toNativePresentationProps(properties);
+    if (Object.keys(presentationProps).length > 0) {
+      nativeViewport.setDisplaySetPresentation(presentationProps);
+    }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@extensions/cornerstone/src/services/ViewportService/backends/NextViewportBackend.ts`
around lines 604 - 610, The LUT replay in mountVolumes is currently skipping
Map-backed presentations, which prevents fusion-specific per-display-set state
from being restored. Update the handling around lutPresentation.properties so
NextViewportBackend can replay Map entries for native fusion bindings instead of
returning early, while still preserving the existing non-Map
getDisplaySetPresentation path. Use the mountVolumes logic and the
setDisplaySetPresentation flow in NextViewportBackend as the place to reapply
each dataId/display-set presentation, including VOI/colormap/threshold state.
extensions/cornerstone/src/commandsModule.ts (1)

1288-1317: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Guard missing viewport before setting colormap.

getCornerstoneViewport(viewportId) can return null; this path then calls ops.setColormap(viewport, ...) and may render a missing viewport. Add the same early return used by the other operation-delegated commands.

Proposed fix
       const viewport = cornerstoneViewportService.getCornerstoneViewport(viewportId);
+      if (!viewport) {
+        return;
+      }
 
       let hpOpacity;
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@extensions/cornerstone/src/commandsModule.ts` around lines 1288 - 1317, The
colormap command path in commandsModule.ts does not guard against
getCornerstoneViewport(viewportId) returning null before calling
ops.setColormap. Add the same early return/null-check pattern used by the other
operation-delegated commands in this module so the command exits when the
viewport is missing. Use the getCornerstoneViewport and ops.setColormap flow
here to locate the fix and keep the orthographic/displaySetInstanceUID logic
unchanged.
extensions/cornerstone/src/hooks/useViewportRendering.tsx (1)

289-311: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Don't substitute the display-set UID for an unresolved dataId. getPresentation() and getDefaultVOIRange() already fall back to the active binding when called with no id; passing activeDisplaySetInstanceUID here bypasses that path and can return the wrong or empty VOI/presentation for unresolved legacy volume bindings. Pass dataId through unchanged instead.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@extensions/cornerstone/src/hooks/useViewportRendering.tsx` around lines 289 -
311, The VOI lookup in useViewportRendering is incorrectly substituting
activeDisplaySetInstanceUID when dataId is unresolved, which can bypass the
adapter’s intended fallback behavior. Update the getPresentation and
getDefaultVOIRange calls to pass dataId through unchanged so the viewport
adapter can resolve the active binding correctly for both native and legacy
viewports. Keep the existing voiRange handling in useViewportRendering and the
adapter methods getPresentation/getDefaultVOIRange as the reference points for
the fix.
🧹 Nitpick comments (1)
extensions/cornerstone/src/services/ViewportService/Viewport.ts (1)

230-239: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Reuse the new getViewportDataShapeType helper instead of re-deriving inline.

This ternary (dataShapeType ?? viewportType) is identical to getViewportDataShapeType just added in extensions/cornerstone/src/utils/viewportDataShape.ts. Consolidating avoids the two copies drifting if the discriminator logic ever changes.

♻️ Suggested consolidation
+import { getViewportDataShapeType } from '../../utils/viewportDataShape';
+
 public hasDisplaySet(displaySetInstanceUID: string): boolean {
   let viewportData = this.getViewportData();
-  const dataShapeType = viewportData.dataShapeType ?? viewportData.viewportType;
+  const dataShapeType = getViewportDataShapeType(viewportData);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@extensions/cornerstone/src/services/ViewportService/Viewport.ts` around lines
230 - 239, The viewport data shape discriminator is duplicated here in the
ViewportService logic, so replace the inline `viewportData.dataShapeType ??
viewportData.viewportType` fallback with the shared `getViewportDataShapeType`
helper from `getViewportDataShapeType` in
`extensions/cornerstone/src/utils/viewportDataShape.ts`. Update the code path
around `Viewport` handling to use that helper wherever `dataShapeType` is
needed, keeping the existing `Enums.ViewportType.ORTHOGRAPHIC` and
`Enums.ViewportType.VOLUME_3D` checks unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@extensions/cornerstone/src/services/ViewportService/adapter/LegacyViewportAdapter.ts`:
- Around line 213-239: `LegacyViewportAdapter.copyDisplayedContentTo` only
copies the first volume and global properties, which causes fusion overlays and
per-volume display state to be dropped in captures/downloads. Update this method
to copy every bound volume from the source viewport onto the target, preserving
the full multi-volume stack used by `CornerstoneViewportDownloadForm.tsx`, and
then apply each volume’s specific properties through the target’s per-volume
`setProperties` path instead of a single shared call. Keep the existing view
presentation and view reference փոխանց, but make sure the target viewport is
fully reconstructed with all source volumes and their individual
VOI/colormap/opacity settings.

In
`@extensions/cornerstone/src/services/ViewportService/adapter/NextViewportAdapter.ts`:
- Around line 186-195: The volume lookup in getVoxelManagerForDisplaySet
currently uses an unanchored includes() match on cache.getVolumes(), which can
select the wrong cached volume when another volumeId contains the same
displaySetInstanceUID substring. Update the matching logic in
NextViewportAdapter so it uses an anchored comparison such as
endsWith(displaySetInstanceUID) or an exact volumeId check against the
`${volumeLoaderSchema}:${displaySetInstanceUID}` format, then keep returning the
matched volume’s voxelManager as before.

In
`@extensions/cornerstone/src/services/ViewportService/backends/LegacyViewportBackend.ts`:
- Around line 137-142: The WSI registration added in mountOther via
csUtils.genericViewportDisplaySetMetadataProvider is never released, leaving
stale global state across viewport reuse. Track the displaySetId registration
when adding the GenericViewport metadata entry, then implement the viewport
disable/destroy lifecycle hooks in LegacyViewportBackend to remove that entry
from csUtils.genericViewportDisplaySetMetadataProvider for the same
displaySetId.
- Around line 156-157: The remount camera snapshot in
LegacyViewportBackend.remount is taken too early, before the viewport family
check, so unsupported families can throw instead of returning undefined. Move
the getCamera() access on vp to after the stack/volume guards in remount, and
only snapshot the camera for IStackViewport and IVolumeViewport paths while
preserving the documented no-op behavior for ECG/video/WSI families.

In
`@extensions/cornerstone/src/services/ViewportService/CornerstoneViewportService.ts`:
- Around line 450-452: The persistence logic in CornerstoneViewportService
currently stores a single presentation object for Generic fusion viewports via
getViewportAdapter(csViewport).getPresentation(), which drops overlay-specific
LUT/PT VOI state. Update the code around isVolumeViewportType(csViewport) and
getViewportAdapter(csViewport) so native fusion persists presentation data per
bound display-set/binding, matching the legacy volume path that saves separate
entries. Make sure the saved state includes all bound display-set presentations,
not just one cleanProperties(getPresentation()) result.
- Around line 1391-1398: The remount flow in CornerstoneViewportService should
not assume this.backend.remount always returns a promise. In the remount
handling block, guard the result from remount(...) before calling .then(), since
it may be undefined for viewport families without a remount path. Either return
early when no promise is returned or normalize the value in displaySetPromise
handling so the existing .then() chain only runs when a real promise exists.

---

Outside diff comments:
In `@extensions/cornerstone/src/commandsModule.ts`:
- Around line 1288-1317: The colormap command path in commandsModule.ts does not
guard against getCornerstoneViewport(viewportId) returning null before calling
ops.setColormap. Add the same early return/null-check pattern used by the other
operation-delegated commands in this module so the command exits when the
viewport is missing. Use the getCornerstoneViewport and ops.setColormap flow
here to locate the fix and keep the orthographic/displaySetInstanceUID logic
unchanged.

In `@extensions/cornerstone/src/hooks/useViewportRendering.tsx`:
- Around line 289-311: The VOI lookup in useViewportRendering is incorrectly
substituting activeDisplaySetInstanceUID when dataId is unresolved, which can
bypass the adapter’s intended fallback behavior. Update the getPresentation and
getDefaultVOIRange calls to pass dataId through unchanged so the viewport
adapter can resolve the active binding correctly for both native and legacy
viewports. Keep the existing voiRange handling in useViewportRendering and the
adapter methods getPresentation/getDefaultVOIRange as the reference points for
the fix.

In
`@extensions/cornerstone/src/services/ViewportService/backends/NextViewportBackend.ts`:
- Around line 604-610: The LUT replay in mountVolumes is currently skipping
Map-backed presentations, which prevents fusion-specific per-display-set state
from being restored. Update the handling around lutPresentation.properties so
NextViewportBackend can replay Map entries for native fusion bindings instead of
returning early, while still preserving the existing non-Map
getDisplaySetPresentation path. Use the mountVolumes logic and the
setDisplaySetPresentation flow in NextViewportBackend as the place to reapply
each dataId/display-set presentation, including VOI/colormap/threshold state.

---

Nitpick comments:
In `@extensions/cornerstone/src/services/ViewportService/Viewport.ts`:
- Around line 230-239: The viewport data shape discriminator is duplicated here
in the ViewportService logic, so replace the inline `viewportData.dataShapeType
?? viewportData.viewportType` fallback with the shared
`getViewportDataShapeType` helper from `getViewportDataShapeType` in
`extensions/cornerstone/src/utils/viewportDataShape.ts`. Update the code path
around `Viewport` handling to use that helper wherever `dataShapeType` is
needed, keeping the existing `Enums.ViewportType.ORTHOGRAPHIC` and
`Enums.ViewportType.VOLUME_3D` checks unchanged.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 8ad9ad55-ce7a-4804-bc2d-4052670b7c75

📥 Commits

Reviewing files that changed from the base of the PR and between 880b346 and 812e823.

📒 Files selected for processing (50)
  • extensions/cornerstone-dicom-seg/src/commandsModule.ts
  • extensions/cornerstone/src/Viewport/Overlays/CustomizableViewportOverlay.tsx
  • extensions/cornerstone/src/Viewport/Overlays/ViewportImageScrollbar.tsx
  • extensions/cornerstone/src/Viewport/Overlays/ViewportOrientationMarkers.tsx
  • extensions/cornerstone/src/Viewport/Overlays/ViewportSliceProgressScrollbar/helpers.ts
  • extensions/cornerstone/src/Viewport/Overlays/ViewportSliceProgressScrollbar/hooks.ts
  • extensions/cornerstone/src/commandsModule.ts
  • extensions/cornerstone/src/components/ViewportOrientationMenu/ViewportOrientationMenu.tsx
  • extensions/cornerstone/src/components/ViewportWindowLevel/utils.ts
  • extensions/cornerstone/src/getToolbarModule.tsx
  • extensions/cornerstone/src/hooks/useViewportRendering.tsx
  • extensions/cornerstone/src/index.tsx
  • extensions/cornerstone/src/init.tsx
  • extensions/cornerstone/src/services/ColorbarService/ColorbarService.ts
  • extensions/cornerstone/src/services/CornerstoneCacheService/CornerstoneCacheService.ts
  • extensions/cornerstone/src/services/SegmentationService/SegmentationService.ts
  • extensions/cornerstone/src/services/SegmentationService/backends/ISegmentationBackend.ts
  • extensions/cornerstone/src/services/SegmentationService/backends/NextSegmentationBackend.ts
  • extensions/cornerstone/src/services/ViewportService/CornerstoneViewportService.ts
  • extensions/cornerstone/src/services/ViewportService/Viewport.ts
  • extensions/cornerstone/src/services/ViewportService/adapter/IViewportAdapter.contract.test.ts
  • extensions/cornerstone/src/services/ViewportService/adapter/IViewportAdapter.ts
  • extensions/cornerstone/src/services/ViewportService/adapter/LegacyViewportAdapter.ts
  • extensions/cornerstone/src/services/ViewportService/adapter/NextViewportAdapter.ts
  • extensions/cornerstone/src/services/ViewportService/adapter/getViewportAdapter.ts
  • extensions/cornerstone/src/services/ViewportService/adapter/index.ts
  • extensions/cornerstone/src/services/ViewportService/backends/IViewportBackend.ts
  • extensions/cornerstone/src/services/ViewportService/backends/IViewportOperations.ts
  • extensions/cornerstone/src/services/ViewportService/backends/IViewportServiceInternals.ts
  • extensions/cornerstone/src/services/ViewportService/backends/LegacyViewportBackend.ts
  • extensions/cornerstone/src/services/ViewportService/backends/NextViewportBackend.ts
  • extensions/cornerstone/src/services/ViewportService/backends/NextViewportOperations.ts
  • extensions/cornerstone/src/services/ViewportService/backends/README.md
  • extensions/cornerstone/src/services/ViewportService/backends/dataIdRegistry.ts
  • extensions/cornerstone/src/services/ViewportService/backends/viewportOperations.ts
  • extensions/cornerstone/src/utils/CornerstoneViewportDownloadForm.tsx
  • extensions/cornerstone/src/utils/getCornerstoneViewportType.test.ts
  • extensions/cornerstone/src/utils/getCornerstoneViewportType.ts
  • extensions/cornerstone/src/utils/getDataIdForViewport.ts
  • extensions/cornerstone/src/utils/getLegacyViewportType.ts
  • extensions/cornerstone/src/utils/nextViewportPolicies.ts
  • extensions/cornerstone/src/utils/nextViewports.ts
  • extensions/cornerstone/src/utils/viewportDataShape.ts
  • extensions/default/src/utils/layerConfigurationUtils.ts
  • extensions/tmtv/src/getHangingProtocolModule.ts
  • extensions/tmtv/src/utils/hpViewports.ts
  • platform/app/src/utils/preserveQueryParameters.ts
  • platform/ui-next/src/components/SegmentationTable/SegmentationTableConfig.tsx
  • scripts/check-next-viewport-boundaries.sh
  • tests/Scoord3dProbe.spec.ts
💤 Files with no reviewable changes (4)
  • tests/Scoord3dProbe.spec.ts
  • scripts/check-next-viewport-boundaries.sh
  • platform/ui-next/src/components/SegmentationTable/SegmentationTableConfig.tsx
  • extensions/cornerstone/src/utils/getDataIdForViewport.ts
✅ Files skipped from review due to trivial changes (2)
  • extensions/cornerstone/src/services/ViewportService/backends/README.md
  • extensions/tmtv/src/utils/hpViewports.ts
🚧 Files skipped from review as they are similar to previous changes (15)
  • extensions/tmtv/src/getHangingProtocolModule.ts
  • extensions/cornerstone/src/services/ViewportService/backends/IViewportServiceInternals.ts
  • extensions/cornerstone/src/Viewport/Overlays/ViewportOrientationMarkers.tsx
  • extensions/cornerstone/src/Viewport/Overlays/ViewportImageScrollbar.tsx
  • extensions/cornerstone/src/init.tsx
  • extensions/cornerstone/src/Viewport/Overlays/CustomizableViewportOverlay.tsx
  • extensions/cornerstone/src/services/ViewportService/backends/IViewportOperations.ts
  • extensions/cornerstone/src/services/SegmentationService/backends/ISegmentationBackend.ts
  • extensions/cornerstone/src/services/SegmentationService/backends/NextSegmentationBackend.ts
  • extensions/cornerstone/src/services/ViewportService/backends/dataIdRegistry.ts
  • extensions/cornerstone/src/services/CornerstoneCacheService/CornerstoneCacheService.ts
  • extensions/cornerstone/src/utils/getCornerstoneViewportType.test.ts
  • extensions/cornerstone/src/utils/getCornerstoneViewportType.ts
  • extensions/cornerstone-dicom-seg/src/commandsModule.ts
  • extensions/cornerstone/src/services/SegmentationService/SegmentationService.ts

Comment on lines +213 to +239
async copyDisplayedContentTo(target: CoreTypes.IViewport): Promise<void> {
const targetViewport = target as unknown as LegacyViewport;
const viewRef = this.viewport.getViewReference?.();

// - properties: VOI, colormap, interpolation, etc.
// - viewPresentation: flip/rotate/zoom presentation state (preserves flip/rotate)
const properties = this.viewport.getProperties?.();
const viewPresentation = this.viewport.getViewPresentation?.();

if (isStackViewportType(targetViewport)) {
const imageId = this.viewport.getCurrentImageId?.();
await targetViewport.setStack?.([imageId]);
} else if (isVolumeViewportType(targetViewport)) {
const volumeIds = this.getVolumeIds();
await targetViewport.setVolumes?.([{ volumeId: volumeIds[0] }]);
}

if (viewPresentation && targetViewport.setViewPresentation) {
targetViewport.setViewPresentation(viewPresentation);
}

targetViewport.setProperties?.(properties);

if (viewRef && targetViewport.setViewReference) {
targetViewport.setViewReference(viewRef);
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Fusion overlay lost in captured/downloaded image for multi-volume viewports.

copyDisplayedContentTo only mounts volumeIds[0] on the target and applies a single, non-per-volume getProperties() call. For a legacy PT/CT fusion (multi-volume) viewport, this drops the overlay volume entirely from the target, and even if it were mounted, its per-volume colormap/VOI/opacity/threshold wouldn't be copied (since setProperties is called without a volumeId). The download/capture form (CornerstoneViewportDownloadForm.tsx) depends on this method to reproduce the source's full displayed state, so fusion captures will silently lose the overlay.

This is consistent with the PR's own review note that native fusion LUT presentation capture around the download path is incomplete — the legacy lane has the analogous gap.

🐛 Proposed fix to copy all bound volumes and their per-volume properties
     if (isStackViewportType(targetViewport)) {
       const imageId = this.viewport.getCurrentImageId?.();
       await targetViewport.setStack?.([imageId]);
+      targetViewport.setProperties?.(properties);
     } else if (isVolumeViewportType(targetViewport)) {
       const volumeIds = this.getVolumeIds();
-      await targetViewport.setVolumes?.([{ volumeId: volumeIds[0] }]);
+      await targetViewport.setVolumes?.(volumeIds.map(volumeId => ({ volumeId })));
+      volumeIds.forEach(volumeId => {
+        const volumeProperties = this.viewport.getProperties?.(volumeId);
+        if (volumeProperties) {
+          targetViewport.setProperties?.(volumeProperties, volumeId);
+        }
+      });
     }

     if (viewPresentation && targetViewport.setViewPresentation) {
       targetViewport.setViewPresentation(viewPresentation);
     }

-    targetViewport.setProperties?.(properties);
-
     if (viewRef && targetViewport.setViewReference) {
       targetViewport.setViewReference(viewRef);
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
async copyDisplayedContentTo(target: CoreTypes.IViewport): Promise<void> {
const targetViewport = target as unknown as LegacyViewport;
const viewRef = this.viewport.getViewReference?.();
// - properties: VOI, colormap, interpolation, etc.
// - viewPresentation: flip/rotate/zoom presentation state (preserves flip/rotate)
const properties = this.viewport.getProperties?.();
const viewPresentation = this.viewport.getViewPresentation?.();
if (isStackViewportType(targetViewport)) {
const imageId = this.viewport.getCurrentImageId?.();
await targetViewport.setStack?.([imageId]);
} else if (isVolumeViewportType(targetViewport)) {
const volumeIds = this.getVolumeIds();
await targetViewport.setVolumes?.([{ volumeId: volumeIds[0] }]);
}
if (viewPresentation && targetViewport.setViewPresentation) {
targetViewport.setViewPresentation(viewPresentation);
}
targetViewport.setProperties?.(properties);
if (viewRef && targetViewport.setViewReference) {
targetViewport.setViewReference(viewRef);
}
}
async copyDisplayedContentTo(target: CoreTypes.IViewport): Promise<void> {
const targetViewport = target as unknown as LegacyViewport;
const viewRef = this.viewport.getViewReference?.();
// - properties: VOI, colormap, interpolation, etc.
// - viewPresentation: flip/rotate/zoom presentation state (preserves flip/rotate)
const properties = this.viewport.getProperties?.();
const viewPresentation = this.viewport.getViewPresentation?.();
if (isStackViewportType(targetViewport)) {
const imageId = this.viewport.getCurrentImageId?.();
await targetViewport.setStack?.([imageId]);
targetViewport.setProperties?.(properties);
} else if (isVolumeViewportType(targetViewport)) {
const volumeIds = this.getVolumeIds();
await targetViewport.setVolumes?.(volumeIds.map(volumeId => ({ volumeId })));
volumeIds.forEach(volumeId => {
const volumeProperties = this.viewport.getProperties?.(volumeId);
if (volumeProperties) {
targetViewport.setProperties?.(volumeProperties, volumeId);
}
});
}
if (viewPresentation && targetViewport.setViewPresentation) {
targetViewport.setViewPresentation(viewPresentation);
}
if (viewRef && targetViewport.setViewReference) {
targetViewport.setViewReference(viewRef);
}
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@extensions/cornerstone/src/services/ViewportService/adapter/LegacyViewportAdapter.ts`
around lines 213 - 239, `LegacyViewportAdapter.copyDisplayedContentTo` only
copies the first volume and global properties, which causes fusion overlays and
per-volume display state to be dropped in captures/downloads. Update this method
to copy every bound volume from the source viewport onto the target, preserving
the full multi-volume stack used by `CornerstoneViewportDownloadForm.tsx`, and
then apply each volume’s specific properties through the target’s per-volume
`setProperties` path instead of a single shared call. Keep the existing view
presentation and view reference փոխանց, but make sure the target viewport is
fully reconstructed with all source volumes and their individual
VOI/colormap/opacity settings.

Comment on lines +186 to +195
getVoxelManagerForDisplaySet(
displaySetInstanceUID: string
): { getRange?: () => [number, number]; [key: string]: unknown } | undefined {
// No getAllVolumeIds/getImageData(volumeId); resolve the display set's volume
// from the cornerstone cache instead.
const volume = cache.getVolumes().find(v => v.volumeId?.includes(displaySetInstanceUID));
return volume?.voxelManager as unknown as
| { getRange?: () => [number, number]; [key: string]: unknown }
| undefined;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find where native volumeIds are constructed to confirm the naming convention is collision-safe.
rg -n "volumeId" extensions/cornerstone/src/services/ViewportService/backends -A3 -B3

Repository: OHIF/Viewers

Length of output: 18470


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the native backend where volumeIds are passed through or generated.
ast-grep outline extensions/cornerstone/src/services/ViewportService/backends/NextViewportBackend.ts --view expanded

echo "---- references to volumeId / displaySetInstanceUID in backend ----"
rg -n "volumeId|displaySetInstanceUID|getVolumes\(|includes\(displaySetInstanceUID\)" \
  extensions/cornerstone/src/services/ViewportService/backends/NextViewportBackend.ts \
  extensions/cornerstone/src/services/ViewportService/backends -A3 -B3

echo "---- file that uses the lookup in question ----"
sed -n '180,205p' extensions/cornerstone/src/services/ViewportService/adapter/NextViewportAdapter.ts

echo "---- data-id registration semantics ----"
sed -n '1,120p' extensions/cornerstone/src/services/ViewportService/backends/dataIdRegistry.ts

Repository: OHIF/Viewers

Length of output: 50368


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Find where volumeIds are created or normalized, and whether they embed the displaySetInstanceUID
# with a delimiter/prefix that makes substring matching unambiguous.
rg -n "volumeId\s*[:=]|create.*volumeId|make.*volumeId|volumeId.*displaySetInstanceUID|displaySetInstanceUID.*volumeId|::overlay|cached volumeId|already-cached volumeId" \
  extensions/cornerstone/src/services -A3 -B3

echo "---- dataIdRegistry (focused) ----"
sed -n '35,90p' extensions/cornerstone/src/services/ViewportService/backends/dataIdRegistry.ts

echo "---- NextViewportBackend volume-mount area (focused) ----"
sed -n '413,490p' extensions/cornerstone/src/services/ViewportService/backends/NextViewportBackend.ts

Repository: OHIF/Viewers

Length of output: 29745


Use an anchored volumeId match here. volumeId is built as ${volumeLoaderSchema}:${displaySetInstanceUID}, so includes() can still return the wrong cached volume when another id contains the same UID substring. endsWith(displaySetInstanceUID) or an exact id compare would be safer.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@extensions/cornerstone/src/services/ViewportService/adapter/NextViewportAdapter.ts`
around lines 186 - 195, The volume lookup in getVoxelManagerForDisplaySet
currently uses an unanchored includes() match on cache.getVolumes(), which can
select the wrong cached volume when another volumeId contains the same
displaySetInstanceUID substring. Update the matching logic in
NextViewportAdapter so it uses an anchored comparison such as
endsWith(displaySetInstanceUID) or an exact volumeId check against the
`${volumeLoaderSchema}:${displaySetInstanceUID}` format, then keep returning the
matched volume’s voxelManager as before.

Comment on lines +137 to +142
const webClient = metaData.get(csEnums.MetadataModules.WADO_WEB_CLIENT, displaySetId);
csUtils.genericViewportDisplaySetMetadataProvider.add(displaySetId, {
imageIds: displaySet.imageIds,
kind: 'wsi',
options: { webClient },
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Release legacy WSI provider registrations.

mountOther adds a WSI entry to the global GenericViewport metadata provider, but the lifecycle hooks remain no-ops. Track these registrations and remove them on viewport disable/destroy to avoid stale provider state across viewport reuse.

Also applies to: 244-254

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@extensions/cornerstone/src/services/ViewportService/backends/LegacyViewportBackend.ts`
around lines 137 - 142, The WSI registration added in mountOther via
csUtils.genericViewportDisplaySetMetadataProvider is never released, leaving
stale global state across viewport reuse. Track the displaySetId registration
when adding the GenericViewport metadata entry, then implement the viewport
disable/destroy lifecycle hooks in LegacyViewportBackend to remove that entry
from csUtils.genericViewportDisplaySetMetadataProvider for the same
displaySetId.

Comment on lines 450 to +452
const properties = isVolumeViewportType(csViewport)
? new Map()
: cleanProperties(csViewport.getProperties());
: cleanProperties(getViewportAdapter(csViewport).getPresentation());

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Persist native fusion LUT state per binding, not as one presentation object.

For Generic volume/fusion viewports, isVolumeViewportType(csViewport) can be false, so this path stores only getViewportAdapter(csViewport).getPresentation(). That loses overlay-specific PT VOI/colormap/threshold across navigation; capture all bound display-set presentations for native fusion the same way legacy volumes persist per-volume entries.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@extensions/cornerstone/src/services/ViewportService/CornerstoneViewportService.ts`
around lines 450 - 452, The persistence logic in CornerstoneViewportService
currently stores a single presentation object for Generic fusion viewports via
getViewportAdapter(csViewport).getPresentation(), which drops overlay-specific
LUT/PT VOI state. Update the code around isVolumeViewportType(csViewport) and
getViewportAdapter(csViewport) so native fusion persists presentation data per
bound display-set/binding, matching the legacy volume path that saves separate
entries. Make sure the saved state includes all bound display-set presentations,
not just one cleanProperties(getPresentation()) result.

sedghi added 2 commits July 2, 2026 12:31
- CornerstoneViewportService.updateViewport: backend.remount() is typed
  Promise<void> | undefined and returns undefined for viewport families with
  no re-mount path; guard before .then() so those families no longer throw.
- LegacyViewportBackend.remount: take the camera snapshot inside the volume
  branch (the only consumer) instead of before the family checks, so families
  without a camera surface no-op safely and the stack path skips a dead call.

Addresses CodeRabbit review findings on PR #6101.
Mirrors the ?useNextViewports opt-in: a cpu URL query param overrides
appConfig.useCPURendering per-session (?cpu, =true, or =1 enable it).
Wired through cornerstone.setUseCPURendering in init, whose global flag is
consulted by the GenericViewport PlanarRenderPathDecisionService for both
the image (CPU_IMAGE) and volume (CPU_VOLUME) paths, so under
useNextViewports a single ?cpu=true forces the next viewport onto CPU.

Extracted the shared query-param parsing into resolveBooleanUrlOptIn.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ohif-integration Causes a CS3D/OHIF integraiton build to be run

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant