feature: support OPM with palette color LUT #5522
Conversation
✅ Deploy Preview for ohif-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
tunglt1810
changed the title
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| } else { | ||
| // If bitsPerEntry < 8, shift left | ||
| lut[i] = (lutValue16 << Math.abs(offsetBits)) & 0xff; | ||
| } |
There was a problem hiding this comment.
Bug: Palette Color Retrieval Breaks 8-bit Compatibility
The _getPaletteColor function now always reads palette data as 16-bit values, assuming the buffer is sized accordingly. This differs from the previous logic that handled both 8-bit and 16-bit data based on the bits parameter. For 8-bit palette data, this can cause buffer overruns and incorrect interpretation, impacting backward compatibility.
There was a problem hiding this comment.
Hi @IbrahimCSAE ,
If you have another testcase, can you share it. So I can test this logic.
|
I have to test this with other color images I have to make sure they didn’t break @sedghi |
|
Hi @IbrahimCSAE , |
| lut[i] = (lutValue16 >> offsetBits) & 0xff; | ||
| } else { | ||
| // If bitsPerEntry < 8, shift left | ||
| lut[i] = (lutValue16 << Math.abs(offsetBits)) & 0xff; |
There was a problem hiding this comment.
Why is a shift needed? offsetBits will be 0 for this case, so
lut[i] = lutValue16 && 0xff;
should be sufficient.
| // Read 16-bit value (LUT is always 16-bit) | ||
| const lutValue16 = view.getUint16(i * 2, true); | ||
|
|
||
| // Apply Orthanc shift logic |
There was a problem hiding this comment.
This should not be orthanc specific - either reference the DICOM standard or specify that this is the format for sop class ...
| for (let i = 0; i < numLutEntries; i++) { | ||
| lut[i] = data[i]; | ||
| // Read 16-bit value (LUT is always 16-bit) | ||
| const lutValue16 = view.getUint16(i * 2, true); |
There was a problem hiding this comment.
According to PS3.3 C.8.17.14.1.6 Bits Allocated, Bits Stored, and High Bit
you can have either 8 or 16 bit data. Please ensure that 8 bit data is handled.
| // const data = bits === 16 ? new Uint16Array(buffer) : new Uint8Array(buffer); | ||
|
|
||
| const view = new DataView(buffer); | ||
| const offsetBits = bits - 8; |
There was a problem hiding this comment.
Suggest isTwoByte = bits > 8
|
Here is an image previously provided for fixing in OHIF: |
|
The attached image is 8 bit data, and is the test data we used previously for checking out the 8 bit palette colour fixes. |
4 participants
Context
Currently, Viewer does not support render dicom CornealTopographyMapStorage with SOPClassUID 1.2.840.10008.5.1.4.1.1.82.1
If I only add SOP class uid to default extension, this is the image
Changes & Results
fetchPaletteColorLookupTableDataTesting
Checklist
PR
semantic-release format and guidelines.
Code
etc.)
Public Documentation Updates
additions or removals.
Tested Environment
Dataset:
HIDDEN_PID-HIDDEN_PNAME-20250908-AS-OCT Corneal Mapping.zip
OS: macOS 26.0.1
Node version: v20.18.1
Browser: Chrome Version 141.0.7390.108 (Official Build) (arm64)