Skip to content

POC: bring viv-dicomweb-test work into SLIM and add basic bulk annotations support - #381

Open
igoroctaviano wants to merge 80 commits into
masterfrom
feat/viv-loader
Open

POC: bring viv-dicomweb-test work into SLIM and add basic bulk annotations support #381
igoroctaviano wants to merge 80 commits into
masterfrom
feat/viv-loader

Conversation

@igoroctaviano

@igoroctaviano igoroctaviano commented Apr 8, 2026

Copy link
Copy Markdown
Collaborator

Summary

POC port of viv-dicomweb-test into SLIM: Viv + Deck.gl slide viewing on dedicated /viv/... routes plus bulk simple annotations (polygons) wired through the same DICOMweb + DMV decode path as OpenLayers.

What changed (vs master)

Verified against feat/viv-loadermaster in the Slim repo (18 files, ~4.3k insertions).

  • Routing: New React Router paths that mirror existing case URLs but prefix /viv: /viv/studies/:studyInstanceUID/* and the GCP Healthcare store variant. ParametrizedCaseViewer passes isVivRoute and optional vivSettings from config when the pathname starts with /viv/.

  • src/viv/ module: dicomLoader.ts (DICOMweb tiling + DMV integration), VivSlideViewport.tsx, loadBulkAnnotationLayers.ts, vivDisplayDefaults.ts, README.md. Includes request abort/cancel handling and viewport work (incl. 8-bit SM tile positioning).

  • UI integration: CaseViewer.tsx switches the slide viewport to VivSlideViewport on Viv routes, adds VivSettingsDrawer and an annotation rail wired to AnnotationGroupList, and adjusts study URLs so navigation stays under /viv/….

  • Dependencies: @deck.gl/*, @vivjs/layers, @vivjs/loaders, @luma.gl/*; dicomweb-client raised to ^0.11.2 (plus types/dicomweb-client). Overrides pin Deck versions; bun.lock updated.

  • Build: craco.config.js — Babel transpilation for node_modules under @deck.gl, @luma.gl, @math.gl, @probe.gl, @hms-dbmi/viv.

  • DICOM web client hook: DicomWebManager.applyToPrimaryDicomwebClient so Viv tiles can attach to the primary store’s requestHooks (same XHR pipeline as VolumeImageViewer).

  • Config types: AppConfig.d.ts — optional vivSettings (selections, channelsVisible, contrastLimits, colors, initialViewState). There is no useViv flag; Viv is strictly /viv/... URLs.

  • ICC profiles: iccProfilesPreference.ts and updates in SlideViewer.tsx so ICC enable/disable persists (fixes ICC behavior with the Viv path).

  • Types: types/dicom-microscopy-viewer/index.d.ts extended for DMV APIs the Viv loader uses.

  • App.tsx: registers the Viv routes.

Branch & local setup

  • SLIM branch: feat/viv-loader.
  • POC still expects a linked dicom-microscopy-viewer checkout with Viv-facing API changes until a matching release is published. From DMV (correct branch): bun link. From the Slim checkout: bun link dicom-microscopy-viewer, then bun install if needed. bun unlink dicom-microscopy-viewer in Slim restores registry installs. Details: src/viv/README.md.

How to test

  1. Add this server URL: https://proxy.imaging.datacommons.cancer.gov/current/viewer-only-no-downloads-see-tinyurl-dot-com-slash-3j3d9jyp/dicomWeb

  2. Open the Viv-prefixed study URL (same study/series as before, /viv after the origin):

    http://localhost:3000/viv/studies/2.25.106918873973188798943205935727506273925/series/1.3.6.1.4.1.5962.99.1.1088146757.1503397867.1637470719301.2.0

  3. (Optional) In public/config/*.js, set vivSettings for channel/contrast/colors/initialViewState on /viv/....

  4. Toggle annotation group visibility. Expect a long first pass while every polygon is built (~23 s in one run); after that, pan/zoom stays responsive and memory behaved better than earlier OpenLayers attempts for this POC.

Screenshot 2026-04-07 at 21 39 51

Possible TODO's

  • Faster annotation loads: stream or commit geometry incrementally as it’s decoded; push heavy parsing/decoding to workers without breaking coordinate math.
  • Clear busy state: loading / progress (and cancel/retry) for long annotation jobs.
  • Lazy / viewport-aware loading: bulk annotations for matching series still load all at once; explore per-view / LOD loading on huge cases.
  • Tighter layering in src/viv/: split network/DICOMweb, decode (DMV), and Deck.gl layer build so dicomLoader / viewport files do not become one mega-module; document slide space → Deck props for the next contributor.
  • One source of truth with OpenLayers path: align behavior and coordinates with SlideViewer / VolumeImageViewer; avoid duplicated fetch/decode when DMV already exposes primitives.
  • Dependency hygiene: consume a published DMV release with Viv-facing exports instead of bun link, and note which DMV version this tracks.
  • Annotations beyond bulk simple: no TID1500 SR, SEG, optical-path sidebar, or presentation states on /viv yet — prioritize or gate behind flags (see README limitations).
  • Docs / example config: add a vivSettings snippet to public/config example docs (paths are /viv/..., not a boolean config switch).
  • README vs code: align src/viv/README.md limitations with dicomLoader (8-/16-bit handling and mixed-bit-depth restriction per series).
  • Automated checks: smoke tests (build + minimal Viv route) and, if feasible, a perf budget for toggling annotation groups on a fixed IDC fixture.
  • Failure modes: clearer errors when WebGL/Deck fails, DICOMweb errors, or malformed annotation corpora—without wedging the case view.

@deepsource-io

deepsource-io Bot commented Apr 8, 2026

Copy link
Copy Markdown

DeepSource Code Review

We reviewed changes in f726405...d0f9fef on this pull request. Below is the summary for the review, and you can see the individual issues we found as inline review comments.

See full review on DeepSource ↗

PR Report Card

Overall Grade   Security  

Reliability  

Complexity  

Hygiene  

Code Review Summary

Analyzer Status Updated (UTC) Details
JavaScript Aug 14, 2026 4:52p.m. Review ↗

Important

AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.

@igoroctaviano
igoroctaviano requested a review from fedorov April 8, 2026 00:54
@igoroctaviano

Copy link
Copy Markdown
Collaborator Author

@fedorov

fedorov commented May 7, 2026

Copy link
Copy Markdown
Member

@igoroctaviano it would be helpful to indicate somewhere (in the "info" panel perhaps?) whether given instance is using viv or not.

I deployed the viewer with useViv: true at https://andrey-slim-test.web.app/studies/2.25.147491078891690248552488589436202370211/series/1.3.6.1.4.1.5962.99.1.1471805016.1464679391.1637854377560.2.0, and I have no clue if it worked and what backend is being used!

@igoroctaviano

Copy link
Copy Markdown
Collaborator Author

@igoroctaviano it would be helpful to indicate somewhere (in the "info" panel perhaps?) whether given instance is using viv or not.

I deployed the viewer with useViv: true at https://andrey-slim-test.web.app/studies/2.25.147491078891690248552488589436202370211/series/1.3.6.1.4.1.5962.99.1.1471805016.1464679391.1637854377560.2.0, and I have no clue if it worked and what backend is being used!

PR description outdated. Just use the viv route before /studies

@sonarqubecloud

sonarqubecloud Bot commented May 7, 2026

Copy link
Copy Markdown

@github-actions

Copy link
Copy Markdown

📦 Firebase Preview - Using Published DMV

This preview is using the published dicom-microscopy-viewer from package.json:

Version ^0.48.23

To link a DMV branch for testing (requires an open PR in DMV):

  • Add dmv-branch: <branch-name> to the PR description, OR
  • Use the same branch name in both repos (automatic matching)

@github-actions

Copy link
Copy Markdown

📦 Firebase Preview - Using Published DMV

This preview is using the published dicom-microscopy-viewer from package.json:

Version ^0.48.23

To link a DMV branch for testing (requires an open PR in DMV):

  • Add dmv-branch: <branch-name> to the PR description, OR
  • Use the same branch name in both repos (automatic matching)

- Return undefined explicitly in useEffect (VivBulkAnnotationLoadIndicator)
- Simplify boolean returns and rename short variables (iccProfilesPreference)
- Rename short variables to descriptive names (vivBulkLodPreference, dicomLoader)
- Use template literal for worker source (centerOutAnnotationOrder)
- Reorder variable declarations to avoid used-before-defined (centerOutAnnotationOrder)
- Add skipcq for valid patterns (CaseViewer default import, void operator)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

📦 Firebase Preview - Using Published DMV

This preview is using the published dicom-microscopy-viewer from package.json:

Version ^0.48.23

To link a DMV branch for testing (requires an open PR in DMV):

  • Add dmv-branch: <branch-name> to the PR description, OR
  • Use the same branch name in both repos (automatic matching)

- Reorder settle/onMessage/onError to fix used-before-defined (JS-0357)
- Rename short variables: v→viewerRecord, m→affineMatrix, b→imageBits,
  s→imageSpp, r→ratio for JS-C1002
- Add explicit return at end of _ensureTileDecodeReady (JS-0045)
- Change SyntheticDyadicPixelSource.getRaster to use Promise.reject (JS-0116)
- Add skipcq comments for intentional patterns (JS-0105, JS-R1005)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

📦 Firebase Preview - Using Published DMV

This preview is using the published dicom-microscopy-viewer from package.json:

Version ^0.48.23

To link a DMV branch for testing (requires an open PR in DMV):

  • Add dmv-branch: <branch-name> to the PR description, OR
  • Use the same branch name in both repos (automatic matching)

- Use function declarations instead of let+assignment for hoisting (JS-0242)
- Rename short variables: v→indexValue, o→api, r→result (JS-C1002)
- Add skipcq for dcmjs namespace import (JS-C1003)
- Add skipcq for high-complexity functions (JS-R1005)
- Use regular strings instead of template literals where no interpolation (JS-R1004)
- Use property shorthand for annotationGroupWrapper (JS-0240)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

📦 Firebase Preview - Using Published DMV

This preview is using the published dicom-microscopy-viewer from package.json:

Version ^0.48.23

To link a DMV branch for testing (requires an open PR in DMV):

  • Add dmv-branch: <branch-name> to the PR description, OR
  • Use the same branch name in both repos (automatic matching)

igoroctaviano and others added 3 commits August 14, 2026 11:15
- Replace Math.random() with crypto.getRandomValues() for retry jitter (S2245)
- Remove identical conditional branches in isMultipart check (S3923)
- Remove unnecessary void operators in useEffect dependencies (S3735)
- Extract duplicate ensureCapacity to createPayloadBuffer helper (S4144)
- Add skipcq comment for getRaster interface compliance (JS-0105)
- Convert template strings without interpolations to regular strings (JS-R1004)
- Rename single-letter variables to descriptive names (JS-C1002)
- Remove unnecessary async keyword from function returning promise (JS-0116)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

📦 Firebase Preview - Using Published DMV

This preview is using the published dicom-microscopy-viewer from package.json:

Version ^0.48.23

To link a DMV branch for testing (requires an open PR in DMV):

  • Add dmv-branch: <branch-name> to the PR description, OR
  • Use the same branch name in both repos (automatic matching)

igoroctaviano and others added 2 commits August 14, 2026 11:36
The e2e visual regression workflow is for testing bulk annotation
rendering, which belongs in the feat/bump-dmv-bulk-ann branch (PR #405),
not in this viv-loader branch. This branch doesn't have the e2e test
infrastructure (build:e2e, test:e2e scripts).

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Remove e2e-visual.yml workflow (belongs in PR #405, not this branch)
- Add skipcq for void operator usage (JS-0098)
- Add skipcq for high complexity functions (JS-R1005):
  - buildPointLayersFromGraphicData
  - buildPathLayersFromGraphicData
  - loadBulkAnnotationMetadataAndJobs
- Rename short variables (JS-C1002):
  - g→typedGeom, t→geomType
  - b→bulkApi
  - d→typedDataset
- Use regular strings instead of template literals (JS-R1004)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

📦 Firebase Preview - Using Published DMV

This preview is using the published dicom-microscopy-viewer from package.json:

Version ^0.48.23

To link a DMV branch for testing (requires an open PR in DMV):

  • Add dmv-branch: <branch-name> to the PR description, OR
  • Use the same branch name in both repos (automatic matching)

- Move skipcq comment directly above void statement (JS-0098)
- Fix more template strings without interpolation (JS-R1004)
- Remove async from deprecated function that doesn't await (JS-0116)
- Add skipcq for buildStyledBulkOverlayLayers complexity (JS-R1005)
- Rename `a` to `alpha` for opacity calculation (JS-C1002)
- Add skipcq for delete statements on ref-tracked dynamic keys (JS-0320)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

📦 Firebase Preview - Using Published DMV

This preview is using the published dicom-microscopy-viewer from package.json:

Version ^0.48.23

To link a DMV branch for testing (requires an open PR in DMV):

  • Add dmv-branch: <branch-name> to the PR description, OR
  • Use the same branch name in both repos (automatic matching)

Add skipcq comments for void statements (JS-0098), delete operations
on dynamic keys (JS-0320), and cleanup function returns (JS-0045).
Rename short variable names: a -> alpha, g -> geom (JS-C1002).

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

📦 Firebase Preview - Using Published DMV

This preview is using the published dicom-microscopy-viewer from package.json:

Version ^0.48.23

To link a DMV branch for testing (requires an open PR in DMV):

  • Add dmv-branch: <branch-name> to the PR description, OR
  • Use the same branch name in both repos (automatic matching)

- Add skipcq for delete operations in loops (JS-0320)
- Add skipcq for useEffect cleanup functions (JS-0045)
- Rename short variables: r -> obj, w/h -> width/height, z -> tileZ,
  a/b -> spacingX/spacingY, t -> ratio/factor (JS-C1002)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

📦 Firebase Preview - Using Published DMV

This preview is using the published dicom-microscopy-viewer from package.json:

Version ^0.48.23

To link a DMV branch for testing (requires an open PR in DMV):

  • Add dmv-branch: <branch-name> to the PR description, OR
  • Use the same branch name in both repos (automatic matching)

- Rename short variables: d -> displayOpts, w/h -> width/height,
  v -> viv, z -> zoom, t -> tgt (JS-C1002)
- Add skipcq for void statements on fire-and-forget async ops (JS-0098)
- Add skipcq for useEffect cleanup functions (JS-0045)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

📦 Firebase Preview - Using Published DMV

This preview is using the published dicom-microscopy-viewer from package.json:

Version ^0.48.23

To link a DMV branch for testing (requires an open PR in DMV):

  • Add dmv-branch: <branch-name> to the PR description, OR
  • Use the same branch name in both repos (automatic matching)

igoroctaviano and others added 2 commits August 14, 2026 13:44
The lockfile was missing the entry for dicomweb-client@0.10.3 which is
a transitive dependency from dicom-microscopy-viewer. This caused CI
failures with ERR_PNPM_LOCKFILE_MISSING_DEPENDENCY.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@sonarqubecloud

Copy link
Copy Markdown

@github-actions

Copy link
Copy Markdown

📦 Firebase Preview - Using Published DMV

This preview is using the published dicom-microscopy-viewer from package.json:

Version ^0.48.23

To link a DMV branch for testing (requires an open PR in DMV):

  • Add dmv-branch: <branch-name> to the PR description, OR
  • Use the same branch name in both repos (automatic matching)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants