cornerstone3d redo viewports

[sedghi]

ohif_ref: feat/use-beta-5.0-cs3d

Why This Is Long Overdue

Cornerstone's viewport layer has been carrying too many responsibilities for too long. The existing StackViewport, VolumeViewport, VideoViewport, WSI, ECG, CPU, VTK, and segmentation paths all grew around real product needs, but the ownership boundaries became blurry: viewport classes load data, choose render implementations, preserve camera state, manage actors, project overlays, dispatch image events, and special-case segmentation behavior.

That made common workflows harder than they should be. A stack image and a volume slice can represent the same plane but still travel through different APIs. Fusion overlays and labelmaps need to know whether the base viewport is stack-like, volume-like, CPU-backed, or VTK-backed. Camera fields are used both as user-facing navigation state and renderer commands. Adding a new rendering path requires touching viewport behavior that should not care how the pixels are drawn.

This PR introduces the Generic Viewport architecture to cleanly separate those concerns. The new model keeps the existing rendering power, but moves it behind explicit boundaries: logical data ids, data providers, render paths, viewport data bindings, semantic view state, and per-data presentation. It is also designed for incremental adoption through compatibility adapters instead of forcing every existing example and tool to migrate at once.

How To Try It

Existing examples can be routed through the Next viewport adapters with:

• ?type=next to enable rendering.useViewportNext
• ?type=next&cpu=true to force CPU planar rendering paths where supported

There are also dedicated Next examples for stack, volume slice, scale/position, video, ECG, WSI, multi-volume, annotation tools, and labelmap workflows.

Viewport

This adds the new viewport family and compatibility path:

• ViewportType.PLANAR_NEXT for 2D stack-like and volume-slice workflows
• ViewportType.VIDEO_NEXT, ECG_NEXT, WHOLE_SLIDE_NEXT, and VOLUME_3D_NEXT
• compatibility adapters so legacy STACK, ORTHOGRAPHIC, VIDEO, ECG, WHOLE_SLIDE, and VOLUME_3D inputs can be backed by Next implementations when useViewportNext is enabled

The main change is PlanarViewport: one viewport model for stack images, volume slices, CPU image rendering, CPU volume sampling, VTK image rendering, and VTK volume-slice rendering. Applications bind logical data; the viewport and render-path decision service choose the runtime path from the data shape, orientation, rendering config, thresholds, and runtime support.

Viewport data is now mounted as source or overlay bindings. The viewport owns binding order and navigation. Each binding owns its runtime resources and receives view/presentation updates without forcing application code to reach into actors and mappers.

Camera

Next viewports use semantic viewState as durable navigation state instead of treating VTK-style camera fields as the source of truth.

For planar rendering, view state tracks concepts like orientation, slice identity, anchor world/canvas points, scale, rotation, flips, and display area. A computed ResolvedView projects that state into whatever the active render path needs: VTK camera fields, CPU canvas transforms, canvas/world conversion, or overlay sampling information.

Legacy camera APIs remain available at the adapter boundary. Existing tools and synchronizers can still ask for an ICamera-compatible shape, while the clean Next path avoids persisting renderer-specific camera commands as viewport truth. 3D and WSI keep their runtime-specific camera behavior where that is the right ownership model.

Loading

Loading now starts from logical data ids instead of direct actor or viewport-type-specific setup.

A data id is registered with metadata such as image ids, volume id, acquisition orientation, or semantic reference information. The viewport asks its data provider to resolve that id, the render-path resolver selects the matching runtime implementation, and the render path returns a binding. Calls like setDataList, setData, and addData then operate on mounted data ids.

This separates four things that were previously tangled together:

• loading source data
• deciding CPU vs GPU and image vs volume-slice render paths
• mounting runtime resources
• updating view state and per-data presentation

Per-data presentation now lives on the binding: VOI, opacity, colormap, blend mode, interpolation, visibility, and related settings can be updated independently for the source and each overlay.

Segmentation

Segmentation needed to move with the viewport work because labelmaps are the hardest overlay case. The old paths had separate behavior for stack labelmap images, volume labelmap actors, overlap handling, and special volume-viewport image-mapper rendering.

This PR introduces a normalized labelmap model with explicit labelmap layers and segment bindings. A segment can be associated with a labelmap layer and label value, which gives us a clearer foundation for stack-backed labelmaps, volume-backed labelmaps, overlapping segments, and future multi-layer behavior.

Labelmap display now builds a render plan for the viewport. It can use the legacy stack/volume actor paths where needed, or mount compatible labelmaps as slice-rendered overlay data for Next/volume workflows when useSliceRendering is enabled. The important architectural change is that segmentation overlays are treated as viewport data bindings instead of independent actor side effects whenever the viewport can support that path.

The PR also updates brush/edit helpers, labelmap update listeners, actor styling, overlap utilities, examples, and screenshot coverage so segmentation can exercise both legacy compatibility and the Next rendering paths.

Docs And Tests

Added documentation for the Next Viewport concepts, API, camera model, data bindings/loading, render paths, migration notes, and viewport accessor migration.

Coverage includes focused unit tests, browser/Playwright coverage, compatibility-mode screenshots, and dedicated Next examples for core viewport behavior and segmentation workflows.

Checklist

• My Pull Request title is descriptive, accurate and follows the semantic-release format and guidelines.
• My code has been well-documented (function documentation, inline comments, etc.)
• The documentation page has been updated as necessary for any public API additions or removals.

[wayfarer3130]

I had claude look through the review - will make notes prefixed with BW> in some of these:

Review: PR #2666 — cornerstone3d redo viewports

Author: sedghi · Base: beta · State: open · Scope: 966 files, +62,638 / −4,593 · Companion: OHIF feat/use-beta-5.0-cs3d

Disclaimer: at this size no single review catches everything. I focused on the new viewport/camera architecture, OHIF impact, and the items explicitly flagged in the request (long-term API stability + performance). I read the architecture, camera/view-state, adapters, and CPU/VTK render-path code closely; the rest of the diff (codemods, examples, screenshots, segmentation strategies, tests) I only skimmed.

---

1. What this PR does

Introduces a "Generic Viewport" / ViewportNext family that re-layers the rendering stack:

DataProvider  →  RenderPathResolver  →  RenderPath  →  MountedData / Binding
                                                ↘  ResolvedView (camera projection)

• New viewport classes: PlanarViewport (replaces stack + volume-slice), ECGViewport, VideoViewport, VolumeViewport3DV2, WSIViewport — all under packages/core/src/RenderingEngine/ViewportNext/.
• New ViewportType enum members: PLANAR_NEXT, VIDEO_NEXT, ECG_NEXT, WHOLE_SLIDE_NEXT, VOLUME_3D_NEXT.
• Camera split into three layers: semantic PlanarViewState → geometric PlanarSliceBasis → renderer ICamera, plumbed through ResolvedViewportView.
• Legacy adapters (e.g. PlanarViewportLegacyAdapter, PlanarLegacyCompatibilityController) so the legacy STACK / ORTHOGRAPHIC / VIDEO / ECG / WHOLE_SLIDE / VOLUME_3D types can be backed by Next when rendering.useViewportNext is on.
• Segmentation reworked around a labelmap render plan and "binding" model; brush/edit helpers, overlap utilities, and label rendering all touched.
• Adds packages/docs/docs/concepts/cornerstone-core/next-viewport/ and a migration guide.

The high-level layering is good and overdue. The criticism below is about edges, not the direction.

---

2. Long-term stability of the new viewport / camera API

2.1 The author has explicitly listed unfinished renames

tests/todo.md (new file) reads, verbatim:

[] - rename viewport next -> generic viewport
[] - rename the data id to displaySets
[] - rename 3dViewport to viewport3D
[] why we have render mode in options

BW> These need updating before commit.

2.2 The new public surface is enormous and exports implementation detail

packages/core/src/RenderingEngine/ViewportNext/index.ts re-exports ~70 symbols. Many are render-path internals: PlanarViewportRenderContext, PlanarPayload, PlanarEffectiveRenderMode, PlanarRegisteredDataSet, PlanarDataLoadOptions, CpuImageSlicePath, VtkImageMapperPath, etc. These appear in user-facing function signatures (e.g. the DataProvider interface forces consumers to traffic in PlanarPayload).

Once these are exported they are effectively frozen. Recommendations:

• Mark with @internal JSDoc and exclude from the type rollup, OR
• Hide them behind a narrower public surface (e.g. only the viewport classes, the RenderPath / DataProvider interfaces, and a small set of *Input / *Presentation types).

BW> I definitely have some concerns about the size of the API surface. We should try reducing the size of the API surface at least some. I also wonder if the camera representation should still be made a bit more abstract/re-useable without needing to tweak the camera round trip stuff. Sometimes for things like figuring out the zoom, to actually use, there are several ways to get it, and I don't quite always know how to declare that in an extensible fashion. It would be very nice to not require the current scale values, but be able to have a pluggable mechanism that works across several types of image areas to create new types of scaling. The same with positioning. The current design feels close, but not QUITE right yet.

2.3 ICamera no longer round-trips on PlanarViewport

The most consequential stability concern. The legacy contract downstream tools rely on is:

const c = vp.getCamera(); vp.setCamera(c);     // identity

For PlanarViewport / PlanarViewportLegacyAdapter this is no longer true:

• getViewState() returns aliased internal arrays. packages/core/src/RenderingEngine/ViewportNext/Planar/PlanarViewport.ts L784-L789 returns { ...this.viewState, displayArea: clone(displayArea) }. Everything else — anchorCanvas, anchorWorld, scale, slice.sliceWorldPoint — is shared by reference. A consumer that mutates state.scale[0] = 2 mutates the viewport.
  BW> High area of concern
• Bogus fallback camera. PlanarViewport.ts L1728-L1745 returns hard-coded values (position: [0,0,1], viewUp: [0,-1,0]) when no data is mounted. These ride out as previousCamera in CAMERA_MODIFIED events. The first event after data loads will report a bogus delta.
• setViewPresentation does two writes. PlanarViewport.ts L884-L930 calls setViewState and then setPan, which is itself a second setViewState. Two events / two render requests per logical write. Worse, pan is scaled by nextScale while getViewPresentation divides by currentScale (L862-L865). If the consumer round-trips through get/set with zoom changing in between, the pan lands at a different point.
  BW> This is kind of intrinsic in the fact that pan is scaled by the scale values so I'm not quite sure how to deal with that. IN many ways, pan is an artificial construct - really useful for users to understand, but not obvious what it always means.
• projectAnchorWorldToCurrentPlane is not invertible by design. planarRenderCamera.ts L179-L198 — a zoom-to-cursor anchor placed on slice N gets projected onto slice M when the user scrolls. Going back to slice N does not restore the original anchor. Documented in the file comment but deserves to be in the migration guide too — cine/sync that stores and replays camera state across slices will see drift.
• resetCamera({ resetPan, resetZoom }) in PlanarViewport.ts L1176-L1193 silently does not reset orientation, flips, or slice. Legacy StackViewport.resetCamera resets to the default acquisition orientation. Behavior divergence to call out.
• setPan fallback in PlanarViewport.ts L725-L749, when there is no resolved view, uses currentPan = getPan() which itself returns [0,0] in the fallback path — so the "delta" is nextPan - 0, wrong if the viewport already has a non-default anchor.

These are not theoretical. OHIF stores getViewPresentation() in a Zustand store and replays it on layout change — see §3.

2.4 Camera projection helpers exist but are not exported

resolvePlanarICamera, derivePlanarPresentation, applyPlanarICameraToRenderer are in planarRenderCamera.ts but not in the top-level barrel. A downstream consumer that wants to compute a camera from a view-state (e.g. a custom synchronizer) has no stable entry point. Either commit to exporting them or make them genuinely internal — right now they're advertised in the type system (PlanarResolvedICamera is exported) without being constructible.

BW> These should be exported as helper modules, probably indicating that they are a little less stable than the core behaviour.

BW> Would it be possible to make an external handler/plugin to allow differentiating between camera types in a generic fashion? Something like being able to get the camera of a given type given the current parameters, or the underlying/base camera type. That would allow pluging in the transforms earlier by keyword/type and being able to transform bidirectionally on different viewport types things like the stack viewport camera types versus the 3d camera types. That might be a way to get different types of pan values. Not quite sure this works, but if it does, I think it would resolve some of the export size differences since there would be mroe smaller exports, but they would allow for better re-use.

2.5 ResolvedViewportView snapshots aren't cached, but tools call them in tight loops

PlanarViewport.getResolvedView() (PlanarViewport.ts L795-L810) calls resolvePlanarViewportView and constructs a fresh PlanarStackResolvedView or PlanarVolumeResolvedView on every call. The class caches sliceBasis and presentation per-instance, but the instance is thrown away the moment the caller returns.

canvasToWorld, worldToCanvas, getZoom, getPan, getRotation, getScale, getViewPresentation, getCameraForEvent all call getResolvedView() — frequently more than once per call (getViewPresentation calls getZoom, getScale, getPan, getRotation separately and each rebuilds). For a mouse-drag tool that calls canvasToWorld twice plus worldToCanvas once per move event, you're doing 3+ resolvePlanarViewportView constructions per move, each of which builds a PlanarSliceBasis from scratch (gl-matrix allocations, orientation math).

This is the kind of regression that doesn't show up in screenshot tests but shows up as scroll/pan/window-level latency on mid-range hardware. Cache the resolved view on the viewport, invalidate on setViewState / resize / binding change.

BW> Definitely a concern in mouse move etc. You will need to test on slower hardware.

2.6 Two-layer state machine in the legacy adapter

PlanarLegacyCompatibilityController (~830 lines) maintains its own properties, globalDefaultProperties, perImageIdDefaultProperties maps keyed by dataId and imageId. This is parallel to data state owned by PlanarMountedData and view state owned by PlanarViewport. No single source of truth, and the divergence cases are real:

• Mixing legacy setStack() with modern addData() is undefined.
• perImageIdDefaultProperties grows unbounded as users scroll a long stack (cleared on destroy() so not a permanent leak, but real memory pressure during long sessions).
• Per-imageId caches are tempting for window/level memory, but unless they participate in cache eviction, they tie viewport memory to study size.

Recommend either documenting "do not mix legacy and modern APIs on the same viewport" loudly, or unifying them.

---

3. OHIF impact

OHIF (extensions/cornerstone*) currently has:

• 21+ instanceof StackViewport | VolumeViewport | VolumeViewport3D checks across commandsModule.ts, CornerstoneViewportService.ts, useViewportRendering.tsx, CornerstoneViewportDownloadForm.tsx, several overlay components.
• ~57 viewport.type === ViewportType.STACK | ORTHOGRAPHIC | VOLUME_3D checks in 16 files.
• Deep dependence on getCamera() / setCamera() (74 call sites), getViewReference() / setViewReference(), getViewPresentation() / setViewPresentation(). The presentation API is canonical in OHIF — usePositionPresentationStore / useLutPresentationStore / useSegmentationPresentationStore persist these objects across layout changes.
• Segmentation logic that branches ViewportType.VOLUME_3D ? Surface : Labelmap (e.g. SegmentationService.ts:1560-1565, createHydrateSegmentationSynchronizer.ts:91-95), and reaches into representationData[Labelmap].imageIds to feed DICOM SEG export.

When useViewportNext is on, OHIF instantiates PlanarViewport (or a *LegacyAdapter thereof) which:

• Does not extend StackViewport or VolumeViewport — every instanceof becomes false. The legacy adapter is a separate class.
• Reports type = PLANAR_NEXT (or whatever *_NEXT value the adapter forwards). Every viewport.type === ViewportType.STACK becomes false unless explicit mapping is added in OHIF.
• Exposes getViewPresentation / setViewPresentation but with the round-trip caveats in §2.3 — the OHIF presentation-store replay path is exactly the case that suffers most.

