Skip to content

fix(dicomImageLoader): normalize stored pixel bits - #2868

Open
daker wants to merge 1 commit into
cornerstonejs:mainfrom
daker:stored-pixel-bits
Open

fix(dicomImageLoader): normalize stored pixel bits#2868
daker wants to merge 1 commit into
cornerstonejs:mainfrom
daker:stored-pixel-bits

Conversation

@daker

@daker daker commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Context

Mask unsigned pixels to Bits Stored before range calculation. Use High Bit to locate the stored field and preserve signed values. Fix rendering when unused upper bits contain nonzero data.

Fixes #781

Changes & Results

image

Testing

Checklist

PR

  • [] My Pull Request title is descriptive, accurate and follows the
    semantic-release format and guidelines.

Code

  • [] My code has been well-documented (function documentation, inline comments,
    etc.)

Public Documentation Updates

  • [] The documentation page has been updated as necessary for any public API
    additions or removals.

Tested Environment

  • [] "OS:
  • [] "Node version:
  • [] "Browser:

Summary by CodeRabbit

  • Bug Fixes

    • Improved DICOM image pixel decoding for unsigned and signed pixel data.
    • Correctly applies stored-bit ranges and High Bit metadata, improving image values when pixel data uses nonstandard bit layouts.
    • Preserves already-normalized data and safely handles invalid pixel metadata.
  • Enhancements

    • Image frame metadata now includes High Bit information for more accurate pixel interpretation.

Mask unsigned pixels to Bits Stored before range calculation.
Use High Bit to locate the stored field and preserve signed values.
Fix rendering when unused upper bits contain nonzero data.

Fixes cornerstonejs#781
@coderabbitai

coderabbitai Bot commented Aug 17, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: c146eddb-92a8-4dc8-9dc7-0660b8114cd8

📥 Commits

Reviewing files that changed from the base of the PR and between 7034957 and 90f7fc1.

📒 Files selected for processing (5)
  • packages/core/src/types/IImageFrame.ts
  • packages/dicomImageLoader/src/__tests__/normalizeStoredPixelData.spec.ts
  • packages/dicomImageLoader/src/decodeImageFrameWorker.js
  • packages/dicomImageLoader/src/imageLoader/getImageFrame.ts
  • packages/dicomImageLoader/src/shared/normalizeStoredPixelData.ts

Included review availability: Your plan includes up to 4 reviews per rolling hour; 2 remain after this review.


📝 Walkthrough

Walkthrough

The change adds optional HighBit metadata, centralizes stored pixel normalization, applies it during decoded-pixel post-processing, and adds coverage for masking, shifting, sign extension, and unchanged data.

Changes

Decoded pixel normalization

Layer / File(s) Summary
HighBit metadata contract
packages/core/src/types/IImageFrame.ts, packages/dicomImageLoader/src/imageLoader/getImageFrame.ts
The ImageFrame type accepts highBit, and image-frame creation includes the DICOM HighBit value.
Stored pixel normalization
packages/dicomImageLoader/src/shared/normalizeStoredPixelData.ts
The shared function validates metadata, masks and shifts stored bits, and sign-extends signed values in place.
Decoder integration and validation
packages/dicomImageLoader/src/decodeImageFrameWorker.js, packages/dicomImageLoader/src/__tests__/normalizeStoredPixelData.spec.ts
Decoded-pixel post-processing uses the shared normalizer. Tests cover unsigned masking, signed sign extension, HighBit extraction, and unchanged data.

Estimated code review effort: 3 (Moderate) | ~15 minutes

Merge Risk: ⚪ Minimal · up to 90f7f

No actionable merge-blocking risk remains; the PR is merge-ready after normal checks and review.

Possibly related PRs

Suggested reviewers: sedghi

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description explains the bug and links issue #781, but it omits testing details and leaves all checklist items unchecked. Add concrete testing steps and tested-environment details, then mark each applicable checklist item as complete.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the stored-pixel normalization fix and follows the repository's semantic-release format.
Linked Issues check ✅ Passed The changes address issue #781 by normalizing stored bits, using HighBit, preserving signed values, and adding targeted tests.
Out of Scope Changes check ✅ Passed The code, metadata update, integration change, and tests are directly related to correcting DX pixel rendering described in issue #781.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@daker

daker commented Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

@jbocce can i have a review?

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.

[Bug] Some DX Images Not Loading Correctly

1 participant