-
-
Notifications
You must be signed in to change notification settings - Fork 4.3k
feat(next): native Generic Viewport migration (useNextViewports) #6101
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
sedghi
wants to merge
64
commits into
master
Choose a base branch
from
ohifohifnextapi
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+5,115
−806
Open
Changes from 32 commits
Commits
Show all changes
64 commits
Select commit
Hold shift + click to select a range
6abe023
feat(core): add appConfig.useNextViewports flag (Generic Viewport M0 …
sedghi 6c1aad1
feat(cornerstone): map viewport types to native *_NEXT under useNextV…
sedghi ef25b55
feat(cornerstone): native-next stack mount behind useNextViewports (M…
sedghi a3fa6ad
feat(cornerstone): render native Generic (next) stack viewport behind…
sedghi 13af1cd
feat(cornerstone): render native Generic (next) volume/MPR behind use…
sedghi f78d01e
feat(cornerstone): allow setViewportOrientation on native volume view…
sedghi e164f8c
feat: Add temporary support for native GenericViewport ("next") migra…
sedghi 6d9e46f
Add plan viewer HTML page for migration master plan display
sedghi ddd4ccc
feat(cornerstone): fork viewport presentation read/write into the bac…
sedghi c51fe4c
feat(cornerstone): persist native viewport pan/zoom/rotation/flip via…
sedghi 93e15bd
feat(cornerstone): restore native volume/MPR pan/zoom on mount
sedghi 99b23cb
fix(next): bridge invert/flip/window-level commands for native viewports
sedghi 4a1b964
fix(next): apply setViewportColormap on native viewports
sedghi 461c015
fix(next): make ColorbarService native-safe
sedghi d282026
fix(next): guard per-volume histogram WL panel for native viewports
sedghi 55091cf
fix(next): make resetViewport/scaleViewport/rotate commands native-safe
sedghi 26273b3
fix(next): guard jumpToMeasurement camera-centering for native viewports
sedghi 5d39eca
fix(next): make getViewportAlignmentData + updateViewport native-safe
sedghi 984e6ac
refactor(next): move viewport interaction ops into a Legacy/Next oper…
sedghi 3963b57
feat(next): render native 3D volume rendering + enable its VR operations
sedghi 05e0df0
docs(next): refresh migration plan to HEAD (2026-06-19 audit)
sedghi 7b61e08
fix(next): target fusion colormap at the overlay binding
sedghi a19bd78
fix(next): make residual native-unsafe viewport sites safe
sedghi ca746e2
feat(next): mount native video/WSI/ECG viewports
sedghi b5784ca
chore(next): guard the useNextViewports flag-read allowlist (M7 prep)
sedghi d5d03d8
docs(next): mark CS-12 native calibration done; refresh CS-20/M6 status
sedghi 138dc17
docs(next): re-verify migration status at HEAD; correct stale prose
sedghi aad8cb9
Refactor SegmentationService to support dual backends for segmentatio…
sedghi cc9883d
temp
sedghi ec3cba4
Refactor CornerstoneViewportService to optimize setDisplaySets handli…
sedghi 55352f2
d
sedghi 053f63c
Refactor viewport handling and opacity management for native volume r…
sedghi 9cbe6b1
fix(WindowLevel): re-sync fusion tab to foreground default after asyn…
sedghi dd9e8c3
Merge remote-tracking branch 'origin/master' into ohifohifnextapi
sedghi b8efc7c
chore(next): remove WIP migration plan artifacts from the branch
sedghi 880b346
fix(next): address review findings (guards + correctness)
sedghi fd2abfa
fix(overlay): show instance number on next viewports
sedghi 0f9df44
fix(overlay): refresh window level on series change for next viewports
sedghi 4374847
fix(scrollbar): seed slice state on orientation change for next viewp…
sedghi 946671b
refactor(next): call resetDisplaySetPresentation on reset
sedghi 1469efe
fix(segmentation): make border/outline thickness slider integer-only
sedghi 7fadeea
fix(crosshairs): guard resetCrosshairs against unregistered Crosshair…
sedghi ec034e4
fix(next-fusion): promote source to volume slice when a data overlay …
sedghi 03c2bf1
fix(next-rtss): keep referenced CT in stack mode on RTSTRUCT hydrate
sedghi b82ffc5
fix(next-fusion): match legacy initial data-overlay opacity (~40%) on…
sedghi 9571c5f
fix(next-fusion): preserve fusion on orientation change in next viewp…
sedghi b8c6f80
fix(next-seg): preserve base image window level through SEG hydration
sedghi a8be6fa
fix(next-mpr): re-seed slice scrollbar after post-mount camera carry
sedghi fa6ddde
Merge remote-tracking branch 'origin/master' into ohifohifnextapi
sedghi 2389f55
Merge remote-tracking branch 'origin/master' into ohifohifnextapi
sedghi 1ac4a09
chore(deps): bump @cornerstonejs/* to 5.1.2
sedghi e5c5f73
chore: empty commit
sedghi ee9bff2
fix(next): use published genericViewportDisplaySetMetadataProvider ex…
sedghi 9153297
test(e2e): extend Scoord3dProbe jump screenshot retry window
sedghi 3392e02
fix(next): address review findings (scaleBy 3D crash, fusion W/L targ…
sedghi b16129e
fix(next): address CodeRabbit review (dispatch discriminator, seg ret…
sedghi d0c8d50
test(next): update getCornerstoneViewportType invalid-type assertion
sedghi 084b6b9
chore(next): replace dev toggle + flag-read guard with URL opt-in
sedghi dab2f1b
fix(tmtv): keep legacy fusion PT opacity ramp; flatten only on next path
sedghi 113a20e
redo
sedghi 812e823
Merge remote-tracking branch 'origin/master' into ohifohifnextapi
sedghi 641dac1
fix(next): avoid top-level dicomWebUtils destructure crash on boot
sedghi df7aa4b
fix(next): guard remount no-op path and defer legacy camera snapshot
sedghi 9e5ab84
feat(next): add ?cpu=true URL opt-in to force the CPU render path
sedghi File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,99 @@ | ||
| import fs from 'node:fs'; | ||
| import path from 'node:path'; | ||
|
|
||
| /** | ||
| * Guards the `useNextViewports` flag-read surface (migration plan §4.2). | ||
| * | ||
| * The native ("next") GenericViewport opt-in flag must be read in only a small, | ||
| * fixed set of places so the legacy (flag-off) path cannot drift. This check fails | ||
| * if any OTHER file under extensions/cornerstone/src reads the flag, via either | ||
| * `isNextViewportsEnabled()` or `appConfig.useNextViewports`. | ||
| * | ||
| * Sanctioned readers (keep this list and the plan §4.2 in sync): | ||
| * - utils/getCornerstoneViewportType.ts — sanctioned read #1 (viewport type selection) | ||
| * - services/ViewportService/CornerstoneViewportService.ts — sanctioned read #2 (backend selection) | ||
| * - services/SegmentationService/SegmentationService.ts — sanctioned read #3 (the one viewport-less | ||
| * seg-backend dispatch: a DICOM SEG's single- vs multi-layer | ||
| * overlap data shape is fixed at load, before any target | ||
| * viewport exists, so it cannot use a capability predicate) | ||
| * - utils/nextViewports.ts — the accessor module itself (defines get/set) | ||
| * - init.tsx — the single setup site that reads appConfig.useNextViewports | ||
| * to seed the accessor | ||
| * - getToolbarModule.tsx — TEMP dev toggle evaluator (remove at M7; see | ||
| * TODO_BEFORE_MERGE.md, and drop this entry then) | ||
| * - *.test.ts(x) — unit tests exercising the mapping | ||
| * | ||
| * Everywhere else, branch on cornerstone CONTENT/CAPABILITY predicates | ||
| * (csUtils.isGenericViewport / viewportIsInVolumeMode / viewportIsInStackMode), or | ||
| * route the viewport through the Legacy/Next backend twins — never the flag. | ||
| */ | ||
|
|
||
| const SRC_DIR = path.resolve(process.cwd(), 'extensions', 'cornerstone', 'src'); | ||
|
|
||
| // Allowlisted files, relative to SRC_DIR (POSIX separators). | ||
| const ALLOWLIST = new Set([ | ||
| 'utils/getCornerstoneViewportType.ts', | ||
| 'services/ViewportService/CornerstoneViewportService.ts', | ||
| 'services/SegmentationService/SegmentationService.ts', | ||
| 'utils/nextViewports.ts', | ||
| 'init.tsx', | ||
| // TEMP — remove together with the dev toggle button (TODO_BEFORE_MERGE.md): | ||
| 'getToolbarModule.tsx', | ||
| ]); | ||
|
|
||
| // Matches an actual flag READ: the accessor call or the appConfig field access. | ||
| const READ_PATTERNS = [/\bisNextViewportsEnabled\s*\(/, /\bappConfig[?.]*\.useNextViewports\b/]; | ||
|
|
||
| function isCommentLine(line) { | ||
| const t = line.trim(); | ||
| return t.startsWith('//') || t.startsWith('*') || t.startsWith('/*'); | ||
| } | ||
|
|
||
| function walk(dir, out) { | ||
| for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { | ||
| const full = path.join(dir, entry.name); | ||
| if (entry.isDirectory()) { | ||
| walk(full, out); | ||
| } else if (/\.tsx?$/.test(entry.name)) { | ||
| out.push(full); | ||
| } | ||
| } | ||
| return out; | ||
| } | ||
|
|
||
| if (!fs.existsSync(SRC_DIR)) { | ||
| console.error(`[next:check-flag-reads] Source dir not found: ${SRC_DIR}`); | ||
| process.exit(1); | ||
| } | ||
|
|
||
| const offenders = []; | ||
|
|
||
| for (const file of walk(SRC_DIR, [])) { | ||
| const rel = path.relative(SRC_DIR, file).split(path.sep).join('/'); | ||
| if (ALLOWLIST.has(rel) || /\.test\.tsx?$/.test(rel)) { | ||
| continue; | ||
| } | ||
|
|
||
| const lines = fs.readFileSync(file, 'utf8').split('\n'); | ||
| lines.forEach((line, i) => { | ||
| if (isCommentLine(line)) { | ||
| return; | ||
| } | ||
| if (READ_PATTERNS.some(re => re.test(line))) { | ||
| offenders.push(`${rel}:${i + 1}: ${line.trim()}`); | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| if (offenders.length > 0) { | ||
| console.error( | ||
| '[next:check-flag-reads] The useNextViewports flag is read outside the sanctioned set (plan §4.2).\n' + | ||
| 'Branch on cornerstone content/capability predicates or the Legacy/Next backend twins instead,\n' + | ||
| 'or add the file to the allowlist in .scripts/check-next-viewports-flag-reads.mjs if the read is\n' + | ||
| 'genuinely a sanctioned seam. Offending reads:\n' + | ||
| offenders.map(o => ` - ${o}`).join('\n') | ||
| ); | ||
| process.exit(1); | ||
| } | ||
|
|
||
| console.log('[next:check-flag-reads] OK: useNextViewports is read only in the sanctioned set.'); | ||
Large diffs are not rendered by default.
Oops, something went wrong.
Large diffs are not rendered by default.
Oops, something went wrong.
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| # TODO before merge | ||
|
|
||
| Temporary / dev-only changes added while building the native GenericViewport | ||
| ("next") migration behind `appConfig.useNextViewports`. Remove or revert all of | ||
| these before merging to `master`. They exist only to make the native path easy | ||
| to exercise and compare locally; none of them should ship. | ||
|
|
||
| ## 1. Dev config flag (flips the default for everyone) | ||
| - `platform/app/public/config/default.js` — remove the `useNextViewports: true` | ||
| line. The flag must stay opt-in (default off); committing it on would change | ||
| the default backend for every deployment using this config. | ||
|
|
||
| ## 2. In-toolbar legacy/next backend toggle button (debug only) | ||
| A throwaway toolbar button + supporting plumbing to flip the backend at runtime | ||
| and reload. Remove all of: | ||
| - `extensions/cornerstone/src/utils/nextViewports.ts` — remove | ||
| `resolveNextViewportsEnabled`, `toggleNextViewportsAndReload`, and the | ||
| `NEXT_VIEWPORTS_OVERRIDE_KEY` localStorage block. Keep | ||
| `set/isNextViewportsEnabled`. | ||
| - `extensions/cornerstone/src/init.tsx` — revert to | ||
| `setNextViewportsEnabled(Boolean(appConfig.useNextViewports));` (drop the | ||
| `resolveNextViewportsEnabled` override and its import). | ||
| - `extensions/cornerstone/src/commandsModule.ts` — remove the | ||
| `toggleNextViewports` action, its entry in the commands map, and the | ||
| `toggleNextViewportsAndReload` import. | ||
| - `extensions/cornerstone/src/getToolbarModule.tsx` — remove the | ||
| `evaluate.cornerstone.nextViewportsToggle` evaluator and the | ||
| `isNextViewportsEnabled` import. Then also drop `getToolbarModule.tsx` from the | ||
| allowlist in `.scripts/check-next-viewports-flag-reads.mjs` (it is the one | ||
| TEMP-only entry there). | ||
| - `modes/basic/src/toolbarButtons.ts` — remove the `ToggleNextViewport` button. | ||
| - `modes/basic/src/index.tsx` — remove `'ToggleNextViewport'` from the primary | ||
| toolbar section. | ||
|
|
||
| Each temporary block is marked with a `TEMP (remove before merge ...)` comment, | ||
| so `git grep "remove before merge"` finds every site. | ||
|
|
||
| ## 3. Flag-read allowlist guard (permanent — keep) | ||
|
|
||
| `yarn next:check-flag-reads` (`.scripts/check-next-viewports-flag-reads.mjs`) | ||
| enforces migration plan §4.2: the `useNextViewports` flag may be read only in the | ||
| sanctioned set — `getCornerstoneViewportType.ts` (type selection), | ||
| `CornerstoneViewportService.ts` (backend selection), `nextViewports.ts` (the | ||
| accessor), `init.tsx` (the one `appConfig.useNextViewports` read that seeds the | ||
| accessor), and — temporarily — `getToolbarModule.tsx` (the dev toggle). Everywhere | ||
| else, branch on cornerstone content/capability predicates or the Legacy/Next | ||
| backend twins. This guard is NOT a dev-only revert; it stays after merge. | ||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||
|
|
||
| ## 4. Verify | ||
| After removing the above: | ||
| - `git grep "remove before merge"` returns nothing. | ||
| - `git grep useNextViewports -- platform/app/public/config` returns nothing. | ||
| - `yarn next:check-flag-reads` still passes (with `getToolbarModule.tsx` dropped | ||
| from its allowlist). | ||
| - The native path is still reachable purely by setting `useNextViewports: true` | ||
| in a deployment config (the feature itself stays). | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.