Cornerstone3D itself has 51 instanceof checks across 36 internal files (grep on (StackViewport|VolumeViewport|VolumeViewport3D)). These are addressed inside this PR via the adapter classes and viewportTypeToViewportClass.ts rewrites. OHIF's 21+ external checks are not addressed — they will need a companion OHIF PR before useViewportNext flips on by default.

Suggested actions for cs3d → OHIF compatibility:

1. Document a "viewport capability" pattern. The PR introduces viewportHasZoom, viewportHasPan, viewportHasFrameOfReferenceUID, viewportHasCanvasWorldTransform guard functions — good! Push OHIF to use these instead of instanceof. The guard functions should be the canonical recommendation in the migration guide.
2. Provide a "behaves-like-stack" / "behaves-like-volume" predicate so OHIF can map PLANAR_NEXT to its existing branches without inventing its own logic.
3. Lock down the getViewPresentation / setViewPresentation round-trip contract with a test that does set(get()) on a viewport with non-trivial pan + zoom + rotation + flip and asserts identity. Right now it doesn't.

---

4. Concrete bugs / smells

Likely bugs

• Aliased viewState arrays leak from getViewState() — PlanarViewport.ts L784-L789. Only displayArea is cloned; scale, anchorCanvas, anchorWorld, slice.sliceWorldPoint are shared by reference.
• Bogus fallback camera in getCameraForEvent() — PlanarViewport.ts L1735-L1745 — when no resolved view. Those values ride into CAMERA_MODIFIED payloads as previousCamera.
• setPan fallback uses currentPan = [0,0] instead of the actual anchor-derived pan — PlanarViewport.ts L735.
• setViewPresentation fires two state writes — PlanarViewport.ts L925-L929 → two CAMERA_MODIFIED events, two render requests.
• renderImageObject collisions — PlanarViewport.ts L405-L416 registers metadata under image.imageId with no de-duplication; back-to-back calls overwrite silently.
• displayArea.scaleMode precedence is subtle — planarRenderCamera.ts L127-L129 and planarViewState.ts L68-L70. displayArea.scaleMode silently overrides viewState.scaleMode. Setting either one alone is fine; setting both can produce surprising results.
• No validation that user-supplied OrientationVectors are orthonormal in clonePlanarOrientation. Bad input renders silently wrong.

Smells

• Constructor throws after partial DOM mutation. this.element.style.position/overflow/background are written (PlanarViewport.ts L141-L143) before the renderer check (L177-L181). Leaves a host element in a half-initialized state if construction fails.
• Many as unknown as ICamera / as unknown as vec3 casts — fine in isolation but indicate the type system is not actually checking the shape. PlanarResolvedICamera extends ICamera with presentationScale, scale, scaleMode, flipHorizontal, flipVertical — either ICamera should be updated, or the resolved type should not be cast back.
  BW> For the vec3, suggest defining a new point type that is valid input for both vec3 methods/results and valid for passing as a Point
• time.txt (4640 lines) is checked in at repo root. Looks like a Playwright JSON dump from the author's machine (path /Users/alireza/...). Strip before merge.
• tests/todo.md ships TODOs in the repo. Move to the PR description or an issue.
• PlanarViewport.type === PLANAR_NEXT is hard-coded (L109) but the legacy adapters report STACK / ORTHOGRAPHIC / etc. This duality is what every consumer's viewport.type === ... branch will hit; document the rule plainly.
• createHydrateSegmentationSynchronizer uses an OHIF-flavored Surface/Labelmap branch that is mirrored inside cs3d's labelmap render plan. The PR adds significant complexity here; would want explicit test coverage for stack-labelmap with the lazy brush path (added in LazyBrushEditController.ts), particularly the failure modes documented there.

Performance hotspots worth profiling before merge

• CPU slice samplers allocate fresh typed arrays per render frame — PlanarCPUVolumeSampler.ts L550, L755 — new SliceArrayConstructor(width * height * components) on every sample. For 512² × 1 component that's ~1 MB / frame; scrolling produces a steady stream into GC. Pool by (w, h, components) shape.
• Per-pixel scalar loop with Math.min/Math.max calls inline — PlanarCPUVolumeSampler.ts L585-L617. Already has a deferred path (DEFERRED_VIEWPORT_RESAMPLE_DELAY_MS); promote it for large viewports or offload to a worker.
• drawImageSync redraws full canvas — CpuVolumeSliceRenderPath.ts L466 — regardless of whether anything but pan/zoom changed. The renderingInvalidated flag is plumbed but doesn't gate the actual pixel write.
• getResolvedView() rebuild storm discussed in §2.5.

VTK actor lifecycle and event-listener cleanup do look correct (removeStreamingSubscriptions?.(), removeEventListener on IMAGE_VOLUME_MODIFIED, bindings.clear() in destroy). No leak found there.

---

5. Tests

• The diff adds a large amount of test infrastructure: Playwright next-screenshot-server.py, git-image-diff-server.py, checkForCanvasSnapshot.ts, compatibilityMode.ts, expectViewportNextRuntime.ts. Good.
• New nextStackApi.browser.test.ts (Vitest browser). Good.
• However: I did not see an explicit set(get()) identity test for getCamera/setCamera or getViewPresentation/setViewPresentation on PlanarViewport. Given §2.3, this is the single test I would add. A property-based fuzzer over pan × zoom × rotation × flip × scaleMode would catch the rounding/aliasing/scaling issues above.
• 51 internal instanceof checks; the diff updates many, but viewportTypeToViewportClass.ts is the only place I see that systematically routes new types. A grep audit for remaining instanceof StackViewport against Next adapters would be worthwhile.

---

6. Concrete recommendations before merging

1. Settle the renames in tests/todo.md — or commit publicly that the current names are final. Don't ship ViewportNext, dataId, and VolumeViewport3DV2 as the stable names if you intend to change them.
2. Strip time.txt and tests/todo.md from the diff.
3. Add a getCamera / setCamera and getViewPresentation / setViewPresentation round-trip test on PlanarViewport with non-trivial pan + zoom + rotation + flip + scaleMode. Document the cross-slice anchor-projection caveat explicitly in the migration guide.
4. Deep-clone what getViewState() returns (PlanarViewport.ts L784-L789). Cheap fix, prevents quiet state corruption.
5. Cache getResolvedView() on the viewport instance with invalidation tied to viewState / bindings / resize. This is the single biggest perf win for tool interaction; right now every tool that touches canvasToWorld per mouse-move rebuilds the slice basis several times.
6. Pool typed arrays in PlanarCPUVolumeSampler (see hotspots above).
7. Batch the two state writes in setViewPresentation into one setViewState({ ...nextCamera, anchorCanvas }).
8. Fix the setPan no-resolved-view path and the bogus fallback ICamera in getCameraForEvent. Or have those methods throw / return undefined instead of pretending to work pre-load.
9. Mark internal types @internal (PlanarPayload, PlanarViewportRenderContext, PlanarEffectiveRenderMode, PlanarRegisteredDataSet, *Path class names) to give yourself room to evolve without a breaking change.
10. OHIF coordination: ship a small "viewport-capability check" migration recipe alongside the feat/use-beta-5.0-cs3d PR — OHIF's ~21 instanceof checks and ~57 viewport.type === STACK checks are the long pole for useViewportNext ever becoming the default. The capability guard functions (viewportHasZoom, etc.) already exist in this PR; just need them threaded into OHIF.

---

7. Verdict

The architectural shape (DataProvider → RenderPathResolver → RenderPath → Binding, with semantic view state separated from renderer camera) is the right move and worth doing. The implementation is mostly thorough, the legacy adapter approach is the right migration strategy, and the docs/tests scaffolding is encouraging.

What I would not do is merge this as a stable surface. The author's own TODO list says the central names are not final; the round-trip semantics of the camera API have real holes; OHIF will need a paired PR before useViewportNext becomes the default. Land it on beta as is — explicitly behind useViewportNext — and burn down items 1–10 above before flipping the default or releasing as 5.0.